AliasValue, name & type, improved Record KeyError handling

octodns/record.py

@@ -71,7 +71,7 @@ class Record(object):
             _type = {
                 'A': ARecord,
                 'AAAA': AaaaRecord,
-                # alias
+                'ALIAS': AliasRecord,
                 # cert
                 'CNAME': CnameRecord,
                 # dhcid
@@ -185,13 +185,14 @@ class _ValuesMixin(object):
     def __init__(self, zone, name, data, source=None):
         super(_ValuesMixin, self).__init__(zone, name, data, source=source)
         try:
-            self.values = sorted(self._process_values(data['values']))
+            values = data['values']
         except KeyError:
             try:
-                self.values = self._process_values([data['value']])
+                values = [data['value']]
             except KeyError:
                 raise Exception('Invalid record {}, missing value(s)'
                                 .format(self.fqdn))
+        self.values = sorted(self._process_values(values))
 
     def changes(self, other, target):
         if self.values != other.values:
@@ -290,10 +291,11 @@ class _ValueMixin(object):
     def __init__(self, zone, name, data, source=None):
         super(_ValueMixin, self).__init__(zone, name, data, source=source)
         try:
-            self.value = self._process_value(data['value'])
+            value = data['value']
         except KeyError:
             raise Exception('Invalid record {}, missing value'
                             .format(self.fqdn))
+        self.value = self._process_value(value)
 
     def changes(self, other, target):
         if self.value != other.value:
@@ -311,14 +313,50 @@ class _ValueMixin(object):
                                            self.fqdn, self.value)
 
 
-class AliasRecord(_ValueMixin, Record):
+class AliasValue(object):
+
+    def __init__(self, value):
+        self.name = value['name'].lower()
+        self._type = value['type']
+
+    @property
+    def data(self):
+        return {
+            'name': self.name,
+            'type': self._type,
+        }
+
+    def __cmp__(self, other):
+        if self.name == other.name:
+            return cmp(self._type, other._type)
+        return cmp(self.name, other.name)
+
+    def __repr__(self):
+        return "'{} {}'".format(self.name, self._type)
+
+
+class AliasRecord(_ValuesMixin, Record):
     _type = 'ALIAS'
 
-    def _process_value(self, value):
+    def __init__(self, zone, name, data, source=None):
-        if not value.endswith(self.zone.name):
+        data = dict(data)
-            raise Exception('Invalid record {}, value ({}) must be in '
+        # TODO: this is an ugly way to fake the lack of ttl :-(
-                            'same zone.'.format(self.fqdn, value))
+        data['ttl'] = 0
-        return value.lower()
+        super(AliasRecord, self).__init__(zone, name, data, source)
+
+    def _process_values(self, values):
+        ret = []
+        for value in values:
+            try:
+                value = AliasValue(value)
+            except KeyError as e:
+                raise Exception('Invalid value in record {}, missing {}'
+                                .format(self.fqdn, e.args[0]))
+            if not value.name.endswith(self.zone.name):
+                raise Exception('Invalid value in record {}, name must be in '
+                                'same zone.'.format(self.fqdn))
+            ret.append(value)
+        return ret
 
 
 class CnameRecord(_ValueMixin, Record):
@@ -326,7 +364,7 @@ class CnameRecord(_ValueMixin, Record):
 
     def _process_value(self, value):
         if not value.endswith('.'):
-            raise Exception('Invalid record {}, value {} missing trailing .'
+            raise Exception('Invalid record {}, value ({}) missing trailing .'
                             .format(self.fqdn, value))
         return value.lower()
 
@@ -444,7 +482,7 @@ class PtrRecord(_ValueMixin, Record):
 
     def _process_value(self, value):
         if not value.endswith('.'):
-            raise Exception('Invalid record {}, value {} missing trailing .'
+            raise Exception('Invalid record {}, value ({}) missing trailing .'
                             .format(self.fqdn, value))
         return value.lower()
 

tests/test_octodns_record.py

@@ -243,15 +243,68 @@ class TestRecord(TestCase):
         a.__repr__()
 
     def test_alias(self):
-        self.assertSingleValue(AliasRecord, 'foo.unit.tests.',
+        a_values = [{
-                               'other.unit.tests.')
+            'name': 'www.unit.tests.',
+            'type': 'A'
+        }, {
+            'name': 'www.unit.tests.',
+            'type': 'AAAA'
+        }]
+        a_data = {'ttl': 0, 'values': a_values}
+        a = AliasRecord(self.zone, '', a_data)
+        self.assertEquals('', a.name)
+        self.assertEquals('unit.tests.', a.fqdn)
+        self.assertEquals(0, a.ttl)
+        self.assertEquals(a_values[0]['name'], a.values[0].name)
+        self.assertEquals(a_values[0]['type'], a.values[0]._type)
+        self.assertEquals(a_values[1]['name'], a.values[1].name)
+        self.assertEquals(a_values[1]['type'], a.values[1]._type)
+        self.assertEquals(a_data, a.data)
 
+        b_value = {
+            'name': 'www.unit.tests.',
+            'type': 'A',
+        }
+        b_data = {'ttl': 0, 'value': b_value}
+        b = AliasRecord(self.zone, 'b', b_data)
+        self.assertEquals(b_value['name'], b.values[0].name)
+        self.assertEquals(b_value['type'], b.values[0]._type)
+        self.assertEquals(b_data, b.data)
+
+        # missing value
         with self.assertRaises(Exception) as ctx:
-            AliasRecord(self.zone, '', {
+            AliasRecord(self.zone, None, {'ttl': 0})
-                'ttl': 31,
+        self.assertTrue('missing value(s)' in ctx.exception.message)
-                'value': 'foo.bar.com.'
+        # invalid value
-            })
+        with self.assertRaises(Exception) as ctx:
-        self.assertTrue('in same zone' in ctx.exception.message)
+            AliasRecord(self.zone, None, {'ttl': 0, 'value': {}})
+        self.assertTrue('Invalid value' in ctx.exception.message)
+        # bad name
+        with self.assertRaises(Exception) as ctx:
+            AliasRecord(self.zone, None, {'ttl': 0, 'value': {
+                'name': 'foo.bar.com.',
+                'type': 'A'
+            }})
+        self.assertTrue('Invalid value' in ctx.exception.message)
+
+        target = SimpleProvider()
+        # No changes with self
+        self.assertFalse(a.changes(a, target))
+        # Diff in priority causes change
+        other = AliasRecord(self.zone, 'a', {'ttl': 30, 'values': a_values})
+        other.values[0].name = 'foo.unit.tests.'
+        change = a.changes(other, target)
+        self.assertEqual(change.existing, a)
+        self.assertEqual(change.new, other)
+        # Diff in value causes change
+        other.values[0].name = a.values[0].name
+        other.values[0]._type = 'MX'
+        change = a.changes(other, target)
+        self.assertEqual(change.existing, a)
+        self.assertEqual(change.new, other)
+
+        # __repr__ doesn't blow up
+        a.__repr__()
 
     def test_cname(self):
         self.assertSingleValue(CnameRecord, 'target.foo.com.',