validate values for empty string or None value

dump does not write invalid value(s) to yaml

octodns/record.py

@@ -211,9 +211,30 @@ class _ValuesMixin(object):
         values = []
         try:
             values = data['values']
+            if not values:
+                values = []
+                reasons.append('missing value(s)')
+            else:
+                # loop through copy of values
+                # remove invalid value from values
+                for value in list(values):
+                    if value is None:
+                        reasons.append('missing value(s)')
+                        values.remove(value)
+                    elif len(value) == 0:
+                        reasons.append('empty value')
+                        values.remove(value)
         except KeyError:
             try:
-                values = [data['value']]
+                value = data['value']
+                if value is None:
+                    reasons.append('missing value(s)')
+                    values = []
+                elif len(value) == 0:
+                    reasons.append('empty value')
+                    values = []
+                else:
+                    values = [value]
             except KeyError:
                 reasons.append('missing value(s)')
 
@@ -238,10 +259,16 @@ class _ValuesMixin(object):
     def _data(self):
         ret = super(_ValuesMixin, self)._data()
         if len(self.values) > 1:
-            ret['values'] = [getattr(v, 'data', v) for v in self.values]
-        else:
+            values = [getattr(v, 'data', v) for v in self.values if v]
+            if len(values) > 1:
+                ret['values'] = values
+            elif len(values) == 1:
+                ret['value'] = values[0]
+        elif len(self.values) == 1:
             v = self.values[0]
-            ret['value'] = getattr(v, 'data', v)
+            if v:
+                ret['value'] = getattr(v, 'data', v)
+
         return ret
 
     def __repr__(self):
@@ -349,6 +376,10 @@ class _ValueMixin(object):
         value = None
         try:
             value = data['value']
+            if value is None:
+                reasons.append('missing value')
+            elif value == '':
+                reasons.append('empty value')
         except KeyError:
             reasons.append('missing value')
         if value:
@@ -366,7 +397,8 @@ class _ValueMixin(object):
 
     def _data(self):
         ret = super(_ValueMixin, self)._data()
-        ret['value'] = getattr(self.value, 'data', self.value)
+        if self.value:
+            ret['value'] = getattr(self.value, 'data', self.value)
         return ret
 
     def __repr__(self):

tests/test_octodns_provider_dnsimple.py

@@ -96,23 +96,23 @@ class TestDnsimpleProvider(TestCase):
                 mock.get(ANY, text=fh.read())
 
             zone = Zone('unit.tests.', [])
-            provider.populate(zone)
+            provider.populate(zone, lenient=True)
             self.assertEquals(set([
                 Record.new(zone, '', {
                     'ttl': 3600,
                     'type': 'SSHFP',
                     'values': []
-                }),
+                }, lenient=True),
                 Record.new(zone, '_srv._tcp', {
                     'ttl': 600,
                     'type': 'SRV',
                     'values': []
-                }),
+                }, lenient=True),
                 Record.new(zone, 'naptr', {
                     'ttl': 600,
                     'type': 'NAPTR',
                     'values': []
-                }),
+                }, lenient=True),
             ]), zone.records)
 
     def test_apply(self):

tests/test_octodns_record.py

@@ -96,6 +96,57 @@ class TestRecord(TestCase):
 
             DummyRecord().__repr__()
 
+    def test_values_mixin_data(self):
+        # no values, no value or values in data
+        a = ARecord(self.zone, '', {
+            'type': 'A',
+            'ttl': 600,
+            'values': []
+        })
+        self.assertNotIn('values', a.data)
+
+        # empty value, no value or values in data
+        b = ARecord(self.zone, '', {
+            'type': 'A',
+            'ttl': 600,
+            'values': ['']
+        })
+        self.assertNotIn('value', b.data)
+
+        # empty/None values, no value or values in data
+        c = ARecord(self.zone, '', {
+            'type': 'A',
+            'ttl': 600,
+            'values': ['', None]
+        })
+        self.assertNotIn('values', c.data)
+
+        # empty/None values and valid, value in data
+        c = ARecord(self.zone, '', {
+            'type': 'A',
+            'ttl': 600,
+            'values': ['', None, '10.10.10.10']
+        })
+        self.assertNotIn('values', c.data)
+        self.assertEqual('10.10.10.10', c.data['value'])
+
+    def test_value_mixin_data(self):
+        # unspecified value, no value in data
+        a = AliasRecord(self.zone, '', {
+            'type': 'ALIAS',
+            'ttl': 600,
+            'value': None
+        })
+        self.assertNotIn('value', a.data)
+
+        # unspecified value, no value in data
+        a = AliasRecord(self.zone, '', {
+            'type': 'ALIAS',
+            'ttl': 600,
+            'value': ''
+        })
+        self.assertNotIn('value', a.data)
+
     def test_geo(self):
         geo_data = {'ttl': 42, 'values': ['5.2.3.4', '6.2.3.4'],
                     'geo': {'AF': ['1.1.1.1'],
@@ -750,6 +801,13 @@ class TestRecordValidation(TestCase):
             'ttl': 600,
             'value': '1.2.3.4',
         })
+        Record.new(self.zone, '', {
+            'type': 'A',
+            'ttl': 600,
+            'values': [
+                '1.2.3.4',
+            ]
+        })
         Record.new(self.zone, '', {
             'type': 'A',
             'ttl': 600,
@@ -759,13 +817,60 @@ class TestRecordValidation(TestCase):
             ]
         })
 
-        # missing value(s)
+        # missing value(s), no value or value
         with self.assertRaises(ValidationError) as ctx:
             Record.new(self.zone, '', {
                 'type': 'A',
                 'ttl': 600,
             })
         self.assertEquals(['missing value(s)'], ctx.exception.reasons)
+
+        # missing value(s), empty values
+        with self.assertRaises(ValidationError) as ctx:
+            Record.new(self.zone, 'www', {
+                'type': 'A',
+                'ttl': 600,
+                'values': []
+            })
+        self.assertEquals(['missing value(s)'], ctx.exception.reasons)
+
+        # missing value(s), None values
+        with self.assertRaises(ValidationError) as ctx:
+            Record.new(self.zone, 'www', {
+                'type': 'A',
+                'ttl': 600,
+                'values': None
+            })
+        self.assertEquals(['missing value(s)'], ctx.exception.reasons)
+
+        # missing value(s) and empty value
+        with self.assertRaises(ValidationError) as ctx:
+            Record.new(self.zone, 'www', {
+                'type': 'A',
+                'ttl': 600,
+                'values': [None, '']
+            })
+        self.assertEquals(['missing value(s)',
+                           'empty value'], ctx.exception.reasons)
+
+        # missing value(s), None value
+        with self.assertRaises(ValidationError) as ctx:
+            Record.new(self.zone, 'www', {
+                'type': 'A',
+                'ttl': 600,
+                'value': None
+            })
+        self.assertEquals(['missing value(s)'], ctx.exception.reasons)
+
+        # empty value, empty string value
+        with self.assertRaises(ValidationError) as ctx:
+            Record.new(self.zone, 'www', {
+                'type': 'A',
+                'ttl': 600,
+                'value': ''
+            })
+        self.assertEquals(['empty value'], ctx.exception.reasons)
+
         # missing value(s) & ttl
         with self.assertRaises(ValidationError) as ctx:
             Record.new(self.zone, '', {
@@ -922,6 +1027,24 @@ class TestRecordValidation(TestCase):
             })
         self.assertEquals(['missing value'], ctx.exception.reasons)
 
+        # missing value
+        with self.assertRaises(ValidationError) as ctx:
+            Record.new(self.zone, 'www', {
+                'type': 'ALIAS',
+                'ttl': 600,
+                'value': None
+            })
+        self.assertEquals(['missing value'], ctx.exception.reasons)
+
+        # empty value
+        with self.assertRaises(ValidationError) as ctx:
+            Record.new(self.zone, 'www', {
+                'type': 'ALIAS',
+                'ttl': 600,
+                'value': ''
+            })
+        self.assertEquals(['empty value'], ctx.exception.reasons)
+
         # missing trailing .
         with self.assertRaises(ValidationError) as ctx:
             Record.new(self.zone, '', {