[2018] add find defaults

and reverted the acl check from the previous commit (going to address that separately)

Also add committed check to diff's find and find_all

src/lib/python/isc/ddns/session.py

@@ -190,12 +190,12 @@ class UpdateSession:
         '''
         try:
             self._get_update_zone()
-            self.__check_update_acl(self.__zname, self.__zclass)
             self._create_diff()
             prereq_result = self.__check_prerequisites()
             if prereq_result != Rcode.NOERROR():
                 self.__make_response(prereq_result)
                 return UPDATE_ERROR, self.__zname, self.__zclass
+            self.__check_update_acl(self.__zname, self.__zclass)
             update_result = self.__do_update()
             if update_result != Rcode.NOERROR():
                 self.__make_response(update_result)
@@ -326,9 +326,7 @@ class UpdateSession:
            only return what the result code would be (and not read/copy
            any actual data).
         '''
-        result, _, _ = self.__diff.find(rrset.get_name(), rrset.get_type(),
-                                        ZoneFinder.NO_WILDCARD |
-                                        ZoneFinder.FIND_GLUE_OK)
+        result, _, _ = self.__diff.find(rrset.get_name(), rrset.get_type())
         return result == ZoneFinder.SUCCESS
 
     def __prereq_rrset_exists_value(self, rrset):
@@ -338,9 +336,7 @@ class UpdateSession:
            Returns True if the prerequisite is satisfied, False otherwise.
         '''
         result, found_rrset, _ = self.__diff.find(rrset.get_name(),
-                                                  rrset.get_type(),
-                                                  ZoneFinder.NO_WILDCARD |
-                                                  ZoneFinder.FIND_GLUE_OK)
+                                                  rrset.get_type())
         if result == ZoneFinder.SUCCESS and\
            rrset.get_name() == found_rrset.get_name() and\
            rrset.get_type() == found_rrset.get_type():
@@ -379,9 +375,7 @@ class UpdateSession:
            to only return what the result code would be (and not read/copy
            any actual data).
         '''
-        result, rrsets, flags = self.__diff.find_all(rrset.get_name(),
-                                                     ZoneFinder.NO_WILDCARD |
-                                                     ZoneFinder.FIND_GLUE_OK)
+        result, rrsets, flags = self.__diff.find_all(rrset.get_name())
         if result == ZoneFinder.SUCCESS and\
            (flags & ZoneFinder.RESULT_WILDCARD == 0):
             return True
@@ -606,9 +600,7 @@ class UpdateSession:
         if rrset.get_type() == RRType.SOA():
             return
         result, orig_rrset, _ = self.__diff.find(rrset.get_name(),
-                                                 rrset.get_type(),
-                                                 ZoneFinder.NO_WILDCARD |
-                                                 ZoneFinder.FIND_GLUE_OK)
+                                                 rrset.get_type())
         if result == ZoneFinder.CNAME:
             # Ignore non-cname rrs that try to update CNAME records
             # (if rrset itself is a CNAME, the finder result would be
@@ -640,9 +632,7 @@ class UpdateSession:
            zone's apex, and the type is either SOA or NS, it
            is ignored.'''
         result, to_delete, _ = self.__diff.find(rrset.get_name(),
-                                                rrset.get_type(),
-                                                ZoneFinder.NO_WILDCARD |
-                                                ZoneFinder.FIND_GLUE_OK)
+                                                rrset.get_type())
         if result == ZoneFinder.SUCCESS:
             if to_delete.get_name() == self.__zname and\
                (to_delete.get_type() == RRType.SOA() or\
@@ -665,9 +655,7 @@ class UpdateSession:
         # The related test is currently disabled. When this is fixed,
         # enable that test again.
         result, orig_rrset, _ = self.__diff.find(rrset.get_name(),
-                                                 rrset.get_type(),
-                                                 ZoneFinder.NO_WILDCARD |
-                                                 ZoneFinder.FIND_GLUE_OK)
+                                                 rrset.get_type())
         # Even a real rrset comparison wouldn't help here...
         # The goal is to make sure that after deletion of the
         # given rrset, at least 1 NS record is left (at the apex).
@@ -697,9 +685,7 @@ class UpdateSession:
            Special case: if the name is the zone's apex, SOA and
            NS records are kept.
         '''
-        result, rrsets, flags = self.__diff.find_all(rrset.get_name(),
-                                                     ZoneFinder.NO_WILDCARD |
-                                                     ZoneFinder.FIND_GLUE_OK)
+        result, rrsets, flags = self.__diff.find_all(rrset.get_name())
         if result == ZoneFinder.SUCCESS and\
            (flags & ZoneFinder.RESULT_WILDCARD == 0):
             for to_delete in rrsets:
@@ -749,9 +735,7 @@ class UpdateSession:
         # serial magic and add the newly created one
 
         # get it from DS and to increment and stuff
-        result, old_soa, _ = self.__diff.find(self.__zname, RRType.SOA(),
-                                              ZoneFinder.NO_WILDCARD |
-                                              ZoneFinder.FIND_GLUE_OK)
+        result, old_soa, _ = self.__diff.find(self.__zname, RRType.SOA())
 
         if self.__added_soa is not None:
             new_soa = self.__added_soa

src/lib/python/isc/xfrin/diff.py

@@ -25,6 +25,7 @@ But for now, it lives here.
 
 import isc.dns
 import isc.log
+from isc.datasrc import ZoneFinder
 from isc.log_messages.libxfrin_messages import *
 
 class NoSuchZone(Exception):
@@ -119,7 +120,7 @@ class Diff:
         else:
             self.__buffer = []
 
-    def __check_commited(self):
+    def __check_committed(self):
         """
         This checks if the diff is already commited or broken. If it is, it
         raises ValueError. This check is for methods that need to work only on
@@ -169,7 +170,7 @@ class Diff:
         - in single_update_mode if any later rr is of type SOA (both for
           addition and deletion)
         """
-        self.__check_commited()
+        self.__check_committed()
         if rr.get_rdata_count() != 1:
             raise ValueError('The rrset must contain exactly 1 Rdata, but ' +
                              'it holds ' + str(rr.get_rdata_count()))
@@ -298,7 +299,7 @@ class Diff:
                 else:
                     raise ValueError('Unknown operation ' + operation)
 
-        self.__check_commited()
+        self.__check_committed()
         # First, compact the data
         self.compact()
         try:
@@ -330,7 +331,7 @@ class Diff:
 
         This might raise isc.datasrc.Error.
         """
-        self.__check_commited()
+        self.__check_committed()
         # Push the data inside the data source
         self.apply()
         # Make sure they are visible.
@@ -377,7 +378,8 @@ class Diff:
         else:
             return (self.__deletions, self.__additions)
 
-    def find(self, name, rrtype, options=isc.datasrc.ZoneFinder.FIND_DEFAULT):
+    def find(self, name, rrtype,
+             options=ZoneFinder.NO_WILDCARD | ZoneFinder.FIND_GLUE_OK):
         """
         Calls the find() method in the ZoneFinder associated with this
         Diff's ZoneUpdater, i.e. the find() on the zone as it was on the
@@ -385,10 +387,14 @@ class Diff:
         See the ZoneFinder documentation for a full description.
         Note that the result does not include changes made in this Diff
         instance so far.
+        Options default to NO_WILDCARD and FIND_GLUE_OK.
+        Raises a ValueError if the Diff has been committed already
         """
+        self.__check_committed()
         return self.__updater.find(name, rrtype, options)
 
-    def find_all(self, name, options=isc.datasrc.ZoneFinder.FIND_DEFAULT):
+    def find_all(self, name,
+                 options=ZoneFinder.NO_WILDCARD | ZoneFinder.FIND_GLUE_OK):
         """
         Calls the find() method in the ZoneFinder associated with this
         Diff's ZoneUpdater, i.e. the find_all() on the zone as it was on the
@@ -396,5 +402,8 @@ class Diff:
         See the ZoneFinder documentation for a full description.
         Note that the result does not include changes made in this Diff
         instance so far.
+        Options default to NO_WILDCARD and FIND_GLUE_OK.
+        Raises a ValueError if the Diff has been committed already
         """
+        self.__check_committed()
         return self.__updater.find_all(name, options)

src/lib/python/isc/xfrin/tests/diff_tests.py

@@ -15,7 +15,7 @@
 
 import isc.log
 import unittest
-import isc.datasrc
+from isc.datasrc import ZoneFinder
 from isc.dns import Name, RRset, RRClass, RRType, RRTTL, Rdata
 from isc.xfrin.diff import Diff, NoSuchZone
 
@@ -289,6 +289,9 @@ class DiffTest(unittest.TestCase):
         self.assertRaises(ValueError, diff.commit)
         self.assertRaises(ValueError, diff.add_data, self.__rrset2)
         self.assertRaises(ValueError, diff.delete_data, self.__rrset1)
+        self.assertRaises(ValueError, diff.find, Name('foo.example.org.'),
+                          RRType.A())
+        self.assertRaises(ValueError, diff.find_all, Name('foo.example.org.'))
         diff.apply = orig_apply
         self.assertRaises(ValueError, diff.apply)
         # This one does not state it should raise, so check it doesn't
@@ -626,15 +629,14 @@ class DiffTest(unittest.TestCase):
         self.assertTrue(self.__find_called)
         self.assertEqual(name, self.__find_name)
         self.assertEqual(rrtype, self.__find_type)
-        self.assertEqual(isc.datasrc.ZoneFinder.FIND_DEFAULT,
+        self.assertEqual(ZoneFinder.NO_WILDCARD | ZoneFinder.FIND_GLUE_OK,
                          self.__find_options)
 
     def test_find_options(self):
         diff = Diff(self, Name('example.org.'))
         name = Name('foo.example.org.')
         rrtype = RRType.TXT()
-        options = isc.datasrc.ZoneFinder.NO_WILDCARD |\
-                  isc.datasrc.ZoneFinder.FIND_GLUE_OK
+        options = ZoneFinder.NO_WILDCARD
 
         self.assertEqual("find_return", diff.find(name, rrtype, options))
 
@@ -655,14 +657,13 @@ class DiffTest(unittest.TestCase):
 
         self.assertTrue(self.__find_all_called)
         self.assertEqual(name, self.__find_all_name)
-        self.assertEqual(isc.datasrc.ZoneFinder.FIND_DEFAULT,
+        self.assertEqual(ZoneFinder.NO_WILDCARD | ZoneFinder.FIND_GLUE_OK,
                          self.__find_all_options)
 
     def test_find_all_options(self):
         diff = Diff(self, Name('example.org.'))
         name = Name('www.example.org.')
-        options = isc.datasrc.ZoneFinder.NO_WILDCARD |\
-                  isc.datasrc.ZoneFinder.FIND_GLUE_OK
+        options = isc.datasrc.ZoneFinder.NO_WILDCARD
 
         self.assertFalse(self.__find_all_called)
         self.assertEqual(None, self.__find_all_name)