diff --git a/CHANGELOG.md b/CHANGELOG.md index 830c7f1..dc3fef1 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/octodns/provider/yaml.py b/octodns/provider/yaml.py index 70c6f8c..3fc6102 100644 --- a/octodns/provider/yaml.py +++ b/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) diff --git a/octodns/record/base.py b/octodns/record/base.py index 700b65d..6f83578 100644 --- a/octodns/record/base.py +++ b/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, diff --git a/tests/test_octodns_record.py b/tests/test_octodns_record.py index abd886d..66f0b9f 100644 --- a/tests/test_octodns_record.py +++ b/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'}