API: change PATCH/PUT on zones to return 204 No Content instead of full zone

Breaks backwards compat, obviously.

diff --git a/docs/markdown/httpapi/api_spec.md b/docs/markdown/httpapi/api_spec.md
index 9ee46c148222082cb43665dcf32156321e20e665..bdcb78cff14ee2d410cef853184cc922c9d447a2 100644 (file)
--- a/docs/markdown/httpapi/api_spec.md
+++ b/docs/markdown/httpapi/api_spec.md
@@ -35,7 +35,7 @@ REST
 
 * GET: List/Retrieve. Success reply: `200 OK`
 * POST: Create. Success reply: `201 Created`, with new object as body.
-* PUT: Update. Success reply: `200 OK`, with modified object as body.
+* PUT: Update. Success reply: `200 OK`, with modified object as body. For some operations, `204 No Content` is returned instead (and the modified object is not given in the body).
 * DELETE: Delete. Success reply: `200 OK`, no body.
 
 not-so-REST
@@ -467,6 +467,7 @@ Deletes this zone, all attached metadata and rrsets.
 #### PATCH
 
 Modifies present RRsets and comments.
+Returns `204 No Content` on success.
 
 **Note**: Authoritative only.
 
@@ -536,6 +537,7 @@ Client body for PATCH:
 Modifies basic zone data (metadata).
 
 Allowed fields in client body: all except `id` and `url`.
+Returns `204 No Content` on success.
 
 Changing `name` renames the zone, as expected.
 

diff --git a/pdns/ws-auth.cc b/pdns/ws-auth.cc
index 22f2363487e7930983b1f628c8208e62e4c6e8da..a568c2e5a7d5e10d44d9e5182c963c4855b63ad2 100644 (file)
--- a/pdns/ws-auth.cc
+++ b/pdns/ws-auth.cc
@@ -792,7 +792,8 @@ static void apiServerZoneDetail(HttpRequest* req, HttpResponse* resp) {
 
     updateDomainSettingsFromDocument(di, zonename, req->json());
 
-    fillZone(zonename, resp);
+    resp->body = "";
+    resp->status = 204; // No Content, but indicate success
     return;
   }
   else if(req->method == "DELETE" && !::arg().mustDo("api-readonly")) {
@@ -1101,8 +1102,9 @@ static void patchZone(HttpRequest* req, HttpResponse* resp) {
   // now the PTRs
   storeChangedPTRs(B, new_ptrs);
 
-  // success
-  fillZone(zonename, resp);
+  resp->body = "";
+  resp->status = 204; // No Content, but indicate success
+  return;
 }
 
 static void apiServerSearchData(HttpRequest* req, HttpResponse* resp) {

diff --git a/pdns/ws-recursor.cc b/pdns/ws-recursor.cc
index 72cdf95e824d35e6f2a2650306cad03dfcc577d0..6dc2b710960e3af300f3ccb221cf09c8330550c2 100644 (file)
--- a/pdns/ws-recursor.cc
+++ b/pdns/ws-recursor.cc
@@ -300,7 +300,8 @@ static void apiServerZoneDetail(HttpRequest* req, HttpResponse* resp)
     doDeleteZone(zonename);
     doCreateZone(document);
     reloadAuthAndForwards();
-    fillZone(apiNameToDNSName(stringFromJson(document, "name")), resp);
+    resp->body = "";
+    resp->status = 204; // No Content, but indicate success
   }
   else if(req->method == "DELETE" && !::arg().mustDo("api-readonly")) {
     if (!doDeleteZone(zonename)) {

diff --git a/regression-tests.api/test_Zones.py b/regression-tests.api/test_Zones.py
index a8cfeb7f0f74203d191e0449b86245b91b58b720..93e345b8c0e70cd61d4579f8c7c4cb169d902180 100644 (file)
--- a/regression-tests.api/test_Zones.py
+++ b/regression-tests.api/test_Zones.py
@@ -567,8 +567,8 @@ fred   IN  A      192.168.0.4
             self.url("/api/v1/servers/localhost/zones/" + name),
             data=json.dumps(payload),
             headers={'content-type': 'application/json'})
-        self.assert_success_json(r)
-        data = r.json()
+        self.assert_success(r)
+        data = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name)).json()
         for k in payload.keys():
             self.assertIn(k, data)
             self.assertEquals(data[k], payload[k])
@@ -582,8 +582,8 @@ fred   IN  A      192.168.0.4
             self.url("/api/v1/servers/localhost/zones/" + name),
             data=json.dumps(payload),
             headers={'content-type': 'application/json'})
-        self.assert_success_json(r)
-        data = r.json()
+        self.assert_success(r)
+        data = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name)).json()
         for k in payload.keys():
             self.assertIn(k, data)
             self.assertEquals(data[k], payload[k])
@@ -612,10 +612,9 @@ fred   IN  A      192.168.0.4
             self.url("/api/v1/servers/localhost/zones/" + name),
             data=json.dumps(payload),
             headers={'content-type': 'application/json'})
-        self.assert_success_json(r)
+        self.assert_success(r)
         # verify that (only) the new record is there
-        r = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name))
-        data = r.json()
+        data = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name)).json()
         self.assertEquals(get_rrset(data, name, 'NS')['records'], rrset['records'])
 
     def test_zone_rr_update_mx(self):
@@ -639,10 +638,9 @@ fred   IN  A      192.168.0.4
             self.url("/api/v1/servers/localhost/zones/" + name),
             data=json.dumps(payload),
             headers={'content-type': 'application/json'})
-        self.assert_success_json(r)
+        self.assert_success(r)
         # verify that (only) the new record is there
-        r = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name))
-        data = r.json()
+        data = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name)).json()
         self.assertEquals(get_rrset(data, name, 'MX')['records'], rrset['records'])
 
     def test_zone_rr_update_multiple_rrsets(self):
@@ -677,10 +675,9 @@ fred   IN  A      192.168.0.4
             self.url("/api/v1/servers/localhost/zones/" + name),
             data=json.dumps(payload),
             headers={'content-type': 'application/json'})
-        self.assert_success_json(r)
+        self.assert_success(r)
         # verify that all rrsets have been updated
-        r = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name))
-        data = r.json()
+        data = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name)).json()
         self.assertEquals(get_rrset(data, name, 'NS')['records'], rrset1['records'])
         self.assertEquals(get_rrset(data, name, 'MX')['records'], rrset2['records'])
 
@@ -697,10 +694,9 @@ fred   IN  A      192.168.0.4
             self.url("/api/v1/servers/localhost/zones/" + name),
             data=json.dumps(payload),
             headers={'content-type': 'application/json'})
-        self.assert_success_json(r)
+        self.assert_success(r)
         # verify that the records are gone
-        r = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name))
-        data = r.json()
+        data = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name)).json()
         self.assertIsNone(get_rrset(data, name, 'NS'))
 
     def test_zone_disable_reenable(self):
@@ -724,9 +720,10 @@ fred   IN  A      192.168.0.4
             self.url("/api/v1/servers/localhost/zones/" + name),
             data=json.dumps(payload),
             headers={'content-type': 'application/json'})
-        self.assert_success_json(r)
+        self.assert_success(r)
         # check SOA serial has been edited
-        soa_serial1 = get_first_rec(r.json(), name, 'SOA')['content'].split()[2]
+        data = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name)).json()
+        soa_serial1 = get_first_rec(data, name, 'SOA')['content'].split()[2]
         self.assertNotEquals(soa_serial1, '1')
         # make sure domain is still in zone list (disabled SOA!)
         r = self.session.get(self.url("/api/v1/servers/localhost/zones"))
@@ -741,10 +738,10 @@ fred   IN  A      192.168.0.4
             self.url("/api/v1/servers/localhost/zones/" + name),
             data=json.dumps(payload),
             headers={'content-type': 'application/json'})
-        self.assert_success_json(r)
+        self.assert_success(r)
         # check SOA serial has been edited again
-        print r.json()
-        soa_serial2 = get_first_rec(r.json(), name, 'SOA')['content'].split()[2]
+        data = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name)).json()
+        soa_serial2 = get_first_rec(data, name, 'SOA')['content'].split()[2]
         self.assertNotEquals(soa_serial2, '1')
         self.assertNotEquals(soa_serial2, soa_serial1)
 
@@ -848,7 +845,7 @@ fred   IN  A      192.168.0.4
             data=json.dumps(payload),
             headers={'content-type': 'application/json'})
         print r.content
-        self.assertEquals(r.status_code, 200)  # succeed so users can fix their wrong, old data
+        self.assert_success(r)  # succeed so users can fix their wrong, old data
 
     def test_zone_delete(self):
         name, payload, zone = self.create_zone()
@@ -879,11 +876,11 @@ fred   IN  A      192.168.0.4
             self.url("/api/v1/servers/localhost/zones/" + name),
             data=json.dumps(payload),
             headers={'content-type': 'application/json'})
-        self.assert_success_json(r)
+        self.assert_success(r)
         # make sure the comments have been set, and that the NS
         # records are still present
-        r = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name))
-        serverset = get_rrset(r.json(), name, 'NS')
+        data = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name)).json()
+        serverset = get_rrset(data, name, 'NS')
         print serverset
         self.assertNotEquals(serverset['records'], [])
         self.assertNotEquals(serverset['comments'], [])
@@ -904,10 +901,10 @@ fred   IN  A      192.168.0.4
             self.url("/api/v1/servers/localhost/zones/" + name),
             data=json.dumps(payload),
             headers={'content-type': 'application/json'})
-        self.assert_success_json(r)
+        self.assert_success(r)
         # make sure the NS records are still present
-        r = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name))
-        serverset = get_rrset(r.json(), name, 'NS')
+        data = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name)).json()
+        serverset = get_rrset(data, name, 'NS')
         print serverset
         self.assertNotEquals(serverset['records'], [])
         self.assertEquals(serverset['comments'], [])
@@ -933,7 +930,7 @@ fred   IN  A      192.168.0.4
             self.url("/api/v1/servers/localhost/zones/" + name),
             data=json.dumps(payload),
             headers={'content-type': 'application/json'})
-        self.assert_success_json(r)
+        self.assert_success(r)
         # replace rrset records
         rrset2 = {
             'changetype': 'replace',
@@ -952,10 +949,10 @@ fred   IN  A      192.168.0.4
             self.url("/api/v1/servers/localhost/zones/" + name),
             data=json.dumps(payload2),
             headers={'content-type': 'application/json'})
-        self.assert_success_json(r)
+        self.assert_success(r)
         # make sure the comments still exist
-        r = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name))
-        serverset = get_rrset(r.json(), name, 'NS')
+        data = self.session.get(self.url("/api/v1/servers/localhost/zones/" + name)).json()
+        serverset = get_rrset(data, name, 'NS')
         print serverset
         self.assertEquals(serverset['records'], rrset2['records'])
         self.assertEquals(serverset['comments'], rrset['comments'])
@@ -1015,7 +1012,7 @@ fred   IN  A      192.168.0.4
             self.url("/api/v1/servers/localhost/zones/" + name),
             data=json.dumps(payload),
             headers={'content-type': 'application/json'})
-        self.assert_success_json(r)
+        self.assert_success(r)
         r = self.session.get(self.url("/api/v1/servers/localhost/zones/" + revzone)).json()
         revsets = [s for s in r['rrsets'] if s['type'] == 'PTR']
         print revsets
@@ -1055,7 +1052,7 @@ fred   IN  A      192.168.0.4
             self.url("/api/v1/servers/localhost/zones/" + name),
             data=json.dumps(payload),
             headers={'content-type': 'application/json'})
-        self.assert_success_json(r)
+        self.assert_success(r)
         r = self.session.get(self.url("/api/v1/servers/localhost/zones/" + revzone)).json()
         revsets = [s for s in r['rrsets'] if s['type'] == 'PTR']
         print revsets
@@ -1159,8 +1156,8 @@ class AuthRootZone(ApiTestCase, AuthZonesHelperMixin):
             self.url("/api/v1/servers/localhost/zones/" + zone_id),
             data=json.dumps(payload),
             headers={'content-type': 'application/json'})
-        self.assert_success_json(r)
-        data = r.json()
+        self.assert_success(r)
+        data = self.session.get(self.url("/api/v1/servers/localhost/zones/" + zone_id)).json()
         for k in payload.keys():
             self.assertIn(k, data)
             self.assertEquals(data[k], payload[k])
@@ -1174,8 +1171,8 @@ class AuthRootZone(ApiTestCase, AuthZonesHelperMixin):
             self.url("/api/v1/servers/localhost/zones/" + zone_id),
             data=json.dumps(payload),
             headers={'content-type': 'application/json'})
-        self.assert_success_json(r)
-        data = r.json()
+        self.assert_success(r)
+        data = self.session.get(self.url("/api/v1/servers/localhost/zones/" + zone_id)).json()
         for k in payload.keys():
             self.assertIn(k, data)
             self.assertEquals(data[k], payload[k])
@@ -1257,8 +1254,8 @@ class RecursorZones(ApiTestCase):
             self.url("/api/v1/servers/localhost/zones/" + name),
             data=json.dumps(payload),
             headers={'content-type': 'application/json'})
-        self.assert_success_json(r)
-        data = r.json()
+        self.assert_success(r)
+        data = self.session.get(self.url("/api/v1/servers/localhost/zones/" + payload['name'])).json()
         for k in payload.keys():
             self.assertEquals(data[k], payload[k])
 

diff --git a/regression-tests.api/test_helper.py b/regression-tests.api/test_helper.py
index fc2fdbd34779c6810288c2178fc6d218bb897b5e..04a851a9710c4d3829d4b259f52cc18ba5314e34 100644 (file)
--- a/regression-tests.api/test_helper.py
+++ b/regression-tests.api/test_helper.py
@@ -30,6 +30,13 @@ class ApiTestCase(unittest.TestCase):
             raise
         self.assertEquals(result.headers['Content-Type'], 'application/json')
 
+    def assert_success(self, result):
+        try:
+            result.raise_for_status()
+        except:
+            print result.content
+            raise
+
 
 def unique_zone_name():
     return 'test-' + datetime.now().strftime('%d%H%S%M%f') + '.org.'