Merge pull request #1102 from octodns/record-data-octodns

Fixed Record.data, now includes the octodns special section data

CHANGELOG.md

@@ -13,6 +13,11 @@
   the in-built `rrs` method
 * Removed code that included sha in module version number when installing from
   repo as it caused problems with non-binary installs.
+* Fixed a bug/shortcoming in Record.data where it didn't include the `octodns`
+  portion of the record's data. Record.copy also omitted it since it relies on
+  .data for the heavy lifting. There are no known actual problems due to this
+  bug, but it's possible (non-public) third party providers are susceptible. The
+  most likely place to hit issues in is tests where data and/or copy are abused.
 
 #### Stuff
 

octodns/provider/yaml.py

@@ -387,8 +387,6 @@ class YamlProvider(BaseProvider):
             if record.ttl == self.default_ttl:
                 # ttl is the default, we don't need to store it
                 del d['ttl']
-            if record._octodns:
-                d['octodns'] = record._octodns
             # we want to output the utf-8 version of the name
             data[record.decoded_name].append(d)
 

octodns/record/base.py

@@ -3,6 +3,7 @@
 #
 
 from collections import defaultdict
+from copy import deepcopy
 from logging import getLogger
 
 from ..context import ContextDict
@@ -167,9 +168,12 @@ class Record(EqualityTupleMixin):
         self._octodns = data.get('octodns', {})
 
     def _data(self):
+        ret = {'ttl': self.ttl}
+        if self._octodns:
+            ret['octodns'] = deepcopy(self._octodns)
         if self.context:
-            return ContextDict({'ttl': self.ttl}, context=self.context)
-        return {'ttl': self.ttl}
+            return ContextDict(ret, context=self.context)
+        return ret
 
     @property
     def data(self):
@@ -244,7 +248,6 @@ class Record(EqualityTupleMixin):
         # data, via _data(), will preserve context
         data = self.data
         data['type'] = self._type
-        data['octodns'] = self._octodns
 
         return Record.new(
             zone if zone else self.zone,

tests/test_octodns_record.py

@@ -279,6 +279,65 @@ class TestRecord(TestCase):
         d.copy()
         self.assertEqual('TXT', d._type)
 
+    def test_record_octodns_with_data_and_copy(self):
+        a = Record.new(
+            self.zone,
+            'a',
+            {
+                'ttl': 44,
+                'type': 'A',
+                'value': '1.2.3.4',
+                'octodns': {'first': 'level', 'key': {'second': 'level'}},
+            },
+        )
+
+        # make a copy
+        b = a.copy()
+        # ensure they're ==
+        self.assertEqual(a.data, b.data)
+
+        # modifying b.data's result doesn't change b's actual data
+        b_data = b.data
+        b_data['added'] = 'thing'
+        # dict is a deep copy
+        b_data['octodns']['added'] = 'thing'
+        b_data['octodns']['key']['added'] = 'thing'
+        self.assertEqual(a.data, b.data)
+
+        # rest of these will use copy, which relies on data for most of the
+        # heavy lifting
+
+        # hand add something at the first level of the copy
+        b = a.copy()
+        b._octodns['added'] = 'thing'
+        b_data = b.data
+        self.assertNotEqual(a.data, b_data)
+
+        # hand modify something at the first level of the copy
+        b = a.copy()
+        b._octodns['first'] = 'unlevel'
+        self.assertNotEqual(a.data, b.data)
+
+        # delete something at the first level of the copy
+        b = a.copy()
+        del b._octodns['first']
+        self.assertNotEqual(a.data, b.data)
+
+        # hand add something deeper in the copy
+        b = a.copy()
+        b._octodns['key']['added'] = 'thing'
+        self.assertNotEqual(a.data, b.data)
+
+        # hand modify something deeper in the copy
+        b = a.copy()
+        b._octodns['key']['second'] = 'unlevel'
+        self.assertNotEqual(a.data, b.data)
+
+        # hand delete something deeper in the copy
+        b = a.copy()
+        del b._octodns['key']['second']
+        self.assertNotEqual(a.data, b.data)
+
     def test_change(self):
         existing = Record.new(
             self.zone, 'txt', {'ttl': 44, 'type': 'TXT', 'value': 'some text'}