Browse Source

[1068] overall documentation updates; also hid the database updater
implementation completely within .cc as even the class name doesn't have to
be visible.

JINMEI Tatuya 13 years ago
parent
commit
2ec9338d84
4 changed files with 272 additions and 82 deletions
  1. 55 36
      src/lib/datasrc/database.cc
  2. 37 31
      src/lib/datasrc/database.h
  3. 2 1
      src/lib/datasrc/sqlite3_accessor.h
  4. 178 14
      src/lib/datasrc/zone.h

+ 55 - 36
src/lib/datasrc/database.cc

@@ -608,49 +608,54 @@ DatabaseClient::getIterator(const isc::dns::Name& name) const {
     return (ZoneIteratorPtr(new DatabaseIterator(context, RRClass::IN())));
 }
 
-ZoneUpdaterPtr
-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));
-    if (!zone.first) {
-        return (ZoneUpdaterPtr());
+//
+// Zone updater using some database system as the underlying data source.
+//
+class DatabaseUpdater : public ZoneUpdater {
+public:
+    DatabaseUpdater(shared_ptr<DatabaseAccessor> accessor, int zone_id,
+            const Name& zone_name, const RRClass& zone_class) :
+        committed_(false), accessor_(accessor), zone_id_(zone_id),
+        db_name_(accessor->getDBName()), zone_name_(zone_name.toText()),
+        zone_class_(zone_class),
+        finder_(new DatabaseClient::Finder::Finder(accessor_, zone_id_,
+                                                   zone_name))
+    {
+        logger.debug(DBG_TRACE_DATA, DATASRC_DATABASE_UPDATER_CREATED)
+            .arg(zone_name_).arg(zone_class_).arg(db_name_);
     }
 
-     return (ZoneUpdaterPtr(new Updater(update_accessor, zone.second,
-                                        name, rrclass_)));
-}
-
-DatabaseClient::Updater::Updater(shared_ptr<DatabaseAccessor> accessor,
-                                 int zone_id, const Name& zone_name,
-                                 const RRClass& zone_class) :
-    committed_(false), accessor_(accessor), zone_id_(zone_id),
-    db_name_(accessor->getDBName()), zone_name_(zone_name.toText()),
-    zone_class_(zone_class),
-    finder_(new Finder(accessor_, zone_id_, zone_name))
-{
-    logger.debug(DBG_TRACE_DATA, DATASRC_DATABASE_UPDATER_CREATED)
-        .arg(zone_name_).arg(zone_class_).arg(db_name_);
-}
+    virtual ~DatabaseUpdater() {
+        if (!committed_) {
+            accessor_->rollbackUpdateZone();
+            logger.info(DATASRC_DATABASE_UPDATER_ROLLBACK)
+                .arg(zone_name_).arg(zone_class_).arg(db_name_);
+        }
 
-DatabaseClient::Updater::~Updater() {
-    if (!committed_) {
-        accessor_->rollbackUpdateZone();
-        logger.info(DATASRC_DATABASE_UPDATER_ROLLBACK)
+        logger.debug(DBG_TRACE_DATA, DATASRC_DATABASE_UPDATER_DESTROYED)
             .arg(zone_name_).arg(zone_class_).arg(db_name_);
     }
 
-    logger.debug(DBG_TRACE_DATA, DATASRC_DATABASE_UPDATER_DESTROYED)
-        .arg(zone_name_).arg(zone_class_).arg(db_name_);
-}
+    virtual ZoneFinder& getFinder() { return (*finder_); }
 
-ZoneFinder&
-DatabaseClient::Updater::getFinder() {
-    return (*finder_);
-}
+    virtual void addRRset(const RRset& rrset);
+    virtual void deleteRRset(const RRset& rrset);
+    virtual void commit();
+
+private:
+    bool committed_;
+    shared_ptr<DatabaseAccessor> accessor_;
+    const int zone_id_;
+    const string db_name_;
+    const string zone_name_;
+    const RRClass zone_class_;
+    boost::scoped_ptr<DatabaseClient::Finder::Finder> finder_;
+    string add_columns_[DatabaseAccessor::ADD_COLUMN_COUNT];
+    string del_params_[DatabaseAccessor::DEL_PARAM_COUNT];
+};
 
 void
-DatabaseClient::Updater::addRRset(const RRset& rrset) {
+DatabaseUpdater::addRRset(const RRset& rrset) {
     if (committed_) {
         isc_throw(DataSourceError, "Add attempt after commit to zone: "
                   << zone_name_ << "/" << zone_class_);
@@ -691,7 +696,7 @@ DatabaseClient::Updater::addRRset(const RRset& rrset) {
 }
 
 void
-DatabaseClient::Updater::deleteRRset(const RRset& rrset) {
+DatabaseUpdater::deleteRRset(const RRset& rrset) {
     if (committed_) {
         isc_throw(DataSourceError, "Delete attempt after commit on zone: "
                   << zone_name_ << "/" << zone_class_);
@@ -719,7 +724,7 @@ DatabaseClient::Updater::deleteRRset(const RRset& rrset) {
 }
 
 void
-DatabaseClient::Updater::commit() {
+DatabaseUpdater::commit() {
     if (committed_) {
         isc_throw(DataSourceError, "Duplicate commit attempt for "
                   << zone_name_ << "/" << zone_class_ << " on "
@@ -736,5 +741,19 @@ DatabaseClient::Updater::commit() {
     logger.debug(DBG_TRACE_DATA, DATASRC_DATABASE_UPDATER_COMMIT)
         .arg(zone_name_).arg(zone_class_).arg(db_name_);
 }
+
+// The updater factory
+ZoneUpdaterPtr
+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));
+    if (!zone.first) {
+        return (ZoneUpdaterPtr());
+    }
+
+     return (ZoneUpdaterPtr(new DatabaseUpdater(update_accessor, zone.second,
+                                                name, rrclass_)));
+}
 }
 }

+ 37 - 31
src/lib/datasrc/database.h

@@ -429,7 +429,33 @@ public:
     /// to the method or internal database error.
     virtual void rollbackUpdateZone() = 0;
 
-    /// TBD
+    /// Clone the accessor with the same configuration.
+    ///
+    /// Each derived class implementation of this method will create a new
+    /// accessor of the same derived class with the same configuration
+    /// (such as the database server address) as that of the caller object
+    /// and return it.
+    ///
+    /// Note that other internal states won't be copied to the new accessor
+    /// even though the name of "clone" may indicate so.  For example, even
+    /// if the calling accessor is in the middle of a update transaction,
+    /// the new accessor will not start a transaction to trace the same
+    /// updates.
+    ///
+    /// The intended use case of cloning is to create a separate context
+    /// where a specific set of database operations can be performed
+    /// independently from the original accessor.  The updater will use it
+    /// so that multiple updaters can be created concurrently even if the
+    /// underlying database system doesn't allow running multiple transactions
+    /// in a single database connection.
+    ///
+    /// The underlying database system may not support the functionality
+    /// that would be needed to implement this method.  For example, it
+    /// may not allow a single thread (or process) to have more than one
+    /// database connections.  In such a case the derived class implementation
+    /// should throw a \c DataSourceError exception.
+    ///
+    /// \return A shared pointer to the cloned accessor.
     virtual boost::shared_ptr<DatabaseAccessor> clone() = 0;
 
     /**
@@ -466,18 +492,18 @@ public:
     /**
      * \brief Constructor
      *
-     * It initializes the client with a database.
+     * It initializes the client with a database via the given accessor.
      *
-     * \exception isc::InvalidParameter if database is NULL. It might throw
+     * \exception isc::InvalidParameter if accessor is NULL. It might throw
      * standard allocation exception as well, but doesn't throw anything else.
      *
      * \param rrclass The RR class of the zones that this client will handle.
-     * \param database The database to use to get data. As the parameter
-     *     suggests, the client takes ownership of the database and will
-     *     delete it when itself deleted.
+     * \param accessor The accessor to the database to use to get data.
+     *  As the parameter suggests, the client takes ownership of the accessor
+     *  and will delete it when itself deleted.
      */
     DatabaseClient(isc::dns::RRClass rrclass,
-                   boost::shared_ptr<DatabaseAccessor> database);
+                   boost::shared_ptr<DatabaseAccessor> accessor);
 
     /**
      * \brief Corresponding ZoneFinder implementation
@@ -642,29 +668,6 @@ public:
         bool hasSubdomains(const std::string& name);
     };
 
-    class Updater : public ZoneUpdater {
-    public:
-        Updater(boost::shared_ptr<DatabaseAccessor> database, int zone_id,
-                const isc::dns::Name& zone_name,
-                const isc::dns::RRClass& zone_class);
-        ~Updater();
-        virtual ZoneFinder& getFinder();
-        virtual void addRRset(const isc::dns::RRset& rrset);
-        virtual void deleteRRset(const isc::dns::RRset& rrset);
-        virtual void commit();
-
-    private:
-        bool committed_;
-        boost::shared_ptr<DatabaseAccessor> accessor_;
-        const int zone_id_;
-        std::string db_name_;
-        const std::string zone_name_;
-        const isc::dns::RRClass zone_class_;
-        boost::scoped_ptr<Finder::Finder> finder_;
-        std::string add_columns_[DatabaseAccessor::ADD_COLUMN_COUNT];
-        std::string del_params_[DatabaseAccessor::DEL_PARAM_COUNT];
-    };
-
     /**
      * \brief Find a zone in the database
      *
@@ -700,7 +703,10 @@ public:
      */
     virtual ZoneIteratorPtr getIterator(const isc::dns::Name& name) const;
 
-    /// TBD
+    /// This implementation internally clones the accessor from the one
+    /// used in the client and starts a separate transaction using the cloned
+    /// 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;
 

+ 2 - 1
src/lib/datasrc/sqlite3_accessor.h

@@ -89,7 +89,8 @@ public:
      */
     ~SQLite3Accessor();
 
-    /// TBD
+    /// 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();
 
     /**

+ 178 - 14
src/lib/datasrc/zone.h

@@ -210,42 +210,206 @@ typedef boost::shared_ptr<ZoneFinder> ZoneFinderPtr;
 typedef boost::shared_ptr<const ZoneFinder> ConstZoneFinderPtr;
 
 /// The base class to make updates to a single zone.
+///
+/// On construction, each derived class object will start a "transaction"
+/// for making updates to a specific zone (this means a constructor of
+/// a derived class would normally take parameters to identify the zone
+/// to be updated).  The underlying realization of a "transaction" will defer
+/// for different derived classes; if it uses a general purpose database
+/// as a backend, it will involve performing some form of "begin transaction"
+/// statement for the database.
+///
+/// Updates (adding or deleting RRs) are made via \c addRRset() and
+/// \c deleteRRset() methods.  Until the \c commit() method is called the
+/// changes are local to the updater object.  For example, they won't be
+/// visible via a \c ZoneFinder object except the one returned by the
+/// updater's own \c getFinder() method.  The \c commit() completes the
+/// transaction and makes the changes visible to others.
+///
+/// This class does not provide an explicit "rollback" interface.  If
+/// something wrong or unexpected happens during the updates and the
+/// caller wants to cancel the intermediate updates, the caller should
+/// simply destruct the updater object without calling \c commit().
+/// The destructor is supposed to perform the "rollback" operation,
+/// depending on the internal details of the derived class.
+///
+/// \note This initial implementation provides a quite simple interface of
+/// adding and deleting RRs (see the description of the related methods).
+/// It may be revisited as we gain more experiences.
 class ZoneUpdater {
 protected:
+    /// The default constructor.
+    ///
+    /// This is intentionally defined as protected to ensure that this base
+    /// class is never instantiated directly.
     ZoneUpdater() {}
 
 public:
+    /// The destructor
+    ///
+    /// Each derived class implementation must ensure that if \c commit()
+    /// has not been performed by the time of the call to it, then it
+    /// "rollbacks" the updates made via the updater so far.
     virtual ~ZoneUpdater() {}
 
-    /// TBD
+    /// Return a finder for the zone being updated.
+    ///
+    /// The returned finder provides the functionalities of \c ZoneFinder
+    /// for the zone as updates are made via the updater.  That is, before
+    /// making any update, the finder will be able to find all RRsets that
+    /// exist in the zone at the time the updater is created.  If RRsets
+    /// are added or deleted via \c addRRset() or \c deleteRRset(),
+    /// this finder will find the added ones or miss the deleted ones
+    /// respectively.
+    ///
+    /// The finder returned by this method is effective only while the updates
+    /// are performed, i.e., from the construction of the corresponding
+    /// updater until \c commit() is performed or the updater is destructed
+    /// without commit.  The result of a subsequent call to this method (or
+    /// the use of the result) after that is undefined.
     ///
-    /// The finder is not expected to provide meaningful data once commit()
-    /// was performed.
+    /// \return A reference to a \c ZoneFinder for the updated zone
     virtual ZoneFinder& getFinder() = 0;
 
-    /// TBD
+    /// Add an RRset to a zone via the updater
+    ///
+    /// This may be revisited in a future version, but right now the intended
+    /// behavior of this method is simple: It "naively" adds the specified
+    /// RRset to the zone specified on creation of the updater.
+    /// It performs minimum level of validation on the specified RRset:
+    /// - Whether the RR class is identical to that for the zone to be updated
+    /// - Whether the RRset is not empty, i.e., it has at least one RDATA
+    ///
+    /// and otherwise does not check any oddity.  For example, it doesn't
+    /// check whether the owner name of the specified RRset is a subdomain
+    /// of the zone's origin; it doesn't care whether or not there is already
+    /// an RRset of the same name and RR type in the zone, and if there is,
+    /// whether any of the existing RRs have duplicate RDATA with the added
+    /// ones.  If these conditions matter the calling application must examine
+    /// the existing data beforehand using the \c ZoneFinder returned by
+    /// \c getFinder().
+    ///
+    /// Conceptually, on successful call to this method, the zone will have
+    /// the specified RRset, and if there is already an RRset of the same
+    /// name and RR type, these two sets will be "merged".  "Merged" means
+    /// that a subsequent call to \c ZoneFinder::find() for the name and type
+    /// will result in success and the returned RRset will contain all
+    /// previously existing and newly added RDATAs with the TTL being the
+    /// minimum of the two RRsets.  The underlying representation of the
+    /// "merged" RRsets may vary depending on the characteristic of the
+    /// underlying data source.  For example, if it uses a general purpose
+    /// database that stores each RR of the same RRset separately, it may
+    /// simply be a larger sets of RRs based on both the existing and added
+    /// RRsets; the TTLs of the RRs may be different within the database, and
+    /// there may even be duplicate RRs in different database rows.  As long
+    /// as the RRset returned via \c ZoneFinder::find() conforms to the
+    /// concept of "merge", the actual internal representation is up to the
+    /// implementation.
     ///
-    /// Notes about unexpected input: class mismatch will be rejected.
-    /// The owner name isn't checked; it's the caller's responsibility.
+    /// This method must not be called once commit() is performed.  If it
+    /// calls after \c commit() the implementation must throw a
+    /// \c DataSourceError exception.
     ///
-    /// Open issues: we may eventually want to return result values such as
-    /// there's a duplicate, etc.
+    /// \todo As noted above we may have to revisit the design details as we
+    /// gain experiences:
     ///
-    /// The added RRset must not be empty (i.e., it must have at least one
-    /// RDATA).
+    /// - we may want to check (and maybe reject) if there is already a
+    /// duplicate RR (that has the same RDATA).
+    /// - we may want to check (and maybe reject) if there is already an
+    /// RRset of the same name and RR type with different TTL
+    /// - we may even want to check if there is already any RRset of the
+    /// same name and RR type.
+    /// - we may want to add an "options" parameter that can control the
+    /// above points
+    /// - we may want to have this method return a value containing the
+    /// information on whether there's a duplicate, etc.
     ///
-    /// This method must not be called once commit() is performed.
+    /// \exception DataSourceError Called after \c commit(), RRset is invalid
+    /// (see above), internal data source error
+    /// \exception std::bad_alloc Resource allocation failure
+    ///
+    /// \param rrset The RRset to be added
     virtual void addRRset(const isc::dns::RRset& rrset) = 0;
 
-    /// TBD
+    /// Delete an RRset from a zone via the updater
+    ///
+    /// Like \c addRRset(), the detailed semantics and behavior of this method
+    /// may have to be revisited in a future version.  The following are
+    /// based on the initial implementation decisions.
+    ///
+    /// On successful completion of this method, it will remove from the zone
+    /// the RRs of the specified owner name and RR type that match one of
+    /// the RDATAs of the specified RRset.  There are several points to be
+    /// noted:
+    /// - Existing RRs that don't match any of the specified RDATAs will
+    ///   remain in the zone.
+    /// - Any RRs of the specified RRset that doesn't exist in the zone will
+    ///   simply be ignored; the implementation of this method is not supposed
+    ///   to check that condition.
+    /// - The TTL of the RRset is ignored; matching is only performed by
+    ///   the owner name, RR type and RDATA
+    ///
+    /// Ignoring the TTL may not look sensible, but it's based on the
+    /// observation that it will result in more intuitive result, especially
+    /// when the underlying data source is a general purpose database.
+    /// See also \c DatabaseAccessor::deleteRecordInZone() on this point.
+    /// It also matches the dynamic update protocol (RFC2136), where TTLs
+    /// are ignored when deleting RRs.
+    ///
+    /// \note Since the TTL is ignored, this method could take the RRset
+    /// to be deleted as a tuple of name, RR type, and a list of RDATAs.
+    /// But in practice, it's quite likely that the caller has the RRset
+    /// in the form of the \c RRset object (e.g., extracted from a dynamic
+    /// update request message), so this interface would rather be more
+    /// convenient.  If it turns out not to be true we can change or extend
+    /// the method signature.
+    ///
+    /// This method performs minimum level of validation on the specified
+    /// RRset:
+    /// - Whether the RR class is identical to that for the zone to be updated
+    /// - Whether the RRset is not empty, i.e., it has at least one RDATA
     ///
-    /// how to handle TTL?
+    /// This method must not be called once commit() is performed.  If it
+    /// calls after \c commit() the implementation must throw a
+    /// \c DataSourceError exception.
+    ///
+    /// \todo As noted above we may have to revisit the design details as we
+    /// gain experiences:
+    ///
+    /// - we may want to check (and maybe reject) if some or all of the RRs
+    ///   for the specified RRset don't exist in the zone
+    /// - we may want to allow an option to "delete everything" for specified
+    ///   name and/or specified name + RR type.
+    /// - as mentioned above, we may want to include the TTL in matching the
+    ///   deleted RRs
+    /// - we may want to add an "options" parameter that can control the
+    ///   above points
+    /// - we may want to have this method return a value containing the
+    ///   information on whether there's any RRs that are specified but don't
+    ///   exit, the number of actually deleted RRs, etc.
+    ///
+    /// \exception DataSourceError Called after \c commit(), RRset is invalid
+    /// (see above), internal data source error
+    /// \exception std::bad_alloc Resource allocation failure
+    ///
+    /// \param rrset The RRset to be deleted
     virtual void deleteRRset(const isc::dns::RRset& rrset) = 0;
 
-    /// TBD
+    /// Commit the updates made in the updater to the zone
+    ///
+    /// This method completes the "transaction" started at the creation
+    /// of the updater.  After successful completion of this method, the
+    /// updates will be visible outside the scope of the updater.
+    /// The actual internal behavior will defer for different derived classes.
+    /// For a derived class with a general purpose database as a backend,
+    /// for example, this method would perform a "commit" statement for the
+    /// database.
     ///
     /// This operation can only be performed at most once.  A duplicate call
     /// must result in a DatasourceError exception.
+    ///
+    /// \exception DataSourceError Duplicate call of the method,
+    /// internal data source error
     virtual void commit() = 0;
 };