Commit ce28b51d authored by JINMEI Tatuya's avatar JINMEI Tatuya
Browse files

[1332] Merge branch 'trac1331' into trac1332

parents 9753568c 7c6c7252
......@@ -272,6 +272,21 @@ public:
/// In such cases this method will result in an \c isc::NotImplemented
/// exception unconditionally or when \c replace is false).
///
/// If \c journaling is true, the data source should store a journal
/// of changes. These can be used later on by, for example, IXFR-out.
/// However, the parameter is a hint only. It might be unable to store
/// them and they would be silently discarded. Or it might need to
/// store them no matter what (for example a git-based data source would
/// store journal implicitly). When the \c journaling is true, it might
/// require that the following update be formatted as IXFR transfer
/// (SOA to be removed, bunch of RRs to be removed, SOA to be added,
/// bunch of RRs to be added, and possibly repeated). If it is false, it
/// must not require so.
///
/// We don't support erasing the whole zone (by replace being true) and
/// saving a journal at the same time. In such situation, BadValue is
/// thrown.
///
/// \note To avoid throwing the exception accidentally with a lazy
/// implementation, we still keep this method pure virtual without
/// an implementation. All derived classes must explicitly define this
......@@ -282,14 +297,18 @@ public:
/// \exception DataSourceError Internal error in the underlying data
/// source.
/// \exception std::bad_alloc Resource allocation failure.
/// \exception BadValue if both replace and journaling are true.
///
/// \param name The zone name to be updated
/// \param replace Whether to delete existing RRs before making updates
/// \param journaling The zone updater should store a journal of the
/// changes.
///
/// \return A pointer to the updater; it will be NULL if the specified
/// zone isn't found.
virtual ZoneUpdaterPtr getUpdater(const isc::dns::Name& name,
bool replace) const = 0;
bool replace, bool journaling = false)
const = 0;
};
}
}
......
......@@ -838,10 +838,12 @@ DatabaseClient::getIterator(const isc::dns::Name& name,
class DatabaseUpdater : public ZoneUpdater {
public:
DatabaseUpdater(shared_ptr<DatabaseAccessor> accessor, int zone_id,
const Name& zone_name, const RRClass& zone_class) :
const Name& zone_name, const RRClass& zone_class,
bool journaling) :
committed_(false), accessor_(accessor), zone_id_(zone_id),
db_name_(accessor->getDBName()), zone_name_(zone_name.toText()),
zone_class_(zone_class),
zone_class_(zone_class), journaling_(journaling),
diff_phase_(NOT_STARTED),
finder_(new DatabaseClient::Finder(accessor_, zone_id_, zone_name))
{
logger.debug(DBG_TRACE_DATA, DATASRC_DATABASE_UPDATER_CREATED)
......@@ -883,6 +885,15 @@ private:
const string db_name_;
const string zone_name_;
const RRClass zone_class_;
const bool journaling_;
// For the journals
enum DiffPhase {
NOT_STARTED,
DELETE,
ADD
};
DiffPhase diff_phase_;
uint32_t serial_;
boost::scoped_ptr<DatabaseClient::Finder> finder_;
};
......@@ -902,6 +913,19 @@ DatabaseUpdater::addRRset(const RRset& rrset) {
<< zone_name_ << "/" << zone_class_ << ": "
<< rrset.toText());
}
if (rrset.getType() == RRType::SOA() && diff_phase_ == ADD &&
journaling_) {
isc_throw(isc::BadValue, "Another SOA added inside an add sequence");
}
if (rrset.getType() != RRType::SOA() && diff_phase_ != ADD &&
journaling_) {
isc_throw(isc::BadValue, "Adding sequence not started by SOA");
}
if (rrset.getType() == RRType::SOA() && diff_phase_ != DELETE &&
journaling_) {
isc_throw(isc::BadValue,
"Adding sequence can follow only after delete");
}
RdataIteratorPtr it = rrset.getRdataIterator();
if (it->isLast()) {
......@@ -916,6 +940,21 @@ DatabaseUpdater::addRRset(const RRset& rrset) {
rrset.getName().reverse().toText();
columns[DatabaseAccessor::ADD_TTL] = rrset.getTTL().toText();
columns[DatabaseAccessor::ADD_TYPE] = rrset.getType().toText();
string journal[DatabaseAccessor::DIFF_PARAM_COUNT];
if (journaling_) {
journal[DatabaseAccessor::DIFF_NAME] =
columns[DatabaseAccessor::ADD_NAME];
journal[DatabaseAccessor::DIFF_TYPE] =
columns[DatabaseAccessor::ADD_TYPE];
journal[DatabaseAccessor::DIFF_TTL] =
columns[DatabaseAccessor::ADD_TTL];
diff_phase_ = ADD;
if (rrset.getType() == RRType::SOA()) {
serial_ =
dynamic_cast<const rdata::generic::SOA&>(it->getCurrent()).
getSerial();
}
}
for (; !it->isLast(); it->next()) {
if (rrset.getType() == RRType::RRSIG()) {
// XXX: the current interface (based on the current sqlite3
......@@ -929,6 +968,16 @@ DatabaseUpdater::addRRset(const RRset& rrset) {
rrsig_rdata.typeCovered().toText();
}
columns[DatabaseAccessor::ADD_RDATA] = it->getCurrent().toText();
if (journaling_) {
journal[DatabaseAccessor::DIFF_RDATA] =
columns[DatabaseAccessor::ADD_RDATA];
try {
accessor_->addRecordDiff(zone_id_, serial_,
DatabaseAccessor::DIFF_ADD, journal);
}
// We ignore not implemented
catch (const isc::NotImplemented&) {}
}
accessor_->addRecordToZone(columns);
}
}
......@@ -949,6 +998,15 @@ DatabaseUpdater::deleteRRset(const RRset& rrset) {
<< zone_name_ << "/" << zone_class_ << ": "
<< rrset.toText());
}
if (rrset.getType() == RRType::SOA() && diff_phase_ == DELETE &&
journaling_) {
isc_throw(isc::BadValue,
"Another SOA delete inside a delete sequence");
}
if (rrset.getType() != RRType::SOA() && diff_phase_ != DELETE &&
journaling_) {
isc_throw(isc::BadValue, "Delete sequence not started by SOA");
}
RdataIteratorPtr it = rrset.getRdataIterator();
if (it->isLast()) {
......@@ -960,8 +1018,33 @@ DatabaseUpdater::deleteRRset(const RRset& rrset) {
string params[DatabaseAccessor::DEL_PARAM_COUNT]; // initialized with ""
params[DatabaseAccessor::DEL_NAME] = rrset.getName().toText();
params[DatabaseAccessor::DEL_TYPE] = rrset.getType().toText();
string journal[DatabaseAccessor::DIFF_PARAM_COUNT];
if (journaling_) {
journal[DatabaseAccessor::DIFF_NAME] =
params[DatabaseAccessor::DEL_NAME];
journal[DatabaseAccessor::DIFF_TYPE] =
params[DatabaseAccessor::DEL_TYPE];
journal[DatabaseAccessor::DIFF_TTL] = rrset.getTTL().toText();
diff_phase_ = DELETE;
if (rrset.getType() == RRType::SOA()) {
serial_ =
dynamic_cast<const rdata::generic::SOA&>(it->getCurrent()).
getSerial();
}
}
for (; !it->isLast(); it->next()) {
params[DatabaseAccessor::DEL_RDATA] = it->getCurrent().toText();
if (journaling_) {
journal[DatabaseAccessor::DIFF_RDATA] =
params[DatabaseAccessor::DEL_RDATA];
try {
accessor_->addRecordDiff(zone_id_, serial_,
DatabaseAccessor::DIFF_DELETE,
journal);
}
// Don't fail if the backend can't store them
catch(const isc::NotImplemented&) {}
}
accessor_->deleteRecordInZone(params);
}
}
......@@ -973,6 +1056,9 @@ DatabaseUpdater::commit() {
<< zone_name_ << "/" << zone_class_ << " on "
<< db_name_);
}
if (journaling_ && diff_phase_ == DELETE) {
isc_throw(isc::Unexpected, "Update sequence not complete");
}
accessor_->commit();
committed_ = true; // make sure the destructor won't trigger rollback
......@@ -986,7 +1072,13 @@ DatabaseUpdater::commit() {
// The updater factory
ZoneUpdaterPtr
DatabaseClient::getUpdater(const isc::dns::Name& name, bool replace) const {
DatabaseClient::getUpdater(const isc::dns::Name& name, bool replace,
bool journaling) const
{
if (replace && journaling) {
isc_throw(isc::BadValue, "Can't store journal and replace the whole "
"zone at the same time");
}
shared_ptr<DatabaseAccessor> update_accessor(accessor_->clone());
const std::pair<bool, int> zone(update_accessor->startUpdateZone(
name.toText(), replace));
......@@ -995,7 +1087,7 @@ DatabaseClient::getUpdater(const isc::dns::Name& name, bool replace) const {
}
return (ZoneUpdaterPtr(new DatabaseUpdater(update_accessor, zone.second,
name, rrclass_)));
name, rrclass_, journaling)));
}
}
}
......@@ -928,7 +928,8 @@ public:
/// accessor. The returned updater will be able to work separately from
/// the original client.
virtual ZoneUpdaterPtr getUpdater(const isc::dns::Name& name,
bool replace) const;
bool replace,
bool journaling = false) const;
private:
/// \brief The RR class that this client handles.
......
......@@ -815,7 +815,7 @@ InMemoryClient::getIterator(const Name& name, bool) const {
}
ZoneUpdaterPtr
InMemoryClient::getUpdater(const isc::dns::Name&, bool) const {
InMemoryClient::getUpdater(const isc::dns::Name&, bool, bool) const {
isc_throw(isc::NotImplemented, "Update attempt on in memory data source");
}
......
......@@ -284,7 +284,8 @@ public:
/// to update via its updater (this may or may not be a good idea and
/// is subject to further discussions).
virtual ZoneUpdaterPtr getUpdater(const isc::dns::Name& name,
bool replace) const;
bool replace, bool journaling = false)
const;
private:
// TODO: Do we still need the PImpl if nobody should manipulate this class
......
......@@ -32,7 +32,9 @@ public:
virtual FindResult findZone(const isc::dns::Name&) const {
return (FindResult(result::NOTFOUND, ZoneFinderPtr()));
}
virtual ZoneUpdaterPtr getUpdater(const isc::dns::Name&, bool) const {
virtual ZoneUpdaterPtr getUpdater(const isc::dns::Name&, bool, bool)
const
{
return (ZoneUpdaterPtr());
}
};
......
......@@ -13,6 +13,7 @@
// PERFORMANCE OF THIS SOFTWARE.
#include <boost/shared_ptr.hpp>
#include <boost/lexical_cast.hpp>
#include <gtest/gtest.h>
......@@ -37,6 +38,7 @@ using namespace std;
// for some systems.
using boost::shared_ptr;
using boost::dynamic_pointer_cast;
using boost::lexical_cast;
using namespace isc::dns;
namespace {
......@@ -269,6 +271,50 @@ private:
};
/**
* Single journal entry in the mock database.
*
* All the members there are public for simplicity, as it only stores data.
* We use the implicit constructor and operator. The members can't be const
* because of the assignment operator (used in the vectors).
*/
struct JournalEntry {
JournalEntry(int id, uint32_t serial,
DatabaseAccessor::DiffOperation operation,
const std::string (&data)[DatabaseAccessor::DIFF_PARAM_COUNT])
: id_(id), serial_(serial), operation_(operation)
{
data_[DatabaseAccessor::DIFF_NAME] = data[DatabaseAccessor::DIFF_NAME];
data_[DatabaseAccessor::DIFF_TYPE] = data[DatabaseAccessor::DIFF_TYPE];
data_[DatabaseAccessor::DIFF_TTL] = data[DatabaseAccessor::DIFF_TTL];
data_[DatabaseAccessor::DIFF_RDATA] =
data[DatabaseAccessor::DIFF_RDATA];
}
JournalEntry(int id, uint32_t serial,
DatabaseAccessor::DiffOperation operation,
const std::string& name, const std::string& type,
const std::string& ttl, const std::string& rdata):
id_(id), serial_(serial), operation_(operation)
{
data_[DatabaseAccessor::DIFF_NAME] = name;
data_[DatabaseAccessor::DIFF_TYPE] = type;
data_[DatabaseAccessor::DIFF_TTL] = ttl;
data_[DatabaseAccessor::DIFF_RDATA] = rdata;
}
int id_;
uint32_t serial_;
DatabaseAccessor::DiffOperation operation_;
std::string data_[DatabaseAccessor::DIFF_PARAM_COUNT];
bool operator==(const JournalEntry& other) const {
bool data_equal(true);
for (size_t i(0); i < DatabaseAccessor::DIFF_PARAM_COUNT; ++ i) {
data_equal = data_equal && (data_[i] == other.data_[i]);
}
return (id_ == other.id_ && serial_ == other.serial_ &&
operation_ == other.operation_ && data_equal);
}
};
/*
* A virtual database accessor that pretends it contains single zone --
* example.org.
......@@ -293,6 +339,7 @@ public:
readonly_records_ = &readonly_records_master_;
update_records_ = &update_records_master_;
empty_records_ = &empty_records_master_;
journal_entries_ = &journal_entries_master_;
fillData();
}
......@@ -301,6 +348,7 @@ public:
cloned_accessor->readonly_records_ = &readonly_records_master_;
cloned_accessor->update_records_ = &update_records_master_;
cloned_accessor->empty_records_ = &empty_records_master_;
cloned_accessor->journal_entries_ = &journal_entries_master_;
latest_clone_ = cloned_accessor;
return (cloned_accessor);
}
......@@ -549,7 +597,13 @@ public:
*update_records_ = *readonly_records_;
}
return (pair<bool, int>(true, WRITABLE_ZONE_ID));
if (zone_name == "bad.example.org.") {
return (pair<bool, int>(true, -1));
} else if (zone_name == "null.example.org.") {
return (pair<bool, int>(true, 13));
} else {
return (pair<bool, int>(true, WRITABLE_ZONE_ID));
}
}
virtual void commit() {
*readonly_records_ = *update_records_;
......@@ -663,6 +717,30 @@ public:
isc_throw(isc::Unexpected, "Unknown zone ID");
}
}
virtual void addRecordDiff(int id, uint32_t serial,
DiffOperation operation,
const std::string (&data)[DIFF_PARAM_COUNT])
{
if (id == 13) { // The null zone doesn't support journaling
isc_throw(isc::NotImplemented, "Test not implemented behaviour");
} else if (id == -1) { // Bad zone throws
isc_throw(DataSourceError, "Test error");
} else {
journal_entries_->push_back(JournalEntry(id, serial, operation,
data));
}
}
// Check the journal is as expected and clear the journal
void checkJournal(const std::vector<JournalEntry> &expected) {
std::vector<JournalEntry> journal;
// Clean the journal, but keep local copy to check
journal.swap(*journal_entries_);
ASSERT_EQ(expected.size(), journal.size());
for (size_t i(0); i < expected.size(); ++ i) {
EXPECT_TRUE(expected[i] == journal[i]);
}
}
private:
// The following member variables are storage and/or update work space
......@@ -682,6 +760,10 @@ private:
const Domains empty_records_master_;
const Domains* empty_records_;
// The journal data
std::vector<JournalEntry> journal_entries_master_;
std::vector<JournalEntry>* journal_entries_;
// used as temporary storage after searchForRecord() and during
// getNextRecord() calls, as well as during the building of the
// fake data
......@@ -799,6 +881,10 @@ public:
rrset_.reset(new RRset(qname_, qclass_, qtype_, rrttl_));
rrset_->addRdata(rdata::createRdata(rrset_->getType(),
rrset_->getClass(), "192.0.2.2"));
soa_.reset(new RRset(zname_, qclass_, RRType::SOA(), rrttl_));
soa_->addRdata(rdata::createRdata(soa_->getType(), soa_->getClass(),
"ns1.example.org. admin.example.org. "
"1234 3600 1800 2419200 7200"));
// And its RRSIG. Also different from the configured one.
rrsigset_.reset(new RRset(qname_, qclass_, RRType::RRSIG(),
......@@ -900,6 +986,7 @@ public:
const RRTTL rrttl_; // commonly used RR TTL
RRsetPtr rrset_; // for adding/deleting an RRset
RRsetPtr rrsigset_; // for adding/deleting an RRset
RRsetPtr soa_; // for adding/deleting an RRset
// update related objects to be tested
ZoneUpdaterPtr updater_;
......@@ -2708,4 +2795,181 @@ TEST_F(MockDatabaseClientTest, badName) {
DataSourceError);
}
/*
* Test correct use of the updater with a journal.
*/
TEST_F(MockDatabaseClientTest, journal) {
updater_ = client_->getUpdater(zname_, false, true);
updater_->deleteRRset(*soa_);
updater_->deleteRRset(*rrset_);
soa_.reset(new RRset(zname_, qclass_, RRType::SOA(), rrttl_));
soa_->addRdata(rdata::createRdata(soa_->getType(), soa_->getClass(),
"ns1.example.org. admin.example.org. "
"1235 3600 1800 2419200 7200"));
updater_->addRRset(*soa_);
updater_->addRRset(*rrset_);
ASSERT_NO_THROW(updater_->commit());
std::vector<JournalEntry> expected;
expected.push_back(JournalEntry(WRITABLE_ZONE_ID, 1234,
DatabaseAccessor::DIFF_DELETE,
"example.org.", "SOA", "3600",
"ns1.example.org. admin.example.org. "
"1234 3600 1800 2419200 7200"));
expected.push_back(JournalEntry(WRITABLE_ZONE_ID, 1234,
DatabaseAccessor::DIFF_DELETE,
"www.example.org.", "A", "3600",
"192.0.2.2"));
expected.push_back(JournalEntry(WRITABLE_ZONE_ID, 1235,
DatabaseAccessor::DIFF_ADD,
"example.org.", "SOA", "3600",
"ns1.example.org. admin.example.org. "
"1235 3600 1800 2419200 7200"));
expected.push_back(JournalEntry(WRITABLE_ZONE_ID, 1235,
DatabaseAccessor::DIFF_ADD,
"www.example.org.", "A", "3600",
"192.0.2.2"));
current_accessor_->checkJournal(expected);
}
/*
* Push multiple delete-add sequences. Checks it is allowed and all is
* saved.
*/
TEST_F(MockDatabaseClientTest, journalMultiple) {
std::vector<JournalEntry> expected;
updater_ = client_->getUpdater(zname_, false, true);
std::string soa_rdata = "ns1.example.org. admin.example.org. "
"1234 3600 1800 2419200 7200";
for (size_t i(1); i < 100; ++ i) {
// Remove the old SOA
updater_->deleteRRset(*soa_);
expected.push_back(JournalEntry(WRITABLE_ZONE_ID, 1234 + i - 1,
DatabaseAccessor::DIFF_DELETE,
"example.org.", "SOA", "3600",
soa_rdata));
// Create a new SOA
soa_rdata = "ns1.example.org. admin.example.org. " +
lexical_cast<std::string>(1234 + i) + " 3600 1800 2419200 7200";
soa_.reset(new RRset(zname_, qclass_, RRType::SOA(), rrttl_));
soa_->addRdata(rdata::createRdata(soa_->getType(), soa_->getClass(),
soa_rdata));
// Add the new SOA
updater_->addRRset(*soa_);
expected.push_back(JournalEntry(WRITABLE_ZONE_ID, 1234 + i,
DatabaseAccessor::DIFF_ADD,
"example.org.", "SOA", "3600",
soa_rdata));
}
ASSERT_NO_THROW(updater_->commit());
// Check the journal contains everything.
current_accessor_->checkJournal(expected);
}
/*
* Test passing a forbidden sequence to it and expect it to throw.
*
* Note that we implicitly test in different testcases (these for add and
* delete) that if the journaling is false, it doesn't expect the order.
*/
TEST_F(MockDatabaseClientTest, journalBadSequence) {
std::vector<JournalEntry> expected;
{
SCOPED_TRACE("Delete A before SOA");
updater_ = client_->getUpdater(zname_, false, true);
EXPECT_THROW(updater_->deleteRRset(*rrset_), isc::BadValue);
// Make sure the journal is empty now
current_accessor_->checkJournal(expected);
}
{
SCOPED_TRACE("Add before delete");
updater_ = client_->getUpdater(zname_, false, true);
EXPECT_THROW(updater_->addRRset(*soa_), isc::BadValue);
// Make sure the journal is empty now
current_accessor_->checkJournal(expected);
}
{
SCOPED_TRACE("Add A before SOA");
updater_ = client_->getUpdater(zname_, false, true);
// So far OK
EXPECT_NO_THROW(updater_->deleteRRset(*soa_));
// But we miss the add SOA here
EXPECT_THROW(updater_->addRRset(*rrset_), isc::BadValue);
// Make sure the journal contains only the first one
expected.push_back(JournalEntry(WRITABLE_ZONE_ID, 1234,
DatabaseAccessor::DIFF_DELETE,
"example.org.", "SOA", "3600",
"ns1.example.org. admin.example.org. "
"1234 3600 1800 2419200 7200"));
current_accessor_->checkJournal(expected);
}
{
SCOPED_TRACE("Commit before add");
updater_ = client_->getUpdater(zname_, false, true);
// So far OK
EXPECT_NO_THROW(updater_->deleteRRset(*soa_));
// Commit at the wrong time
EXPECT_THROW(updater_->commit(), isc::Unexpected);
current_accessor_->checkJournal(expected);
}
{
SCOPED_TRACE("Delete two SOAs");
updater_ = client_->getUpdater(zname_, false, true);
// So far OK
EXPECT_NO_THROW(updater_->deleteRRset(*soa_));
// Delete the SOA again
EXPECT_THROW(updater_->deleteRRset(*soa_), isc::BadValue);
current_accessor_->checkJournal(expected);
}
{
SCOPED_TRACE("Add two SOAs");
updater_ = client_->getUpdater(zname_, false, true);
// So far OK
EXPECT_NO_THROW(updater_->deleteRRset(*soa_));
// Still OK
EXPECT_NO_THROW(updater_->addRRset(*soa_));
// But this one is added again
EXPECT_THROW(updater_->addRRset(*soa_), isc::BadValue);
expected.push_back(JournalEntry(WRITABLE_ZONE_ID, 1234,
DatabaseAccessor::DIFF_ADD,
"example.org.", "SOA", "3600",
"ns1.example.org. admin.example.org. "
"1234 3600 1800 2419200 7200"));
current_accessor_->checkJournal(expected);
}
}
/*
* Test it rejects to store journals when we request it together with
* erasing the whole zone.
*/
TEST_F(MockDatabaseClientTest, journalOnErase) {
EXPECT_THROW(client_->getUpdater(zname_, true, true), isc::BadValue);
}
/*
* Check that the exception isn't raised when the journal is not implemented.
*/
TEST_F(MockDatabaseClientTest, journalNotImplemented) {
updater_ = client_->getUpdater(Name("null.example.org"), false, true);
EXPECT_NO_THROW(updater_->deleteRRset(*soa_));
soa_.reset(new RRset(zname_, qclass_, RRType::SOA(), rrttl_));
soa_->addRdata(rdata::createRdata(soa_->getType(), soa_->getClass(),
"ns1.example.org. admin.example.org. "
"1234 3600 1800 2419201 7200"));
EXPECT_NO_THROW(updater_->addRRset(*soa_));
}
/*
* Test that different exceptions are propagated.
*/
TEST_F(MockDatabaseClientTest, journalException) {
updater_ = client_->getUpdater(Name("bad.example.org"), false, true);
EXPECT_THROW(updater_->deleteRRset(*soa_), DataSourceError);
}
}
......@@ -438,6 +438,10 @@ public:
/// calls after \c commit() the implementation must throw a
/// \c DataSourceError exception.
///
/// If journaling was requested when getting this updater, it might reject
/// to add the RRset if the squence doesn't look like and IXFR. In such
/// case isc::BadValue is thrown.
///
/// \todo As noted above we may have to revisit the design details as we
/// gain experiences:
///