[1068] documented DataSourceClient::getUpdater() (renamed from

startUpdateZone() to be more consistent with getIterator()).

doc/Doxyfile

@@ -568,8 +568,8 @@ WARN_LOGFILE           =
 # directories like "/usr/src/myproject". Separate the files or directories
 # with spaces.
 
-INPUT                  = ../src/lib/cc ../src/lib/config \
-    ../src/lib/cryptolink ../src/lib/dns ../src/lib/datasrc \
+INPUT                  = ../src/lib/exceptions ../src/lib/cc \
+    ../src/lib/config ../src/lib/cryptolink ../src/lib/dns ../src/lib/datasrc \
     ../src/bin/auth ../src/bin/resolver ../src/lib/bench ../src/lib/log \
     ../src/lib/log/compiler ../src/lib/asiolink/ ../src/lib/nsas \
     ../src/lib/testutils ../src/lib/cache ../src/lib/server_common/ \

src/lib/datasrc/client.h

@@ -80,8 +80,8 @@ typedef boost::shared_ptr<ZoneIterator> ZoneIteratorPtr;
 /// disruption with a naive copy it's prohibited explicitly.  For the expected
 /// usage of the client classes the restriction should be acceptable.
 ///
-/// \todo This class is not complete. It needs more factory methods, for
-///     accessing the whole zone, updating it, loading it, etc.
+/// \todo This class is still not complete. It will need more factory methods,
+/// e.g. for (re)loading a zone.
 class DataSourceClient : boost::noncopyable {
 public:
     /// \brief A helper structure to represent the search result of
@@ -181,19 +181,64 @@ public:
                   "Data source doesn't support iteration");
     }
 
-    /// TBD
-    ///
-    /// We allow having a read-only data source.  For such data source
-    /// this method will result in a NotImplemented exception.
-    ///
-    /// To avoid throwing the exception accidentally with a lazy
+    /// Return an updater to make updates to a specific zone.
+    ///
+    /// The RR class of the zone is the one that the client is expected to
+    /// handle (see the detailed description of this class).
+    ///
+    /// If the specified zone is not found via the client, a NULL pointer
+    /// will be returned; in other words a completely new zone cannot be
+    /// created using an updater.  It must be created beforehand (even if
+    /// it's an empty placeholder) in a way specific to the underlying data
+    /// source.
+    ///
+    /// Conceptually, the updater will trigger a separate transaction for
+    /// subsequent updates to the zone within the context of the updater
+    /// (the actual implementation of the "transaction" may vary for the
+    /// specific underlying data source).  Until \c commit() is performed
+    /// on the updater, the intermediate updates won't affect the results
+    /// of other methods (and the result of the object's methods created
+    /// by other factory methods).  Likewise, if the updater is destructed
+    /// without performing \c commit(), the intermediate updates will be
+    /// effectively canceled and will never affect other methods.
+    ///
+    /// If the underlying data source allows concurrent updates, this method
+    /// can be called multiple times while the previously returned updater(s)
+    /// are still active.  In this case each updater triggers a different
+    /// "transaction".  Normally it would be for different zones for such a
+    /// case as handling multiple incoming AXFR streams concurrently, but
+    /// this interface does not even prohibit an attempt of getting more than
+    /// one updater for the same zone, as long as the underlying data source
+    /// allows such an operation (and any conflict resolution is left to the
+    /// specific derived class implementation).
+    ///
+    /// If \c replace is true, any existing RRs of the zone will be
+    /// deleted on successful completion of updates (after \c commit() on
+    /// the updater); if it's false, the existing RRs will be
+    /// intact unless explicitly deleted by \c deleteRRset() on the updater.
+    ///
+    /// A data source can be "read only" or can prohibit partial updates.
+    /// In such cases this method will result in an \c isc::NotImplemented
+    /// exception unconditionally or when \c replace is false).
+    ///
+    /// \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 write the
-    /// definition of this method, even if it simply throws the NotImplemented
-    /// exception.
-    virtual ZoneUpdaterPtr startUpdateZone(const isc::dns::Name& name,
-                                           bool replace)
-        const = 0;
+    /// an implementation.  All derived classes must explicitly define this
+    /// method, even if it simply throws the NotImplemented exception.
+    ///
+    /// \exception NotImplemented The underlying data source does not support
+    /// updates.
+    /// \exception DataSourceError Internal error in the underlying data
+    /// source.
+    /// \exception std::bad_alloc Resource allocation failure.
+    ///
+    /// \param name The zone name to be updated
+    /// \param replace Whether to delete existing RRs before making updates
+    ///
+    /// \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;
 };
 }
 }

src/lib/datasrc/database.cc

@@ -609,9 +609,7 @@ DatabaseClient::getIterator(const isc::dns::Name& name) const {
 }
 
 ZoneUpdaterPtr
-DatabaseClient::startUpdateZone(const isc::dns::Name& name,
-                                bool replace) const
-{
+DatabaseClient::getUpdater(const isc::dns::Name& name, bool replace) const {
     shared_ptr<DatabaseAccessor> update_accessor(accessor_->clone());
     const std::pair<bool, int> zone(update_accessor->startUpdateZone(
                                         name.toText(), replace));

src/lib/datasrc/database.h

@@ -701,8 +701,8 @@ public:
     virtual ZoneIteratorPtr getIterator(const isc::dns::Name& name) const;
 
     /// TBD
-    virtual ZoneUpdaterPtr startUpdateZone(const isc::dns::Name& name,
-                                           bool replace) const;
+    virtual ZoneUpdaterPtr getUpdater(const isc::dns::Name& name,
+                                      bool replace) const;
 
 private:
     /// \brief The RR class that this client handles.

src/lib/datasrc/memory_datasrc.cc

@@ -796,7 +796,7 @@ InMemoryClient::getIterator(const Name& name) const {
 }
 
 ZoneUpdaterPtr
-InMemoryClient::startUpdateZone(const isc::dns::Name&, bool) const {
+InMemoryClient::getUpdater(const isc::dns::Name&, bool) const {
     isc_throw(isc::NotImplemented, "Update attempt on in memory data source");
 }
 } // end of namespace datasrc

src/lib/datasrc/memory_datasrc.h

@@ -268,8 +268,8 @@ public:
 
     /// In-memory data source is read-only, so this derived method will
     /// result in a NotImplemented (once merged) exception.
-    virtual ZoneUpdaterPtr startUpdateZone(const isc::dns::Name& name,
-                                           bool replace) const;
+    virtual ZoneUpdaterPtr getUpdater(const isc::dns::Name& name,
+                                      bool replace) const;
 
 private:
     // TODO: Do we still need the PImpl if nobody should manipulate this class

src/lib/datasrc/tests/client_unittest.cc

@@ -32,7 +32,7 @@ public:
     virtual FindResult findZone(const isc::dns::Name&) const {
         return (FindResult(result::NOTFOUND, ZoneFinderPtr()));
     }
-    virtual ZoneUpdaterPtr startUpdateZone(const isc::dns::Name&, bool) const {
+    virtual ZoneUpdaterPtr getUpdater(const isc::dns::Name&, bool) const {
         return (ZoneUpdaterPtr());
     }
 };

src/lib/datasrc/tests/database_unittest.cc

@@ -1503,7 +1503,7 @@ TEST_F(DatabaseClientTest, getOrigin) {
 }
 
 TEST_F(DatabaseClientTest, updaterFinder) {
-    updater_ = client_->startUpdateZone(zname_, false);
+    updater_ = client_->getUpdater(zname_, false);
     ASSERT_TRUE(updater_);
 
     // If this update isn't replacing the zone, the finder should work
@@ -1519,7 +1519,7 @@ TEST_F(DatabaseClientTest, updaterFinder) {
     // When replacing the zone, the updater's finder shouldn't see anything
     // in the zone until something is added.
     updater_.reset();
-    updater_ = client_->startUpdateZone(zname_, true);
+    updater_ = client_->getUpdater(zname_, true);
     ASSERT_TRUE(updater_);
     EXPECT_EQ(WRITABLE_ZONE_ID, dynamic_cast<DatabaseClient::Finder&>(
                   updater_->getFinder()).zone_id());
@@ -1536,7 +1536,7 @@ TEST_F(DatabaseClientTest, flushZone) {
 
     // start update in the replace mode.  the normal finder should still
     // be able to see the record, but the updater's finder shouldn't.
-    updater_ = client_->startUpdateZone(zname_, true);
+    updater_ = client_->getUpdater(zname_, true);
     setUpdateAccessor();
     EXPECT_EQ(ZoneFinder::SUCCESS,
               finder->find(qname_, qtype_).code);
@@ -1557,7 +1557,7 @@ TEST_F(DatabaseClientTest, updateCancel) {
     ZoneFinderPtr finder = client_->findZone(zname_).zone_finder;
     EXPECT_EQ(ZoneFinder::SUCCESS, finder->find(qname_, qtype_).code);
 
-    updater_ = client_->startUpdateZone(zname_, true);
+    updater_ = client_->getUpdater(zname_, true);
     setUpdateAccessor();
     EXPECT_EQ(ZoneFinder::NXDOMAIN,
               updater_->getFinder().find(qname_, qtype_).code);
@@ -1574,14 +1574,14 @@ TEST_F(DatabaseClientTest, updateCancel) {
 
 TEST_F(DatabaseClientTest, duplicateCommit) {
     // duplicate commit.  should result in exception.
-    updater_ = client_->startUpdateZone(zname_, true);
+    updater_ = client_->getUpdater(zname_, true);
     updater_->commit();
     EXPECT_THROW(updater_->commit(), DataSourceError);
 }
 
 TEST_F(DatabaseClientTest, addRRsetToNewZone) {
     // Add a single RRset to a fresh empty zone
-    updater_ = client_->startUpdateZone(zname_, true);
+    updater_ = client_->getUpdater(zname_, true);
     updater_->addRRset(*rrset_);
 
     expected_rdatas_.clear();
@@ -1595,7 +1595,7 @@ TEST_F(DatabaseClientTest, addRRsetToNewZone) {
 
     // Similar to the previous case, but with RRSIG
     updater_.reset();
-    updater_ = client_->startUpdateZone(zname_, true);
+    updater_ = client_->getUpdater(zname_, true);
     updater_->addRRset(*rrset_);
     updater_->addRRset(*rrsigset_);
 
@@ -1620,7 +1620,7 @@ TEST_F(DatabaseClientTest, addRRsetToCurrentZone) {
     // Similar to the previous test, but not replacing the existing data.
     shared_ptr<DatabaseClient::Finder> finder(getFinder());
 
-    updater_ = client_->startUpdateZone(zname_, false);
+    updater_ = client_->getUpdater(zname_, false);
     updater_->addRRset(*rrset_);
 
     // We should see both old and new data.
@@ -1645,7 +1645,7 @@ TEST_F(DatabaseClientTest, addRRsetToCurrentZone) {
 TEST_F(DatabaseClientTest, addMultipleRRs) {
     // Similar to the previous case, but the added RRset contains multiple
     // RRs.
-    updater_ = client_->startUpdateZone(zname_, false);
+    updater_ = client_->getUpdater(zname_, false);
     rrset_->addRdata(rdata::createRdata(rrset_->getType(), rrset_->getClass(),
                                         "192.0.2.3"));
     updater_->addRRset(*rrset_);
@@ -1665,7 +1665,7 @@ 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(zname_, false);
+    updater_ = client_->getUpdater(zname_, false);
     rrset_->setTTL(RRTTL(7200));
     updater_->addRRset(*rrset_);
 
@@ -1683,7 +1683,7 @@ TEST_F(DatabaseClientTest, addRRsetOfLargerTTL) {
 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(zname_, false);
+    updater_ = client_->getUpdater(zname_, false);
     rrset_->setTTL(RRTTL(1800));
     updater_->addRRset(*rrset_);
 
@@ -1703,7 +1703,7 @@ TEST_F(DatabaseClientTest, addSameRR) {
     // 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(zname_, false);
+    updater_ = client_->getUpdater(zname_, false);
     rrset_.reset(new RRset(qname_, qclass_, qtype_, rrttl_));
     rrset_->addRdata(rdata::createRdata(rrset_->getType(), rrset_->getClass(),
                                         "192.0.2.1"));
@@ -1720,7 +1720,7 @@ TEST_F(DatabaseClientTest, addSameRR) {
 }
 
 TEST_F(DatabaseClientTest, addDeviantRR) {
-    updater_ = client_->startUpdateZone(zname_, false);
+    updater_ = client_->getUpdater(zname_, false);
 
     // RR class mismatch.  This should be detected and rejected.
     rrset_.reset(new RRset(qname_, RRClass::CH(), RRType::TXT(), rrttl_));
@@ -1748,13 +1748,13 @@ TEST_F(DatabaseClientTest, addDeviantRR) {
 }
 
 TEST_F(DatabaseClientTest, addEmptyRRset) {
-    updater_ = client_->startUpdateZone(zname_, false);
+    updater_ = client_->getUpdater(zname_, false);
     rrset_.reset(new RRset(qname_, qclass_, qtype_, rrttl_));
     EXPECT_THROW(updater_->addRRset(*rrset_), DataSourceError);
 }
 
 TEST_F(DatabaseClientTest, addAfterCommit) {
-   updater_ = client_->startUpdateZone(zname_, false);
+   updater_ = client_->getUpdater(zname_, false);
    updater_->commit();
    EXPECT_THROW(updater_->addRRset(*rrset_), DataSourceError);
 }
@@ -1767,7 +1767,7 @@ TEST_F(DatabaseClientTest, deleteRRset) {
                                         "192.0.2.1"));
 
     // Delete one RR from an RRset
-    updater_ = client_->startUpdateZone(zname_, false);
+    updater_ = client_->getUpdater(zname_, false);
     updater_->deleteRRset(*rrset_);
 
     // Delete the only RR of a name
@@ -1826,7 +1826,7 @@ TEST_F(DatabaseClientTest, deleteRRsetToNXDOMAIN) {
     rrset_->addRdata(rdata::createRdata(rrset_->getType(), rrset_->getClass(),
                                         "www.example.org"));
 
-    updater_ = client_->startUpdateZone(zname_, false);
+    updater_ = client_->getUpdater(zname_, false);
     updater_->deleteRRset(*rrset_);
     {
         SCOPED_TRACE("delete RRset to NXDOMAIN");
@@ -1843,7 +1843,7 @@ TEST_F(DatabaseClientTest, deleteMultipleRRs) {
     rrset_->addRdata(rdata::createRdata(rrset_->getType(), rrset_->getClass(),
                                         "2001:db8::2"));
 
-    updater_ = client_->startUpdateZone(zname_, false);
+    updater_ = client_->getUpdater(zname_, false);
     updater_->deleteRRset(*rrset_);
 
     {
@@ -1864,7 +1864,7 @@ TEST_F(DatabaseClientTest, partialDelete) {
 
     // deleteRRset should succeed "silently", and subsequent find() should
     // find the remaining RR.
-    updater_ = client_->startUpdateZone(zname_, false);
+    updater_ = client_->getUpdater(zname_, false);
     updater_->deleteRRset(*rrset_);
     {
         SCOPED_TRACE("partial delete");
@@ -1878,7 +1878,7 @@ TEST_F(DatabaseClientTest, partialDelete) {
 TEST_F(DatabaseClientTest, deleteNoMatch) {
     // similar to the previous test, but there's not even a match in the
     // specified RRset.  Essentially there's no difference in the result.
-    updater_ = client_->startUpdateZone(zname_, false);
+    updater_ = client_->getUpdater(zname_, false);
     updater_->deleteRRset(*rrset_);
     {
         SCOPED_TRACE("delete no match");
@@ -1894,7 +1894,7 @@ TEST_F(DatabaseClientTest, deleteWithDifferentTTL) {
     rrset_.reset(new RRset(qname_, qclass_, qtype_, RRTTL(1800)));
     rrset_->addRdata(rdata::createRdata(rrset_->getType(), rrset_->getClass(),
                                         "192.0.2.1"));
-    updater_ = client_->startUpdateZone(zname_, false);
+    updater_ = client_->getUpdater(zname_, false);
     updater_->deleteRRset(*rrset_);
     {
         SCOPED_TRACE("delete RRset with a different TTL");
@@ -1905,7 +1905,7 @@ TEST_F(DatabaseClientTest, deleteWithDifferentTTL) {
 }
 
 TEST_F(DatabaseClientTest, deleteDeviantRR) {
-    updater_ = client_->startUpdateZone(zname_, false);
+    updater_ = client_->getUpdater(zname_, false);
 
     // RR class mismatch.  This should be detected and rejected.
     rrset_.reset(new RRset(qname_, RRClass::CH(), RRType::TXT(), rrttl_));
@@ -1922,13 +1922,13 @@ TEST_F(DatabaseClientTest, deleteDeviantRR) {
 }
 
 TEST_F(DatabaseClientTest, deleteAfterCommit) {
-   updater_ = client_->startUpdateZone(zname_, false);
+   updater_ = client_->getUpdater(zname_, false);
    updater_->commit();
    EXPECT_THROW(updater_->deleteRRset(*rrset_), DataSourceError);
 }
 
 TEST_F(DatabaseClientTest, deleteEmptyRRset) {
-    updater_ = client_->startUpdateZone(zname_, false);
+    updater_ = client_->getUpdater(zname_, false);
     rrset_.reset(new RRset(qname_, qclass_, qtype_, rrttl_));
     EXPECT_THROW(updater_->deleteRRset(*rrset_), DataSourceError);
 }

src/lib/datasrc/tests/memory_datasrc_unittest.cc

@@ -198,7 +198,7 @@ TEST_F(InMemoryClientTest, getZoneCount) {
 }
 
 TEST_F(InMemoryClientTest, startUpdateZone) {
-    EXPECT_THROW(memory_client.startUpdateZone(Name("example.org"), false),
+    EXPECT_THROW(memory_client.getUpdater(Name("example.org"), false),
                  isc::NotImplemented);
 }