Implement & test Zone.copy (shallow) and utilize it in processors

octodns/processor/acme.py

@@ -33,7 +33,7 @@ class AcmeMangingProcessor(BaseProcessor):
         self._owned = set()
 
     def process_source_zone(self, zone, *args, **kwargs):
-        ret = self._clone_zone(zone)
+        ret = zone.copy()
         for record in zone.records:
             if record._type == 'TXT' and \
                record.name.startswith('_acme-challenge'):
@@ -45,11 +45,11 @@ class AcmeMangingProcessor(BaseProcessor):
                 # This assumes we'll see things as sources before targets,
                 # which is the case...
                 self._owned.add(record)
-            ret.add_record(record)
+                ret.add_record(record, replace=True)
         return ret
 
     def process_target_zone(self, zone, *args, **kwargs):
-        ret = self._clone_zone(zone)
+        ret = zone.copy()
         for record in zone.records:
             # Uses a startswith rather than == to ignore subdomain challenges,
             # e.g. _acme-challenge.foo.domain.com when managing domain.com
@@ -58,7 +58,6 @@ class AcmeMangingProcessor(BaseProcessor):
                '*octoDNS*' not in record.values and \
                record not in self._owned:
                 self.log.info('_process: ignoring %s', record.fqdn)
-                continue
-            ret.add_record(record)
+                ret.remove_record(record)
 
         return ret

octodns/processor/base.py

@@ -13,9 +13,6 @@ class BaseProcessor(object):
     def __init__(self, name):
         self.name = name
 
-    def _clone_zone(self, zone):
-        return Zone(zone.name, sub_zones=zone.sub_zones)
-
     def process_source_zone(self, zone, sources):
         # sources may be empty, as will be the case for aliased zones
         return zone

octodns/processor/filter.py

@@ -15,10 +15,10 @@ class TypeAllowlistFilter(BaseProcessor):
         self.allowlist = set(allowlist)
 
     def _process(self, zone, *args, **kwargs):
-        ret = self._clone_zone(zone)
+        ret = zone.copy()
         for record in zone.records:
-            if record._type in self.allowlist:
-                ret.add_record(record)
+            if record._type not in self.allowlist:
+                ret.remove_record(record)
 
         return ret
 
@@ -33,10 +33,10 @@ class TypeRejectlistFilter(BaseProcessor):
         self.rejectlist = set(rejectlist)
 
     def _process(self, zone, *args, **kwargs):
-        ret = self._clone_zone(zone)
+        ret = zone.copy()
         for record in zone.records:
-            if record._type not in self.rejectlist:
-                ret.add_record(record)
+            if record._type in self.rejectlist:
+                ret.remove_record(record)
 
         return ret
 

octodns/processor/ownership.py

@@ -25,10 +25,8 @@ class OwnershipProcessor(BaseProcessor):
         self._txt_values = [txt_value]
 
     def process_source_zone(self, zone, *args, **kwargs):
-        ret = self._clone_zone(zone)
+        ret = zone.copy()
         for record in zone.records:
-            # Always copy over the source records
-            ret.add_record(record)
             # Then create and add an ownership TXT for each of them
             record_name = record.name.replace('*', '_wildcard')
             if record.name:

octodns/zone.py

@@ -49,16 +49,26 @@ class Zone(object):
         # optional trailing . b/c some sources don't have it on their fqdn
         self._name_re = re.compile(r'\.?{}?$'.format(name))
 
+        # Copy-on-write semantics support, when `not None` this property will
+        # point to a location with records for this `Zone`. Once `hydrated`
+        # this property will be set to None
+        self._origin = None
+
         self.log.debug('__init__: zone=%s, sub_zones=%s', self, sub_zones)
 
     @property
     def records(self):
+        if self._origin:
+            return self._origin.records
         return set([r for _, node in self._records.items() for r in node])
 
     def hostname_from_fqdn(self, fqdn):
         return self._name_re.sub('', fqdn)
 
     def add_record(self, record, replace=False, lenient=False):
+        if self._origin:
+            self.hydrate()
+
         name = record.name
         last = name.split('.')[-1]
 
@@ -94,10 +104,14 @@ class Zone(object):
 
         node.add(record)
 
-    def _remove_record(self, record):
-        'Only for use in tests'
+    def remove_record(self, record):
+        if self._origin:
+            self.hydrate()
         self._records[record.name].discard(record)
 
+    # TODO: delete this
+    _remove_record = remove_record
+
     def changes(self, desired, target):
         self.log.debug('changes: zone=%s, target=%s', self, target)
 
@@ -184,5 +198,42 @@ class Zone(object):
 
         return changes
 
+    def hydrate(self):
+        '''
+        Take a shallow copy Zone and make it a deeper copy holding its own
+        reference to records. These records will still be the originals and
+        they should not be modified. Changes should be made by calling
+        `add_record`, often with `replace=True`, and/or `remove_record`.
+
+        Note: This method does not need to be called under normal circumstances
+        as `add_record` and `remove_record` will automatically call it when
+        appropriate.
+        '''
+        origin = self._origin
+        if origin is None:
+            return False
+        # Need to clear this before the copy to prevent recursion
+        self._origin = None
+        for record in origin.records:
+            # Use lenient as we're copying origin and should take its records
+            # regardless
+            self.add_record(record, lenient=True)
+        return True
+
+    def copy(self):
+        '''
+        Copy-on-write semantics support. This method will create a shallow
+        clone of the zone which will be hydrated the first time `add_record` or
+        `remove_record` is called.
+
+        This allows low-cost copies of things to be made in situations where
+        changes are unlikely and only incurs the "expense" of actually
+        copying the records when required. The actual record copy will not be
+        "deep" meaning that records should not be modified directly.
+        '''
+        copy = Zone(self.name, self.sub_zones)
+        copy._origin = self
+        return copy
+
     def __repr__(self):
         return 'Zone<{}>'.format(self.name)

tests/test_octodns_manager.py

@@ -464,7 +464,7 @@ class TestManager(TestCase):
         class MockProcessor(BaseProcessor):
 
             def process_source_zone(self, zone, sources):
-                zone = self._clone_zone(zone)
+                zone = zone.copy()
                 zone.add_record(record)
                 return zone
 
@@ -480,7 +480,7 @@ class TestManager(TestCase):
         class MockProcessor(BaseProcessor):
 
             def process_target_zone(self, zone, target):
-                zone = self._clone_zone(zone)
+                zone = zone.copy()
                 zone.add_record(record)
                 return zone
 
@@ -496,7 +496,7 @@ class TestManager(TestCase):
         class MockProcessor(BaseProcessor):
 
             def process_target_zone(self, zone, target):
-                zone = self._clone_zone(zone)
+                zone = zone.copy()
                 zone.add_record(record)
                 return zone
 

tests/test_octodns_provider_base.py

@@ -61,11 +61,11 @@ class TrickyProcessor(BaseProcessor):
         self.existing = existing
         self.target = target
 
-        new = self._clone_zone(existing)
+        new = existing.copy()
         for record in existing.records:
-            new.add_record(record)
+            new.add_record(record, replace=True)
         for record in self.add_during_process_target_zone:
-            new.add_record(record)
+            new.add_record(record, replace=True)
         return new
 
 

tests/test_octodns_zone.py

@@ -355,3 +355,59 @@ class TestZone(TestCase):
 
         self.assertTrue(zone_missing.changes(zone_normal, provider))
         self.assertFalse(zone_missing.changes(zone_included, provider))
+
+    def assertEqualsNameAndValues(self, a, b):
+        a = dict([(r.name, r.values[0]) for r in a])
+        b = dict([(r.name, r.values[0]) for r in b])
+        self.assertEquals(a, b)
+
+    def test_copy(self):
+        zone = Zone('unit.tests.', [])
+
+        a = ARecord(zone, 'a', {'ttl': 42, 'value': '1.1.1.1'})
+        zone.add_record(a)
+        b = ARecord(zone, 'b', {'ttl': 42, 'value': '1.1.1.2'})
+        zone.add_record(b)
+
+        # Sanity check
+        self.assertEqualsNameAndValues(set((a, b)), zone.records)
+
+        copy = zone.copy()
+        # We have an origin set and it is the source/original zone
+        self.assertEquals(zone, copy._origin)
+        # Our records are zone's records to start (references)
+        self.assertEqualsNameAndValues(zone.records, copy.records)
+
+        # If we try and change something that's already there we realize and
+        # then get an error about a duplicate
+        b_prime = ARecord(zone, 'b', {'ttl': 42, 'value': '1.1.1.3'})
+        with self.assertRaises(DuplicateRecordException):
+            copy.add_record(b_prime)
+        self.assertIsNone(copy._origin)
+        # Unchanged, straight copies
+        self.assertEqualsNameAndValues(zone.records, copy.records)
+
+        # If we add with replace things will be realized and the record will
+        # have changed
+        copy = zone.copy()
+        copy.add_record(b_prime, replace=True)
+        self.assertIsNone(copy._origin)
+        self.assertEqualsNameAndValues(set((a, b_prime)), copy.records)
+
+        # If we add another record, things are reliazed and it has been added
+        copy = zone.copy()
+        c = ARecord(zone, 'c', {'ttl': 42, 'value': '1.1.1.3'})
+        copy.add_record(c)
+        self.assertEqualsNameAndValues(set((a, b, c)), copy.records)
+
+        # If we remove a record, things are reliazed and it has been removed
+        copy = zone.copy()
+        copy.remove_record(a)
+        self.assertEqualsNameAndValues(set((b,)), copy.records)
+
+        # Re-realizing is a noop
+        copy = zone.copy()
+        # Happens the first time
+        self.assertTrue(copy.hydrate())
+        # Doesn't the second
+        self.assertFalse(copy.hydrate())