From: Jelte Jansen <jelte@isc.org>
Date: Thu, 12 Apr 2012 10:51:35 +0000 (+0200)
Subject: [1789] more review comments
X-Git-Tag: trac2351_base~226^2~116^2~40^2
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=56338ac70f174880ff1ca73bda0afe73dad2e7d4;p=thirdparty%2Fkea.git

[1789] more review comments

in xfrin
- add note about lack of thread-safety
- add note about from-source temporariness
- add test for case-insensitive class strings
- directly replace _memory_zones
---

diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py
index 04df8bf0f8..b88d6a99b9 100644
--- a/src/bin/xfrin/tests/xfrin_test.py
+++ b/src/bin/xfrin/tests/xfrin_test.py
@@ -2597,7 +2597,7 @@ class TextXfrinMemoryZones(unittest.TestCase):
                             ]
                           },
                           { 'type': 'memory',
-                            'class': 'CH',
+                            'class': 'ch',
                             'zones': [
                               { 'origin': 'example.com',
                                 'filetype': 'sqlite3' }
@@ -2655,6 +2655,7 @@ class TextXfrinMemoryZones(unittest.TestCase):
         # make sure it is case insensitive, root-dot-insensitive,
         # and supports CLASSXXX notation
         self.assertTrue(self.xfr._is_memory_zone("EXAMPLE.com", "IN"))
+        self.assertTrue(self.xfr._is_memory_zone("example.com", "in"))
         self.assertTrue(self.xfr._is_memory_zone("example2.com.", "IN"))
         self.assertTrue(self.xfr._is_memory_zone("example.com", "CLASS3"))
 
@@ -2704,9 +2705,6 @@ class TextXfrinMemoryZones(unittest.TestCase):
         self.assertFalse(self.xfr._is_memory_zone("example3.com", "IN"))
         self.assertTrue(self.xfr._is_memory_zone("example.com", "CH"))
 
-
-
-
 def raise_interrupt():
     raise KeyboardInterrupt()
 
diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in
index a8df637f74..a60ad7f707 100755
--- a/src/bin/xfrin/xfrin.py.in
+++ b/src/bin/xfrin/xfrin.py.in
@@ -1332,6 +1332,10 @@ class Xfrin:
         """Returns true if the given zone/class combination is configured
            in the in-memory datasource of the Auth process with file type
            'sqlite3'.
+           Note: this method is not thread-safe. We are considering
+           changing the threaded model here, but if we do not, take
+           care in accessing and updating the memory zone set (or add
+           locks)
         """
         # Normalize them first, if either conversion fails, return false
         # (they won't be in the set anyway)
@@ -1342,15 +1346,15 @@ class Xfrin:
             return False
         return (zone_name_str, zone_class_str) in self._memory_zones
 
-    def _add_memory_zone(self, zone_name_str, zone_class_str):
-        """Add the given zone and class to the internal list of zones
-           in memory datasources with file type 'sqlite3'"""
-        self._memory_zones.add((zone_name_str, zone_class_str))
-
     def _set_memory_zones(self, new_config, config_data):
         """Part of the _auth_config_handler function, keeps an internal set
            of zones in the datasources config subset that have 'sqlite3' as
-           their file type"""
+           their file type.
+           Note: this method is not thread-safe. We are considering
+           changing the threaded model here, but if we do not, take
+           care in accessing and updating the memory zone set (or add
+           locks)
+        """
         # walk through the data and collect the memory zones
         # If this causes any exception, assume we were passed bad data
         # and keep the original set
@@ -1373,10 +1377,8 @@ class Xfrin:
                                 new_memory_zones.add((zone_name_str,
                                                       ds_class.to_text()))
                 # Ok, we can use the data, update our list
-                self._clear_memory_zones()
-                for zone_str, class_str in new_memory_zones:
-                    self._add_memory_zone(zone_str, class_str)
-        except Exception as eee:
+                self._memory_zones = new_memory_zones
+        except Exception:
             # Something is wrong with the data. If this data even reached us,
             # we cannot do more than assume the real module has logged and
             # reported an error. Keep the old set.
@@ -1527,6 +1529,8 @@ class Xfrin:
         if is_default and "B10_FROM_BUILD" in os.environ:
             # override the local database setting if it is default and we
             # are running from the source tree
+            # This should be hidden inside the data source library and/or
+            # done as a configuration, and this special case should be gone).
             db_file = os.environ["B10_FROM_BUILD"] + os.sep +\
                       "bind10_zones.sqlite3"
         self._db_file = db_file