From: Jelte Jansen <jelte@isc.org>
Date: Mon, 10 Dec 2012 19:15:21 +0000 (+0100)
Subject: [2541] add createZone() to DataSourceClient interface
X-Git-Tag: bind10-1.0.0-beta-release~26^2~14
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3a5d06d3901871929d310ea0e968611e7bfa0f27;p=thirdparty%2Fkea.git

[2541] add createZone() to DataSourceClient interface

Note: currently called createZone so as not to interfere with an existing addZone() call in MemoryClient.
Instead of pure virtual I made it have a default 'NotImplemented' method.
Also, the DatabaseClient holds the transaction (not the SQLite3Accessor); we could do it on that level but we already have a transaction API on a higher level and it makes a little bit more sense to me to do it there
---

diff --git a/src/lib/datasrc/client.h b/src/lib/datasrc/client.h
index 3756a6858a..e1cde588a9 100644
--- a/src/lib/datasrc/client.h
+++ b/src/lib/datasrc/client.h
@@ -378,6 +378,20 @@ public:
         isc_throw(isc::NotImplemented,
                   "Data source doesn't support getZoneCount");
     }
+
+    // It first checks if the specified name of the zone exists.  If it
+    // exists it returns false; otherwise it adds information of the
+    // new zone in backend-dependent manner and returns true.
+    // The DB-based version of this method would perform the check and add in
+    // a single transaction.
+    //
+    // Throws on any unexpected failure.
+    // Default implementation throws isc::NotImplemented
+
+    virtual bool createZone(const dns::Name&) {
+        isc_throw(isc::NotImplemented,
+                  "Data source doesn't support addZone");
+    };
 };
 }
 }
diff --git a/src/lib/datasrc/database.cc b/src/lib/datasrc/database.cc
index fbada44a33..de432130df 100644
--- a/src/lib/datasrc/database.cc
+++ b/src/lib/datasrc/database.cc
@@ -43,6 +43,30 @@ using boost::scoped_ptr;
 
 namespace isc {
 namespace datasrc {
+// RAII-style transaction holder; roll back the transaction unless explicitely committed
+namespace {
+class TransactionHolder {
+public:
+    TransactionHolder(DatabaseAccessor& accessor) : accessor_(accessor),
+                                                    committed_(false)
+    {
+        accessor_.startTransaction();
+    }
+    ~TransactionHolder() {
+        if (!committed_) {
+            accessor_.rollback();
+        }
+    }
+    void commit() {
+        accessor_.commit();
+        committed_ = true;
+    }
+private:
+    DatabaseAccessor& accessor_;
+    bool committed_;
+};
+} // end unnamed namespace
+
 
 DatabaseClient::DatabaseClient(RRClass rrclass,
                                boost::shared_ptr<DatabaseAccessor>
@@ -80,6 +104,18 @@ DatabaseClient::findZone(const Name& name) const {
     return (FindResult(result::NOTFOUND, ZoneFinderPtr()));
 }
 
+bool
+DatabaseClient::createZone(const Name& name) {
+    TransactionHolder transaction(*accessor_);
+    std::pair<bool, int> zone(accessor_->getZone(name.toText()));
+    if (zone.first) {
+        return (false);
+    }
+    accessor_->addZone(name.toText());
+    transaction.commit();
+    return (true);
+}
+
 DatabaseClient::Finder::Finder(boost::shared_ptr<DatabaseAccessor> accessor,
                                int zone_id, const isc::dns::Name& origin) :
     accessor_(accessor),
diff --git a/src/lib/datasrc/database.h b/src/lib/datasrc/database.h
index a3b49f56bd..6546bdeb47 100644
--- a/src/lib/datasrc/database.h
+++ b/src/lib/datasrc/database.h
@@ -1393,6 +1393,8 @@ public:
     ///     should use it as a ZoneFinder only.
     virtual FindResult findZone(const isc::dns::Name& name) const;
 
+    virtual bool createZone(const isc::dns::Name&);
+
     /// \brief Get the zone iterator
     ///
     /// The iterator allows going through the whole zone content. If the
diff --git a/src/lib/datasrc/tests/client_unittest.cc b/src/lib/datasrc/tests/client_unittest.cc
index 87ab5e05db..915e3b7ce1 100644
--- a/src/lib/datasrc/tests/client_unittest.cc
+++ b/src/lib/datasrc/tests/client_unittest.cc
@@ -60,4 +60,8 @@ TEST_F(ClientTest, defaultGetZoneCount) {
     EXPECT_THROW(client_.getZoneCount(), isc::NotImplemented);
 }
 
+TEST_F(ClientTest, defaultCreateZone) {
+    EXPECT_THROW(client_.createZone(Name("example.com.")), isc::NotImplemented);
+}
+
 }
diff --git a/src/lib/datasrc/tests/database_unittest.cc b/src/lib/datasrc/tests/database_unittest.cc
index 05d825e2dd..8494096368 100644
--- a/src/lib/datasrc/tests/database_unittest.cc
+++ b/src/lib/datasrc/tests/database_unittest.cc
@@ -334,6 +334,7 @@ public:
         isc_throw(isc::NotImplemented,
                   "This test database knows nothing about NSEC3 nor order");
     }
+
 private:
     const std::string database_name_;
 
@@ -4097,4 +4098,26 @@ TYPED_TEST(DatabaseClientTest, findNSEC3) {
     performNSEC3Test(*finder, true);
 }
 
+TYPED_TEST(DatabaseClientTest, createZone) {
+    const Name new_name("example.com");
+    const DataSourceClient::FindResult
+        zone(this->client_->findZone(new_name));
+    ASSERT_EQ(result::NOTFOUND, zone.code);
+    // The mock implementation does not do createZone,
+    // in which case it should throw NotImplemented (from
+    // the base class)
+    if(this->is_mock_) {
+        ASSERT_THROW(this->client_->createZone(new_name), isc::NotImplemented);
+    } else {
+        // But in the real case, it should work and return true
+        ASSERT_TRUE(this->client_->createZone(new_name));
+        const DataSourceClient::FindResult
+            zone2(this->client_->findZone(new_name));
+        ASSERT_EQ(result::SUCCESS, zone2.code);
+        // And the second call should return false since
+        // it already exists
+        ASSERT_FALSE(this->client_->createZone(new_name));
+    }
+}
+
 }