瀏覽代碼

fix(api): make DELETE on DomainDetail idempotent, fixes #12

- return 204 instead of 404 if domain does not exist
- don't raise an expection if the domain does not exist on nslord
  or nsmaster
- update tests
Peter Thomassen 8 年之前
父節點
當前提交
fd92373104
共有 3 個文件被更改,包括 17 次插入4 次删除
  1. 6 2
      api/desecapi/pdns.py
  2. 4 2
      api/desecapi/tests/testdomains.py
  3. 7 0
      api/desecapi/views.py

+ 6 - 2
api/desecapi/pdns.py

@@ -25,12 +25,16 @@ def _pdns_delete(url):
     # We thus issue a second delete request on nsmaster to delete the zone there immediately.
     # We thus issue a second delete request on nsmaster to delete the zone there immediately.
     r1 = requests.delete(settings.NSLORD_PDNS_API + url, headers=headers_nslord)
     r1 = requests.delete(settings.NSLORD_PDNS_API + url, headers=headers_nslord)
     if r1.status_code < 200 or r1.status_code >= 300:
     if r1.status_code < 200 or r1.status_code >= 300:
-        raise Exception(r1.text)
+        # Deletion technically does not fail if the zone didn't exist in the first place
+        if r1.status_code == 422 and 'Could not find domain' in r1.text:
+            pass
+        else:
+            raise Exception(r1.text)
 
 
     # Delete from nsmaster as well
     # Delete from nsmaster as well
     r2 = requests.delete(settings.NSMASTER_PDNS_API + url, headers=headers_nsmaster)
     r2 = requests.delete(settings.NSMASTER_PDNS_API + url, headers=headers_nsmaster)
     if r2.status_code < 200 or r2.status_code >= 300:
     if r2.status_code < 200 or r2.status_code >= 300:
-        # Allow this to fail if nsmaster does not know the zone yet
+        # Deletion technically does not fail if the zone didn't exist in the first place
         if r2.status_code == 422 and 'Could not find domain' in r2.text:
         if r2.status_code == 422 and 'Could not find domain' in r2.text:
             pass
             pass
         else:
         else:

+ 4 - 2
api/desecapi/tests/testdomains.py

@@ -76,8 +76,9 @@ class AuthenticatedDomainTests(APITestCase):
 
 
         url = reverse('domain-detail', args=(self.otherDomains[1].pk,))
         url = reverse('domain-detail', args=(self.otherDomains[1].pk,))
         response = self.client.delete(url)
         response = self.client.delete(url)
-        self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+        self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)
         self.assertTrue(isinstance(httpretty.last_request(), httpretty.core.HTTPrettyRequestEmpty))
         self.assertTrue(isinstance(httpretty.last_request(), httpretty.core.HTTPrettyRequestEmpty))
+        self.assertTrue(Domain.objects.filter(pk=self.otherDomains[1].pk).exists())
 
 
     def testCanGetOwnedDomains(self):
     def testCanGetOwnedDomains(self):
         url = reverse('domain-detail', args=(self.ownedDomains[1].pk,))
         url = reverse('domain-detail', args=(self.ownedDomains[1].pk,))
@@ -235,8 +236,9 @@ class AuthenticatedDynDomainTests(APITestCase):
 
 
         url = reverse('domain-detail', args=(self.otherDomains[1].pk,))
         url = reverse('domain-detail', args=(self.otherDomains[1].pk,))
         response = self.client.delete(url)
         response = self.client.delete(url)
-        self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+        self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)
         self.assertTrue(isinstance(httpretty.last_request(), httpretty.core.HTTPrettyRequestEmpty))
         self.assertTrue(isinstance(httpretty.last_request(), httpretty.core.HTTPrettyRequestEmpty))
+        self.assertTrue(Domain.objects.filter(pk=self.otherDomains[1].pk).exists())
 
 
     def testCanPostDynDomains(self):
     def testCanPostDynDomains(self):
         url = reverse('domain-list')
         url = reverse('domain-list')

+ 7 - 0
api/desecapi/views.py

@@ -88,6 +88,13 @@ class DomainDetail(generics.RetrieveUpdateDestroyAPIView):
     serializer_class = DomainSerializer
     serializer_class = DomainSerializer
     permission_classes = (permissions.IsAuthenticated, IsOwner,)
     permission_classes = (permissions.IsAuthenticated, IsOwner,)
 
 
+    def delete(self, request, *args, **kwargs):
+        try:
+            super(DomainDetail, self).delete(request, *args, **kwargs)
+        except Http404:
+            pass
+        return Response(status=status.HTTP_204_NO_CONTENT)
+
     def get_queryset(self):
     def get_queryset(self):
         return Domain.objects.filter(owner=self.request.user.pk)
         return Domain.objects.filter(owner=self.request.user.pk)