refactor(api): introduce RR model, replaces RRsets.records items

This commit also comes with a refactoring of the RRset serializers.

After applying this commit, it is required to run the `sync-from-pdns`
Django management command.

api/desecapi/migrations/0017_rr_model.py

@@ -0,0 +1,37 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.4 on 2017-08-29 00:57
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('desecapi', '0016_dyn_flag_default'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='RR',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('content', models.CharField(max_length=4092)),
+            ],
+        ),
+        migrations.RemoveField(
+            model_name='rrset',
+            name='records',
+        ),
+        migrations.AlterField(
+            model_name='rrset',
+            name='domain',
+            field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='desecapi.Domain'),
+        ),
+        migrations.AddField(
+            model_name='rr',
+            name='rrset',
+            field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='records', to='desecapi.RRset'),
+        ),
+    ]

api/desecapi/models.py

@@ -192,14 +192,13 @@ class Domain(models.Model, mixins.SetterMixin):
 
         self._dirtyRecords = False
 
+    @transaction.atomic
     def sync_from_pdns(self):
-        with transaction.atomic():
-            RRset.objects.filter(domain=self).delete()
-            rrsets = pdns.get_rrsets(self)
-            rrsets = [rrset for rrset in rrsets if rrset.type not in RRset.RESTRICTED_TYPES]
-            RRset.objects.bulk_create(rrsets)
+        RRset.objects.filter(domain=self).delete()
+        for rrset in pdns.get_rrsets(self):
+            if rrset['type'] not in RRset.RESTRICTED_TYPES:
+                RRset(**rrset).save(pdns=False)
 
-    @transaction.atomic
     def delete(self, *args, **kwargs):
         super(Domain, self).delete(*args, **kwargs)
 
@@ -219,6 +218,9 @@ class Domain(models.Model, mixins.SetterMixin):
 
         self.pdns_sync(new_domain)
 
+    def __str__(self):
+        return self.name
+
     class Meta:
         ordering = ('created',)
 
@@ -250,7 +252,7 @@ class Donation(models.Model):
 
     def save(self, *args, **kwargs):
         self.iban = self.iban[:6] + "xxx" # do NOT save account details
-        super(Donation, self).save(*args, **kwargs) # Call the "real" save() method.
+        super().save(*args, **kwargs) # Call the "real" save() method.
 
 
     class Meta:
@@ -267,10 +269,9 @@ def validate_upper(value):
 class RRset(models.Model, mixins.SetterMixin):
     created = models.DateTimeField(auto_now_add=True)
     updated = models.DateTimeField(null=True)
-    domain = models.ForeignKey(Domain, on_delete=models.CASCADE, related_name='rrsets')
+    domain = models.ForeignKey(Domain, on_delete=models.CASCADE)
     subname = models.CharField(max_length=178, blank=True)
     type = models.CharField(max_length=10, validators=[validate_upper])
-    records = models.CharField(max_length=64000, blank=True)
     ttl = models.PositiveIntegerField(validators=[MinValueValidator(1)])
 
     _dirty = False
@@ -281,6 +282,7 @@ class RRset(models.Model, mixins.SetterMixin):
         unique_together = (("domain","subname","type"),)
 
     def __init__(self, *args, **kwargs):
+        self.records_data = kwargs.pop('records_data', [])
         self._dirties = set()
         super().__init__(*args, **kwargs)
 
@@ -306,12 +308,6 @@ class RRset(models.Model, mixins.SetterMixin):
 
         return val
 
-    def setter_records(self, val):
-        if val != self.records:
-            self._dirty = True
-
-        return val
-
     def setter_ttl(self, val):
         if val != self.ttl:
             self._dirty = True
@@ -337,18 +333,31 @@ class RRset(models.Model, mixins.SetterMixin):
 
     @transaction.atomic
     def delete(self, *args, **kwargs):
-        # Reset records so that our pdns update later will cause deletion
-        self.records = '[]'
         super().delete(*args, **kwargs)
-
         self.update_pdns()
 
     @transaction.atomic
-    def save(self, *args, **kwargs):
+    def save(self, pdns=True, *args, **kwargs):
         new = self.pk is None
         self.updated = timezone.now()
         self.full_clean()
         super().save(*args, **kwargs)
 
-        if self._dirty or new:
+        records = self.records.all()
+        if self.records_data and self.records_data != [{'content': x.content}
+                                                       for x in records]:
+            self._dirty = True
+            records.delete()
+            while self.records_data:
+                self.records.create(**self.records_data.pop())
+
+        if pdns and (self._dirty or new):
             self.update_pdns()
+
+        self._dirty = False
+
+
+class RR(models.Model):
+    rrset = models.ForeignKey(RRset, on_delete=models.CASCADE, related_name='records')
+    # https://lists.isc.org/pipermail/bind-users/2008-April/070148.html
+    content = models.CharField(max_length=4092)

api/desecapi/pdns.py

@@ -153,22 +153,13 @@ def get_rrsets(domain):
     """
     Retrieves a JSON representation of the RRsets in a given zone, optionally restricting to a name and RRset type 
     """
-    from desecapi.models import RRset
-    from desecapi.serializers import GenericRRsetSerializer
-
-    rrsets = []
-    for rrset in get_zone(domain)['rrsets']:
-        data = {'domain': domain.pk,
-                'subname': rrset['name'][:-(len(domain.name) + 2)],
-                'type': rrset['type'],
-                'records': [record['content'] for record in rrset['records']],
-                'ttl': rrset['ttl']}
-
-        serializer = GenericRRsetSerializer(data=data)
-        serializer.is_valid(raise_exception=True)
-        rrsets.append(RRset(**serializer.validated_data))
-
-    return rrsets
+    return [{'domain': domain,
+             'subname': rrset['name'][:-(len(domain.name) + 2)],
+             'type': rrset['type'],
+             'records_data': [{'content': record['content']}
+                              for record in rrset['records']],
+             'ttl': rrset['ttl']}
+            for rrset in get_zone(domain)['rrsets']]
 
 
 def set_rrset(rrset):
@@ -176,14 +167,11 @@ def set_rrset(rrset):
 
 
 def set_rrsets(domain, rrsets):
-    from desecapi.serializers import GenericRRsetSerializer
-    rrsets = [GenericRRsetSerializer(rrset).data for rrset in rrsets]
-
     data = {'rrsets':
-        [{'name': rrset['name'], 'type': rrset['type'], 'ttl': rrset['ttl'],
+        [{'name': rrset.name, 'type': rrset.type, 'ttl': rrset.ttl,
           'changetype': 'REPLACE',
-          'records': [{'content': record, 'disabled': False}
-                      for record in rrset['records']]
+          'records': [{'content': record.content, 'disabled': False}
+                      for record in rrset.records.all()]
           }
          for rrset in rrsets]
     }

api/desecapi/serializers.py

@@ -1,45 +1,53 @@
 from rest_framework import serializers
-from desecapi.models import Domain, Donation, User, RRset
+from desecapi.models import Domain, Donation, User, RR, RRset
 from djoser import serializers as djoserSerializers
-import json
 
 
-class JSONSerializer(serializers.Field):
-    def to_representation(self, obj):
-        return json.loads(obj)
-
-    def to_internal_value(self, data):
-        return json.dumps(data)
-
-
-class RecordsSerializer(JSONSerializer):
-    def to_internal_value(self, records):
-        if isinstance(records, str) or not all(isinstance(record, str) for record in records):
-            msg = 'Incorrect type. Expected a list of strings'
-            raise serializers.ValidationError(msg)
-
-        # https://lists.isc.org/pipermail/bind-users/2008-April/070148.html
-        if not len(records) < 4092:
-            msg = 'Records too long. Must be less than 4092 characters, but was %d'
-            raise serializers.ValidationError(msg % len(records))
-
-        return super().to_internal_value(records)
+class RRSerializer(serializers.ModelSerializer):
+    class Meta:
+        model = RR
+        fields = ('content',)
 
 
-class GenericRRsetSerializer(serializers.ModelSerializer):
+class RRsetSerializer(serializers.ModelSerializer):
+    domain = serializers.StringRelatedField()
     subname = serializers.CharField(allow_blank=True, required=False)
     type = serializers.CharField(required=False)
-    records = RecordsSerializer()
+    records = serializers.SerializerMethodField()
 
 
     class Meta:
         model = RRset
         fields = ('domain', 'subname', 'name', 'records', 'ttl', 'type',)
 
-
-class RRsetSerializer(GenericRRsetSerializer):
-    # The value of this field is set in RRsetList.perform_create()
-    domain = serializers.SlugRelatedField(read_only=True, slug_field='name')
+    def _inject_records_data(self, validated_data):
+        records_data = [{'content': x}
+                        for x in self.context['request'].data['records']]
+        rrs = RRSerializer(data=records_data, many=True, allow_empty=False)
+        if not rrs.is_valid():
+            errors = rrs.errors
+            if 'non_field_errors' in errors:
+                errors['records'] = errors.pop('non_field_errors')
+            raise serializers.ValidationError(errors)
+
+        return {'records_data': rrs.validated_data, **validated_data}
+
+    def create(self, validated_data):
+        validated_data = self._inject_records_data(validated_data)
+        return super().create(validated_data)
+
+    def update(self, instance, validated_data):
+        validated_data = self._inject_records_data(validated_data)
+        return super().update(instance, validated_data)
+
+    def get_records(self, obj):
+        return [x for x in obj.records.values_list('content', flat=True)]
+
+    def validate_type(self, value):
+        if value in RRset.RESTRICTED_TYPES:
+            raise serializers.ValidationError(
+                "You cannot tinker with the %s RRset." % value)
+        return value
 
 
 class DomainSerializer(serializers.ModelSerializer):

api/desecapi/tests/testrrsets.py

@@ -144,14 +144,14 @@ class AuthenticatedRRsetTests(APITestCase):
         url = reverse('rrsets', args=(self.ownedDomains[1].name,))
         data = {'records': [], 'ttl': 60, 'type': 'A'}
         response = self.client.post(url, json.dumps(data), content_type='application/json')
-        self.assertEqual(response.status_code, status.HTTP_422_UNPROCESSABLE_ENTITY)
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
 
     def testCantPostRestrictedTypes(self):
         for type_ in self.restricted_types:
             url = reverse('rrsets', args=(self.ownedDomains[1].name,))
             data = {'records': ['ns1.desec.io. peter.desec.io. 2584 10800 3600 604800 60'], 'ttl': 60, 'type': type_}
             response = self.client.post(url, json.dumps(data), content_type='application/json')
-            self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+            self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
 
     def testCantPostForeignRRsets(self):
         url = reverse('rrsets', args=(self.otherDomains[1].name,))
@@ -400,7 +400,7 @@ class AuthenticatedRRsetTests(APITestCase):
         # Create record, should cause a pdns PATCH request and a notify
         url = reverse('rrsets', args=(self.ownedDomains[1].name,))
         data = {'records': ['1.2.3.4'], 'ttl': 60, 'type': 'A'}
-        response = self.client.post(url, json.dumps(data), content_type='application/json')
+        self.client.post(url, json.dumps(data), content_type='application/json')
 
         # Delete record, should cause a pdns PATCH request and a notify
         url = reverse('rrset', args=(self.ownedDomains[1].name, '', 'A',))

api/desecapi/views.py

@@ -107,7 +107,7 @@ class DomainDetail(generics.RetrieveUpdateDestroyAPIView):
 
     def delete(self, request, *args, **kwargs):
         try:
-            super(DomainDetail, self).delete(request, *args, **kwargs)
+            super().delete(request, *args, **kwargs)
         except Http404:
             pass
         return Response(status=status.HTTP_204_NO_CONTENT)
@@ -117,7 +117,7 @@ class DomainDetail(generics.RetrieveUpdateDestroyAPIView):
 
     def update(self, request, *args, **kwargs):
         try:
-            return super(DomainDetail, self).update(request, *args, **kwargs)
+            return super().update(request, *args, **kwargs)
         except django.core.exceptions.ValidationError as e:
             ex = ValidationError(detail={"detail": str(e)})
             ex.status_code = status.HTTP_409_CONFLICT
@@ -184,10 +184,6 @@ class RRsetList(generics.ListCreateAPIView):
         return rrsets
 
     def create(self, request, *args, **kwargs):
-        type_ = request.data.get('type', '')
-        if type_ in RRset.RESTRICTED_TYPES:
-            raise PermissionDenied("You cannot tinker with the %s RRset." % type_)
-
         try:
             return super().create(request, *args, **kwargs)
         except Domain.DoesNotExist:

docs/rrsets.rst

@@ -278,14 +278,15 @@ problem to enable such functionality.
     order to secure the data stored in your zones.  RRsets of this type are
     generated and served automatically by our nameservers.  However, you can
     neither read nor manipulate these RRsets through the API.  When attempting
-    such operations, ``403 Forbidden`` is returned.
+    such operations, ``403 Forbidden`` or ``400 Bad Request`` is returned,
+    respectively.
 
 .. _`SOA caveat`:
 
 ``SOA`` record
     The ``SOA`` record cannot be read or written through this interface.  When
-    attempting to create, modify or otherwise access an ``SOA`` record, ``403
-    Forbidden`` is returned.
+    attempting to create, modify or otherwise access an ``SOA`` record, ``400
+    Bad Request`` or ``403 Forbidden`` is returned, respectively.
 
     The rationale behind this is that the content of the ``SOA`` record is
     entirely determined by the DNS operator, and users should not have to bother