[svn] commit: r3637 - in /branches/trac374/src/lib/datasrc: ./ tests/ tests/testdata/

BIND 10 source code commits bind10-changes at lists.isc.org
Thu Nov 25 10:03:59 UTC 2010


Author: jelte
Date: Thu Nov 25 10:03:59 2010
New Revision: 3637

Log:
address (most of the) review comments

Modified:
    branches/trac374/src/lib/datasrc/data_source.cc
    branches/trac374/src/lib/datasrc/data_source.h
    branches/trac374/src/lib/datasrc/sqlite3_datasrc.cc
    branches/trac374/src/lib/datasrc/sqlite3_datasrc.h
    branches/trac374/src/lib/datasrc/tests/sqlite3_unittest.cc
    branches/trac374/src/lib/datasrc/tests/testdata/test.sqlite3
    branches/trac374/src/lib/datasrc/tests/testdata/update4.packet
    branches/trac374/src/lib/datasrc/tests/testdata/update5.packet
    branches/trac374/src/lib/datasrc/tests/testdata/update6.packet

Modified: branches/trac374/src/lib/datasrc/data_source.cc
==============================================================================
--- branches/trac374/src/lib/datasrc/data_source.cc (original)
+++ branches/trac374/src/lib/datasrc/data_source.cc Thu Nov 25 10:03:59 2010
@@ -13,6 +13,11 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 // $Id$
+
+// TODO: to get to a RRset& reference from an RRsetList, we need to
+// dereff twice (or use get() on the shared_ptr). This looks like a
+// bad design of the RRsetList interace. If we change that, we should
+// update the cases here. (search for '*(*' )
 
 #include <config.h>
 
@@ -1275,14 +1280,14 @@
 
 DataSrc::WriteResult
 DataSrc::addRRset(DataSrcTransaction& transaction UNUSED_PARAM,
-                  const isc::dns::ConstRRsetPtr rrset UNUSED_PARAM)
+                  const isc::dns::RRset& rrset UNUSED_PARAM)
 {
     return (W_NOT_IMPLEMENTED);
 }
 
 DataSrc::WriteResult
 DataSrc::delRRset(DataSrcTransaction& transaction UNUSED_PARAM,
-                  isc::dns::ConstRRsetPtr rrset UNUSED_PARAM)
+                  const isc::dns::RRset& rrset UNUSED_PARAM)
 {
     return (W_NOT_IMPLEMENTED);
 }
@@ -1294,22 +1299,28 @@
 
 DataSrc::WriteResult
 DataSrc::replaceZone(DataSrcTransaction& transaction UNUSED_PARAM,
-                     isc::dns::RRsetPtr (*nextRRset)(void*, void*) UNUSED_PARAM,
+                     isc::dns::RRset* (*nextRRset)(void*, void*) UNUSED_PARAM,
                      void* arg1 UNUSED_PARAM, void* arg2 UNUSED_PARAM)
 {
     return (W_NOT_IMPLEMENTED);
 }
 
-static bool
-equalRRsets(ConstRRsetPtr a, ConstRRsetPtr b) {
-    if (a->getName() != b->getName() ||
-        a->getClass() != b->getClass() ||
-        a->getType() != b->getType() ||
-        a->getRdataCount() != b->getRdataCount()) {
+namespace {
+
+//
+// Returns true if the RRsets are equal.
+// TTL is ignored
+//
+bool
+equalRRsets(const RRset& a, const RRset& b) {
+    if (a.getName() != b.getName() ||
+        a.getClass() != b.getClass() ||
+        a.getType() != b.getType() ||
+        a.getRdataCount() != b.getRdataCount()) {
         return (false);
     }
-    RdataIteratorPtr ita = a->getRdataIterator();
-    RdataIteratorPtr itb = b->getRdataIterator();
+    RdataIteratorPtr ita = a.getRdataIterator();
+    RdataIteratorPtr itb = b.getRdataIterator();
     itb->first();
     for (ita->first(); !ita->isLast(); ita->next()) {
         if (ita->getCurrent().compare(itb->getCurrent()) != 0) {
@@ -1320,26 +1331,28 @@
     return (true);
 }
 
+} // local namespace
+
 bool
-DataSrc::haveRRset(DataSrcTransaction& transaction UNUSED_PARAM,
-                   ConstRRsetPtr rrset)
+DataSrc::updateHaveRRset(DataSrcTransaction& transaction UNUSED_PARAM,
+                         const RRset& rrset)
 {
     RRsetList rrset_list;
     Result result;
     uint32_t flags = 0;
 
-    if (rrset->getClass() == RRClass::ANY() || rrset->getClass() == RRClass::NONE()) {
-        result = findExactRRset(rrset->getName(), RRClass::IN(),
-                                         rrset->getType(), rrset_list, flags, NULL);
+    if (rrset.getClass() == RRClass::ANY() || rrset.getClass() == RRClass::NONE()) {
+        result = findExactRRset(rrset.getName(), RRClass::IN(),
+                                rrset.getType(), rrset_list, flags, NULL);
     } else {
-        result = findExactRRset(rrset->getName(), rrset->getClass(),
-                                         rrset->getType(), rrset_list, flags, NULL);
+        result = findExactRRset(rrset.getName(), rrset.getClass(),
+                                         rrset.getType(), rrset_list, flags, NULL);
     }
     if (result != SUCCESS || rrset_list.size() == 0) {
         return (false);
     }
-    if (rrset->getRdataCount() > 0) {
-        return (equalRRsets(rrset, *(rrset_list.begin())));
+    if (rrset.getRdataCount() > 0) {
+        return (equalRRsets(rrset, *(*(rrset_list.begin()))));
     } else {
         return ((*(rrset_list.begin()))->getRdataCount() > 0);
     }
@@ -1347,39 +1360,39 @@
 
 isc::dns::Rcode
 DataSrc::updateCheckPrerequisite(DataSrcTransaction& transaction,
-                                 ConstRRsetPtr prereq)
+                                 const RRset& prereq)
 {
     // section 3.2 of RFC2136
-    if (prereq->getClass() == RRClass::ANY()) {
-        if (prereq->getTTL().getValue() != 0 ||
-            prereq->getRdataCount() != 0) {
+    if (prereq.getClass() == RRClass::ANY()) {
+        if (prereq.getTTL().getValue() != 0 ||
+            prereq.getRdataCount() != 0) {
             return (Rcode::FORMERR());
         }
-        if (prereq->getType() == RRType::ANY()) {
-            if (!haveRRset(transaction, prereq)) {
+        if (prereq.getType() == RRType::ANY()) {
+            if (!updateHaveRRset(transaction, prereq)) {
                 return (Rcode::NXDOMAIN());
             }
-        } else if (!haveRRset(transaction, prereq)) {
+        } else if (!updateHaveRRset(transaction, prereq)) {
             return (Rcode::NXRRSET());
         }
-    } else if (prereq->getClass() == RRClass::NONE()) {
-        if (prereq->getTTL().getValue() != 0 ||
-            prereq->getRdataCount() != 0) {
+    } else if (prereq.getClass() == RRClass::NONE()) {
+        if (prereq.getTTL().getValue() != 0 ||
+            prereq.getRdataCount() != 0) {
             return (Rcode::FORMERR());
         }
-        if (prereq->getType() == RRType::ANY()) {
-            if (haveRRset(transaction, prereq)) {
+        if (prereq.getType() == RRType::ANY()) {
+            if (updateHaveRRset(transaction, prereq)) {
                 return (Rcode::YXDOMAIN());
             }
-        } else if (haveRRset(transaction, prereq)) {
+        } else if (updateHaveRRset(transaction, prereq)) {
             return (Rcode::YXRRSET());
         }
-    } else if (prereq->getClass() == transaction.getZoneClass()) {
-        if (prereq->getTTL().getValue() != 0) {
+    } else if (prereq.getClass() == transaction.getZoneClass()) {
+        if (prereq.getTTL().getValue() != 0) {
             return (Rcode::FORMERR());
         }
         // 3.2.3 talks about rebuilding sets, but we already have full rrsets
-        if (!haveRRset(transaction, prereq)) {
+        if (!updateHaveRRset(transaction, prereq)) {
             return (Rcode::NXRRSET());
         }
     } else {
@@ -1389,48 +1402,55 @@
     return (Rcode::NOERROR());
 }
 
+namespace {
+    // do we have a direct check in rrtype to see if a specific
+    // type is known and not a meta type? should we?
+    bool isMetaType(const RRType type) {
+        return (type == RRType::ANY() ||
+                type == RRType::IXFR() ||
+                type == RRType::AXFR());
+                // MAILA and MAILB are mentioned in rfc2136, but
+                // are not implemented
+    }
+}
+
 Rcode
 DataSrc::updateProcessUpdate(DataSrcTransaction& transaction,
-                                    isc::dns::RRsetPtr update)
+                             const isc::dns::RRset& update)
 {
     // The RFC says to pre-scan them, but since we use a transaction
     // we can roll back, we can process the RRsets one at a time
 
     // TODO, NOTZONE check
-
-    RRType update_type = update->getType();
-    if (update->getClass() != RRClass::ANY()) {
-        // do we have a direct check in rrtype to see if a specific
-        // type is known and not a meta type?
-        if (update_type == RRType::ANY() ||
-            update_type == RRType::IXFR() ||
-            update_type == RRType::AXFR()) {
+    const RRType update_type = update.getType();
+
+    if (update.getClass() == transaction.getZoneClass()) {
+        if (isMetaType(update_type)) {
             return (Rcode::FORMERR());
         }
-    } else if (update->getClass() == RRClass::ANY()) {
-        if (update->getTTL().getValue() != 0) {
+    } else if (update.getClass() == RRClass::ANY()) {
+        if (update.getTTL().getValue() != 0 ||
+            update.getRdataCount() != 0 ||
+            (update_type != RRType::ANY() &&
+             isMetaType(update_type)
+            )
+           ) {
             return (Rcode::FORMERR());
         }
-        if (update_type == RRType::IXFR() ||
-            update_type == RRType::AXFR()) {
+    } else if (update.getClass() == RRClass::NONE()) {
+        if (update.getTTL().getValue() != 0 ||
+            isMetaType(update_type)) {
             return (Rcode::FORMERR());
         }
-        if (update->getRdataCount() > 0) {
-            return (Rcode::FORMERR());
-        }
-    } else if (update->getClass() == RRClass::NONE()) {
-        if (update->getTTL().getValue() != 0) {
-            return (Rcode::FORMERR());
-        }
-    }
-
+    }
+    
     // Most types are blindly added, but some require special handling
     if (update_type == RRType::SOA()) {
         // check serial and delete old
         RRsetList soa_list;
         uint32_t flags = 0;
-        if (findExactRRset(update->getName(),
-                           update->getClass(),
+        if (findExactRRset(update.getName(),
+                           update.getClass(),
                            update_type,
                            soa_list,
                            flags,
@@ -1439,7 +1459,7 @@
             return (Rcode::SERVFAIL());
         } else {
             // TODO: no serial arithmetic yet?
-            if (delRRset(transaction, *(soa_list.begin())) != W_SUCCESS) {
+            if (delRRset(transaction, *(*(soa_list.begin()))) != W_SUCCESS) {
                 return (Rcode::SERVFAIL());
             }
             if (addRRset(transaction, update) != W_SUCCESS) {
@@ -1450,8 +1470,8 @@
     // addRRset and delRRset should do 'the right thing' regarding
     // types (any/none/specific) and rdata count
     } else {
-        if (update->getClass() == RRClass::ANY() ||
-            update->getClass() == RRClass::NONE()) {
+        if (update.getClass() == RRClass::ANY() ||
+            update.getClass() == RRClass::NONE()) {
             if (delRRset(transaction, update) != W_SUCCESS) {
                 return (Rcode::SERVFAIL());
             }
@@ -1464,57 +1484,68 @@
     return (Rcode::NOERROR());
 }
 
-isc::dns::RRsetPtr
+isc::dns::RRset*
 callbackHelperRRsetIterator(void* arg1, void* arg2) {
     RRsetIterator* cur = static_cast<RRsetIterator*>(arg1);
     RRsetIterator* end = static_cast<RRsetIterator*>(arg2);
 
-    if (cur == NULL || end == NULL || *cur == *end) {
-        return (RRsetPtr());
+    if (cur == NULL || end == NULL) {
+        isc_throw(Unexpected, "Callback argument NULL");
+    }
+    if (*cur == *end) {
+        return (NULL);
     } else {
-        RRsetPtr result = **cur;
-        (*cur)++;
+        RRset* result = (*cur)->get();
+        ++(*cur);
         return (result);
     }
 }
 
-isc::dns::RRsetPtr
+isc::dns::RRset*
 callbackHelperRRsetVector(void *arg1, void *arg2) {
     std::vector<RRsetPtr>* v = static_cast<std::vector<RRsetPtr>* >(arg1);
     size_t* i = static_cast<size_t*>(arg2);
 
-    if (v && i && *i < v->size()) {
-        RRsetPtr result = ((*v)[(*i)++]);
+    if (v == NULL || i == NULL) {
+        isc_throw(isc::InvalidParameter, "Callback argument NULL");
+    }
+    if (*i < v->size()) {
+        RRset* result = &(*(v->at(*i)));
+        ++(*i);
         return (result);
     } else {
-        return (RRsetPtr());
+        return (NULL);
     }
 }
 
 DataSrc::WriteResult
 DataSrc::doIXFR(DataSrcTransaction& transaction UNUSED_PARAM,
-                isc::dns::RRsetPtr (*nextRRset)(void*, void*),
+                isc::dns::RRset* (*nextRRset)(void*, void*),
                 void* arg1, void* arg2)
 {
-    WriteResult result;
-
     if (transaction.getState() != DataSrcTransaction::RUNNING) {
-        return (W_ERROR);
-    }
-
-    RRsetPtr final_soa = nextRRset(arg1, arg2);
+        isc_throw(DataSourceTransactionError, "doIXFR() called on "
+                      "non-running transaction");
+    }
+    if (nextRRset == NULL) {
+        isc_throw(DataSourceError, "doIXFR called with NULL nextRRset()");
+    }
+
+    const RRset* final_soa = nextRRset(arg1, arg2);
     if (!final_soa) {
         return (W_ERROR);
     }
-    RRsetPtr first_soa = nextRRset(arg1, arg2);
-    if (!final_soa) {
+    const RRset* first_soa = nextRRset(arg1, arg2);
+    if (!first_soa) {
         return (W_ERROR);
     }
 
-    RRsetPtr next_rrset, last_rrset;
+    RRset* next_rrset = NULL;
+    RRset* last_rrset = NULL;
+    WriteResult result;
 
     if (first_soa->getType() == RRType::SOA()) {
-        if (!haveRRset(transaction, first_soa)) {
+        if (!updateHaveRRset(transaction, *first_soa)) {
             return (W_ERROR);
         }
     } else {
@@ -1523,22 +1554,29 @@
         if (result != W_SUCCESS) {
             return (result);
         }
-
-        while (next_rrset = nextRRset(arg1, arg2)) {
-            result = addRRset(transaction, next_rrset);
+        result = addRRset(transaction, *first_soa);
+        if (result != W_SUCCESS) {
+            return (result);
+        }
+
+        next_rrset = nextRRset(arg1, arg2);
+        while (next_rrset) {
+            result = addRRset(transaction, *next_rrset);
             if (result != W_SUCCESS) {
                 return (result);
             }
+            next_rrset = nextRRset(arg1, arg2);
         }
         return (W_SUCCESS);
     }
     bool deleting = true;
 
-    while (next_rrset = nextRRset(arg1, arg2)) {
+    next_rrset = nextRRset(arg1, arg2);
+    while (next_rrset) {
         // If we see a SOA, it means we are switching operations (either
         // we start deleting or adding depending on what we were doing
         // before.
-        // We don't delete the actual SOA itself,
+        // We don't delete the actual SOA itself.
         if (next_rrset->getType() == RRType::SOA()) {
             // TODO: check if serial has increased compared to the last soa we saw
             deleting = !deleting;
@@ -1546,8 +1584,8 @@
         } else {
             if (deleting) {
                 // check if rrset exists, if not, something is very wrong, abort
-                if (haveRRset(transaction, next_rrset)) {
-                    result = delRRset(transaction, next_rrset);
+                if (updateHaveRRset(transaction, *next_rrset)) {
+                    result = delRRset(transaction, *next_rrset);
                     if (result != W_SUCCESS) {
                         return (result);
                     }
@@ -1555,20 +1593,21 @@
                     return (W_ERROR);
                 }
             } else {
-                result = addRRset(transaction, next_rrset);
+                result = addRRset(transaction, *next_rrset);
                 if (result != W_SUCCESS) {
                     return (result);
                 }
             }
         }
-    }
-    if (equalRRsets(last_rrset, final_soa)) {
+        next_rrset = nextRRset(arg1, arg2);
+    }
+    if (equalRRsets(*last_rrset, *final_soa)) {
         // Finally replace the SOA
-        result = delRRset(transaction, first_soa);
+        result = delRRset(transaction, *first_soa);
         if (result != W_SUCCESS) {
             return (result);
         }
-        addRRset(transaction, final_soa);
+        result = addRRset(transaction, *final_soa);
         if (result != W_SUCCESS) {
             return (result);
         }
@@ -1576,61 +1615,57 @@
     } else {
         return (W_ERROR);
     }
-    return (W_NOT_IMPLEMENTED);
-}
-
-
-DataSrc::WriteResult
+}
+
+Rcode
 DataSrc::doUpdate(DataSrcTransaction& transaction UNUSED_PARAM,
-                         isc::dns::Message& msg UNUSED_PARAM)
-{
+                  isc::dns::Message& msg UNUSED_PARAM)
+{
+    if (transaction.getState() != DataSrcTransaction::RUNNING) {
+        isc_throw(DataSourceTransactionError, "doUpdate() called on "
+                      "non-running transaction");
+    }
     if (msg.getOpcode() != isc::dns::Opcode::UPDATE()) {
-        return (W_ERROR);
+        isc_throw(DataSourceError, "doUpdate() called with opcode != UPDATE");
     }
 
     // hmz, zone already in transaction. should we do transaction here?
     // (and not as an argument) for now, simply check it
     if (msg.getRRCount(isc::dns::Section::QUESTION()) != 1) {
-        return (W_ERROR);
-    }
-    QuestionPtr question = *(msg.beginQuestion());
+        return (Rcode::FORMERR());
+    }
+    const ConstQuestionPtr question = *(msg.beginQuestion());
     if (question->getName() != transaction.getZoneName()) {
-        return (W_ERROR);
+        return (Rcode::FORMERR());
     }
     if (question->getType() != isc::dns::RRType::SOA()) {
-        return (W_ERROR);
+        return (Rcode::FORMERR());
     }
     if (question->getClass() != transaction.getZoneClass()) {
-        return (W_ERROR);
+        return (Rcode::FORMERR());
     }
 
     // check the prerequisites
     RRsetIterator it;
     for (it = msg.beginSection(isc::dns::Section::ANSWER());
          it != msg.endSection(isc::dns::Section::ANSWER());
-         it++) {
-        RRsetPtr cur_prereq = *it;
-        isc::dns::Rcode prereq_result = updateCheckPrerequisite(transaction, cur_prereq);
+         ++it) {
+        isc::dns::Rcode prereq_result = updateCheckPrerequisite(transaction, *(*it));
         if (prereq_result != Rcode::NOERROR()) {
-            msg.clear(Message::RENDER);
-            msg.setRcode(prereq_result);
-            return (W_ERROR);
+            return (prereq_result);
         }
     }
 
     for (it = msg.beginSection(isc::dns::Section::AUTHORITY());
          it != msg.endSection(isc::dns::Section::AUTHORITY());
-         it++) {
-        RRsetPtr cur_update = *it;
-        Rcode result = updateProcessUpdate(transaction, cur_update);
+         ++it) {
+        Rcode result = updateProcessUpdate(transaction, *(*it));
         if (result != Rcode::NOERROR()) {
-            return (W_ERROR);
-        }
-    }
-
-    // do we need to do anything with additional?
-
-    return (W_SUCCESS);
+            return (result);
+        }
+    }
+
+    return (Rcode::NOERROR());
 }
 
 DataSrc::Result
@@ -1693,12 +1728,16 @@
 
 DataSrcTransaction::~DataSrcTransaction() {
     try {
-        if (getState() == RUNNING) {
-            _data_source->rollbackTransaction(*this);
+        if (state_ == RUNNING) {
+            DataSrc::WriteResult result = data_source_->rollbackTransaction(*this);
+            if (result != DataSrc::W_SUCCESS) {
+                // if we have an exception-free logging system we definitely
+                // want to log something.
+            }
         }
     } catch (...) {
-        // if we have an exception-free logging system we might
-        // want to log something. Otherwise ignore
+        // if we have an exception-free logging system we definitely
+        // want to log something.
     }
 }
 
@@ -1722,14 +1761,14 @@
 
 DataSrc::WriteResult
 MetaDataSrc::addRRset(DataSrcTransaction& transaction UNUSED_PARAM,
-                      isc::dns::ConstRRsetPtr rrset UNUSED_PARAM)
+                      const isc::dns::RRset& rrset UNUSED_PARAM)
 {
     return (W_NOT_IMPLEMENTED);
 }
 
 DataSrc::WriteResult
 MetaDataSrc::delRRset(DataSrcTransaction& transaction UNUSED_PARAM,
-                      isc::dns::ConstRRsetPtr rrset UNUSED_PARAM)
+                      const isc::dns::RRset& rrset UNUSED_PARAM)
 {
     return (W_NOT_IMPLEMENTED);
 }
@@ -1742,7 +1781,7 @@
 
 DataSrc::WriteResult
 MetaDataSrc::replaceZone(DataSrcTransaction& transaction UNUSED_PARAM,
-                         isc::dns::RRsetPtr (*nextRRset)(void*, void*) UNUSED_PARAM,
+                         isc::dns::RRset* (*nextRRset)(void*, void*) UNUSED_PARAM,
                          void* arg1 UNUSED_PARAM, void* arg2 UNUSED_PARAM)
 {
     return (W_NOT_IMPLEMENTED);

Modified: branches/trac374/src/lib/datasrc/data_source.h
==============================================================================
--- branches/trac374/src/lib/datasrc/data_source.h (original)
+++ branches/trac374/src/lib/datasrc/data_source.h Thu Nov 25 10:03:59 2010
@@ -27,9 +27,7 @@
 
 #include <dns/name.h>
 #include <dns/rrclass.h>
-#include <dns/rrset.h>
 #include <cc/data.h>
-#include <dns/message.h>
 
 namespace isc {
 
@@ -38,6 +36,8 @@
 class RRType;
 class RRset;
 class RRsetList;
+class Message;
+class Rcode;
 }
 
 namespace datasrc {
@@ -55,6 +55,16 @@
 public:
     DataSourceError(const char* file, size_t line, const char* what) :
         isc::Exception(file, line, what) {}
+    virtual ~DataSourceError() throw() {};
+};
+
+/// This exception is raised when a transaction-related operation is
+/// called while the transaction is not in the correct state for that
+/// operation
+class DataSourceTransactionError : public Exception {
+public:
+    DataSourceTransactionError(const char* file, size_t line,
+        const char* what) : isc::Exception(file, line, what) {}
 };
 
 /// \brief Helper function for use as ready-made callback argument
@@ -83,7 +93,7 @@
 ///             as returned by isc::dns::message::endSection().
 /// \return The RRsetPtr as pointed to by the iterator in arg1, or
 ///         an empty RRsetPtr if the end is reached.
-isc::dns::RRsetPtr
+isc::dns::RRset*
 callbackHelperRRsetIterator(void* arg1, void* arg2);
 
 /// \brief Helper function for use as ready-made callback argument
@@ -99,7 +109,7 @@
 /// \param arg2, pointer to a size_t value. This value is incremented
 ///              on each call to this function
 /// \return The RRsetPtr in the vector *arg1 at position *arg2.
-isc::dns::RRsetPtr
+isc::dns::RRset*
 callbackHelperRRsetVector(void* arg1, void* arg2);
 
 class AbstractDataSrc {
@@ -131,39 +141,28 @@
     // For write-related methods, we have a separate list of
     // return values, since the errors that can occur are different
     enum WriteResult {
-        W_SUCCESS,              // ok
-        W_ERROR,                // general (code) error
-        W_DB_ERROR,             // (write) error in the backend
-        W_NOT_IMPLEMENTED,      // write functions not implemented
-        W_NO_SUCH_ZONE,         // zone not known to this data source
-        W_NO_SUCH_DATA          // attempt to delete nonexistant data
+        W_SUCCESS,              //!< ok
+        W_ERROR,                //!< general (code) error
+        W_DB_ERROR,             //!< (write) error in the backend
+        W_NOT_IMPLEMENTED,      //!< write functions not implemented
+        W_NO_SUCH_ZONE,         //!< zone not known to this data source
+        W_NO_SUCH_DATA          //!< attempt to delete nonexistant data
     };
 
     // These flags indicate conditions encountered while processing a query.
-    //
-    // REFERRAL:       The node contains an NS record
-    // CNAME_FOUND:    The node contains a CNAME record
-    // NAME_NOT_FOUND: The node does not exist in the data source.
-    // TYPE_NOT_FOUND: The node does not contain the requested RRType
-    // NO_SUCH_ZONE:   The zone does not exist in this data source.
-    //
-    // DATA_NOT_FOUND: A combination of the last three, for coding convenience
     enum QueryResponseFlags {
-        REFERRAL = 0x01,
-        CNAME_FOUND = 0x02,
-        NAME_NOT_FOUND = 0x04,
-        TYPE_NOT_FOUND = 0x08,
-        NO_SUCH_ZONE = 0x10,
-        DATA_NOT_FOUND = (NAME_NOT_FOUND|TYPE_NOT_FOUND|NO_SUCH_ZONE)
+        REFERRAL = 0x01,        //!< The node contains an NS record
+        CNAME_FOUND = 0x02,     //!< The node contains a CNAME record
+        NAME_NOT_FOUND = 0x04,  //!< The node does not exist in the data source.
+        TYPE_NOT_FOUND = 0x08,  //!< The node does not contain the requested RRType
+        NO_SUCH_ZONE = 0x10,    //!< The zone does not exist in this data source.
+        DATA_NOT_FOUND = (NAME_NOT_FOUND|TYPE_NOT_FOUND|NO_SUCH_ZONE) 
+        //!< A combination of the last three, for coding convenience
     };
 
     // 'High-level' methods.  These will be implemented by the
     // general DataSrc class, and SHOULD NOT be overwritten by subclasses.
     virtual void doQuery(Query& query) = 0;
-
-    // XXX: High-level methods to be implemented later:
-    // virtual void doUpdate(Update update) = 0;
-    // virtual void doXfr(Query query) = 0;
 
     // 'Medium-level' methods.  This will be implemented by the general
     // DataSrc class but MAY be overwritten by subclasses.
@@ -255,7 +254,15 @@
 ///
 class DataSrcTransaction {
 public:
-    enum states { INIT, RUNNING, DONE };
+    enum States {
+        INIT,       //!< Transaction has been created, but not started
+                    ///  yet.
+        RUNNING,    //!< Transaction in progress. If a transaction
+                    ///  object in this state is destroyed, it will
+                    ///  automatically be rolled back.
+        DONE        //!< Transaction finished (either committed or
+                    ///  rolled back).
+    };
 
     /// \brief Constructs a DataSrcTransaction object
     ///
@@ -275,22 +282,31 @@
     /// \param zone_name The name of the zone to perform operations on
     /// \param zone_class The RRClass of the zone to perform operations on.
     ///
-    explicit DataSrcTransaction(DataSrc *data_source,
-                                const isc::dns::Name& zone_name,
-                                const isc::dns::RRClass& zone_class) :
-                                _zone_name(zone_name),
-                                _zone_class(zone_class),
-                                _data_source(data_source),
-                                _state(INIT) {}
+    DataSrcTransaction(DataSrc *data_source,
+                       const isc::dns::Name& zone_name,
+                       const isc::dns::RRClass& zone_class) :
+                       zone_name_(zone_name),
+                       zone_class_(zone_class),
+                       data_source_(data_source),
+                       state_(INIT) {}
+
+    /// \brief Destruction for transactions.
+    ///
+    /// If the transaction is in \c State RUNNING, destroying it will
+    /// cause the transaction to be rolled back (by calling
+    /// \c rollbackTransaction on the \c DataSrc associated with this
+    /// transaction).
+    /// If the rollback fails for some reason, this is currently
+    /// ignored.
     ~DataSrcTransaction();
 
     /// \brief Returns the current state of the transaction
     /// \return state
-    states getState() { return (_state); }
+    States getState() const { return (state_); }
 
     /// \brief Sets the state of the transaction
     /// \param state The new state
-    void setState(states state) { _state = state; }
+    void setState(States state) { state_ = state; }
 
     /// \brief Returns the base ElementPtr of the arbitrary
     /// datasource-specific data.
@@ -298,33 +314,33 @@
     /// data source, and no assumptions about its contents should
     /// be made by the caller.
     /// \return ElementPtr containing transaction data
-    isc::data::ElementPtr getData() { return (_data); }
+    isc::data::ElementPtr getData() const { return (data_); }
 
     /// \brief Sets the base ElementsPtr of the arbitrary
     /// datasource-specific data. This function should only be called
     /// by the data source, in the startTransaction() method.
     /// \param data The ElementPtr containing data
-    void setData(isc::data::ElementPtr data) { _data = data; }
+    void setData(isc::data::ElementPtr data) { data_ = data; }
 
     /// \brief Returns the name of the zone this transaction is about
     /// \return zone name
-    const isc::dns::Name getZoneName() { return (_zone_name); }
+    const isc::dns::Name& getZoneName() const { return (zone_name_); }
 
     /// \brief Returns the RRClass of the zone this transaction is about
     /// \return RRClass
-    const isc::dns::RRClass getZoneClass() { return (_zone_class); }
+    isc::dns::RRClass getZoneClass() const { return (zone_class_); }
 
     /// \brief Returns a pointer to the data source the zone was
     /// found in.
     /// \return DataSrc* pointer
-    DataSrc* getDataSource() { return (_data_source); }
+    //DataSrc* getDataSource() const { return (data_source_); }
 
 private:
-    isc::data::ElementPtr _data;
-    isc::dns::Name _zone_name;
-    isc::dns::RRClass _zone_class;
-    DataSrc* _data_source;
-    states _state;
+    isc::data::ElementPtr data_;
+    const isc::dns::Name zone_name_;
+    const isc::dns::RRClass zone_class_;
+    DataSrc* data_source_;
+    States state_;
 };
 
 // Base class for a DNS Data Source
@@ -402,12 +418,19 @@
     /// A data source that supports writes MUST override these methods.
     ///
     //@{
-    /// \brief Start a transaction
+    /// \brief Start a transaction.
+    ///
     /// The given transaction must be in state INIT.
     /// On success, the transaction must be in state RUNNING.
     /// The data source checks for the zone, starts a backend
     /// transaction, and sets any data it will need later in the given
     /// transaction object.
+    ///
+    /// Raises a DataSourceTransactionError if the transaction is not
+    /// in state INIT.
+    /// Raises a DataSourceTransactionError if create_zone is false,
+    /// the creation of the zone fails, and rollback fails too.
+    ///
     /// \param transaction a transaction that has been initialized for
     /// a zone held by this data source.
     /// \param create_zone if set to true, and the zone does not exist,
@@ -418,44 +441,105 @@
     virtual WriteResult startTransaction(DataSrcTransaction& transaction,
                                          const bool create_zone = false);
 
-    /// \brief Commit the changes made in the given transaction
+    /// \brief Commit the changes made in the given transaction.
+    ///
     /// The transaction object must be in state RUNNING.
     /// On successs, the transaction will be in state DONE.
+    ///
+    /// Raises a DataSourceTransactionError if the transaction is not
+    /// in state RUNNING.
+    ///
     /// \param transaction to be committed.
     /// \return W_SUCCESS on success, error value otherwise
     virtual WriteResult commitTransaction(DataSrcTransaction& transaction);
 
-    /// \brief Roll back the changes made in the given transaction
+    /// \brief Roll back the changes made in the given transaction.
+    ///
     /// The transaction object must be in state RUNNING.
     /// On successs, the transaction will be in state DONE.
+    ///
+    /// Raises a DataSourceTransactionError if the transaction is not
+    /// in state RUNNING.
+    ///
     /// \param transaction to be rolled back.
     /// \return W_SUCCESS on success, error value otherwise
     virtual WriteResult rollbackTransaction(DataSrcTransaction& transaction);
 
     /// \brief Add an RRset to the zone.
+    ///
+    /// The implementations of this function do not perform
+    /// zone integrity checks, these should be done by the caller
+    /// The implementations of this function should not raise
+    /// exceptions, but log the error and return an error code.
+    ///
+    /// Raises a DataSourceTransactionError if the transaction is not
+    /// in state RUNNING.
+    ///
+    /// If the result is not W_SUCCESS, or if an exception is raised,
+    /// the corresponding transaction should be rolled back, to prevent
+    /// the database to get into an inconsistent state
+    ///
     /// \param transaction The transaction in which this addition is
     ///        performed.
-    /// \param RRsetPtr The RRset to add
-    /// \param W_SUCCESS on success.
+    /// \param rrset The RRset to add
+    /// \return W_SUCCESS on success.
+    ///         W_NOT_IMPLEMENTED if the DataSrc does not support
+    ///         writing
+    ///         Error value otherwise
     virtual WriteResult addRRset(DataSrcTransaction& transaction,
-                                       isc::dns::ConstRRsetPtr rrset);
+                                 const isc::dns::RRset& rrset);
+
     /// \brief Delete an RRset from the zone.
+    ///
     /// If the given RRset contains no Rdata parts, all RRs with
     /// the name, type and class of the given rrset are deleted.
     /// Otherwise every RR in the given RRset is deleted.
+    ///
+    /// The implementations of this function do not perform
+    /// zone integrity checks, these should be done by the caller
+    /// The implementations of this function should not raise
+    /// exceptions, but log the error and return an error code.
+    ///
+    /// Raises a DataSourceTransactionError if the transaction is not
+    /// in state RUNNING.
+    ///
+    /// If the result is not W_SUCCESS, or if an exception is raised,
+    /// the corresponding transaction should be rolled back, to prevent
+    /// the database to get into an inconsistent state
+    ///
     /// \param transaction The transaction in which this deletion is
     ///        performed.
-    /// \param RRsetPtr The RRset to delete
-    /// \param W_SUCCESS on success.
+    /// \param rrset The RRset to delete
+    /// \return W_SUCCESS on success.
+    ///         W_NOT_IMPLEMENTED if the DataSrc does not support
+    ///         writing
+    ///         Error value otherwise
     virtual WriteResult delRRset(DataSrcTransaction& transaction,
-                                       isc::dns::ConstRRsetPtr rrset);
-
-    /// \brief Remove an entire zone from the data source
-    /// Removes all records for the zone, and the zone itself from
-    /// the datasource.
+                                       const isc::dns::RRset& rrset);
+
+    /// \brief Remove an entire zone from the data source.
+    ///
+    /// Removes all records for the zone, and the zone itself
+    /// from the datasource.
+    ///
+    /// Since the start of a transaction either requires the
+    /// zone to exist or creates the zone, the implementations
+    /// of this function do not check if the zones exist. Rather
+    /// nonexistance is ignored.
+    ///
+    /// Raises a DataSourceTransactionError if the transaction is not
+    /// in state RUNNING.
+    ///
+    /// If the result is not W_SUCCESS, or if an exception is raised,
+    /// the corresponding transaction should be rolled back, to prevent
+    /// the database to get into an inconsistent state
+    ///
     /// \param transaction The transaction in which this deletion is
     ///        performed.
     /// \return W_SUCCESS on success
+    ///         W_NOT_IMPLEMENTED if the DataSrc does not support
+    ///         writing
+    ///         Error value otherwise
     virtual WriteResult delZone(DataSrcTransaction& transaction);
 
     /// \brief Replace the contents of the zone by the rrsets returned
@@ -469,16 +553,24 @@
     /// you can use the helper function callbackHelperRRsetVector. See its
     /// description for more information on how to do this.
     ///
+    /// Raises a DataSourceTransactionError if the transaction is not
+    /// in state RUNNING.
+    ///
+    /// If the result is not W_SUCCESS, or if an exception is raised,
+    /// the corresponding transaction should be rolled back, to prevent
+    /// the database to get into an inconsistent state
+    ///
     /// \param transaction The transaction to perform the operations on
     /// \param nextRRset Function that returns the next RRsetPtr on
-    ///        each call to it
+    ///        each call to it. If NULL is passed here, all records in
+    ///        the zone will be deleted, and none will be added.
     /// \param arg1 This will be passed as the first argument to the
     ///             nextRRset function (defaults to NULL)
     /// \param arg2 This will be passed as the second argument to the
     ///             nextRRset function (defaults to NULL)
     /// \return W_SUCCESS on success
     virtual WriteResult replaceZone(DataSrcTransaction& transaction,
-                                    isc::dns::RRsetPtr (*nextRRset)(void*, void*),
+                                    isc::dns::RRset* (*nextRRset)(void*, void*),
                                     void* arg1 = NULL, void* arg2 = NULL);
     //@}
 
@@ -486,9 +578,11 @@
     /// These functions have a default implementation, and do not
     /// need to be overridden. They can be, if the data source can make
     /// use of specific knowledge about the backend.
+    /// They should NOT be called by datasource users.
 
     //@{
-    /// \brief Checks whether an RRset exists
+    /// \brief Checks whether an RRset exists for the purposes of
+    ///        dynamic updates and IXFR.
     /// If the given RRset is of class NONE or ANY, it will check if
     /// there are rrs for class IN.
     /// If the given rrset has 0 rdata records; any RR with the given
@@ -503,8 +597,8 @@
     ///         database, or if the given rrset has a 0 rdata count,
     ///         and there are records in the database with the same
     ///         name, class and type. False otherwise.
-    virtual bool haveRRset(DataSrcTransaction& transaction,
-                           isc::dns::ConstRRsetPtr rrset);
+    virtual bool updateHaveRRset(DataSrcTransaction& transaction,
+                                 const isc::dns::RRset& rrset);
 
     /// \brief Checks a prerequisite as described in section 3.2 of
     /// RFC2136
@@ -512,7 +606,7 @@
     /// \param prereq the prerequisite RRset
     /// \return The DNS Rcode as defined by the rules in RFC2136
     virtual isc::dns::Rcode updateCheckPrerequisite(DataSrcTransaction& transaction,
-                                            isc::dns::ConstRRsetPtr prereq);
+                                            const isc::dns::RRset& prereq);
 
     /// \brief Handles a single update as described in section 3.4 of
     /// RFC2136
@@ -520,7 +614,7 @@
     /// \param update the update RRset
     /// \return The DNS Rcode as defined by the rules in RFC2136
     virtual isc::dns::Rcode updateProcessUpdate(DataSrcTransaction& transaction,
-                                        isc::dns::RRsetPtr update);
+                                                const isc::dns::RRset& update);
 
     /// \brief Perform an IXFR operation with data provided by the
     ///        given function.
@@ -532,24 +626,42 @@
     /// callbackHelperRRsetIterator() here. See the documentation
     /// of that function on how to use it.
     ///
+    /// Raises a DataSourceTransactionError if the transaction is not
+    /// in state RUNNING.
+    /// Raises a DataSourceError if nextRRset is NULL
+    ///
+    /// If the result is not W_SUCCESS, or if an exception is raised,
+    /// the corresponding transaction should be rolled back, to prevent
+    /// the database to get into an inconsistent state
+    ///
     /// \param transaction the transaction to perform the operation in
     /// \param nextRRset Function that returns the next RRsetPtr on
-    ///        each call to it
+    ///        each call to it (may not be NULL).
     /// \param arg1 This will be passed as the first argument to the
     ///             nextRRset function (defaults to NULL)
     /// \param arg2 This will be passed as the second argument to the
     ///             nextRRset function (defaults to NULL)
     /// \return W_SUCCESS on success, error value on failure.
     virtual WriteResult doIXFR(DataSrcTransaction& transaction,
-                               isc::dns::RRsetPtr (*nextRRset)(void*, void*),
+                               isc::dns::RRset* (*nextRRset)(void*, void*),
                                void* arg1 = NULL, void* arg2 = NULL);
 
     /// \brief Perform a DNS Dynamic update as desribed in RFC2136
+    ///
+    /// Raises a DataSourceTransactionError if the transaction is not
+    /// in state RUNNING.
+    /// Raises a DataSourceError if the given message does not have
+    /// opcode UPDATE.
+    ///
+    /// If the result is not W_SUCCESS, or if an exception is raised,
+    /// the corresponding transaction should be rolled back, to prevent
+    /// the database to get into an inconsistent state
+    ///
     /// \param transaction the transaction to perform the udpate in
     /// \param msg the DNS UPDATE msg to handle
-    /// \return SUCCESS on success, ERROR on failure.
-    virtual WriteResult doUpdate(DataSrcTransaction& transaction,
-                                 isc::dns::Message& msg);
+    /// \return NOERROR on success, an error Rcode on failure.
+    virtual isc::dns::Rcode doUpdate(DataSrcTransaction& transaction,
+                                     isc::dns::Message& msg);
     //@}
 
 private:
@@ -630,12 +742,12 @@
     virtual WriteResult rollbackTransaction(DataSrcTransaction& transaction);
 
     virtual WriteResult addRRset(DataSrcTransaction& transaction,
-                                       isc::dns::ConstRRsetPtr rrset);
+                                       const isc::dns::RRset& rrset);
     virtual WriteResult delRRset(DataSrcTransaction& transaction,
-                                       isc::dns::ConstRRsetPtr rrset);
+                                       const isc::dns::RRset& rrset);
     virtual WriteResult delZone(DataSrcTransaction& transaction);
     virtual WriteResult replaceZone(DataSrcTransaction& transaction,
-                                    isc::dns::RRsetPtr (*nextRRset)(void*, void*),
+                                    isc::dns::RRset* (*nextRRset)(void*, void*),
                                     void* arg1 = NULL, void* arg2 = NULL);
     // end of writable data sources part
 private:

Modified: branches/trac374/src/lib/datasrc/sqlite3_datasrc.cc
==============================================================================
--- branches/trac374/src/lib/datasrc/sqlite3_datasrc.cc (original)
+++ branches/trac374/src/lib/datasrc/sqlite3_datasrc.cc Thu Nov 25 10:03:59 2010
@@ -30,8 +30,6 @@
 #include <dns/rrsetlist.h>
 
 #include <cc/data.h>
-
-#include <boost/foreach.hpp>
 
 using namespace std;
 using namespace isc::dns;
@@ -154,16 +152,18 @@
 //
 int
 Sqlite3DataSrc::hasExactZone(const char* const name) const {
-    int rc;
-
+    if (name == NULL) {
+        return (-1);
+    }
     sqlite3_reset(dbparameters->q_zone_);
-    rc = sqlite3_bind_text(dbparameters->q_zone_, 1, name, -1, SQLITE_STATIC);
-    if (rc != SQLITE_OK) {
+
+    if (sqlite3_bind_text(dbparameters->q_zone_, 1, name, -1,
+                          SQLITE_STATIC) != SQLITE_OK) {
         isc_throw(Sqlite3Error, "Could not bind " << name <<
                   " to SQL statement (zone)");
     }
 
-    rc = sqlite3_step(dbparameters->q_zone_);
+    const int rc = sqlite3_step(dbparameters->q_zone_);
     const int i = (rc == SQLITE_ROW) ?
         sqlite3_column_int(dbparameters->q_zone_, 0) : -1; 
     sqlite3_reset(dbparameters->q_zone_);
@@ -643,7 +643,16 @@
         if (params_.w_add_rr_ != NULL) {
             sqlite3_finalize(params_.w_add_rr_);
         }
+        if (params_.w_add_zone_ != NULL) {
+            sqlite3_finalize(params_.w_add_rr_);
+        }
         if (params_.w_del_rr_ != NULL) {
+            sqlite3_finalize(params_.w_del_rr_);
+        }
+        if (params_.w_del_all_ != NULL) {
+            sqlite3_finalize(params_.w_del_rr_);
+        }
+        if (params_.w_del_zone_ != NULL) {
             sqlite3_finalize(params_.w_del_rr_);
         }
         if (params_.db_ != NULL) {
@@ -718,9 +727,14 @@
 
     Sqlite3Initializer initializer;
 
-    int sq_result = sqlite3_open(name.c_str(), &initializer.params_.db_);
+    const int sq_result = sqlite3_open(name.c_str(),
+                                       &initializer.params_.db_);
     if (sq_result != 0) {
-        isc_throw(Sqlite3Error, "Cannot open SQLite database file " << name << ": error " << sq_result << " (" << sqlite3_errmsg(dbparameters->db_) << ")");
+        isc_throw(Sqlite3Error, "Cannot open SQLite database file " <<
+                                name << ": error " << sq_result <<
+                                " (" <<
+                                sqlite3_errmsg(dbparameters->db_) <<
+                                 ")");
     }
 
     checkAndSetupSchema(&initializer);
@@ -787,25 +801,29 @@
 }
 
 DataSrc::WriteResult
-Sqlite3DataSrc::startTransaction(DataSrcTransaction& transaction, bool new_zone)
+Sqlite3DataSrc::startTransaction(DataSrcTransaction& transaction,
+                                 const bool new_zone)
 {
     if (transaction.getState() != DataSrcTransaction::INIT) {
-        return (DataSrc::W_ERROR);
+        isc_throw(DataSourceTransactionError, "startTransaction "
+                      "called but transaction state not INIT");
     }
     int result = sqlite3_exec(dbparameters->db_,
                               w_start_transaction_str,
                               NULL, NULL, NULL);
     if (result == SQLITE_OK) {
-        int zone_id = hasExactZone(transaction.getZoneName().toText().c_str());
-        if (zone_id <= 0) {
+        std::string zone_name = transaction.getZoneName().toText();
+        int zone_id = hasExactZone(zone_name.c_str());
+        if (zone_id < 0) {
             if (new_zone) {
                 DataSrc::WriteResult add_zone_res =
                     addZone(transaction.getZoneName(),
-                            isc::dns::RRClass::IN());
+                            transaction.getZoneClass());
                 if (add_zone_res != DataSrc::W_SUCCESS) {
                     return (add_zone_res);
                 }
-                zone_id = hasExactZone(transaction.getZoneName().toText().c_str());
+                zone_id = hasExactZone(zone_name.c_str());
+                assert(zone_id >= 0);
             } else {
                 // roll back the transaction we started
                 result = sqlite3_exec(dbparameters->db_,
@@ -814,7 +832,9 @@
                 if (result == SQLITE_OK) {
                     return (DataSrc::W_NO_SUCH_ZONE);
                 } else {
-                    return (DataSrc::W_DB_ERROR);
+                    isc_throw(DataSourceTransactionError,
+                              "Error on rollback after failed zone "
+                              "creation");
                 }
             }
         }
@@ -832,11 +852,13 @@
 Sqlite3DataSrc::commitTransaction(DataSrcTransaction& transaction UNUSED_PARAM)
 {
     if (transaction.getState() != DataSrcTransaction::RUNNING) {
-        return (DataSrc::W_ERROR);
-    }
-
-    int result = sqlite3_exec(dbparameters->db_, w_commit_transaction_str,
-                              NULL, NULL, NULL);
+        isc_throw(DataSourceTransactionError, "commitTransaction "
+                      "called but transaction state not RUNNING");
+    }
+
+    const int result = sqlite3_exec(dbparameters->db_,
+                                    w_commit_transaction_str,
+                                    NULL, NULL, NULL);
 
     if (result == SQLITE_OK) {
         transaction.setState(DataSrcTransaction::DONE);
@@ -849,7 +871,8 @@
 DataSrc::WriteResult
 Sqlite3DataSrc::rollbackTransaction(DataSrcTransaction& transaction) {
     if (transaction.getState() != DataSrcTransaction::RUNNING) {
-        return (DataSrc::W_ERROR);
+        isc_throw(DataSourceTransactionError, "commitTransaction "
+                      "called but transaction state not RUNNING");
     }
     int result = sqlite3_exec(dbparameters->db_, w_rollback_transaction_str, NULL, NULL, NULL);
 
@@ -870,22 +893,19 @@
     sqlite3_reset(query);
     sqlite3_clear_bindings(query);
 
-    const string s_name = name.toText();
-    int rc;
-    rc = sqlite3_bind_text(query, 1, s_name.c_str(), -1, SQLITE_TRANSIENT);
-    if (rc != SQLITE_OK) {
-        isc_throw(Sqlite3Error, "Could not bind name " << s_name <<
-                  " to SQL statement (query)");
-    }
-
-    rc = sqlite3_bind_text(query, 2, rrclass.toText().c_str(), -1,
-                           SQLITE_TRANSIENT);
-    if (rc != SQLITE_OK) {
+    if (sqlite3_bind_text(query, 1, name.toText().c_str(), -1,
+                          SQLITE_TRANSIENT) != SQLITE_OK) {
+        isc_throw(Sqlite3Error, "Could not bind name " << name.toText()
+                                << " to SQL statement (query)");
+    }
+
+    if (sqlite3_bind_text(query, 2, rrclass.toText().c_str(), -1,
+                          SQLITE_TRANSIENT) != SQLITE_OK) {
         isc_throw(Sqlite3Error, "Could not bind zone class " <<
                   rrclass.toText() << " to SQL statement (query)");
     }
 
-    int result = sqlite3_step(query);
+    const int result = sqlite3_step(query);
 
     sqlite3_reset(query);
 
@@ -908,50 +928,45 @@
     sqlite3_reset(query);
     sqlite3_clear_bindings(query);
 
-    int rc;
-    rc = sqlite3_bind_int(query, 1, zone_id);
-    if (rc != SQLITE_OK) {
+    if (sqlite3_bind_int(query, 1, zone_id) != SQLITE_OK) {
         isc_throw(Sqlite3Error, "Could not bind zone ID " << zone_id <<
                   " to SQL statement (query)");
     }
-    const string s_name = name.toText();
-    rc = sqlite3_bind_text(query, 2, s_name.c_str(), -1, SQLITE_TRANSIENT);
-    if (rc != SQLITE_OK) {
-        isc_throw(Sqlite3Error, "Could not bind name " << s_name <<
+
+    if (sqlite3_bind_text(query, 2, name.toText().c_str(), -1,
+                          SQLITE_TRANSIENT) != SQLITE_OK) {
+        isc_throw(Sqlite3Error, "Could not bind name " << name.toText()
+                  << " to SQL statement (query)");
+    }
+
+    // reverse name
+    if (sqlite3_bind_text(query, 3, name.reverse().toText().c_str(),
+                          -1, SQLITE_TRANSIENT) != SQLITE_OK) {
+        isc_throw(Sqlite3Error, "Could not bind rname " <<
+                  name.reverse().toText() <<
                   " to SQL statement (query)");
     }
 
-    // reverse name
-    const string r_name = name.reverse().toText();
-    rc = sqlite3_bind_text(query, 3, r_name.c_str(), -1, SQLITE_TRANSIENT);
-    if (rc != SQLITE_OK) {
-        isc_throw(Sqlite3Error, "Could not bind name " << s_name <<
-                  " to SQL statement (query)");
-    }
-
-    rc = sqlite3_bind_int(query, 4, rrttl.getValue());
-    if (rc != SQLITE_OK) {
+    if (sqlite3_bind_int(query, 4, rrttl.getValue()) != SQLITE_OK) {
         isc_throw(Sqlite3Error, "Could not bind RR TTL " <<
                   rrttl.toText() << " to SQL statement (query)");
     }
 
-    rc = sqlite3_bind_text(query, 5, rrtype.toText().c_str(), -1,
-                           SQLITE_TRANSIENT);
-    if (rc != SQLITE_OK) {
+    if (sqlite3_bind_text(query, 5, rrtype.toText().c_str(), -1,
+                          SQLITE_TRANSIENT) != SQLITE_OK) {
         isc_throw(Sqlite3Error, "Could not bind RR type " <<
                   rrtype.toText() << " to SQL statement (query)");
     }
 
     // TODO: sigtype
 
-    rc = sqlite3_bind_text(query, 7, rdata.toText().c_str(), -1,
-                           SQLITE_TRANSIENT);
-    if (rc != SQLITE_OK) {
+    if (sqlite3_bind_text(query, 7, rdata.toText().c_str(), -1,
+                          SQLITE_TRANSIENT) != SQLITE_OK) {
         isc_throw(Sqlite3Error, "Could not bind RR rdata " <<
                   rdata.toText() << " to SQL statement (query)");
     }
 
-    int result = sqlite3_step(query);
+    const int result = sqlite3_step(query);
 
     sqlite3_reset(query);
 
@@ -965,23 +980,27 @@
 
 DataSrc::WriteResult
 Sqlite3DataSrc::addRRset(DataSrcTransaction& transaction,
-                         isc::dns::ConstRRsetPtr rrset)
+                         const isc::dns::RRset& rrset)
 {
     if (transaction.getState() != DataSrcTransaction::RUNNING) {
-        return (DataSrc::W_ERROR);
-    }
-    if (rrset->getRdataCount() == 0) {
-        return (DataSrc::W_ERROR);
-    }
-    int zone_id = transaction.getData()->get("zone_id")->intValue();
-    RdataIteratorPtr rdp = rrset->getRdataIterator();
+        isc_throw(DataSourceTransactionError, "addRRset called but "
+                      "transaction state not RUNNING");
+    }
+    if (rrset.getRdataCount() == 0) {
+        isc_throw(DataSourceError, "addRRset called with empty RRset");
+    }
+
+    const int zone_id = transaction.getData()->get("zone_id")->intValue();
+    RdataIteratorPtr rdp = rrset.getRdataIterator();
     rdp->first();
 
     DataSrc::WriteResult result = DataSrc::W_SUCCESS;
     do {
-        result = addRR(zone_id, rrset->getName(), rrset->getType(),
-                       rrset->getTTL(), rdp->getCurrent());
+        result = addRR(zone_id, rrset.getName(), rrset.getType(),
+                       rrset.getTTL(), rdp->getCurrent());
         if (result != DataSrc::W_SUCCESS) {
+            // Simply return, we are in a transaction which
+            // should be rolled back (see data_source.h)
             return (result);
         } else {
             rdp->next();
@@ -1000,19 +1019,19 @@
     sqlite3_reset(query);
     sqlite3_clear_bindings(query);
 
-    int rc;
-    rc = sqlite3_bind_int(query, 1, zone_id);
-    if (rc != SQLITE_OK) {
+    if (sqlite3_bind_int(query, 1, zone_id) != SQLITE_OK) {
         isc_throw(Sqlite3Error, "Could not bind zone ID " << zone_id <<
                   " to SQL statement (query)");
     }
-    const string s_name = name.toText();
-    rc = sqlite3_bind_text(query, 2, s_name.c_str(), -1, SQLITE_TRANSIENT);
-    if (rc != SQLITE_OK) {
-        isc_throw(Sqlite3Error, "Could not bind name " << s_name <<
-                  " to SQL statement (query)");
-    }
-
+
+    if (sqlite3_bind_text(query, 2, name.toText().c_str(), -1,
+                          SQLITE_TRANSIENT) != SQLITE_OK) {
+        isc_throw(Sqlite3Error, "Could not bind name " <<
+                                name.toText() <<
+                                " to SQL statement (query)");
+    }
+
+    int rc;
     if (rrtype == RRType::ANY()) {
         rc = sqlite3_bind_text(query, 3, "%", -1,
                                SQLITE_STATIC);
@@ -1025,15 +1044,14 @@
                   rrtype.toText() << " to SQL statement (query)");
     }
 
-    rc = sqlite3_bind_text(query, 4, "%", -1,
-                           SQLITE_STATIC);
-    if (rc != SQLITE_OK) {
+    if (sqlite3_bind_text(query, 4, "%", -1,SQLITE_STATIC)
+        != SQLITE_OK) {
         isc_throw(Sqlite3Error, "Could not bind '%' rdata "
                                 " to SQL statement (query)");
     }
 
-    int result = sqlite3_step(query);
-    int deleted_rows =  sqlite3_total_changes(dbparameters->db_);
+    const int result = sqlite3_step(query);
+    const int deleted_rows = sqlite3_changes(dbparameters->db_);
 
     sqlite3_reset(query);
 
@@ -1059,35 +1077,31 @@
     sqlite3_reset(query);
     sqlite3_clear_bindings(query);
 
-    int rc;
-    rc = sqlite3_bind_int(query, 1, zone_id);
-    if (rc != SQLITE_OK) {
+    if (sqlite3_bind_int(query, 1, zone_id) != SQLITE_OK) {
         isc_throw(Sqlite3Error, "Could not bind zone ID " << zone_id <<
                   " to SQL statement (query)");
     }
-    const string s_name = name.toText();
-    rc = sqlite3_bind_text(query, 2, s_name.c_str(), -1, SQLITE_TRANSIENT);
-    if (rc != SQLITE_OK) {
-        isc_throw(Sqlite3Error, "Could not bind name " << s_name <<
-                  " to SQL statement (query)");
-    }
-
-    rc = sqlite3_bind_text(query, 3, rrtype.toText().c_str(), -1,
-                           SQLITE_TRANSIENT);
-    if (rc != SQLITE_OK) {
+    
+    if (sqlite3_bind_text(query, 2, name.toText().c_str(), -1,
+                          SQLITE_TRANSIENT) != SQLITE_OK) {
+        isc_throw(Sqlite3Error, "Could not bind name " << name.toText()
+                                << " to SQL statement (query)");
+    }
+
+    if (sqlite3_bind_text(query, 3, rrtype.toText().c_str(), -1,
+                           SQLITE_TRANSIENT) != SQLITE_OK) {
         isc_throw(Sqlite3Error, "Could not bind RR type " <<
                   rrtype.toText() << " to SQL statement (query)");
     }
 
-    rc = sqlite3_bind_text(query, 4, rdata.toText().c_str(), -1,
-                           SQLITE_TRANSIENT);
-    if (rc != SQLITE_OK) {
+    if (sqlite3_bind_text(query, 4, rdata.toText().c_str(), -1,
+                          SQLITE_TRANSIENT) != SQLITE_OK) {
         isc_throw(Sqlite3Error, "Could not bind RR rdata " <<
                   rdata.toText() << " to SQL statement (query)");
     }
 
-    int result = sqlite3_step(query);
-    int deleted_rows =  sqlite3_total_changes(dbparameters->db_);
+    const int result = sqlite3_step(query);
+    const int deleted_rows =  sqlite3_changes(dbparameters->db_);
 
     sqlite3_reset(query);
 
@@ -1109,34 +1123,33 @@
     sqlite3_reset(query);
     sqlite3_clear_bindings(query);
 
-    int rc;
-    rc = sqlite3_bind_int(query, 1, zone_id);
-    if (rc != SQLITE_OK) {
+    if (sqlite3_bind_int(query, 1, zone_id) != SQLITE_OK) {
         isc_throw(Sqlite3Error, "Could not bind zone ID " << zone_id <<
                   " to SQL statement (query)");
     }
 
-    int result = sqlite3_step(query);
+    const int result = sqlite3_step(query);
 
     sqlite3_reset(query);
 
     if (result == SQLITE_DONE) {
-        return (DataSrc::W_SUCCESS);
-    } else {
-        return (DataSrc::W_DB_ERROR);
+        return (W_SUCCESS);
+    } else {
+        return (W_DB_ERROR);
     }
 }
 
 DataSrc::WriteResult
 Sqlite3DataSrc::replaceZone(DataSrcTransaction& transaction,
-                            isc::dns::RRsetPtr (*nextRRset)(void*, void*),
+                            isc::dns::RRset* (*nextRRset)(void*, void*),
                             void* arg1, void* arg2)
 {
     if (transaction.getState() != DataSrcTransaction::RUNNING) {
-        return (DataSrc::W_ERROR);
-    }
-
-    int zone_id = transaction.getData()->get("zone_id")->intValue();
+        isc_throw(DataSourceTransactionError, "replaceZone called but "
+                  "transaction state not RUNNING");
+    }
+
+    const int zone_id = transaction.getData()->get("zone_id")->intValue();
 
     DataSrc::WriteResult result = delAll(zone_id);
     if (result != DataSrc::W_SUCCESS) {
@@ -1146,10 +1159,13 @@
     if (!nextRRset) {
         return (DataSrc::W_SUCCESS);
     }
-    RRsetPtr next_rrset = nextRRset(arg1, arg2);
+
+    RRset* next_rrset = nextRRset(arg1, arg2);
     while (next_rrset) {
-        result = addRRset(transaction, next_rrset);
+        result = addRRset(transaction, *next_rrset);
         if (result != DataSrc::W_SUCCESS) {
+            // Simply return, we are in a transaction which
+            // should be rolled back (see data_source.h)
             return (result);
         }
         next_rrset = nextRRset(arg1, arg2);
@@ -1160,10 +1176,11 @@
 DataSrc::WriteResult
 Sqlite3DataSrc::delZone(DataSrcTransaction& transaction) {
     if (transaction.getState() != DataSrcTransaction::RUNNING) {
-        return (DataSrc::W_ERROR);
-    }
-
-    int zone_id = transaction.getData()->get("zone_id")->intValue();
+        isc_throw(DataSourceTransactionError, "delZone called but "
+                      "transaction state not RUNNING");
+    }
+
+    const int zone_id = transaction.getData()->get("zone_id")->intValue();
 
     DataSrc::WriteResult result = delAll(zone_id);
     if (result != DataSrc::W_SUCCESS) {
@@ -1175,14 +1192,12 @@
     sqlite3_reset(query);
     sqlite3_clear_bindings(query);
 
-    int rc;
-    rc = sqlite3_bind_int(query, 1, zone_id);
-    if (rc != SQLITE_OK) {
+    if (sqlite3_bind_int(query, 1, zone_id) != SQLITE_OK) {
         isc_throw(Sqlite3Error, "Could not bind zone ID " << zone_id <<
                   " to SQL statement (query)");
     }
 
-    int q_result = sqlite3_step(query);
+    const int q_result = sqlite3_step(query);
 
     sqlite3_reset(query);
 
@@ -1195,29 +1210,32 @@
 
 DataSrc::WriteResult
 Sqlite3DataSrc::delRRset(DataSrcTransaction& transaction,
-                         isc::dns::ConstRRsetPtr rrset)
+                         const isc::dns::RRset& rrset)
 {
     if (transaction.getState() != DataSrcTransaction::RUNNING) {
-        return (DataSrc::W_ERROR);
+        isc_throw(DataSourceTransactionError, "delRRset called but "
+                      "transaction state not RUNNING");
     }
 
     int zone_id = transaction.getData()->get("zone_id")->intValue();
 
     DataSrc::WriteResult result = DataSrc::W_SUCCESS;
 
-    if (rrset->getRdataCount() > 0) {
-        RdataIteratorPtr rdp = rrset->getRdataIterator();
+    if (rrset.getRdataCount() > 0) {
+        RdataIteratorPtr rdp = rrset.getRdataIterator();
         rdp->first();
         do {
-            result = delRR(zone_id, rrset->getName(), rrset->getType(), rdp->getCurrent());
+            result = delRR(zone_id, rrset.getName(), rrset.getType(), rdp->getCurrent());
             if (result != DataSrc::W_SUCCESS) {
+                // Simply return, we are in a transaction which
+                // should be rolled back (see data_source.h)
                 return (result);
             } else {
                 rdp->next();
             }
         } while (!rdp->isLast());
     } else {
-        result = delRR(zone_id, rrset->getName(), rrset->getType());
+        result = delRR(zone_id, rrset.getName(), rrset.getType());
     }
     return (result);
 }

Modified: branches/trac374/src/lib/datasrc/sqlite3_datasrc.h
==============================================================================
--- branches/trac374/src/lib/datasrc/sqlite3_datasrc.h (original)
+++ branches/trac374/src/lib/datasrc/sqlite3_datasrc.h Thu Nov 25 10:03:59 2010
@@ -23,19 +23,20 @@
 
 #include <datasrc/data_source.h>
 
-#include <dns/name.h>
-#include <dns/rrttl.h>
-#include <dns/rdata.h>
-#include <dns/rrset.h>
-#include <dns/message.h>
-
 namespace isc {
 
 namespace dns {
 class Name;
 class RRClass;
 class RRType;
+class RRTTL;
+class RRset;
 class RRsetList;
+
+namespace rdata {
+class Rdata;
+}
+
 }
 
 namespace datasrc {
@@ -43,10 +44,13 @@
 class Query;
 struct Sqlite3Parameters;
 
-class Sqlite3Error : public Exception {
+/// This exception is thrown if there is a problem with the
+/// sqlite-specific calls (for instance if a call to sqlite3_bind()
+/// fails.
+class Sqlite3Error : public DataSourceError {
 public:
     Sqlite3Error(const char* file, size_t line, const char* what) :
-        isc::Exception(file, line, what) {}
+        DataSourceError(file, line, what) {}
 };
 
 class Sqlite3DataSrc : public DataSrc {
@@ -106,18 +110,20 @@
 
 
     // write access
-
+    // These functions may also raise Sqlite3Error, but since
+    // that is a subclass of DataSourceError, no extra handling
+    // should be required by the caller
     WriteResult startTransaction(DataSrcTransaction& transaction,
-                                 bool create_zone = false);
+                                 const bool create_zone = false);
     WriteResult commitTransaction(DataSrcTransaction& transaction);
     WriteResult rollbackTransaction(DataSrcTransaction& transaction);
 
     WriteResult addRRset(DataSrcTransaction& transaction,
-                               isc::dns::ConstRRsetPtr rrset);
+                         const isc::dns::RRset& rrset);
     WriteResult delRRset(DataSrcTransaction& transaction,
-                               isc::dns::ConstRRsetPtr rrset);
+                         const isc::dns::RRset& rrset);
     WriteResult replaceZone(DataSrcTransaction& transaction,
-                            isc::dns::RRsetPtr (*nextRRset)(void*, void*),
+                            isc::dns::RRset* (*nextRRset)(void*, void*),
                             void* arg1 = NULL, void* arg2 = NULL);
     WriteResult delZone(DataSrcTransaction& transaction);
 
@@ -130,12 +136,12 @@
     DataSrc::WriteResult addZone(const isc::dns::Name& name,
                                  const isc::dns::RRClass& rrclass);
     DataSrc::WriteResult delRR(int zone_id,
-                                     const isc::dns::Name& name,
-                                     const isc::dns::RRType& rrtype);
+                               const isc::dns::Name& name,
+                               const isc::dns::RRType& rrtype);
     DataSrc::WriteResult delRR(int zone_id,
-                                     const isc::dns::Name& name,
-                                     const isc::dns::RRType& rrtype,
-                                     const isc::dns::rdata::Rdata& rdata);
+                               const isc::dns::Name& name,
+                               const isc::dns::RRType& rrtype,
+                               const isc::dns::rdata::Rdata& rdata);
     DataSrc::WriteResult delAll(int zone_id);
 
 private:

Modified: branches/trac374/src/lib/datasrc/tests/sqlite3_unittest.cc
==============================================================================
--- branches/trac374/src/lib/datasrc/tests/sqlite3_unittest.cc (original)
+++ branches/trac374/src/lib/datasrc/tests/sqlite3_unittest.cc Thu Nov 25 10:03:59 2010
@@ -41,6 +41,8 @@
 #include <datasrc/data_source.h>
 #include <datasrc/sqlite3_datasrc.h>
 
+#include <boost/foreach.hpp>
+
 using namespace std;
 using namespace isc::dns;
 using namespace isc::dns::rdata;
@@ -58,7 +60,7 @@
     "{ \"database_file\": \"" TEST_DATA_DIR "/brokendb.sqlite3\"}");
 ConstElementPtr SQLITE_DBFILE_MEMORY = Element::fromJSON(
     "{ \"database_file\": \":memory:\"}");
-ElementPtr SQLITE_DBFILE_WRITE = Element::fromJSON(
+ConstElementPtr SQLITE_DBFILE_WRITE = Element::fromJSON(
     "{ \"database_file\": \"" TEST_DATA_OUT_DIR "/write_test.sqlite3\"}");
 
 // The following file must be non existent and must be non"creatable";
@@ -104,14 +106,33 @@
     REFERRAL
 } FindMode;
 
-class Sqlite3DataSourceTest : public ::testing::Test {
+class Sqlite3DataSourceTestBase : public ::testing::Test {
 protected:
-    Sqlite3DataSourceTest() : rrclass(RRClass::IN()),
-                              rrclass_notmatch(RRClass::CH()),
-                              rrtype(RRType::A()), rrttl(RRTTL(3600)),
-                              find_flags(0), rrset_matched(0), rrset_count(0),
-                              zone_name("example.com")
-    {
+    Sqlite3DataSourceTestBase() : rrclass(RRClass::IN()),
+                                  rrclass_notmatch(RRClass::CH()),
+                                  rrtype(RRType::A()),
+                                  rrttl(RRTTL(3600)), find_flags(0),
+                                  rrset_matched(0), rrset_count(0) {}
+
+    virtual ~Sqlite3DataSourceTestBase() {};
+
+    Sqlite3DataSrc data_source;
+
+    // we allow these to be modified in the test
+    RRClass rrclass;
+    RRClass rrclass_notmatch;
+    RRType rrtype;
+    RRTTL rrttl;
+    RRsetList result_sets;
+    uint32_t find_flags;
+    unsigned rrset_matched;
+    unsigned rrset_count;
+};
+
+class Sqlite3DataSourceTest : public Sqlite3DataSourceTestBase {
+
+protected:
+    Sqlite3DataSourceTest() {
         data_source.init(SQLITE_DBFILE_EXAMPLE);
 
         common_a_data.push_back("192.0.2.1");
@@ -180,24 +201,7 @@
         nsec3_sig_data.push_back("NSEC3 5 4 7200 20100410172647 "
                                  "20100311172647 63192 sql2.example.com. "
                                  + nsec3_signature);
-
-        // initialize data for writable tests
-        new_rrset = RRsetPtr(new RRset(Name("new_rr.example.com"),
-                                       RRClass::IN(),
-                                       RRType::A(),
-                                       RRTTL(3600)));
-        new_rrset->addRdata(createRdata(RRType::A(), RRClass::IN(), "192.0.2.3")); 
-    }
-    Sqlite3DataSrc data_source;
-    // we allow these to be modified in the test
-    RRClass rrclass;
-    RRClass rrclass_notmatch;
-    RRType rrtype;
-    RRTTL rrttl;
-    RRsetList result_sets;
-    uint32_t find_flags;
-    unsigned rrset_matched;
-    unsigned rrset_count;
+    }
 
     vector<RRType> types;
     vector<RRTTL> ttls;
@@ -243,12 +247,39 @@
     vector<string> nsec3_data;
     vector<string> nsec3_sig_data;
 
-    // common data for writable tests
-    Name zone_name;
-    RRsetPtr new_rrset;
-
     void findReferralRRsetCommon(const Name& qname, const RRClass& qclass);
     void findAddressRRsetCommon(const RRClass& qclass);
+};
+
+class Sqlite3DataSourceWriteTest : public Sqlite3DataSourceTestBase {
+    
+protected:
+    Sqlite3DataSourceWriteTest() : new_rrset(Name("new_rr.example.com"),
+                                            RRClass::IN(),
+                                            RRType::A(),
+                                            RRTTL(3600))
+    {
+        installWritableDatabase();
+        data_source.init(SQLITE_DBFILE_WRITE);
+        // initialize data for writable tests
+        new_rrset.addRdata(createRdata(RRType::A(), RRClass::IN(), "192.0.2.3")); 
+    }
+
+    //
+    // We copy one of the example databases to a writable one
+    // (the source one may not be writable, and we modify it, so if
+    // one of the tests fail halfway, we want to restart with a clean
+    // database next time)
+    //
+    int
+    installWritableDatabase()
+    {
+        return (system(INSTALL_PROG " " TEST_DATA_DIR "/test.sqlite3 "
+                       TEST_DATA_OUT_DIR "/write_test.sqlite3" ));
+    }
+
+    // common data for writable tests
+    RRset new_rrset;
 };
 
 void
@@ -954,78 +985,53 @@
     findAddressRRsetCommon(RRClass::ANY());
 }
 
-//
-// We copy one of the example databases to a writable one
-// (the source one may not be writable, and we modify it, so if
-// one of the tests fail halfway, we want to restart with a clean
-// database next time)
-//
-int
-install_writable_database()
-{
-    return (system(INSTALL_PROG " " TEST_DATA_DIR "/test.sqlite3 " TEST_DATA_OUT_DIR "/write_test.sqlite3" ));
-}
-
-TEST_F(Sqlite3DataSourceTest, Transactions) {
-    // first writables test, reset database
-    ASSERT_EQ(0, install_writable_database());
-
-    // use our copied writable datasource db
-    ASSERT_EQ(DataSrc::SUCCESS, data_source.close());
-    ASSERT_EQ(DataSrc::SUCCESS, data_source.init(SQLITE_DBFILE_WRITE));
-
+TEST_F(Sqlite3DataSourceWriteTest, Transactions) {
     DataSrcTransaction transaction(&data_source, zone_name, RRClass::IN());
-    EXPECT_EQ(DataSrcTransaction::INIT, transaction.getState());
+    ASSERT_EQ(DataSrcTransaction::INIT, transaction.getState());
 
     // start doing things without calling startTransaction()
-    EXPECT_EQ(DataSrc::W_ERROR,
-              data_source.commitTransaction(transaction));
-    EXPECT_EQ(DataSrc::W_ERROR,
-              data_source.rollbackTransaction(transaction));
-    EXPECT_EQ(DataSrc::W_ERROR,
-              data_source.addRRset(transaction, new_rrset));
+    EXPECT_THROW(data_source.commitTransaction(transaction),
+                 DataSourceTransactionError);
+    EXPECT_THROW(data_source.rollbackTransaction(transaction),
+                 DataSourceTransactionError);
+    EXPECT_THROW(data_source.addRRset(transaction, new_rrset),
+                 DataSourceTransactionError);
     RRsetIterator rrset_it = RRsetIterator();
-    EXPECT_EQ(DataSrc::W_ERROR,
-              data_source.replaceZone(transaction, NULL));
-    EXPECT_EQ(DataSrc::W_ERROR,
-              data_source.delZone(transaction));
-    EXPECT_EQ(DataSrc::W_ERROR,
-              data_source.delRRset(transaction, new_rrset));
-    // need rrsetit for doIXFR (but doIXFR should probably be moved up to datasource itself)
-
-    EXPECT_EQ(DataSrc::W_SUCCESS,
+    EXPECT_THROW(data_source.replaceZone(transaction, NULL),
+                 DataSourceTransactionError);
+    EXPECT_THROW(data_source.delZone(transaction),
+                 DataSourceTransactionError);
+    EXPECT_THROW(data_source.delRRset(transaction, new_rrset),
+                 DataSourceTransactionError);
+
+    ASSERT_EQ(DataSrc::W_SUCCESS,
               data_source.startTransaction(transaction));
-    EXPECT_EQ(DataSrcTransaction::RUNNING, transaction.getState());
-    EXPECT_EQ(DataSrc::W_ERROR,
-              data_source.startTransaction(transaction));
+    ASSERT_EQ(DataSrcTransaction::RUNNING, transaction.getState());
+    EXPECT_THROW(data_source.startTransaction(transaction),
+                 DataSourceTransactionError);
     EXPECT_EQ(DataSrc::W_SUCCESS,
               data_source.rollbackTransaction(transaction));
     EXPECT_EQ(DataSrcTransaction::DONE, transaction.getState());
 
     // state now 'done', everything should error again
-    EXPECT_EQ(DataSrc::W_ERROR,
-              data_source.commitTransaction(transaction));
-    EXPECT_EQ(DataSrc::W_ERROR,
-              data_source.rollbackTransaction(transaction));
-    EXPECT_EQ(DataSrc::W_ERROR,
-              data_source.addRRset(transaction, new_rrset));
-    EXPECT_EQ(DataSrc::W_ERROR,
-              data_source.replaceZone(transaction, NULL));
-    EXPECT_EQ(DataSrc::W_ERROR,
-              data_source.delZone(transaction));
-    EXPECT_EQ(DataSrc::W_ERROR,
-              data_source.delRRset(transaction, new_rrset));
-}
-
-TEST_F(Sqlite3DataSourceTest, addRRset) {
-
-    // use our copied writable datasource db
-    ASSERT_EQ(DataSrc::SUCCESS, data_source.close());
-    ASSERT_EQ(DataSrc::SUCCESS, data_source.init(SQLITE_DBFILE_WRITE));
-
+    EXPECT_THROW(data_source.commitTransaction(transaction),
+                 DataSourceTransactionError);
+    EXPECT_THROW(data_source.rollbackTransaction(transaction),
+                 DataSourceTransactionError);
+    EXPECT_THROW(data_source.addRRset(transaction, new_rrset),
+                 DataSourceTransactionError);
+    EXPECT_THROW(data_source.replaceZone(transaction, NULL),
+                 DataSourceTransactionError);
+    EXPECT_THROW(data_source.delZone(transaction),
+                 DataSourceTransactionError);
+    EXPECT_THROW(data_source.delRRset(transaction, new_rrset),
+                 DataSourceTransactionError);
+}
+
+TEST_F(Sqlite3DataSourceWriteTest, addRRset) {
     // check whether our new record does not exist
     EXPECT_EQ(DataSrc::SUCCESS,
-              data_source.findAddrs(new_rrset->getName(), new_rrset->getClass(),
+              data_source.findAddrs(new_rrset.getName(), new_rrset.getClass(),
                                     result_sets, find_flags, &zone_name));
     EXPECT_EQ(DataSrc::NAME_NOT_FOUND, find_flags);
 
@@ -1040,7 +1046,7 @@
 
     // check whether our new record does not exist
     EXPECT_EQ(DataSrc::SUCCESS,
-              data_source.findAddrs(new_rrset->getName(), new_rrset->getClass(),
+              data_source.findAddrs(new_rrset.getName(), new_rrset.getClass(),
                                     result_sets, find_flags, &zone_name));
     EXPECT_EQ(DataSrc::NAME_NOT_FOUND, find_flags);
 
@@ -1055,65 +1061,85 @@
 
     // now check whether it does exist
     EXPECT_EQ(DataSrc::SUCCESS,
-              data_source.findAddrs(new_rrset->getName(), new_rrset->getClass(),
+              data_source.findAddrs(new_rrset.getName(), new_rrset.getClass(),
                                     result_sets, find_flags, &zone_name));
     EXPECT_EQ(DataSrc::SUCCESS, find_flags);
     EXPECT_EQ(result_sets.size(), 1);
 }
 
-TEST_F(Sqlite3DataSourceTest, delRRset) {
-
-    // use our copied writable datasource db
-    ASSERT_EQ(DataSrc::SUCCESS, data_source.close());
-    ASSERT_EQ(DataSrc::SUCCESS, data_source.init(SQLITE_DBFILE_WRITE));
-
-    // check whether our new record exists (added by previous test)
-    EXPECT_EQ(DataSrc::SUCCESS,
-              data_source.findAddrs(new_rrset->getName(), new_rrset->getClass(),
+TEST_F(Sqlite3DataSourceWriteTest, delRRset) {
+    RRset del_rrset(Name("www.example.com."), RRClass::IN(),
+                    RRType::A(), RRTTL(3600));
+    del_rrset.addRdata(rdata::createRdata(RRType::A(), RRClass::IN(),
+                                          "192.0.2.1"));
+
+    // check whether our the record we will delete exists
+    ASSERT_EQ(DataSrc::SUCCESS,
+              data_source.findAddrs(del_rrset.getName(), del_rrset.getClass(),
                                     result_sets, find_flags, &zone_name));
     EXPECT_EQ(DataSrc::SUCCESS, find_flags);
     EXPECT_EQ(result_sets.size(), 1);
 
-    // add it, but roll back
+    // delete it, but roll back
     DataSrcTransaction transaction1(&data_source, zone_name, RRClass::IN());
-    EXPECT_EQ(DataSrc::W_SUCCESS,
+    ASSERT_EQ(DataSrc::W_SUCCESS,
               data_source.startTransaction(transaction1));
     EXPECT_EQ(DataSrc::W_SUCCESS,
-              data_source.delRRset(transaction1, new_rrset));
+              data_source.delRRset(transaction1, del_rrset));
     EXPECT_EQ(DataSrc::W_SUCCESS,
               data_source.rollbackTransaction(transaction1));
 
     // check whether our new record still exists
-    EXPECT_EQ(DataSrc::SUCCESS,
-              data_source.findAddrs(new_rrset->getName(), new_rrset->getClass(),
+    ASSERT_EQ(DataSrc::SUCCESS,
+              data_source.findAddrs(del_rrset.getName(), del_rrset.getClass(),
                                     result_sets, find_flags, &zone_name));
     EXPECT_EQ(DataSrc::SUCCESS, find_flags);
     EXPECT_EQ(result_sets.size(), 1);
 
-    // add it, and commit
+    // delete it, and commit
     DataSrcTransaction transaction2(&data_source, zone_name, RRClass::IN());
+    ASSERT_EQ(DataSrc::W_SUCCESS,
+              data_source.startTransaction(transaction2));
+    ASSERT_EQ(DataSrc::W_SUCCESS,
+              data_source.delRRset(transaction2, del_rrset));
+    ASSERT_EQ(DataSrc::W_SUCCESS,
+              data_source.commitTransaction(transaction2));
+
+    // now check whether it does not exist now
+    EXPECT_EQ(DataSrc::SUCCESS,
+              data_source.findAddrs(del_rrset.getName(), del_rrset.getClass(),
+                                    result_sets, find_flags, &zone_name));
+    EXPECT_EQ(DataSrc::SUCCESS, find_flags);
+    EXPECT_EQ(result_sets.size(), 1);
+
+    // delete all rrsets for this name
+    RRset del_name(Name("www.example.com."), RRClass::IN(),
+                   RRType::ANY(), RRTTL(3600));
+    DataSrcTransaction transaction3(&data_source, zone_name, RRClass::IN());
+    ASSERT_EQ(DataSrc::W_SUCCESS,
+              data_source.startTransaction(transaction3));
+    ASSERT_EQ(DataSrc::W_SUCCESS,
+              data_source.delRRset(transaction3, del_name));
+    ASSERT_EQ(DataSrc::W_SUCCESS,
+              data_source.commitTransaction(transaction3));
+
+
+    EXPECT_EQ(DataSrc::SUCCESS,
+              data_source.findAddrs(del_rrset.getName(), del_rrset.getClass(),
+                                    result_sets, find_flags, &zone_name));
+    EXPECT_EQ(DataSrc::NAME_NOT_FOUND, find_flags);
+
+    // deleting it again should fail
+    DataSrcTransaction transaction4(&data_source, zone_name, RRClass::IN());
     EXPECT_EQ(DataSrc::W_SUCCESS,
-              data_source.startTransaction(transaction2));
+              data_source.startTransaction(transaction4));
+    EXPECT_EQ(DataSrc::W_NO_SUCH_DATA,
+              data_source.delRRset(transaction4, del_rrset));
     EXPECT_EQ(DataSrc::W_SUCCESS,
-              data_source.delRRset(transaction2, new_rrset));
-    EXPECT_EQ(DataSrc::W_SUCCESS,
-              data_source.commitTransaction(transaction2));
-
-    // now check whether it does not exist now
-    EXPECT_EQ(DataSrc::SUCCESS,
-              data_source.findAddrs(new_rrset->getName(), new_rrset->getClass(),
-                                    result_sets, find_flags, &zone_name));
-    EXPECT_EQ(DataSrc::NAME_NOT_FOUND, find_flags);
-}
-
-TEST_F(Sqlite3DataSourceTest, replaceZone_container) {
-    // reset database
-    ASSERT_EQ(0, install_writable_database());
-
-    // use our copied writable datasource db
-    ASSERT_EQ(DataSrc::SUCCESS, data_source.close());
-    ASSERT_EQ(DataSrc::SUCCESS, data_source.init(SQLITE_DBFILE_WRITE));
-
+              data_source.rollbackTransaction(transaction4));
+}
+
+TEST_F(Sqlite3DataSourceWriteTest, replaceZone_container) {
     // check whether an A exists
     EXPECT_EQ(DataSrc::SUCCESS,
               data_source.findRRset(www_name, RRClass::IN(), RRType::A(),
@@ -1124,8 +1150,8 @@
     // Replace them, roll back
     DataSrcTransaction transaction1(&data_source, zone_name, RRClass::IN());
     ASSERT_EQ(DataSrc::W_SUCCESS, data_source.startTransaction(transaction1));
-    ASSERT_EQ(DataSrc::W_SUCCESS, data_source.replaceZone(transaction1, NULL));
-    ASSERT_EQ(DataSrc::W_SUCCESS, data_source.rollbackTransaction(transaction1));
+    EXPECT_EQ(DataSrc::W_SUCCESS, data_source.replaceZone(transaction1, NULL));
+    EXPECT_EQ(DataSrc::W_SUCCESS, data_source.rollbackTransaction(transaction1));
 
     // check whether it still exists
     EXPECT_EQ(DataSrc::SUCCESS,
@@ -1148,19 +1174,12 @@
     EXPECT_EQ(1, result_sets.size());
 }
 
-RRsetPtr
-getRRsetCallback_empty(void *arg1 UNUSED_PARAM, void* arg2 UNUSED_PARAM) {
-    return (RRsetPtr());
-}
-
-TEST_F(Sqlite3DataSourceTest, replaceZone_callback_empty) {
-    // reset database
-    ASSERT_EQ(0, install_writable_database());
-
-    // use our copied writable datasource db
-    ASSERT_EQ(DataSrc::SUCCESS, data_source.close());
-    ASSERT_EQ(DataSrc::SUCCESS, data_source.init(SQLITE_DBFILE_WRITE));
-
+RRset*
+getRRsetCallbackEmpty(void *arg1 UNUSED_PARAM, void* arg2 UNUSED_PARAM) {
+    return (NULL);
+}
+
+TEST_F(Sqlite3DataSourceWriteTest, replaceZoneCallbackEmpty) {
     // check whether an A exists
     EXPECT_EQ(DataSrc::SUCCESS,
               data_source.findRRset(www_name, RRClass::IN(), RRType::A(),
@@ -1171,8 +1190,8 @@
     // Replace them, roll back
     DataSrcTransaction transaction1(&data_source, zone_name, RRClass::IN());
     ASSERT_EQ(DataSrc::W_SUCCESS, data_source.startTransaction(transaction1));
-    ASSERT_EQ(DataSrc::W_SUCCESS, data_source.replaceZone(transaction1, getRRsetCallback_empty));
-    ASSERT_EQ(DataSrc::W_SUCCESS, data_source.rollbackTransaction(transaction1));
+    EXPECT_EQ(DataSrc::W_SUCCESS, data_source.replaceZone(transaction1, getRRsetCallbackEmpty));
+    EXPECT_EQ(DataSrc::W_SUCCESS, data_source.rollbackTransaction(transaction1));
 
     // check whether it still exists
     EXPECT_EQ(DataSrc::SUCCESS,
@@ -1184,7 +1203,7 @@
     // Replace them, commit
     DataSrcTransaction transaction2(&data_source, zone_name, RRClass::IN());
     ASSERT_EQ(DataSrc::W_SUCCESS, data_source.startTransaction(transaction2));
-    ASSERT_EQ(DataSrc::W_SUCCESS, data_source.replaceZone(transaction2, getRRsetCallback_empty));
+    ASSERT_EQ(DataSrc::W_SUCCESS, data_source.replaceZone(transaction2, getRRsetCallbackEmpty));
     ASSERT_EQ(DataSrc::W_SUCCESS, data_source.commitTransaction(transaction2));
 
     // check whether it's gone now
@@ -1196,45 +1215,43 @@
 
 }
 
-TEST_F(Sqlite3DataSourceTest, replaceZone_new_zone) {
-    // reset database
-    ASSERT_EQ(0, install_writable_database());
-
-    // use our copied writable datasource db
-    ASSERT_EQ(DataSrc::SUCCESS, data_source.close());
-    ASSERT_EQ(DataSrc::SUCCESS, data_source.init(SQLITE_DBFILE_WRITE));
-
-    std::vector<RRsetPtr> rrsets;
-
-    RRsetPtr soa_rrset = RRsetPtr(new RRset(Name("new.example.com"),
-                                  RRClass::IN(),
-                                  RRType::SOA(),
-                                  RRTTL(3600)));
-    RdataPtr soa_rdata = isc::dns::rdata::createRdata(RRType::SOA(),
-                                                      RRClass::IN(),
-                                                      "new.example.com. new.example.com. 760 28800 7200 604800 3600");
+TEST_F(Sqlite3DataSourceWriteTest, replaceZoneNewZone) {
+    std::vector<ConstRRsetPtr> rrsets;
+
+    RRsetPtr soa_rrset(new RRset(Name("new.example.com"),
+                                 RRClass::IN(),
+                                 RRType::SOA(),
+                                 RRTTL(3600)));
+    RdataPtr soa_rdata = createRdata(RRType::SOA(),
+                                     RRClass::IN(),
+                                     "new.example.com. new.example.com."
+                                     " 760 28800 7200 604800 3600");
     soa_rrset->addRdata(soa_rdata);
     rrsets.push_back(soa_rrset);
 
-    RRsetPtr a_rrset = RRsetPtr(new RRset(Name("new.example.com"),
-                                RRClass::IN(),
-                                RRType::A(),
-                                RRTTL(3600)));
-    RdataPtr a_rdata = isc::dns::rdata::createRdata(RRType::A(),
-                                                    RRClass::IN(),
-                                                    "192.0.2.1");
+    RRsetPtr a_rrset(new RRset(Name("new.example.com"),
+                               RRClass::IN(),
+                               RRType::A(),
+                               RRTTL(3600)));
+    RdataPtr a_rdata = createRdata(RRType::A(),
+                                   RRClass::IN(),
+                                   "192.0.2.1");
     a_rrset->addRdata(a_rdata);
     rrsets.push_back(a_rrset);
 
     // Try to start transaction without create_zone set to true
-    DataSrcTransaction transaction1(&data_source, soa_rrset->getName(), RRClass::IN());
-    ASSERT_EQ(DataSrc::W_NO_SUCH_ZONE, data_source.startTransaction(transaction1));
+    DataSrcTransaction transaction1(&data_source, soa_rrset->getName(),
+                                    RRClass::IN());
+    EXPECT_EQ(DataSrc::W_NO_SUCH_ZONE,
+              data_source.startTransaction(transaction1));
 
     // Replace them, commit
     size_t i = 0;
 
-    DataSrcTransaction transaction2(&data_source, soa_rrset->getName(), RRClass::IN());
-    ASSERT_EQ(DataSrc::W_SUCCESS, data_source.startTransaction(transaction2, true));
+    DataSrcTransaction transaction2(&data_source, soa_rrset->getName(),
+                                    RRClass::IN());
+    ASSERT_EQ(DataSrc::W_SUCCESS,
+              data_source.startTransaction(transaction2, true));
     ASSERT_EQ(DataSrc::W_SUCCESS,
               data_source.replaceZone(transaction2,
                                       isc::datasrc::callbackHelperRRsetVector,
@@ -1245,19 +1262,12 @@
     EXPECT_EQ(DataSrc::SUCCESS,
               data_source.findExactRRset(a_rrset->getName(), a_rrset->getClass(), a_rrset->getType(),
                                     result_sets, find_flags, &a_rrset->getName()));
-    ASSERT_EQ(DataSrc::SUCCESS, find_flags);
-    ASSERT_EQ(1, result_sets.size());
-}
-
-
-TEST_F(Sqlite3DataSourceTest, delZone) {
-    // reset database
-    ASSERT_EQ(0, install_writable_database());
-
-    // use our copied writable datasource db
-    ASSERT_EQ(DataSrc::SUCCESS, data_source.close());
-    ASSERT_EQ(DataSrc::SUCCESS, data_source.init(SQLITE_DBFILE_WRITE));
-
+    EXPECT_EQ(DataSrc::SUCCESS, find_flags);
+    EXPECT_EQ(1, result_sets.size());
+}
+
+
+TEST_F(Sqlite3DataSourceWriteTest, delZone) {
     // check whether an A exists
     EXPECT_EQ(DataSrc::SUCCESS,
               data_source.findRRset(www_name, RRClass::IN(), RRType::A(),
@@ -1268,8 +1278,8 @@
     // Delete zone, roll back
     DataSrcTransaction transaction1(&data_source, zone_name, RRClass::IN());
     ASSERT_EQ(DataSrc::W_SUCCESS, data_source.startTransaction(transaction1));
-    ASSERT_EQ(DataSrc::W_SUCCESS, data_source.delZone(transaction1));
-    ASSERT_EQ(DataSrc::W_SUCCESS, data_source.rollbackTransaction(transaction1));
+    EXPECT_EQ(DataSrc::W_SUCCESS, data_source.delZone(transaction1));
+    EXPECT_EQ(DataSrc::W_SUCCESS, data_source.rollbackTransaction(transaction1));
 
     // check whether it still exists
     EXPECT_EQ(DataSrc::SUCCESS,
@@ -1294,9 +1304,21 @@
     // try to start a transaction for the zone that is now gone
     DataSrcTransaction transaction3(&data_source, zone_name, RRClass::IN());
     EXPECT_EQ(DataSrc::W_NO_SUCH_ZONE, data_source.startTransaction(transaction3));
-}
-
-QuestionPtr
+
+    // if we try to delete it again, it should already fail on
+    // startTransaction
+    DataSrcTransaction transaction4(&data_source, zone_name, RRClass::IN());
+    EXPECT_EQ(DataSrc::W_NO_SUCH_ZONE, data_source.startTransaction(transaction4));
+}
+
+
+//
+// Helper functions. More generalized versions of these should probably
+// go into the main libraries somewhere. For simplicity here,
+// we omitted some error checks (like after >>)
+//
+
+ConstQuestionPtr
 questionFromFile(ifstream& file) {
     string s;
     file >> s;
@@ -1316,7 +1338,7 @@
     string line;
     getline(file, line);
 
-    QuestionPtr question(new Question(n, rrclass, rrtype));
+    ConstQuestionPtr question(new Question(n, rrclass, rrtype));
     return (question);
 }
 
@@ -1355,10 +1377,7 @@
         rrtype = RRType(s);
     }
 
-    RRsetPtr rrset = RRsetPtr(new RRset(n,
-                              rrclass,
-                              rrtype,
-                              ttl));
+    RRsetPtr rrset(new RRset(n, rrclass, rrtype, ttl));
     string line;
     getline(file, line);
     while (line[0] == ' ' || line[0] == '\t') {
@@ -1372,42 +1391,41 @@
     return (rrset);
 }
 
-// initialize the message with flags and codes,
-// read a list of rrs and put them in that order in the answer
-// section of the given message
 
 // reads rrsets from the given file into the given section
 // stops at eof or if empty line or line starting with ; is
 // read
 // returns 1 if empty line/comment has been seen
 // returns 0 if eof reached
-
 int
 rrsetsFromFile(ifstream& file, Message& msg, Section section) {
     RRsetPtr prev_rrset = RRsetPtr();
     while (! file.eof() ) {
-        RRsetPtr rrset = rrsetFromFile(file);
+        const RRsetPtr rrset = rrsetFromFile(file);
+        // SOA is treated as a different type; we want SOA records
+        // to show up as separate RRsets for correct handling
+        const bool same_type =
+            (rrset && prev_rrset &&
+            prev_rrset->getName() == rrset->getName() &&
+            prev_rrset->getType() == rrset->getType() &&
+            prev_rrset->getType() != RRType::SOA() &&
+            prev_rrset->getClass() == rrset->getClass());
+        // if this is the end of the section or we've got a new type of RRset,
+        // add the RRset we've constructed so far, if any.
+        if (prev_rrset && (!rrset || !same_type)) {
+            msg.addRRset(section, prev_rrset);
+        }
+        // if this is the end of the section we are done.
         if (!rrset) {
-            if (prev_rrset) {
-                msg.addRRset(section, prev_rrset);
-            }
             return (1);
         }
-        if (prev_rrset) {
-            if (rrset) {
-                if (prev_rrset->getName() == rrset->getName() &&
-                    prev_rrset->getType() == rrset->getType() &&
-                    prev_rrset->getType() != RRType::SOA() &&
-                    prev_rrset->getClass() == rrset->getClass()) {
-                    RdataIteratorPtr it = rrset->getRdataIterator();
-                    for (it->first(); !it->isLast(); it->next()) {
-                        prev_rrset->addRdata(it->getCurrent());
-                    }
-                    rrset = RRsetPtr();
-                }
+        // update prev_rrset: if this is a new RR of the same type, merge it.
+        // otherwise replace it.
+        if (same_type) {
+            RdataIteratorPtr it = rrset->getRdataIterator();
+            for (it->first(); !it->isLast(); it->next()) {
+                prev_rrset->addRdata(it->getCurrent());
             }
-            msg.addRRset(section, prev_rrset);
-            prev_rrset = rrset;
         } else {
             prev_rrset = rrset;
         }
@@ -1416,31 +1434,34 @@
 }
 
 int
-rrsetsFromFile(ifstream& file, std::vector<RRsetPtr>& container) {
+rrsetsFromFile(ifstream& file, std::vector<ConstRRsetPtr>& container) {
     RRsetPtr prev_rrset = RRsetPtr();
     while (! file.eof() ) {
-        RRsetPtr rrset = rrsetFromFile(file);
+        const RRsetPtr rrset = rrsetFromFile(file);
+        // SOA is treated as a different type; we want SOA records
+        // to show up as separate RRsets for correct handling
+        const bool same_type =
+            (rrset && prev_rrset &&
+            prev_rrset->getName() == rrset->getName() &&
+            prev_rrset->getType() == rrset->getType() &&
+            prev_rrset->getType() != RRType::SOA() &&
+            prev_rrset->getClass() == rrset->getClass());
+        // if this is the end of the section or we've got a new type of RRset,
+        // add the RRset we've constructed so far, if any.
+        if (prev_rrset && (!rrset || !same_type)) {
+            container.push_back(prev_rrset);
+        }
+        // if this is the end of the section we are done.
         if (!rrset) {
-            if (prev_rrset) {
-                container.push_back(prev_rrset);
-            }
             return (1);
         }
-        if (prev_rrset) {
-            if (rrset) {
-                if (prev_rrset->getName() == rrset->getName() &&
-                    prev_rrset->getType() == rrset->getType() &&
-                    prev_rrset->getType() != RRType::SOA() &&
-                    prev_rrset->getClass() == rrset->getClass()) {
-                    RdataIteratorPtr it = rrset->getRdataIterator();
-                    for (it->first(); !it->isLast(); it->next()) {
-                        prev_rrset->addRdata(it->getCurrent());
-                    }
-                    rrset = RRsetPtr();
-                }
+        // update prev_rrset: if this is a new RR of the same type, merge it.
+        // otherwise replace it.
+        if (same_type) {
+            RdataIteratorPtr it = rrset->getRdataIterator();
+            for (it->first(); !it->isLast(); it->next()) {
+                prev_rrset->addRdata(it->getCurrent());
             }
-            container.push_back(prev_rrset);
-            prev_rrset = rrset;
         } else {
             prev_rrset = rrset;
         }
@@ -1449,7 +1470,7 @@
 }
 
 int
-rrsetsFromFile(const char* file_name, std::vector<RRsetPtr>& container) {
+rrsetsFromFile(const char* file_name, std::vector<ConstRRsetPtr>& container) {
     ifstream myfile(file_name);
     if (myfile.is_open()) {
         rrsetsFromFile(myfile, container);
@@ -1487,14 +1508,14 @@
     m.setOpcode(isc::dns::Opcode::UPDATE());
     m.setRcode(isc::dns::Rcode::NOERROR());
     int stage = 1;
-    QuestionPtr question;
+    ConstQuestionPtr question;
     if (myfile.is_open()) {
         while (! myfile.eof() ) {
             switch (stage) {
             case 1:
                 question = questionFromFile(myfile);
                 if (question) {
-                    m.addQuestion(question);
+                    m.addQuestion(*question);
                 } else {
                     stage++;
                 }
@@ -1540,14 +1561,12 @@
     }
 }
 
-TEST_F(Sqlite3DataSourceTest, ixfr_ok_message) {
-    // reset database
-    ASSERT_EQ(0, install_writable_database());
-
-    // use our copied writable datasource db
-    ASSERT_EQ(DataSrc::SUCCESS, data_source.close());
-    ASSERT_EQ(DataSrc::SUCCESS, data_source.init(SQLITE_DBFILE_WRITE));
-
+//
+// End of helper functions
+//
+
+
+TEST_F(Sqlite3DataSourceWriteTest, ixfr_ok_message) {
     checkSingleRRset(data_source,
                      "www.example.com. 3600 IN A 192.0.2.1\n",
                      www_name, RRClass::IN(), RRType::A());
@@ -1567,7 +1586,7 @@
                                  &begin,
                                  &end)
     );
-    ASSERT_EQ(DataSrc::W_SUCCESS, data_source.rollbackTransaction(transaction1));
+    EXPECT_EQ(DataSrc::W_SUCCESS, data_source.rollbackTransaction(transaction1));
 
     checkSingleRRset(data_source,
                      "www.example.com. 3600 IN A 192.0.2.1\n",
@@ -1601,33 +1620,30 @@
                                  &begin,
                                  &end)
     );
-    ASSERT_EQ(DataSrc::W_SUCCESS, data_source.rollbackTransaction(transaction3));
-}
-
-TEST_F(Sqlite3DataSourceTest, ixfr_ok_function) {
-    // reset database
-    ASSERT_EQ(0, install_writable_database());
-
-    // use our copied writable datasource db
-    ASSERT_EQ(DataSrc::SUCCESS, data_source.close());
-    ASSERT_EQ(DataSrc::SUCCESS, data_source.init(SQLITE_DBFILE_WRITE));
-
+    EXPECT_EQ(DataSrc::W_SUCCESS, data_source.rollbackTransaction(transaction3));
+}
+
+TEST_F(Sqlite3DataSourceWriteTest, ixfr_ok_function) {
     checkSingleRRset(data_source,
                      "www.example.com. 3600 IN A 192.0.2.1\n",
                      www_name, RRClass::IN(), RRType::A());
 
-    std::vector<RRsetPtr> rrsets;
+    std::vector<ConstRRsetPtr> rrsets;
     ASSERT_EQ(0, rrsetsFromFile(TEST_DATA_DIR "/ixfr_ok.rrs", rrsets));
 
     // do IXFR, roll back
     size_t i = 0;
     DataSrcTransaction transaction1(&data_source, zone_name, RRClass::IN());
     ASSERT_EQ(DataSrc::W_SUCCESS, data_source.startTransaction(transaction1));
-    ASSERT_EQ(DataSrc::W_SUCCESS,
+    EXPECT_THROW(data_source.doIXFR(transaction1,
+                                 NULL, &rrsets, &i),
+                 DataSourceError);
+    EXPECT_EQ(DataSrc::W_SUCCESS,
               data_source.doIXFR(transaction1,
-                                 isc::datasrc::callbackHelperRRsetVector, &rrsets, &i)
+                                 isc::datasrc::callbackHelperRRsetVector,
+                                 &rrsets, &i)
     );
-    ASSERT_EQ(DataSrc::W_SUCCESS, data_source.rollbackTransaction(transaction1));
+    EXPECT_EQ(DataSrc::W_SUCCESS, data_source.rollbackTransaction(transaction1));
 
     checkSingleRRset(data_source,
                      "www.example.com. 3600 IN A 192.0.2.1\n",
@@ -1651,21 +1667,14 @@
     i = 0;
     DataSrcTransaction transaction3(&data_source, zone_name, RRClass::IN());
     ASSERT_EQ(DataSrc::W_SUCCESS, data_source.startTransaction(transaction3));
-    ASSERT_EQ(DataSrc::W_ERROR,
+    EXPECT_EQ(DataSrc::W_ERROR,
               data_source.doIXFR(transaction3,
                                  isc::datasrc::callbackHelperRRsetVector, &rrsets, &i)
     );
-    ASSERT_EQ(DataSrc::W_SUCCESS, data_source.rollbackTransaction(transaction3));
-}
-
-TEST_F(Sqlite3DataSourceTest, ixfr_bad) {
-    // reset database
-    ASSERT_EQ(0, install_writable_database());
-
-    // use our copied writable datasource db
-    ASSERT_EQ(DataSrc::SUCCESS, data_source.close());
-    ASSERT_EQ(DataSrc::SUCCESS, data_source.init(SQLITE_DBFILE_WRITE));
-
+    EXPECT_EQ(DataSrc::W_SUCCESS, data_source.rollbackTransaction(transaction3));
+}
+
+TEST_F(Sqlite3DataSourceWriteTest, ixfr_bad) {
     // a bad ixfr, tries to remove a non-existant RR
     Message ixfr_msg(Message::RENDER);
     ASSERT_EQ(0, ixfrFromFile(ixfr_msg, TEST_DATA_DIR
@@ -1681,17 +1690,26 @@
                                  &begin,
                                  &end)
     );
-    ASSERT_EQ(DataSrc::W_SUCCESS, data_source.rollbackTransaction(transaction1));
-}
-
-TEST_F(Sqlite3DataSourceTest, dynamic_update) {
-    // reset database
-    ASSERT_EQ(0, install_writable_database());
-
-    // use our copied writable datasource db
-    ASSERT_EQ(DataSrc::SUCCESS, data_source.close());
-    ASSERT_EQ(DataSrc::SUCCESS, data_source.init(SQLITE_DBFILE_WRITE));
-
+    EXPECT_EQ(DataSrc::W_SUCCESS, data_source.rollbackTransaction(transaction1));
+}
+
+TEST_F(Sqlite3DataSourceWriteTest, dynamic_update_check_prerequisite) {
+    DataSrcTransaction transaction(&data_source, zone_name, RRClass::IN());
+
+    // Test name exists
+    RRset prereq1(Name("www.example.com"), RRClass::ANY(), RRType::ANY(), RRTTL(0));
+    EXPECT_EQ(Rcode::NOERROR(), data_source.updateCheckPrerequisite(transaction, prereq1));
+
+    // Test empty non-terminal does not
+    RRset prereq2(Name("ent.example.com"), RRClass::ANY(), RRType::ANY(), RRTTL(0));
+    EXPECT_EQ(Rcode::NXDOMAIN(), data_source.updateCheckPrerequisite(transaction, prereq2));
+
+    // But the data that makes it should
+    RRset prereq3(Name("a.ent.example.com"), RRClass::ANY(), RRType::ANY(), RRTTL(0));
+    EXPECT_EQ(Rcode::NOERROR(), data_source.updateCheckPrerequisite(transaction, prereq3));
+}
+
+TEST_F(Sqlite3DataSourceWriteTest, dynamic_update) {
     Message update_msg(Message::RENDER);
 
     DataSrcTransaction transaction1(&data_source, zone_name, RRClass::IN());
@@ -1700,24 +1718,10 @@
 
     // bad qtype
     update_msg.setOpcode(isc::dns::Opcode::QUERY());
-    ASSERT_EQ(DataSrc::W_ERROR, data_source.doUpdate(transaction1, update_msg));
+    EXPECT_THROW(data_source.doUpdate(transaction1, update_msg),
+                 DataSourceError);
 
     update_msg.setOpcode(isc::dns::Opcode::UPDATE());
-
-    // no/bad question section
-    ASSERT_EQ(DataSrc::W_ERROR, data_source.doUpdate(transaction1, update_msg));
-
-    QuestionPtr bad_question(new Question(Name("example.com"), RRClass::CH(), RRType::A()));
-    update_msg.addQuestion(bad_question);
-    ASSERT_EQ(DataSrc::W_ERROR, data_source.doUpdate(transaction1, update_msg));
-
-    // removeQuestion not implemented, reset msg
-    // checks, we delete one set, one name, one rr, add a set, and add to a set
-    // delete ip46 A rrset
-    // delete all from www.example.com
-    // delete 1 mx record
-    // add newaddr.example.com 192.0.2.21
-    // add dns04.example.com
 
     // check if they are (not) there first
     checkSingleRRset(data_source,
@@ -1741,7 +1745,7 @@
                      Name("example.com"), RRClass::IN(), RRType::NS());
 
     ASSERT_EQ(0, updateFromFile(update_msg, TEST_DATA_DIR "/update1.packet"));
-    ASSERT_EQ(DataSrc::W_SUCCESS,
+    ASSERT_EQ(Rcode::NOERROR(),
               data_source.doUpdate(transaction1, update_msg));
 
     ASSERT_EQ(DataSrc::W_SUCCESS, data_source.commitTransaction(transaction1));
@@ -1766,14 +1770,7 @@
                      Name("example.com"), RRClass::IN(), RRType::NS());
 }
 
-TEST_F(Sqlite3DataSourceTest, dynamic_update_bad_class) {
-    // reset database
-    ASSERT_EQ(0, install_writable_database());
-
-    // use our copied writable datasource db
-    ASSERT_EQ(DataSrc::SUCCESS, data_source.close());
-    ASSERT_EQ(DataSrc::SUCCESS, data_source.init(SQLITE_DBFILE_WRITE));
-
+TEST_F(Sqlite3DataSourceWriteTest, dynamic_update_bad_class) {
     Message update_msg(Message::RENDER);
 
     DataSrcTransaction transaction1(&data_source, zone_name, RRClass::CH());
@@ -1782,59 +1779,48 @@
 
     update_msg.setOpcode(isc::dns::Opcode::UPDATE());
 
-    // no/bad question section
-    ASSERT_EQ(DataSrc::W_ERROR, data_source.doUpdate(transaction1, update_msg));
-}
-
-TEST_F(Sqlite3DataSourceTest, dynamic_update_prereq_fails) {
-    // reset database
-    ASSERT_EQ(0, install_writable_database());
-
-    // use our copied writable datasource db
-    ASSERT_EQ(DataSrc::SUCCESS, data_source.close());
-    ASSERT_EQ(DataSrc::SUCCESS, data_source.init(SQLITE_DBFILE_WRITE));
-
+    // no question section
+    EXPECT_EQ(Rcode::FORMERR(), data_source.doUpdate(transaction1, update_msg));
+
+    // bad class
+    QuestionPtr bad_question(new Question(Name("example.com"),
+                             RRClass::CH(), RRType::A()));
+    update_msg.addQuestion(bad_question);
+    EXPECT_EQ(Rcode::FORMERR(),
+              data_source.doUpdate(transaction1, update_msg));
+
+}
+
+TEST_F(Sqlite3DataSourceWriteTest, dynamic_update_prereq_fails) {
     Message update_msg(Message::RENDER);
 
     DataSrcTransaction transaction1(&data_source, zone_name, RRClass::IN());
     ASSERT_EQ(DataSrc::W_SUCCESS, data_source.startTransaction(transaction1));
 
     ASSERT_EQ(0, updateFromFile(update_msg, TEST_DATA_DIR "/update2.packet"));
-    ASSERT_EQ(DataSrc::W_ERROR, data_source.doUpdate(transaction1, update_msg));
-    ASSERT_EQ(Rcode::NXRRSET(), update_msg.getRcode());
+    EXPECT_EQ(Rcode::NXRRSET(), data_source.doUpdate(transaction1, update_msg));
 
     ASSERT_EQ(0, updateFromFile(update_msg, TEST_DATA_DIR "/update3.packet"));
-    ASSERT_EQ(DataSrc::W_ERROR, data_source.doUpdate(transaction1, update_msg));
-    ASSERT_EQ(Rcode::NXDOMAIN(), update_msg.getRcode());
+    EXPECT_EQ(Rcode::NXDOMAIN(), data_source.doUpdate(transaction1, update_msg));
 
     ASSERT_EQ(0, updateFromFile(update_msg, TEST_DATA_DIR "/update4.packet"));
-    ASSERT_EQ(DataSrc::W_ERROR, data_source.doUpdate(transaction1, update_msg));
-    ASSERT_EQ(Rcode::NXRRSET(), update_msg.getRcode());
+    EXPECT_EQ(Rcode::NXRRSET(), data_source.doUpdate(transaction1, update_msg));
 
     ASSERT_EQ(0, updateFromFile(update_msg, TEST_DATA_DIR "/update5.packet"));
-    ASSERT_EQ(DataSrc::W_ERROR, data_source.doUpdate(transaction1, update_msg));
-    ASSERT_EQ(Rcode::YXRRSET(), update_msg.getRcode());
+    EXPECT_EQ(Rcode::YXRRSET(), data_source.doUpdate(transaction1, update_msg));
 
     ASSERT_EQ(0, updateFromFile(update_msg, TEST_DATA_DIR "/update6.packet"));
-    ASSERT_EQ(DataSrc::W_ERROR, data_source.doUpdate(transaction1, update_msg));
-    ASSERT_EQ(Rcode::YXDOMAIN(), update_msg.getRcode());
-}
-
-TEST_F(Sqlite3DataSourceTest, dynamic_update_bad_update) {
-    // reset database
-    ASSERT_EQ(0, install_writable_database());
-
-    // use our copied writable datasource db
-    ASSERT_EQ(DataSrc::SUCCESS, data_source.close());
-    ASSERT_EQ(DataSrc::SUCCESS, data_source.init(SQLITE_DBFILE_WRITE));
-
+    EXPECT_EQ(Rcode::YXDOMAIN(), data_source.doUpdate(transaction1, update_msg));
+}
+
+TEST_F(Sqlite3DataSourceWriteTest, dynamic_update_bad_update) {
     Message update_msg(Message::RENDER);
 
     DataSrcTransaction transaction1(&data_source, zone_name, RRClass::IN());
     ASSERT_EQ(DataSrc::W_SUCCESS, data_source.startTransaction(transaction1));
 
     ASSERT_EQ(0, updateFromFile(update_msg, TEST_DATA_DIR "/update7.packet"));
-    ASSERT_EQ(DataSrc::W_ERROR, data_source.doUpdate(transaction1, update_msg));
-}
-
-}
+    EXPECT_EQ(Rcode::SERVFAIL(), data_source.doUpdate(transaction1, update_msg));
+}
+
+}

Modified: branches/trac374/src/lib/datasrc/tests/testdata/test.sqlite3
==============================================================================
Binary files - no diff available.

Modified: branches/trac374/src/lib/datasrc/tests/testdata/update4.packet
==============================================================================
--- branches/trac374/src/lib/datasrc/tests/testdata/update4.packet (original)
+++ branches/trac374/src/lib/datasrc/tests/testdata/update4.packet Thu Nov 25 10:03:59 2010
@@ -1,5 +1,5 @@
 example.com IN  SOA
-; third prereq should fail
+; third prereq (rrset) should fail
 ip46.example.com 0 ANY A
 www.example.com 0 ANY ANY
 example.com 0 IN MX 10 mail.example.com.

Modified: branches/trac374/src/lib/datasrc/tests/testdata/update5.packet
==============================================================================
--- branches/trac374/src/lib/datasrc/tests/testdata/update5.packet (original)
+++ branches/trac374/src/lib/datasrc/tests/testdata/update5.packet Thu Nov 25 10:03:59 2010
@@ -1,5 +1,5 @@
 example.com IN  SOA
-; fourth prereq should fail
+; fourth prereq (rrset) should fail
 ip46.example.com 0 ANY A
 www.example.com 0 ANY ANY
 example.com 0 IN MX 10 mail.example.com.

Modified: branches/trac374/src/lib/datasrc/tests/testdata/update6.packet
==============================================================================
--- branches/trac374/src/lib/datasrc/tests/testdata/update6.packet (original)
+++ branches/trac374/src/lib/datasrc/tests/testdata/update6.packet Thu Nov 25 10:03:59 2010
@@ -1,5 +1,5 @@
 example.com IN  SOA
-; fifth prereq should fail
+; fifth prereq (rrset) should fail
 ip46.example.com 0 ANY A
 www.example.com 0 ANY ANY
 example.com 0 IN MX 10 mail.example.com.




More information about the bind10-changes mailing list