Browse Source

[1384] change adjust_ttl to separate_rrs

Jelte Jansen 13 years ago
parent
commit
8780f99820

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

@@ -215,18 +215,19 @@ public:
     ///
     /// \param name The name of zone apex to be traversed. It doesn't do
     ///     nearest match as findZone.
-    /// \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.
+    /// \param separate_rrs If true, the iterator will return each RR as a
+    ///                     new RRset object. If false, the iterator will
+    ///                     combine consecutive RRs with the name and type
+    ///                     into 1 RRset. The capitalization of the RRset will
+    ///                     be that of the first RR read, and TTLs will be
+    ///                     adjusted to the lowest one found.
     /// \return Pointer to the iterator.
     virtual ZoneIteratorPtr getIterator(const isc::dns::Name& name,
-                                        bool adjust_ttl = true) const {
+                                        bool separate_rrs = false) const {
         // This is here to both document the parameter in doxygen (therefore it
         // needs a name) and avoid unused parameter warning.
         static_cast<void>(name);
-        static_cast<void>(adjust_ttl);
+        static_cast<void>(separate_rrs);
 
         isc_throw(isc::NotImplemented,
                   "Data source doesn't support iteration");

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

@@ -707,11 +707,11 @@ public:
     DatabaseIterator(shared_ptr<DatabaseAccessor> accessor,
                      const Name& zone_name,
                      const RRClass& rrclass,
-                     bool adjust_ttl) :
+                     bool separate_rrs) :
         accessor_(accessor),
         class_(rrclass),
         ready_(true),
-        adjust_ttl_(adjust_ttl)
+        separate_rrs_(separate_rrs)
     {
         // Get the zone
         const pair<bool, int> zone(accessor_->getZone(zone_name.toText()));
@@ -769,20 +769,19 @@ public:
         const RRType rtype(rtype_str);
         RRsetPtr rrset(new RRset(name, class_, rtype, RRTTL(ttl)));
         while (data_ready_ && name_ == name_str && rtype_str == rtype_) {
-            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());
+            if (ttl_ != ttl) {
+                if (ttl < ttl_) {
+                    ttl_ = ttl;
+                    rrset->setTTL(RRTTL(ttl));
                 }
-            } else if (ttl_ != ttl) {
-                break;
+                LOG_WARN(logger, DATASRC_DATABASE_ITERATE_TTL_MISMATCH).
+                    arg(name_).arg(class_).arg(rtype_).arg(rrset->getTTL());
             }
             rrset->addRdata(rdata::createRdata(rtype, class_, rdata_));
             getData();
+            if (separate_rrs_) {
+                break;
+            }
         }
         LOG_DEBUG(logger, DBG_TRACE_DETAILED, DATASRC_DATABASE_ITERATE_NEXT).
             arg(rrset->getName()).arg(rrset->getType());
@@ -814,18 +813,18 @@ private:
     string name_, rtype_, rdata_, ttl_;
     // Whether to modify differing TTL values, or treat a different TTL as
     // a different RRset
-    bool adjust_ttl_;
+    bool separate_rrs_;
 };
 
 }
 
 ZoneIteratorPtr
 DatabaseClient::getIterator(const isc::dns::Name& name,
-                            bool adjust_ttl) const
+                            bool separate_rrs) const
 {
     ZoneIteratorPtr iterator = ZoneIteratorPtr(new DatabaseIterator(
                                                    accessor_->clone(), name,
-                                                   rrclass_, adjust_ttl));
+                                                   rrclass_, separate_rrs));
     LOG_DEBUG(logger, DBG_TRACE_DETAILED, DATASRC_DATABASE_ITERATE).
         arg(name);
 

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

@@ -863,15 +863,16 @@ public:
      * \exception Anything else the underlying DatabaseConnection might
      *     want to throw.
      * \param name The origin of the zone to iterate.
-     * \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.
+     * \param separate_rrs If true, the iterator will return each RR as a
+     *                     new RRset object. If false, the iterator will
+     *                     combine consecutive RRs with the name and type
+     *                     into 1 RRset. The capitalization of the RRset will
+     *                     be that of the first RR read, and TTLs will be
+     *                     adjusted to the lowest one found.
      * \return Shared pointer to the iterator (it will never be NULL)
      */
     virtual ZoneIteratorPtr getIterator(const isc::dns::Name& name,
-                                        bool adjust_ttl = true) const;
+                                        bool separate_rrs = false) const;
 
     /// This implementation internally clones the accessor from the one
     /// used in the client and starts a separate transaction using the cloned

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

@@ -1246,8 +1246,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_no_adjust_ttl) {
-    ZoneIteratorPtr it(this->client_->getIterator(Name("example.org"), false));
+TEST_F(MockDatabaseClientTest, ttldiff_separate_rrsets) {
+    ZoneIteratorPtr it(this->client_->getIterator(Name("example.org"), true));
 
     // Walk through the full iterator, we should see 1 rrset with name
     // ttldiff1.example.org., and two rdatas. Same for ttldiff2

+ 7 - 6
src/lib/python/isc/datasrc/client_inc.cc

@@ -89,7 +89,7 @@ None\n\
 ";
 
 const char* const DataSourceClient_getIterator_doc = "\
-get_iterator(name, adjust_ttl=True) -> ZoneIterator\n\
+get_iterator(name, separate_rrs=False) -> ZoneIterator\n\
 \n\
 Returns an iterator to the given zone.\n\
 \n\
@@ -111,11 +111,12 @@ anything else.\n\
 Parameters:\n\
   isc.dns.Name The name of zone apex to be traversed. It doesn't do\n\
                nearest match as find_zone.\n\
-  adjust_ttl   If True, the iterator will treat RRs with the same\n\
-               name and type but different TTL values to be of the\n\
-               same RRset, and will adjust the TTL to the lowest\n\
-               value found. If false, it will consider the RR to\n\
-               belong to a different RRset.\n\
+  separate_rrs If true, the iterator will return each RR as a\n\
+               new RRset object. If false, the iterator will\n\
+               combine consecutive RRs with the name and type\n\
+               into 1 RRset. The capitalization of the RRset will\n\
+               be that of the first RR read, and TTLs will be\n\
+               adjusted to the lowest one found.\n\
 \n\
 Return Value(s): Pointer to the iterator.\n\
 ";

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

@@ -84,26 +84,26 @@ PyObject*
 DataSourceClient_getIterator(PyObject* po_self, PyObject* args) {
     s_DataSourceClient* const self = static_cast<s_DataSourceClient*>(po_self);
     PyObject* name_obj;
-    PyObject* adjust_ttl_obj = NULL;
+    PyObject* separate_rrs_obj = NULL;
     if (PyArg_ParseTuple(args, "O!|O", &name_type, &name_obj,
-                         &adjust_ttl_obj)) {
+                         &separate_rrs_obj)) {
         try {
-            bool adjust_ttl = true;
-            if (adjust_ttl_obj != NULL) {
+            bool separate_rrs = false;
+            if (separate_rrs_obj != NULL) {
                 // store result in local var so we can explicitely check for
                 // -1 error return value
-                int adjust_ttl_no = PyObject_Not(adjust_ttl_obj);
-                if (adjust_ttl_no == 1) {
-                    adjust_ttl = false;
-                } else if (adjust_ttl_no == -1) {
+                int separate_rrs_true = PyObject_IsTrue(separate_rrs_obj);
+                if (separate_rrs_true == 1) {
+                    separate_rrs = true;
+                } else if (separate_rrs_true == -1) {
                     PyErr_SetString(getDataSourceException("Error"),
-                                    "Error getting value of adjust_ttl");
+                                    "Error getting value of separate_rrs");
                     return (NULL);
                 }
             }
             return (createZoneIteratorObject(
                 self->cppobj->getInstance().getIterator(PyName_ToName(name_obj),
-                                                        adjust_ttl),
+                                                        separate_rrs),
                 po_self));
         } catch (const isc::NotImplemented& ne) {
             PyErr_SetString(getDataSourceException("NotImplemented"),

+ 27 - 11
src/lib/python/isc/datasrc/tests/datasrc_test.py

@@ -88,7 +88,7 @@ class DataSrcClient(unittest.TestCase):
 
         # for RRSIGS, the TTL's are currently modified. This test should
         # start failing when we fix that.
-        rrs = dsc.get_iterator(isc.dns.Name("sql1.example.com."), False)
+        rrs = dsc.get_iterator(isc.dns.Name("sql1.example.com."), True)
 
         # we do not know the order in which they are returned by the iterator
         # but we do want to check them, so we put all records into one list
@@ -115,7 +115,11 @@ class DataSrcClient(unittest.TestCase):
                      "256 3 5 AwEAAdYdRhBAEY67R/8G1N5AjGF6asIiNh/pNGeQ8xDQP13J"+
                      "N2lo+sNqWcmpYNhuVqRbLB+mamsU1XcCICSBvAlSmfz/ZUdafX23knAr"+
                      "TlALxMmspcfdpqun3Yr3YYnztuj06rV7RqmveYckWvAUXVYMSMQZfJ30"+
-                     "5fs0dE/xLztL/CzZ",
+                     "5fs0dE/xLztL/CzZ"
+                  ])
+        add_rrset(expected_rrset_list, name, rrclass,
+                  isc.dns.RRType.DNSKEY(), isc.dns.RRTTL(3600),
+                  [
                      "257 3 5 AwEAAbaKDSa9XEFTsjSYpUTHRotTS9Tz3krfDucugW5UokGQ"+
                      "KC26QlyHXlPTZkC+aRFUs/dicJX2kopndLcnlNAPWiKnKtrsFSCnIJDB"+
                      "ZIyvcKq+9RXmV3HK3bUdHnQZ88IZWBRmWKfZ6wnzHo53kdYKAemTErkz"+
@@ -127,8 +131,16 @@ class DataSrcClient(unittest.TestCase):
         add_rrset(expected_rrset_list, name, rrclass,
                   isc.dns.RRType.NS(), isc.dns.RRTTL(3600),
                   [
-                    "dns01.example.com.",
-                    "dns02.example.com.",
+                    "dns01.example.com."
+                  ])
+        add_rrset(expected_rrset_list, name, rrclass,
+                  isc.dns.RRType.NS(), isc.dns.RRTTL(3600),
+                  [
+                    "dns02.example.com."
+                  ])
+        add_rrset(expected_rrset_list, name, rrclass,
+                  isc.dns.RRType.NS(), isc.dns.RRTTL(3600),
+                  [
                     "dns03.example.com."
                   ])
         add_rrset(expected_rrset_list, name, rrclass,
@@ -139,15 +151,19 @@ class DataSrcClient(unittest.TestCase):
         # For RRSIGS, we can't add the fake data through the API, so we
         # simply pass no rdata at all (which is skipped by the check later)
         
-        # Since we passed adjust_ttl = False to get_iterator, we get several
+        # Since we passed separate_rrs = True to get_iterator, we get several
         # sets of RRSIGs, one for each TTL
         add_rrset(expected_rrset_list, name, rrclass,
                   isc.dns.RRType.RRSIG(), isc.dns.RRTTL(3600), None)
         add_rrset(expected_rrset_list, name, rrclass,
-                  isc.dns.RRType.RRSIG(), isc.dns.RRTTL(7200), None)
+                  isc.dns.RRType.RRSIG(), isc.dns.RRTTL(3600), None)
         add_rrset(expected_rrset_list, name, rrclass,
                   isc.dns.RRType.RRSIG(), isc.dns.RRTTL(3600), None)
         add_rrset(expected_rrset_list, name, rrclass,
+                  isc.dns.RRType.RRSIG(), isc.dns.RRTTL(3600), None)
+        add_rrset(expected_rrset_list, name, rrclass,
+                  isc.dns.RRType.RRSIG(), isc.dns.RRTTL(7200), None)
+        add_rrset(expected_rrset_list, name, rrclass,
                   isc.dns.RRType.SOA(), isc.dns.RRTTL(3600),
                   [
                      "master.example.com. admin.example.com. 678 3600 1800 2419200 7200"
@@ -191,26 +207,26 @@ class DataSrcClient(unittest.TestCase):
         # instead of failing?
         self.assertRaises(isc.datasrc.Error, rrs.get_next_rrset)
 
-        # Without the adjust_ttl argument, it should return 55 RRsets
+        # Without the separate_rrs argument, it should return 55 RRsets
         dsc = isc.datasrc.DataSourceClient("sqlite3", READ_ZONE_DB_CONFIG)
         rrets = dsc.get_iterator(isc.dns.Name("example.com"))
         # there are more than 80 RRs in this zone... let's just count them
         # (already did a full check of the smaller zone above)
         self.assertEqual(55, len(list(rrets)))
 
-        # same test, but now with explicit True argument for adjust_ttl
+        # same test, but now with explicit False argument for separate_rrs
         dsc = isc.datasrc.DataSourceClient("sqlite3", READ_ZONE_DB_CONFIG)
-        rrets = dsc.get_iterator(isc.dns.Name("example.com"), True)
+        rrets = dsc.get_iterator(isc.dns.Name("example.com"), False)
         # there are more than 80 RRs in this zone... let's just count them
         # (already did a full check of the smaller zone above)
         self.assertEqual(55, len(list(rrets)))
 
         # Count should be 71 if we request individual rrsets for differing ttls
         dsc = isc.datasrc.DataSourceClient("sqlite3", READ_ZONE_DB_CONFIG)
-        rrets = dsc.get_iterator(isc.dns.Name("example.com"), False)
+        rrets = dsc.get_iterator(isc.dns.Name("example.com"), True)
         # there are more than 80 RRs in this zone... let's just count them
         # (already did a full check of the smaller zone above)
-        self.assertEqual(71, len(list(rrets)))
+        self.assertEqual(84, len(list(rrets)))
         # TODO should we catch this (iterating past end) and just return None
         # instead of failing?
         self.assertRaises(isc.datasrc.Error, rrs.get_next_rrset)