[1068] implemented adding RRsets

src/lib/datasrc/database.cc

@@ -21,6 +21,7 @@
 #include <dns/name.h>
 #include <dns/rrclass.h>
 #include <dns/rrttl.h>
+#include <dns/rrset.h>
 #include <dns/rdata.h>
 #include <dns/rdataclass.h>
 
@@ -31,8 +32,8 @@
 
 using namespace std;
 using boost::shared_ptr;
-using isc::dns::Name;
-using isc::dns::RRClass;
+using namespace isc::dns;
+using namespace isc::dns::rdata;
 
 namespace isc {
 namespace datasrc {
@@ -327,30 +328,31 @@ DatabaseClient::startUpdateZone(const isc::dns::Name& name,
     }
 
      return (ZoneUpdaterPtr(new Updater(accessor_, zone.second,
-                                        name.toText(), rrclass_.toText())));
+                                        name.toText(), rrclass_)));
 }
 
 DatabaseClient::Updater::Updater(shared_ptr<DatabaseAccessor> accessor,
                                  int zone_id, const string& zone_name,
-                                 const string& class_name) :
+                                 const RRClass& zone_class) :
     committed_(false), accessor_(accessor), zone_id_(zone_id),
     db_name_(accessor->getDBName()), zone_name_(zone_name),
-    class_name_(class_name),
-    finder_(new Finder(accessor_, zone_id_))
+    zone_class_(zone_class),
+    finder_(new Finder(accessor_, zone_id_)),
+    add_columns_(DatabaseAccessor::ADD_COLUMN_COUNT)
 {
     logger.debug(DBG_TRACE_DATA, DATASRC_DATABASE_UPDATER_CREATED)
-        .arg(zone_name_).arg(class_name_).arg(db_name_);
+        .arg(zone_name_).arg(zone_class_).arg(db_name_);
 }
 
 DatabaseClient::Updater::~Updater() {
     if (!committed_) {
         accessor_->rollbackUpdateZone();
         logger.info(DATASRC_DATABASE_UPDATER_ROLLBACK)
-            .arg(zone_name_).arg(class_name_).arg(db_name_);
+            .arg(zone_name_).arg(zone_class_).arg(db_name_);
     }
 
     logger.debug(DBG_TRACE_DATA, DATASRC_DATABASE_UPDATER_DESTROYED)
-        .arg(zone_name_).arg(class_name_).arg(db_name_);
+        .arg(zone_name_).arg(zone_class_).arg(db_name_);
 }
 
 ZoneFinder&
@@ -359,10 +361,52 @@ DatabaseClient::Updater::getFinder() {
 }
 
 void
+DatabaseClient::Updater::addRRset(const RRset& rrset) {
+    if (committed_) {
+        isc_throw(DataSourceError, "Add attempt after commit to zone: "
+                  << zone_name_ << "/" << zone_class_);
+    }
+    if (rrset.getClass() != zone_class_) {
+        isc_throw(DataSourceError, "An RRset of a different class is being "
+                  << "added to " << zone_name_ << "/" << zone_class_ << ": "
+                  << rrset.toText());
+    }
+
+    RdataIteratorPtr it = rrset.getRdataIterator();
+    if (it->isLast()) {
+        isc_throw(DataSourceError, "An empty RRset is being added for "
+                  << rrset.getName() << "/" << zone_class_ << "/"
+                  << rrset.getType());
+    }
+
+    add_columns_.clear();
+    add_columns_[DatabaseAccessor::ADD_NAME] = rrset.getName().toText();
+    add_columns_[DatabaseAccessor::ADD_REV_NAME] =
+        rrset.getName().reverse().toText();
+    add_columns_[DatabaseAccessor::ADD_TTL] = rrset.getTTL().toText();
+    add_columns_[DatabaseAccessor::ADD_TYPE] = rrset.getType().toText();
+    for (; !it->isLast(); it->next()) {
+        if (rrset.getType() == RRType::RRSIG()) {
+            // XXX: the current interface (based on the current sqlite3
+            // data source schema) requires a separate "sigtype" column,
+            // even though it won't be used in a newer implementation.
+            // We should eventually clean up the schema design and simplify
+            // the interface, but until then we have to conform to the schema.
+            const generic::RRSIG& rrsig_rdata =
+                dynamic_cast<const generic::RRSIG&>(it->getCurrent());
+            add_columns_[DatabaseAccessor::ADD_SIGTYPE] =
+                rrsig_rdata.typeCovered().toText();
+        }
+        add_columns_[DatabaseAccessor::ADD_RDATA] = it->getCurrent().toText();
+        accessor_->addRecordToZone(add_columns_);
+    }
+}
+
+void
 DatabaseClient::Updater::commit() {
     if (committed_) {
         isc_throw(DataSourceError, "Duplicate commit attempt for "
-                  << zone_name_ << "/" << class_name_ << " on "
+                  << zone_name_ << "/" << zone_class_ << " on "
                   << db_name_);
     }
     accessor_->commitUpdateZone();
@@ -374,7 +418,7 @@ DatabaseClient::Updater::commit() {
     committed_ = true;
 
     logger.debug(DBG_TRACE_DATA, DATASRC_DATABASE_UPDATER_COMMIT)
-        .arg(zone_name_).arg(class_name_).arg(db_name_);
+        .arg(zone_name_).arg(zone_class_).arg(db_name_);
 }
 }
 }

src/lib/datasrc/database.h

@@ -20,6 +20,8 @@
 #include <boost/scoped_ptr.hpp>
 
 #include <dns/rrclass.h>
+#include <dns/rrclass.h>
+#include <dns/rrset.h>
 
 #include <datasrc/client.h>
 
@@ -153,6 +155,16 @@ public:
         RDATA_COLUMN = 3    ///< Full text representation of the record's RDATA
     };
 
+    enum AddRecordColumns {
+        ADD_NAME = 0, ///< The owner name of the record (a domain name)
+        ADD_REV_NAME = 1, ///< Reversed name of NAME (used for DNSSEC)
+        ADD_TTL = 2,     ///< The TTL of the record (an integer)
+        ADD_TYPE = 3,    ///< The RRType of the record (A/NS/TXT etc.)
+        ADD_SIGTYPE = 4, ///< For RRSIG records, this contains the RRTYPE
+                            ///< the RRSIG covers.
+        ADD_RDATA = 5    ///< Full text representation of the record's RDATA
+    };
+
     /// TBD
     virtual std::pair<bool, int> startUpdateZone(const std::string& zone_name,
                                                  bool replace) = 0;
@@ -345,9 +357,11 @@ public:
     class Updater : public ZoneUpdater {
     public:
         Updater(boost::shared_ptr<DatabaseAccessor> database, int zone_id,
-                const std::string& zone_name, const std::string& class_name);
+                const std::string& zone_name,
+                const isc::dns::RRClass& zone_class);
         ~Updater();
         virtual ZoneFinder& getFinder();
+        virtual void addRRset(const isc::dns::RRset& rrset);
         virtual void commit();
 
     private:
@@ -356,8 +370,9 @@ public:
         const int zone_id_;
         std::string db_name_;
         std::string zone_name_;
-        std::string class_name_;
+        isc::dns::RRClass zone_class_;
         boost::scoped_ptr<Finder::Finder> finder_;
+        std::vector<std::string> add_columns_;
     };
 
     /**

src/lib/datasrc/tests/database_unittest.cc

@@ -12,9 +12,12 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
+#include <boost/foreach.hpp>
+
 #include <gtest/gtest.h>
 
 #include <dns/name.h>
+#include <dns/rdata.h>
 #include <dns/rrttl.h>
 #include <dns/rrset.h>
 #include <exceptions/exceptions.h>
@@ -30,10 +33,7 @@
 using namespace isc::datasrc;
 using namespace std;
 using namespace boost;
-using isc::dns::Name;
-using isc::dns::RRType;
-using isc::dns::RRClass;
-using isc::dns::RRTTL;
+using namespace isc::dns;
 
 namespace {
 
@@ -119,6 +119,47 @@ public:
         search_running_ = false;
     }
 
+    virtual pair<bool, int> startUpdateZone(const std::string& zone_name,
+                                            bool replace)
+    {
+        const pair<bool, int> zone_info = getZone(zone_name);
+        if (!zone_info.first) {
+            return (pair<bool, int>(false, 0));
+        }
+
+        // Prepare the record set for update.  If replacing the existing one,
+        // we use an empty set; otherwise we use a writable copy of the
+        // original.
+        if (replace) {
+            update_records.clear();
+        } else {
+            update_records = readonly_records;
+        }
+
+        return (pair<bool, int>(true, WRITABLE_ZONE_ID));
+    }
+    virtual void commitUpdateZone() {
+        readonly_records = update_records;
+    }
+    virtual void rollbackUpdateZone() {
+        rollbacked_ = true;
+    }
+    virtual void addRecordToZone(const vector<string>& columns) {
+        // Copy the current value to cur_name.  If it doesn't exist,
+        // operator[] will create a new one.
+        cur_name = update_records[columns[DatabaseAccessor::ADD_NAME]];
+        addRecord(columns[DatabaseAccessor::ADD_TYPE],
+                  columns[DatabaseAccessor::ADD_TTL],
+                  columns[DatabaseAccessor::ADD_SIGTYPE],
+                  columns[DatabaseAccessor::ADD_RDATA]);
+        // copy back the added entry.
+        update_records[columns[DatabaseAccessor::ADD_NAME]] = cur_name;
+
+        // remember this one so that test cases can check it.
+        columns_lastadded = columns;
+    }
+    virtual void deleteRecordInZone(const vector<string>&) {}
+
     bool searchRunning() const {
         return (search_running_);
     }
@@ -130,6 +171,10 @@ public:
     virtual const std::string& getDBName() const {
         return (database_name_);
     }
+
+    const vector<string>& getLastAdded() const {
+        return (columns_lastadded);
+    }
 private:
     RECORDS readonly_records;
     RECORDS update_records;
@@ -142,6 +187,9 @@ private:
     // fake data
     std::vector< std::vector<std::string> > cur_name;
 
+    // The columns that were most recently added via addRecordToZone()
+    vector<string> columns_lastadded;
+
     // This boolean is used to make sure find() calls resetSearch
     // when it encounters an error
     bool search_running_;
@@ -168,13 +216,13 @@ private:
     // Adds one record to the current name in the database
     // The actual data will not be added to 'records' until
     // addCurName() is called
-    void addRecord(const std::string& name,
-                   const std::string& type,
+    void addRecord(const std::string& type,
+                   const std::string& ttl,
                    const std::string& sigtype,
                    const std::string& rdata) {
         std::vector<std::string> columns;
-        columns.push_back(name);
         columns.push_back(type);
+        columns.push_back(ttl);
         columns.push_back(sigtype);
         columns.push_back(rdata);
         cur_name.push_back(columns);
@@ -292,40 +340,33 @@ private:
         addRecord("RRSIG", "3600", "TXT", "A 5 3 3600 20000101000000 20000201000000 12345 example.org. FAKEFAKEFAKE");
         addCurName("badsigtype.example.org.");
     }
-
-    virtual pair<bool, int> startUpdateZone(const std::string& zone_name,
-                                            bool replace)
-    {
-        const pair<bool, int> zone_info = getZone(zone_name);
-        if (!zone_info.first) {
-            return (pair<bool, int>(false, 0));
-        }
-
-        // Prepare the record set for update.  If replacing the existing one,
-        // we use an empty set; otherwise we use a writable copy of the
-        // original.
-        if (replace) {
-            update_records.clear();
-        } else {
-            update_records = readonly_records;
-        }
-
-        return (pair<bool, int>(true, WRITABLE_ZONE_ID));
-    }
-    virtual void commitUpdateZone() {
-        readonly_records = update_records;
-    }
-    virtual void rollbackUpdateZone() {
-        rollbacked_ = true;
-    }
-    virtual void addRecordToZone(const vector<string>&) {}
-    virtual void deleteRecordInZone(const vector<string>&) {}
 };
 
 class DatabaseClientTest : public ::testing::Test {
 protected:
-    DatabaseClientTest() : qname("www.example.org"), qtype("A") {
+    DatabaseClientTest() : qname("www.example.org"), qtype("A"), rrttl(3600) {
         createClient();
+
+        // set up the commonly used finder.
+        DataSourceClient::FindResult zone(
+            client_->findZone(Name("example.org")));
+        assert(zone.code == result::SUCCESS);
+        finder = dynamic_pointer_cast<DatabaseClient::Finder>(
+            zone.zone_finder);
+
+        rrset.reset(new RRset(qname, RRClass::IN(), qtype, rrttl));
+        // Adding an IN/A RDATA.  Intentionally using different data
+        // than the initial data configured MockAccessor::fillData().
+        rrset->addRdata(rdata::createRdata(rrset->getType(), rrset->getClass(),
+                                           "192.0.2.2"));
+
+        rrsigset.reset(new RRset(qname, RRClass::IN(), RRType::RRSIG(),
+                                 rrttl));
+        rrsigset->addRdata(rdata::createRdata(rrsigset->getType(),
+                                              rrsigset->getClass(),
+                                              "A 5 3 0 20000101000000 "
+                                              "20000201000000 0 example.org. "
+                                              "FAKEFAKEFAKE"));
     }
     /*
      * We initialize the client from a function, so we can call it multiple
@@ -342,8 +383,14 @@ protected:
     shared_ptr<DatabaseClient> client_;
     const std::string database_name_;
 
+    // The zone finder of the test zone commonly used in various tests.
+    shared_ptr<DatabaseClient::Finder> finder;
+
     const Name qname;           // commonly used name to be found
     const RRType qtype;         // commonly used RR type with qname
+    const RRTTL rrttl;          // commonly used RR TTL
+    RRsetPtr rrset;             // for adding/deleting an RRset
+    RRsetPtr rrsigset;          // for adding/deleting an RRset
 
     ZoneUpdaterPtr updater;
     const std::vector<std::string> empty_rdatas; // for NXRRSET/NXDOMAIN
@@ -450,10 +497,6 @@ doFindTest(ZoneFinder& finder,
 } // end anonymous namespace
 
 TEST_F(DatabaseClientTest, find) {
-    DataSourceClient::FindResult zone(client_->findZone(Name("example.org")));
-    ASSERT_EQ(result::SUCCESS, zone.code);
-    shared_ptr<DatabaseClient::Finder> finder(
-        dynamic_pointer_cast<DatabaseClient::Finder>(zone.zone_finder));
     EXPECT_EQ(READONLY_ZONE_ID, finder->zone_id());
     EXPECT_FALSE(current_accessor_->searchRunning());
 
@@ -766,7 +809,7 @@ TEST_F(DatabaseClientTest, updaterFinder) {
     expected_rdatas.clear();
     expected_rdatas.push_back("192.0.2.1");
     doFindTest(updater->getFinder(), Name("www.example.org."),
-               RRType::A(), RRType::A(), RRTTL(3600), ZoneFinder::SUCCESS,
+               qtype, qtype, rrttl, ZoneFinder::SUCCESS,
                expected_rdatas, empty_rdatas);
 
     // When replacing the zone, the updater's finder shouldn't see anything
@@ -829,6 +872,160 @@ TEST_F(DatabaseClientTest, duplicateCommit) {
     EXPECT_THROW(updater->commit(), DataSourceError);
 }
 
-// add/delete after commit.  should error
+TEST_F(DatabaseClientTest, addRRsetToNewZone) {
+    // Add a single RRset to a fresh empty zone
+    updater = client_->startUpdateZone(Name("example.org"), true);
+    updater->addRRset(*rrset);
+
+    expected_rdatas.clear();
+    expected_rdatas.push_back("192.0.2.2");
+    {
+        SCOPED_TRACE("add RRset");
+        doFindTest(updater->getFinder(), qname, qtype, qtype, rrttl,
+                   ZoneFinder::SUCCESS, expected_rdatas, empty_rdatas);
+    }
+
+    // Similar to the previous case, but with RRSIG
+    updater = client_->startUpdateZone(Name("example.org"), true);
+    updater->addRRset(*rrset);
+    updater->addRRset(*rrsigset);
+
+    // confirm the expected columns were passed to the mock accessor.
+    const char* const rrsig_added[] = {
+        "www.example.org.", "org.example.www.", "3600", "RRSIG", "A",
+        "A 5 3 0 20000101000000 20000201000000 0 example.org. FAKEFAKEFAKE"
+    };
+    int i = 0;
+    BOOST_FOREACH(const string& column, current_accessor_->getLastAdded()) {
+        EXPECT_EQ(rrsig_added[i++], column);
+    }
+
+    expected_sig_rdatas.clear();
+    expected_sig_rdatas.push_back(rrsig_added[DatabaseAccessor::ADD_RDATA]);
+    {
+        SCOPED_TRACE("add RRset with RRSIG");
+        doFindTest(updater->getFinder(), qname, qtype, qtype, rrttl,
+                   ZoneFinder::SUCCESS, expected_rdatas, expected_sig_rdatas);
+    }
+}
+
+TEST_F(DatabaseClientTest, addRRsetToCurrentZone) {
+    // Similar to the previous test, but not replacing the existing data.
+    updater = client_->startUpdateZone(Name("example.org"), false);
+    updater->addRRset(*rrset);
+
+    // We should see both old and new data.
+    expected_rdatas.clear();
+    expected_rdatas.push_back("192.0.2.1");
+    expected_rdatas.push_back("192.0.2.2");
+    {
+        SCOPED_TRACE("add RRset");
+        doFindTest(updater->getFinder(), qname, qtype, qtype, rrttl,
+                   ZoneFinder::SUCCESS, expected_rdatas, empty_rdatas);
+    }
+    updater->commit();
+    {
+        SCOPED_TRACE("add RRset after commit");
+        doFindTest(*finder, qname, qtype, qtype, rrttl,
+                   ZoneFinder::SUCCESS, expected_rdatas, empty_rdatas);
+    }
+}
+
+TEST_F(DatabaseClientTest, addRRsetOfLargerTTL) {
+    // Similar to the previous one, but the TTL of the added RRset is larger
+    // than that of the existing record.  The finder should use the smaller
+    // one.
+    updater = client_->startUpdateZone(Name("example.org"), false);
+    rrset->setTTL(RRTTL(7200));
+    updater->addRRset(*rrset);
+
+    expected_rdatas.clear();
+    expected_rdatas.push_back("192.0.2.1");
+    expected_rdatas.push_back("192.0.2.2");
+    {
+        SCOPED_TRACE("add RRset of larger TTL");
+        doFindTest(updater->getFinder(), qname, qtype, qtype, rrttl,
+                   ZoneFinder::SUCCESS, expected_rdatas, empty_rdatas);
+    }
+}
+
+TEST_F(DatabaseClientTest, addRRsetOfSmallerTTL) {
+    // Similar to the previous one, but the added RRset has a smaller TTL.
+    // The added TTL should be used by the finder.
+    updater = client_->startUpdateZone(Name("example.org"), false);
+    rrset->setTTL(RRTTL(1800));
+    updater->addRRset(*rrset);
+
+    expected_rdatas.clear();
+    expected_rdatas.push_back("192.0.2.1");
+    expected_rdatas.push_back("192.0.2.2");
+    {
+        SCOPED_TRACE("add RRset of smaller TTL");
+        doFindTest(updater->getFinder(), qname, qtype, qtype, RRTTL(1800),
+                   ZoneFinder::SUCCESS, expected_rdatas, empty_rdatas);
+    }
+}
+
+TEST_F(DatabaseClientTest, addSameRR) {
+    // Add the same RR as that is already in the data source.
+    // Currently the add interface doesn't try to suppress the duplicate,
+    // neither does the finder.  We may want to revisit it in future versions.
+
+    updater = client_->startUpdateZone(Name("example.org"), false);
+    rrset.reset(new RRset(qname, RRClass::IN(), qtype, rrttl));
+    rrset->addRdata(rdata::createRdata(rrset->getType(), rrset->getClass(),
+                                       "192.0.2.1"));
+    updater->addRRset(*rrset);
+    expected_rdatas.clear();
+    expected_rdatas.push_back("192.0.2.1");
+    expected_rdatas.push_back("192.0.2.1");
+    {
+        SCOPED_TRACE("add same RR");
+        doFindTest(updater->getFinder(), qname, qtype, qtype, rrttl,
+                   ZoneFinder::SUCCESS, expected_rdatas, empty_rdatas);
+    }
+}
+
+TEST_F(DatabaseClientTest, addDeviantRR) {
+    updater = client_->startUpdateZone(Name("example.org"), false);
+
+    // RR class mismatch.  This should be detected and rejected.
+    rrset.reset(new RRset(qname, RRClass::CH(), RRType::TXT(), rrttl));
+    rrset->addRdata(rdata::createRdata(rrset->getType(), rrset->getClass(),
+                                       "test text"));
+    EXPECT_THROW(updater->addRRset(*rrset), DataSourceError);
+
+    // Out-of-zone owner name.  At a higher level this should be rejected,
+    // but it doesn't happen in this interface.
+    rrset.reset(new RRset(Name("example.com"), RRClass::IN(), qtype, rrttl));
+    rrset->addRdata(rdata::createRdata(rrset->getType(), rrset->getClass(),
+                                       "192.0.2.100"));
+    updater->addRRset(*rrset);
+
+    expected_rdatas.clear();
+    expected_rdatas.push_back("192.0.2.100");
+    {
+        // Note: with the find() implementation being more strict about
+        // zone cuts, this test may fail.  Then the test should be updated.
+        SCOPED_TRACE("add out-of-zone RR");
+        doFindTest(updater->getFinder(), Name("example.com"), qtype, qtype,
+                   rrttl, ZoneFinder::SUCCESS, expected_rdatas, empty_rdatas);
+    }
+}
+
+TEST_F(DatabaseClientTest, addEmptyRRset) {
+    updater = client_->startUpdateZone(Name("example.org"), false);
+    rrset.reset(new RRset(qname, RRClass::IN(), qtype, rrttl));
+    EXPECT_THROW(updater->addRRset(*rrset), DataSourceError);
+}
+
+TEST_F(DatabaseClientTest, addAfterCommit) {
+   updater = client_->startUpdateZone(Name("example.org"), false);
+   updater->commit();
+   EXPECT_THROW(updater->addRRset(*rrset), DataSourceError);
+}
+
+// delete rrset without rdata; not necessarily harmful, but treat it as an error.
+// delete after commit.  should error
 
 }

src/lib/datasrc/zone.h

@@ -15,9 +15,11 @@
 #ifndef __ZONE_H
 #define __ZONE_H 1
 
-#include <datasrc/result.h>
+#include <dns/rrset.h>
 #include <dns/rrsetlist.h>
 
+#include <datasrc/result.h>
+
 namespace isc {
 namespace datasrc {
 
@@ -216,10 +218,27 @@ public:
     virtual ~ZoneUpdater() {}
 
     /// TBD
+    ///
+    /// The finder is not expected to provide meaningful data once commit()
+    /// was performed.
     virtual ZoneFinder& getFinder() = 0;
 
     /// TBD
     ///
+    /// Notes about unexpected input: class mismatch will be rejected.
+    /// The owner name isn't checked; it's the caller's responsibility.
+    ///
+    /// Open issues: we may eventually want to return result values such as
+    /// there's a duplicate, etc.
+    ///
+    /// The added RRset must not be empty (i.e., it must have at least one
+    /// RDATA).
+    ///
+    /// This method must not be called once commit() is performed.
+    virtual void addRRset(const isc::dns::RRset& rrset) = 0;
+
+    /// TBD
+    ///
     /// This operation can only be performed at most once.  A duplicate call
     /// must result in a DatasourceError exception.
     virtual void commit() = 0;