Browse Source

[1217] change 'individual_rrs' to 'adjust_ttl'

with a slightly different interpretation; if false, treat differing TTLs as an indicator of different RRsets
Jelte Jansen 13 years ago
parent
commit
64d4ac8b0f

+ 7 - 5
src/lib/datasrc/client.h

@@ -215,16 +215,18 @@ public:
     ///
     /// \param name The name of zone apex to be traversed. It doesn't do
     ///     nearest match as findZone.
-    /// \param individual_rrs If true, the iterator will return each RR found
-    ///        in the data as a separate RRset, instead of combining them into
-    ///        RRsets.
+    /// \param adjust_ttl If true, the iterator will treat RRs with the same
+    ///                   name and type but different TTL values to be of the
+    ///                   same RRset, and will adjust the TTL to the lowest
+    ///                   value found. If false, it will consider the RR to
+    ///                   belong to a different RRset.
     /// \return Pointer to the iterator.
     virtual ZoneIteratorPtr getIterator(const isc::dns::Name& name,
-                                        bool individual_rrs = false) const {
+                                        bool adjust_ttl = true) const {
         // This is here to both document the parameter in doxygen (therefore it
         // needs a name) and avoid unused parameter warning.
         static_cast<void>(name);
-        static_cast<void>(individual_rrs);
+        static_cast<void>(adjust_ttl);
 
         isc_throw(isc::NotImplemented,
                   "Data source doesn't support iteration");

+ 17 - 15
src/lib/datasrc/database.cc

@@ -705,11 +705,11 @@ namespace {
 class DatabaseIterator : public ZoneIterator {
 public:
     DatabaseIterator(const DatabaseAccessor::IteratorContextPtr& context,
-             const RRClass& rrclass, bool individual_rrs) :
+             const RRClass& rrclass, bool adjust_ttl) :
         context_(context),
         class_(rrclass),
         ready_(true),
-        individual_rrs_(individual_rrs)
+        adjust_ttl_(adjust_ttl)
     {
         // Prepare data for the next time
         getData();
@@ -731,19 +731,20 @@ public:
         RRType rtype(rtype_str);
         RRsetPtr rrset(new RRset(name, class_, rtype, RRTTL(ttl)));
         while (data_ready_ && name_ == name_str && rtype_str == rtype_) {
-            if (ttl_ != ttl) {
-                if (ttl < ttl_) {
-                    ttl_ = ttl;
-                    rrset->setTTL(RRTTL(ttl));
+            if (adjust_ttl_) {
+                if (ttl_ != ttl) {
+                    if (ttl < ttl_) {
+                        ttl_ = ttl;
+                        rrset->setTTL(RRTTL(ttl));
+                    }
+                    LOG_WARN(logger, DATASRC_DATABASE_ITERATE_TTL_MISMATCH).
+                        arg(name_).arg(class_).arg(rtype_).arg(rrset->getTTL());
                 }
-                LOG_WARN(logger, DATASRC_DATABASE_ITERATE_TTL_MISMATCH).
-                    arg(name_).arg(class_).arg(rtype_).arg(rrset->getTTL());
+            } else if (ttl_ != ttl) {
+                break;
             }
             rrset->addRdata(rdata::createRdata(rtype, class_, rdata_));
             getData();
-            if (individual_rrs_) {
-                break;
-            }
         }
         LOG_DEBUG(logger, DBG_TRACE_DETAILED, DATASRC_DATABASE_ITERATE_NEXT).
             arg(rrset->getName()).arg(rrset->getType());
@@ -768,15 +769,16 @@ private:
     bool ready_, data_ready_;
     // Data of the next row
     string name_, rtype_, rdata_, ttl_;
-    // Whether to return individual RRsets
-    bool individual_rrs_;
+    // Whether to modify differing TTL values, or treat a different TTL as
+    // a different RRset
+    bool adjust_ttl_;
 };
 
 }
 
 ZoneIteratorPtr
 DatabaseClient::getIterator(const isc::dns::Name& name,
-                            bool individual_rrs) const
+                            bool adjust_ttl) const
 {
     // Get the zone
     std::pair<bool, int> zone(accessor_->getZone(name.toText()));
@@ -802,7 +804,7 @@ DatabaseClient::getIterator(const isc::dns::Name& name,
     LOG_DEBUG(logger, DBG_TRACE_DETAILED, DATASRC_DATABASE_ITERATE).
         arg(name);
     return (ZoneIteratorPtr(new DatabaseIterator(context, RRClass::IN(),
-                                                 individual_rrs)));
+                                                 adjust_ttl)));
 }
 
 //

+ 6 - 4
src/lib/datasrc/database.h

@@ -741,13 +741,15 @@ public:
      * \exception Anything else the underlying DatabaseConnection might
      *     want to throw.
      * \param name The origin of the zone to iterate.
-     * \param individual_rrs If true, the iterator will return each RR found
-     *        in the data as a separate RRset, instead of combining them into
-     *        RRsets.
+     * \param adjust_ttl If true, the iterator will treat RRs with the same
+     *                   name and type but different TTL values to be of the
+     *                   same RRset, and will adjust the TTL to the lowest
+     *                   value found. If false, it will consider the RR to
+     *                   belong to a different RRset.
      * \return Shared pointer to the iterator (it will never be NULL)
      */
     virtual ZoneIteratorPtr getIterator(const isc::dns::Name& name,
-                                        bool individual_rrs = false) const;
+                                        bool adjust_ttl = true) const;
 
     /// This implementation internally clones the accessor from the one
     /// used in the client and starts a separate transaction using the cloned

+ 4 - 0
src/lib/datasrc/memory_datasrc.cc

@@ -786,6 +786,10 @@ public:
 
 ZoneIteratorPtr
 InMemoryClient::getIterator(const Name& name, bool) const {
+    // note: adjust_ttl argument is ignored, as the RRsets are already
+    // individually stored, and hence cannot have different TTLs anymore at
+    // this point
+
     ZoneTable::FindResult result(impl_->zone_table.findZone(name));
     if (result.code != result::SUCCESS) {
         isc_throw(DataSourceError, "No such zone: " + name.toText());

+ 1 - 1
src/lib/datasrc/memory_datasrc.h

@@ -273,7 +273,7 @@ public:
 
     /// \brief Implementation of the getIterator method
     virtual ZoneIteratorPtr getIterator(const isc::dns::Name& name,
-                                        bool individual_rrs = false) const;
+                                        bool adjust_ttl = true) const;
 
     /// In-memory data source is read-only, so this derived method will
     /// result in a NotImplemented exception.

+ 5 - 2
src/lib/datasrc/tests/database_unittest.cc

@@ -1120,8 +1120,8 @@ TEST_F(MockDatabaseClientTest, ttldiff) {
 
 // Unless we ask for individual RRs in our iterator request. In that case
 // every RR should go into its own 'rrset'
-TEST_F(MockDatabaseClientTest, ttldiff_individual) {
-    ZoneIteratorPtr it(this->client_->getIterator(Name("example.org"), true));
+TEST_F(MockDatabaseClientTest, ttldiff_no_adjust_ttl) {
+    ZoneIteratorPtr it(this->client_->getIterator(Name("example.org"), false));
 
     // Walk through the full iterator, we should see 1 rrset with name
     // ttldiff1.example.org., and two rdatas. Same for ttldiff2
@@ -1139,6 +1139,9 @@ TEST_F(MockDatabaseClientTest, ttldiff_individual) {
             } else if (rrset->getTTL() == RRTTL(600)) {
                 ASSERT_FALSE(found2);
                 found2 = true;
+            } else {
+                FAIL() << "Found unexpected TTL: " <<
+                          rrset->getTTL().toText();
             }
         }
         rrset = it->getNextRRset();

+ 7 - 7
src/lib/python/isc/datasrc/client_python.cc

@@ -84,18 +84,18 @@ PyObject*
 DataSourceClient_getIterator(PyObject* po_self, PyObject* args) {
     s_DataSourceClient* const self = static_cast<s_DataSourceClient*>(po_self);
     PyObject* name_obj;
-    PyObject* individual_rrs_obj = NULL;
+    PyObject* adjust_ttl_obj = NULL;
     if (PyArg_ParseTuple(args, "O!|O", &name_type, &name_obj,
-                         &individual_rrs_obj)) {
+                         &adjust_ttl_obj)) {
         try {
-            bool individual_rrs = false;
-            if (individual_rrs_obj != NULL &&
-                PyObject_IsTrue(individual_rrs_obj)) {
-                individual_rrs = true;
+            bool adjust_ttl = true;
+            if (adjust_ttl_obj != NULL &&
+                PyObject_Not(adjust_ttl_obj)) {
+                adjust_ttl = false;
             }
             return (createZoneIteratorObject(
                 self->cppobj->getInstance().getIterator(PyName_ToName(name_obj),
-                                                        individual_rrs),
+                                                        adjust_ttl),
                 po_self));
         } catch (const isc::NotImplemented& ne) {
             PyErr_SetString(getDataSourceException("NotImplemented"),