BIND 10 master, updated. 6260781b99d5b7c1f495e584abe9f74ea4e33e03 [master] Merge branch 'trac2541'

BIND 10 source code commits bind10-changes at lists.isc.org
Thu Dec 13 21:27:10 UTC 2012


The branch, master has been updated
       via  6260781b99d5b7c1f495e584abe9f74ea4e33e03 (commit)
       via  2c565f3b83fd09baffbeb82a7e8a4c56990f45dd (commit)
       via  57ad11888879be61a4d877b5ea4a29d504263e90 (commit)
       via  3666cb4d97d21903f1637796edab95bb466c213e (commit)
       via  42c5261913f5277e51be25a4e3de08223cf8949a (commit)
       via  fe8a9e7c2c315207cf643f27115a78e8b2b4eb7f (commit)
       via  f4c12ef1784ce3abe462de0328446ab5c7702339 (commit)
       via  d7441b27a527262614446fd678513b8d6a819214 (commit)
       via  bc8aae68ec152b7dce35c158758537bd9912688f (commit)
       via  8aa69ddf3d2d99fd9050a07c20eb01dd4287de95 (commit)
       via  eb65b11ca44fb2b2c128b317449272240ce337c3 (commit)
       via  5c72873a1145a7bcdc70d8b1f9ddce1198548c3c (commit)
       via  b193ad87c312d2e6ceb54f0f2746425ba4f59d63 (commit)
       via  b1c2cce67f50c0da6ff45744f98142353166482f (commit)
       via  ea1a4ef9ac12b377a8c6b407ac488a1cb3805ba0 (commit)
       via  3a5d06d3901871929d310ea0e968611e7bfa0f27 (commit)
       via  e5ae1675a59c6116c1d0c471b760e647bf6d31b5 (commit)
       via  0ece883214d68bb606764dd8c6572487af976833 (commit)
      from  b25d6802e2236e06c9eb83a9508cb9bfca048563 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 6260781b99d5b7c1f495e584abe9f74ea4e33e03
Merge: b25d680 2c565f3
Author: Jelte Jansen <jelte at isc.org>
Date:   Thu Dec 13 21:33:01 2012 +0100

    [master] Merge branch 'trac2541'

-----------------------------------------------------------------------

Summary of changes:
 src/lib/datasrc/Makefile.am                        |    2 +-
 src/lib/datasrc/{exceptions.h => client.cc}        |   55 +++---
 src/lib/datasrc/client.h                           |   44 +++--
 src/lib/datasrc/database.cc                        |   55 +++++-
 src/lib/datasrc/database.h                         |   28 +++
 src/lib/datasrc/datasrc_messages.mes               |   10 +
 src/lib/datasrc/sqlite3_accessor.cc                |   35 +++-
 src/lib/datasrc/sqlite3_accessor.h                 |  201 ++++++++++----------
 src/lib/datasrc/tests/client_unittest.cc           |    4 +
 src/lib/datasrc/tests/database_unittest.cc         |   61 +++++-
 src/lib/datasrc/tests/sqlite3_accessor_unittest.cc |   77 ++++++++
 11 files changed, 421 insertions(+), 151 deletions(-)
 copy src/lib/datasrc/{exceptions.h => client.cc} (50%)

-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/Makefile.am b/src/lib/datasrc/Makefile.am
index 340c4f4..a4edd4e 100644
--- a/src/lib/datasrc/Makefile.am
+++ b/src/lib/datasrc/Makefile.am
@@ -31,7 +31,7 @@ libb10_datasrc_la_SOURCES += zonetable.h zonetable.cc
 libb10_datasrc_la_SOURCES += zone.h zone_finder.cc zone_finder_context.cc
 libb10_datasrc_la_SOURCES += result.h
 libb10_datasrc_la_SOURCES += logger.h logger.cc
-libb10_datasrc_la_SOURCES += client.h iterator.h
+libb10_datasrc_la_SOURCES += client.h client.cc iterator.h
 libb10_datasrc_la_SOURCES += database.h database.cc
 libb10_datasrc_la_SOURCES += factory.h factory.cc
 libb10_datasrc_la_SOURCES += client_list.h client_list.cc
diff --git a/src/lib/datasrc/client.cc b/src/lib/datasrc/client.cc
new file mode 100644
index 0000000..919e9ab
--- /dev/null
+++ b/src/lib/datasrc/client.cc
@@ -0,0 +1,48 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <datasrc/client.h>
+
+#include <exceptions/exceptions.h>
+
+/// This file defines a few default implementations for methods.
+///
+/// While some of the detail of the API are worked out, we define
+/// default implementations to ease development for some of the
+/// more tentative methods (those that are not (yet) pure virtual)
+/// They should all throw NotImplemented
+
+namespace isc {
+namespace datasrc {
+
+ZoneIteratorPtr
+DataSourceClient::getIterator(const isc::dns::Name&, bool) const {
+    isc_throw(isc::NotImplemented,
+              "Data source doesn't support iteration");
+}
+
+unsigned int
+DataSourceClient::getZoneCount() const {
+    isc_throw(isc::NotImplemented,
+              "Data source doesn't support getZoneCount");
+}
+
+bool
+DataSourceClient::createZone(const dns::Name&) {
+    isc_throw(isc::NotImplemented,
+              "Data source doesn't support addZone");
+}
+
+} // end namespace datasrc
+} // end namespace isc
diff --git a/src/lib/datasrc/client.h b/src/lib/datasrc/client.h
index 3756a68..f65d87d 100644
--- a/src/lib/datasrc/client.h
+++ b/src/lib/datasrc/client.h
@@ -20,8 +20,6 @@
 #include <boost/noncopyable.hpp>
 #include <boost/shared_ptr.hpp>
 
-#include <exceptions/exceptions.h>
-
 #include <datasrc/zone.h>
 
 /// \file
@@ -225,15 +223,7 @@ public:
     ///                     adjusted to the lowest one found.
     /// \return Pointer to the iterator.
     virtual ZoneIteratorPtr getIterator(const isc::dns::Name& name,
-                                        bool separate_rrs = false) const {
-        // This is here to both document the parameter in doxygen (therefore it
-        // needs a name) and avoid unused parameter warning.
-        static_cast<void>(name);
-        static_cast<void>(separate_rrs);
-
-        isc_throw(isc::NotImplemented,
-                  "Data source doesn't support iteration");
-    }
+                                        bool separate_rrs = false) const;
 
     /// Return an updater to make updates to a specific zone.
     ///
@@ -368,16 +358,36 @@ public:
     /// This is an optional convenience method, currently only implemented
     /// by the InMemory datasource. By default, it throws NotImplemented
     ///
+    /// \note This is a tentative API, and this method is likely to change
+    /// or be removed in the near future. For that reason, it currently
+    /// provides a default implementation that throws NotImplemented.
+    ///
     /// \exception NotImplemented Thrown if this method is not supported
     ///            by the datasource
     ///
-    /// \note This is a tentative API, and this method may likely to be
-    ///       removed in the near future.
     /// \return The number of zones known to this datasource
-    virtual unsigned int getZoneCount() const {
-        isc_throw(isc::NotImplemented,
-                  "Data source doesn't support getZoneCount");
-    }
+    virtual unsigned int getZoneCount() const;
+
+    /// \brief Create a zone in the database
+    ///
+    /// Creates a new (empty) zone in the data source backend, which
+    /// can subsequently be filled with data (through getUpdater()).
+    ///
+    /// \note This is a tentative API, and this method is likely to change
+    /// or be removed in the near future. For that reason, it currently
+    /// provides a default implementation that throws NotImplemented.
+    ///
+    /// Apart from the two exceptions mentioned below, in theory this
+    /// call can throw anything, depending on the implementation of
+    /// the datasource backend.
+    ///
+    /// \throw NotImplemented If the datasource backend does not support
+    ///                       direct zone creation.
+    /// \throw DataSourceError If something goes wrong in the data source
+    ///                        while creating the zone.
+    /// \param name The (fully qualified) name of the zone to create
+    /// \return True if the zone was added, false if it already existed
+    virtual bool createZone(const dns::Name& name);
 };
 }
 }
diff --git a/src/lib/datasrc/database.cc b/src/lib/datasrc/database.cc
index fbada44..b65bc08 100644
--- a/src/lib/datasrc/database.cc
+++ b/src/lib/datasrc/database.cc
@@ -43,6 +43,42 @@ using boost::scoped_ptr;
 
 namespace isc {
 namespace datasrc {
+// RAII-style transaction holder; roll back the transaction unless explicitly
+// committed
+namespace {
+class TransactionHolder {
+public:
+    TransactionHolder(DatabaseAccessor& accessor) : accessor_(accessor),
+                                                    committed_(false)
+    {
+        accessor_.startTransaction();
+    }
+    ~TransactionHolder() {
+        if (!committed_) {
+            try {
+                accessor_.rollback();
+            } catch (const DataSourceError& e) {
+                // We generally expect that rollback always succeeds, and
+                // it should in fact succeed in a way we execute it.  But
+                // as the public API allows rollback() to fail and
+                // throw, we should expect it.  Obviously we cannot re-throw
+                // it.  The best we can do is to log it as a critical error.
+                logger.error(DATASRC_DATABASE_TRANSACTION_ROLLBACKFAIL).
+                            arg(accessor_.getDBName()).
+                            arg(e.what());
+            }
+        }
+    }
+    void commit() {
+        accessor_.commit();
+        committed_ = true;
+    }
+private:
+    DatabaseAccessor& accessor_;
+    bool committed_;
+};
+} // end unnamed namespace
+
 
 DatabaseClient::DatabaseClient(RRClass rrclass,
                                boost::shared_ptr<DatabaseAccessor>
@@ -80,6 +116,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),
@@ -1349,11 +1397,8 @@ public:
                 logger.info(DATASRC_DATABASE_UPDATER_ROLLBACK)
                     .arg(zone_name_).arg(zone_class_).arg(db_name_);
             } catch (const DataSourceError& e) {
-                // We generally expect that rollback always succeeds, and
-                // it should in fact succeed in a way we execute it.  But
-                // as the public API allows rollback() to fail and
-                // throw, we should expect it.  Obviously we cannot re-throw
-                // it.  The best we can do is to log it as a critical error.
+                // See The destructor ~TransactionHolder() for the
+                // reason to catch this.
                 logger.error(DATASRC_DATABASE_UPDATER_ROLLBACKFAIL)
                     .arg(zone_name_).arg(zone_class_).arg(db_name_)
                     .arg(e.what());
diff --git a/src/lib/datasrc/database.h b/src/lib/datasrc/database.h
index 320f327..9db8a8f 100644
--- a/src/lib/datasrc/database.h
+++ b/src/lib/datasrc/database.h
@@ -177,6 +177,24 @@ public:
     ///     an opaque handle.
     virtual std::pair<bool, int> getZone(const std::string& name) const = 0;
 
+    /// \brief Add a new zone to the database
+    ///
+    /// This method creates a new (and empty) zone in the database.
+    ///
+    /// Like for addRecordToZone, implementations are not required to
+    /// check for the existence of the given zone name, it is the
+    /// responsibility of the caller to do so.
+    ///
+    /// Callers must also start a transaction before calling this method,
+    /// implementations should throw DataSourceError if this has not been
+    /// done. Callers should also expect DataSourceErrors for other potential
+    /// problems.
+    ///
+    /// \param name The (fully qualified) domain name of the zone to add.
+    /// \return The internal zone id of the zone (whether is existed already
+    ///         or was created by this call).
+    virtual int addZone(const std::string& name) = 0;
+
     /// \brief This holds the internal context of ZoneIterator for databases
     ///
     /// While the ZoneIterator implementation from DatabaseClient does all the
@@ -1373,6 +1391,16 @@ public:
     ///     should use it as a ZoneFinder only.
     virtual FindResult findZone(const isc::dns::Name& name) const;
 
+    /// \brief Create a zone in the database
+    ///
+    /// This method implements \c DataSourceClient::createZone()
+    ///
+    /// It starts a transaction, checks if the zone exists, and if it
+    /// does not, creates it, commits, and returns true. If the zone
+    /// does exist already, it does nothing (except abort the transaction)
+    /// and returns false.
+    virtual bool createZone(const isc::dns::Name& name);
+
     /// \brief Get the zone iterator
     ///
     /// The iterator allows going through the whole zone content. If the
diff --git a/src/lib/datasrc/datasrc_messages.mes b/src/lib/datasrc/datasrc_messages.mes
index e7cb9d3..e9b4c90 100644
--- a/src/lib/datasrc/datasrc_messages.mes
+++ b/src/lib/datasrc/datasrc_messages.mes
@@ -216,6 +216,16 @@ to find any invalid data and fix it.
 No match (not even a wildcard) was found in the named data source for the given
 name/type/class in the data source.
 
+% DATASRC_DATABASE_TRANSACTION_ROLLBACKFAIL failed to roll back transaction on %1: %2
+A transaction on the database was rolled back without committing the
+changes to the database, but the rollback itself unexpectedly fails.
+The higher level implementation does not expect it to fail, so this means
+either a serious operational error in the underlying data source (such as a
+system failure of a database) or software bug in the underlying data source
+implementation.  In either case if this message is logged the administrator
+should carefully examine the underlying data source to see what exactly
+happens and whether the data is still valid.
+
 % DATASRC_DATABASE_UPDATER_COMMIT updates committed for '%1/%2' on %3
 Debug information.  A set of updates to a zone has been successfully
 committed to the corresponding database backend.  The zone name,
diff --git a/src/lib/datasrc/sqlite3_accessor.cc b/src/lib/datasrc/sqlite3_accessor.cc
index 68d6554..4ef49d0 100644
--- a/src/lib/datasrc/sqlite3_accessor.cc
+++ b/src/lib/datasrc/sqlite3_accessor.cc
@@ -78,7 +78,8 @@ enum StatementID {
     ADD_NSEC3_RECORD = 19,
     DEL_ZONE_NSEC3_RECORDS = 20,
     DEL_NSEC3_RECORD = 21,
-    NUM_STATEMENTS = 22
+    ADD_ZONE = 22,
+    NUM_STATEMENTS = 23
 };
 
 const char* const text_statements[NUM_STATEMENTS] = {
@@ -161,7 +162,10 @@ const char* const text_statements[NUM_STATEMENTS] = {
     "DELETE FROM nsec3 WHERE zone_id=?1",
     // DEL_NSEC3_RECORD: delete specified NSEC3-related records
     "DELETE FROM nsec3 WHERE zone_id=?1 AND hash=?2 "
-    "AND rdtype=?3 AND rdata=?4"
+    "AND rdtype=?3 AND rdata=?4",
+
+    // ADD_ZONE: add a zone to the zones table
+    "INSERT INTO zones (name, rdclass) VALUES (?1, ?2)" // ADD_ZONE
 };
 
 struct SQLite3Parameters {
@@ -612,6 +616,33 @@ SQLite3Accessor::getZone(const std::string& name) const {
     return (std::pair<bool, int>(false, 0));
 }
 
+int
+SQLite3Accessor::addZone(const std::string& name) {
+    // Transaction should have been started by the caller
+    if (!dbparameters_->in_transaction) {
+        isc_throw(DataSourceError, "performing addZone on SQLite3 "
+                  "data source without transaction");
+    }
+
+    StatementProcessor proc(*dbparameters_, ADD_ZONE, "add zone");
+    // note: using TRANSIENT here, STATIC would be slightly more
+    // efficient, but TRANSIENT is safer and performance is not
+    // as important in this specific code path.
+    proc.bindText(1, name.c_str(), SQLITE_TRANSIENT);
+    proc.bindText(2, class_.c_str(), SQLITE_TRANSIENT);
+    proc.exec();
+
+    // There are tricks to getting this in one go, but it is safer
+    // to do a new lookup (sqlite3_last_insert_rowid is unsafe
+    // regarding threads and triggers). This requires two
+    // statements, and is unpredictable in the case a zone is added
+    // twice, but this method assumes the caller does not do that
+    // anyway
+    std::pair<bool, int> getzone_result = getZone(name);
+    assert(getzone_result.first);
+    return (getzone_result.second);
+}
+
 namespace {
 
 // Conversion to plain char
diff --git a/src/lib/datasrc/sqlite3_accessor.h b/src/lib/datasrc/sqlite3_accessor.h
index a8112d4..c5773d0 100644
--- a/src/lib/datasrc/sqlite3_accessor.h
+++ b/src/lib/datasrc/sqlite3_accessor.h
@@ -34,13 +34,11 @@ class RRClass;
 
 namespace datasrc {
 
-/**
- * \brief Low-level database error
- *
- * This exception is thrown when the SQLite library complains about something.
- * It might mean corrupt database file, invalid request or that something is
- * rotten in the library.
- */
+/// \brief Low-level database error
+///
+/// This exception is thrown when the SQLite library complains about something.
+/// It might mean corrupt database file, invalid request or that something is
+/// rotten in the library.
 class SQLite3Error : public DataSourceError {
 public:
     SQLite3Error(const char* file, size_t line, const char* what) :
@@ -53,24 +51,20 @@ public:
         isc::Exception(file, line, what) {}
 };
 
-/**
- * \brief Too Much Data
- *
- * Thrown if a query expecting a certain number of rows back returned too
- * many rows.
- */
+/// \brief Too Much Data
+///
+/// Thrown if a query expecting a certain number of rows back returned too
+/// many rows.
 class TooMuchData : public DataSourceError {
 public:
     TooMuchData(const char* file, size_t line, const char* what) :
         DataSourceError(file, line, what) {}
 };
 
-/**
- * \brief Too Little Data
- *
- * Thrown if a query expecting a certain number of rows back returned too
- * few rows (including none).
- */
+/// \brief Too Little Data
+///
+/// Thrown if a query expecting a certain number of rows back returned too
+/// few rows (including none).
 class TooLittleData : public DataSourceError {
 public:
     TooLittleData(const char* file, size_t line, const char* what) :
@@ -79,70 +73,83 @@ public:
 
 struct SQLite3Parameters;
 
-/**
- * \brief Concrete implementation of DatabaseAccessor for SQLite3 databases
- *
- * This opens one database file with our schema and serves data from there.
- * According to the design, it doesn't interpret the data in any way, it just
- * provides unified access to the DB.
- */
+/// \brief Concrete implementation of DatabaseAccessor for SQLite3 databases
+///
+/// This opens one database file with our schema and serves data from there.
+/// According to the design, it doesn't interpret the data in any way, it just
+/// provides unified access to the DB.
 class SQLite3Accessor : public DatabaseAccessor,
     public boost::enable_shared_from_this<SQLite3Accessor> {
 public:
-    /**
-     * \brief Constructor
-     *
-     * This opens the database and becomes ready to serve data from there.
-     *
-     * \exception SQLite3Error will be thrown if the given database file
-     * doesn't work (it is broken, doesn't exist and can't be created, etc).
-     *
-     * \param filename The database file to be used.
-     * \param rrclass Textual representation of RR class ("IN", "CH", etc),
-     *     specifying which class of data it should serve (while the database
-     *     file can contain multiple classes of data, a single accessor can
-     *     work with only one class).
-     */
+    /// \brief Constructor
+    ///
+    /// This opens the database and becomes ready to serve data from there.
+    ///
+    /// \exception SQLite3Error will be thrown if the given database file
+    /// doesn't work (it is broken, doesn't exist and can't be created, etc).
+    ///
+    /// \param filename The database file to be used.
+    /// \param rrclass Textual representation of RR class ("IN", "CH", etc),
+    ///    specifying which class of data it should serve (while the database
+    ///    file can contain multiple classes of data, a single accessor can
+    ///    work with only one class).
     SQLite3Accessor(const std::string& filename, const std::string& rrclass);
 
-    /**
-     * \brief Destructor
-     *
-     * Closes the database.
-     */
-    ~SQLite3Accessor();
+    /// \brief Destructor
+    ///
+    /// Closes the database.
+    virtual ~SQLite3Accessor();
 
     /// This implementation internally opens a new sqlite3 database for the
     /// same file name specified in the constructor of the original accessor.
     virtual boost::shared_ptr<DatabaseAccessor> clone();
 
-    /**
-     * \brief Look up a zone
-     *
-     * This implements the getZone from DatabaseAccessor and looks up a zone
-     * in the data. It looks for a zone with the exact given origin and class
-     * passed to the constructor.
-     *
-     * \exception SQLite3Error if something about the database is broken.
-     *
-     * \param name The (fully qualified) domain name of zone to look up
-     * \return The pair contains if the lookup was successful in the first
-     *     element and the zone id in the second if it was.
-     */
+    /// \brief Look up a zone
+    ///
+    /// This implements the getZone from DatabaseAccessor and looks up a zone
+    /// in the data. It looks for a zone with the exact given origin and class
+    /// passed to the constructor.
+    ///
+    /// \exception SQLite3Error if something about the database is broken.
+    ///
+    /// \param name The (fully qualified) domain name of zone to look up
+    /// \return The pair contains if the lookup was successful in the first
+    ///    element and the zone id in the second if it was.
     virtual std::pair<bool, int> getZone(const std::string& name) const;
 
-    /** \brief Look up all resource records for a name
-     *
-     * This implements the getRecords() method from DatabaseAccessor
-     *
-     * \exception SQLite3Error if there is an sqlite3 error when performing
-     *                         the query
-     *
-     * \param name the name to look up
-     * \param id the zone id, as returned by getZone()
-     * \param subdomains Match subdomains instead of the name.
-     * \return Iterator that contains all records with the given name
-     */
+    /// \brief Add a zone
+    ///
+    /// This implements the addZone from DatabaseAccessor and adds a zone
+    /// into the zones table. If the zone exists already, it is still added,
+    /// so the caller should make sure this does not happen (by making
+    /// sure the zone does not exist). In the case of duplicate addition,
+    /// it is undefined which zone id is returned.
+    ///
+    /// The class of the newly created zone is the class passed at construction
+    /// time of the accessor.
+    ///
+    /// This method requires a transaction has been started (with
+    /// \c beginTransaction) by the caller.
+    ///
+    /// \exception DataSourceError if no transaction is active, or if there
+    ///                           is an SQLite3 error when performing the
+    ///                           queries.
+    ///
+    /// \param name The origin name of the zone to add
+    /// \return the id of the zone that has been added
+    virtual int addZone(const std::string& name);
+
+    /// \brief Look up all resource records for a name
+    ///
+    /// This implements the getRecords() method from DatabaseAccessor
+    ///
+    /// \exception SQLite3Error if there is an sqlite3 error when performing
+    ///                        the query
+    ///
+    /// \param name the name to look up
+    /// \param id the zone id, as returned by getZone()
+    /// \param subdomains Match subdomains instead of the name.
+    /// \return Iterator that contains all records with the given name
     virtual IteratorContextPtr getRecords(const std::string& name,
                                           int id,
                                           bool subdomains = false) const;
@@ -155,35 +162,33 @@ public:
     virtual IteratorContextPtr getNSEC3Records(const std::string& hash,
                                                int id) const;
 
-    /** \brief Look up all resource records for a zone
-     *
-     * This implements the getRecords() method from DatabaseAccessor
-     *
-     * \exception SQLite3Error if there is an sqlite3 error when performing
-     *                         the query
-     *
-     * \param id the zone id, as returned by getZone()
-     * \return Iterator that contains all records in the given zone
-     */
+    /// \brief Look up all resource records for a zone
+    ///
+    /// This implements the getRecords() method from DatabaseAccessor
+    ///
+    /// \exception SQLite3Error if there is an sqlite3 error when performing
+    ///                        the query
+    ///
+    /// \param id the zone id, as returned by getZone()
+    /// \return Iterator that contains all records in the given zone
     virtual IteratorContextPtr getAllRecords(int id) const;
 
-    /** \brief Creates an iterator context for a set of differences.
-     *
-     * Implements the getDiffs() method from DatabaseAccessor
-     *
-     * \exception NoSuchSerial if either of the versions do not exist in
-     *            the difference table.
-     * \exception SQLite3Error if there is an sqlite3 error when performing
-     *            the query
-     *
-     * \param id The ID of the zone, returned from getZone().
-     * \param start The SOA serial number of the version of the zone from
-     *        which the difference sequence should start.
-     * \param end The SOA serial number of the version of the zone at which
-     *        the difference sequence should end.
-     *
-     * \return Iterator containing difference records.
-     */
+    /// \brief Creates an iterator context for a set of differences.
+    ///
+    /// Implements the getDiffs() method from DatabaseAccessor
+    ///
+    /// \exception NoSuchSerial if either of the versions do not exist in
+    ///           the difference table.
+    /// \exception SQLite3Error if there is an sqlite3 error when performing
+    ///           the query
+    ///
+    /// \param id The ID of the zone, returned from getZone().
+    /// \param start The SOA serial number of the version of the zone from
+    ///       which the difference sequence should start.
+    /// \param end The SOA serial number of the version of the zone at which
+    ///       the difference sequence should end.
+    ///
+    /// \return Iterator containing difference records.
     virtual IteratorContextPtr
     getDiffs(int id, uint32_t start, uint32_t end) const;
 
diff --git a/src/lib/datasrc/tests/client_unittest.cc b/src/lib/datasrc/tests/client_unittest.cc
index 87ab5e0..915e3b7 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 dda4de2..174da04 100644
--- a/src/lib/datasrc/tests/database_unittest.cc
+++ b/src/lib/datasrc/tests/database_unittest.cc
@@ -274,6 +274,11 @@ public:
         }
     }
 
+    virtual int addZone(const std::string&) {
+        isc_throw(isc::NotImplemented,
+                  "This database datasource can't add zones");
+    }
+
     virtual boost::shared_ptr<DatabaseAccessor> clone() {
         // This accessor is stateless, so we can simply return a new instance.
         return (boost::shared_ptr<DatabaseAccessor>(new NopAccessor));
@@ -329,6 +334,7 @@ public:
         isc_throw(isc::NotImplemented,
                   "This test database knows nothing about NSEC3 nor order");
     }
+
 private:
     const std::string database_name_;
 
@@ -1780,7 +1786,7 @@ doFindAllTestResult(ZoneFinder& finder, const isc::dns::Name& name,
               expected_name, result->rrset->getName());
 }
 
-// When asking for an RRset where RRs somehow have different TTLs, it should 
+// When asking for an RRset where RRs somehow have different TTLs, it should
 // convert to the lowest one.
 TEST_F(MockDatabaseClientTest, ttldiff) {
     ZoneIteratorPtr it(this->client_->getIterator(Name("example.org")));
@@ -4092,4 +4098,57 @@ 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));
+    }
+}
+
+TYPED_TEST(DatabaseClientTest, createZoneRollbackOnLocked) {
+    // skip test for mock
+    if (this->is_mock_) {
+        return;
+    }
+
+    const Name new_name("example.com");
+    isc::datasrc::ZoneUpdaterPtr updater =
+        this->client_->getUpdater(this->zname_, true);
+    ASSERT_THROW(this->client_->createZone(new_name), DataSourceError);
+    // createZone started a transaction as well, but since it failed,
+    // it should have been rolled back. Roll back the other one as
+    // well, and the next attempt should succeed
+    updater.reset();
+    ASSERT_TRUE(this->client_->createZone(new_name));
+}
+
+TYPED_TEST(DatabaseClientTest, createZoneRollbackOnExists) {
+    // skip test for mock
+    if (this->is_mock_) {
+        return;
+    }
+
+    const Name new_name("example.com");
+    ASSERT_FALSE(this->client_->createZone(this->zname_));
+    // createZone started a transaction, but since it failed,
+    // it should have been rolled back, and the next attempt should succeed
+    ASSERT_TRUE(this->client_->createZone(new_name));
+}
+
 }
diff --git a/src/lib/datasrc/tests/sqlite3_accessor_unittest.cc b/src/lib/datasrc/tests/sqlite3_accessor_unittest.cc
index 100a0dd..8dff6d5 100644
--- a/src/lib/datasrc/tests/sqlite3_accessor_unittest.cc
+++ b/src/lib/datasrc/tests/sqlite3_accessor_unittest.cc
@@ -667,6 +667,64 @@ TEST_F(SQLite3Create, creationtest) {
     ASSERT_TRUE(isReadable(SQLITE_NEW_DBFILE));
 }
 
+// Test addZone works. This is done on the 'createtest' fixture so we
+// can easily be sure it does not exist yet.
+TEST_F(SQLite3Create, addZone) {
+    // Need shared_ptr for the getAllRecords at the end of the test
+    boost::shared_ptr<SQLite3Accessor> accessor(
+        new SQLite3Accessor(SQLITE_NEW_DBFILE, "IN"));
+
+    const std::string zone_name("example.com");
+    EXPECT_FALSE(accessor->getZone(zone_name).first);
+
+    // Calling addZone without transaction should fail
+    EXPECT_THROW(accessor->addZone(zone_name), DataSourceError);
+
+    // Add the zone. Returns the new zone id
+    accessor->startTransaction();
+    const int new_zone_id = accessor->addZone(zone_name);
+    accessor->commit();
+
+    // Check that it exists now, but has no records at this point
+    const std::pair<bool, int> zone_info(accessor->getZone(zone_name));
+    ASSERT_TRUE(zone_info.first);
+    EXPECT_EQ(new_zone_id, zone_info.second);
+
+    DatabaseAccessor::IteratorContextPtr context =
+        accessor->getAllRecords(zone_info.second);
+    string data[DatabaseAccessor::COLUMN_COUNT];
+    EXPECT_NE(DatabaseAccessor::IteratorContextPtr(), context);
+    EXPECT_FALSE(context->getNext(data));
+}
+
+// Test addZone does not get confused with different classes
+TEST_F(SQLite3Create, addZoneDifferentClass) {
+    const std::string zone_name("example.com.");
+
+    // Add IN zone
+    boost::shared_ptr<SQLite3Accessor> accessor(
+        new SQLite3Accessor(SQLITE_NEW_DBFILE, "IN"));
+    accessor->startTransaction();
+    const int new_zone_id_IN = accessor->addZone(zone_name);
+    accessor->commit();
+
+    // Add CH zone
+    accessor.reset(new SQLite3Accessor(SQLITE_NEW_DBFILE, "CH"));
+    accessor->startTransaction();
+    const int new_zone_id_CH = accessor->addZone(zone_name);
+    accessor->commit();
+
+    // id's should differ
+    ASSERT_NE(new_zone_id_IN, new_zone_id_CH);
+
+    // Reopen the database for both classes, and make sure
+    // we get the right zone id on searches
+    accessor.reset(new SQLite3Accessor(SQLITE_NEW_DBFILE, "IN"));
+    EXPECT_EQ(new_zone_id_IN, accessor->getZone(zone_name).second);
+    accessor.reset(new SQLite3Accessor(SQLITE_NEW_DBFILE, "CH"));
+    EXPECT_EQ(new_zone_id_CH, accessor->getZone(zone_name).second);
+}
+
 TEST_F(SQLite3Create, emptytest) {
     ASSERT_FALSE(isReadable(SQLITE_NEW_DBFILE));
 
@@ -1538,4 +1596,23 @@ TEST_F(SQLite3Update, addDiffWithUpdate) {
 
     checkDiffs(expected_stored, accessor->getDiffs(zone_id, 1234, 1300));
 }
+
+TEST_F(SQLite3Update, addZoneWhileLocked) {
+    const std::string existing_zone = "example.com.";
+    const std::string new_zone = "example2.com.";
+
+    // Start 'replacing' an existing zone, it should lock the db
+    zone_id = accessor->startUpdateZone(existing_zone, true).second;
+
+    // addZone should throw an exception that it is locked
+    another_accessor->startTransaction();
+    EXPECT_THROW(another_accessor->addZone(new_zone), DataSourceError);
+    // Commit should do nothing, but not fail
+    another_accessor->commit();
+
+    accessor->rollback();
+    // New zone should not exist
+    EXPECT_FALSE(accessor->getZone(new_zone).first);
+}
+
 } // end anonymous namespace



More information about the bind10-changes mailing list