From e4a004097eb286a7bb8a631183dce110300d2410 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Mon, 5 Jun 2017 09:04:58 +0300 Subject: [PATCH] MXS-1220: Add support for PATCH The PATCH method should now be used instead the PUT method to update resources. As PUT request bodies should represent complete resources, the use of PUT to update resources is no longer supported. Altered tests to use PATCH instead of PUT for updating resources. --- Documentation/REST-API/API.md | 14 ++++++++++---- Documentation/REST-API/Resources-Monitor.md | 2 +- Documentation/REST-API/Resources-Server.md | 14 +++++++------- Documentation/REST-API/Resources-Service.md | 2 +- server/core/admin.cc | 2 +- server/core/resource.cc | 18 ++++++++++++------ server/core/test/rest-api/test/auth.js | 2 +- server/core/test/rest-api/test/core.js | 2 +- server/core/test/rest-api/test/errors.js | 2 +- server/core/test/rest-api/test/http.js | 4 ++-- server/core/test/rest-api/test/logs.js | 2 +- server/core/test/rest-api/test/monitor.js | 10 +++++----- server/core/test/rest-api/test/server.js | 4 ++-- server/core/test/rest-api/test/service.js | 6 +++--- 14 files changed, 48 insertions(+), 36 deletions(-) diff --git a/Documentation/REST-API/API.md b/Documentation/REST-API/API.md index 1d3986aa9..c203e0f55 100644 --- a/Documentation/REST-API/API.md +++ b/Documentation/REST-API/API.md @@ -111,10 +111,16 @@ Credentials for authentication. #### Content-Type All PUT and POST requests must use the `Content-Type: application/json` media -type and the request body must be a valid JSON representation of a resource. All -PATCH requests must use the `Content-Type: application/json` media type and the -request body must be a JSON document containing a partial definition of the -original resource. +type and the request body must be a complete and valid JSON representation of a +resource. All PATCH requests must use the `Content-Type: application/json` media +type and the request body must be a JSON document containing a partial +definition of the original resource. + +The current version of the API supports PATCH-like PUT requests with +partial definitions of resources in the request body. This is discouraged +as it goes against the intended use of the PUT method. Future versions of +the MaxScale REST API can remove this support which means that this +functionality is deprecated. #### Host diff --git a/Documentation/REST-API/Resources-Monitor.md b/Documentation/REST-API/Resources-Monitor.md index cb7a70f6e..832577c74 100644 --- a/Documentation/REST-API/Resources-Monitor.md +++ b/Documentation/REST-API/Resources-Monitor.md @@ -267,7 +267,7 @@ The :name in the URI must map to a monitor name with all whitespace replaced wit hyphens. The request body must be a valid JSON document representing the modified monitor. ``` -PUT /v1/monitor/:name +PATCH /v1/monitor/:name ``` ### Modifiable Fields diff --git a/Documentation/REST-API/Resources-Server.md b/Documentation/REST-API/Resources-Server.md index 8254fa896..c50f186ca 100644 --- a/Documentation/REST-API/Resources-Server.md +++ b/Documentation/REST-API/Resources-Server.md @@ -312,7 +312,7 @@ Status: 403 Forbidden ### Update a server ``` -PUT /v1/servers/:name +PATCH /v1/servers/:name ``` The _:name_ in the URI must map to a server name with all whitespace replaced @@ -443,12 +443,12 @@ Request for `PUT /v1/server/server1`: } ``` -The current implementation accepts both PUT and PATCH requests with partially -defined resources as request body. If parts of the resource are not defined -(e.g. the `attributes` field in the above example), those parts of the resource -are not modified. All parts that are defined are interpreted as the new -definition of those part of the resource. In the above example, the -`relationships` of the resource are completely redefined. +The current implementation accepts PATCH requests with partially defined +resources as request body. If parts of the resource are not defined (e.g. the +`attributes` field in the above example), those parts of the resource are not +modified. All parts that are defined are interpreted as the new definition of +those part of the resource. In the above example, the `relationships` of the +resource are completely redefined. #### Response diff --git a/Documentation/REST-API/Resources-Service.md b/Documentation/REST-API/Resources-Service.md index 0f34f550b..e43c585b8 100644 --- a/Documentation/REST-API/Resources-Service.md +++ b/Documentation/REST-API/Resources-Service.md @@ -265,7 +265,7 @@ The _:name_ in the URI must map to a service name and the request body must be a valid JSON Patch document which is applied to the resource. ``` -PUT /v1/services/:name +PATCH /v1/services/:name ``` The following standard service parameters can be modified. diff --git a/server/core/admin.cc b/server/core/admin.cc index 809f01343..fc484dda4 100644 --- a/server/core/admin.cc +++ b/server/core/admin.cc @@ -75,7 +75,7 @@ static inline size_t request_data_length(MHD_Connection *connection) static bool modifies_data(MHD_Connection *connection, string method) { return (method == MHD_HTTP_METHOD_POST || method == MHD_HTTP_METHOD_PUT || - method == MHD_HTTP_METHOD_DELETE) && + method == MHD_HTTP_METHOD_DELETE || method == MHD_HTTP_METHOD_PATCH) && request_data_length(connection); } diff --git a/server/core/resource.cc b/server/core/resource.cc index dc9da38e9..f69542b8b 100644 --- a/server/core/resource.cc +++ b/server/core/resource.cc @@ -677,11 +677,11 @@ public: m_post.push_back(SResource(new Resource(cb_modulecmd, 4, "maxscale", "modules", ":module", "?"))); /** Update resources */ - m_put.push_back(SResource(new Resource(cb_alter_server, 2, "servers", ":server"))); - m_put.push_back(SResource(new Resource(cb_alter_monitor, 2, "monitors", ":monitor"))); - m_put.push_back(SResource(new Resource(cb_alter_service, 2, "services", ":service"))); - m_put.push_back(SResource(new Resource(cb_alter_logs, 2, "maxscale", "logs"))); - m_put.push_back(SResource(new Resource(cb_alter_maxscale, 1, "maxscale"))); + m_patch.push_back(SResource(new Resource(cb_alter_server, 2, "servers", ":server"))); + m_patch.push_back(SResource(new Resource(cb_alter_monitor, 2, "monitors", ":monitor"))); + m_patch.push_back(SResource(new Resource(cb_alter_service, 2, "services", ":service"))); + m_patch.push_back(SResource(new Resource(cb_alter_logs, 2, "maxscale", "logs"))); + m_patch.push_back(SResource(new Resource(cb_alter_maxscale, 1, "maxscale"))); /** Change resource states */ m_put.push_back(SResource(new Resource(cb_stop_monitor, 3, "monitors", ":monitor", "stop"))); @@ -774,6 +774,10 @@ public: { return process_request_type(m_put, request); } + else if (request.get_verb() == MHD_HTTP_METHOD_PATCH) + { + return process_request_type(m_patch, request); + } else if (request.get_verb() == MHD_HTTP_METHOD_POST) { return process_request_type(m_post, request); @@ -810,6 +814,7 @@ private: ResourceList m_put; /**< PUT request handlers */ ResourceList m_post; /**< POST request handlers */ ResourceList m_delete; /**< DELETE request handlers */ + ResourceList m_patch; /**< PATCH request handlers */ }; static RootResource resources; /**< Core resource set */ @@ -820,7 +825,8 @@ static bool request_modifies_data(const string& verb) { return verb == MHD_HTTP_METHOD_POST || verb == MHD_HTTP_METHOD_PUT || - verb == MHD_HTTP_METHOD_DELETE; + verb == MHD_HTTP_METHOD_DELETE || + verb == MHD_HTTP_METHOD_PATCH; } static bool request_reads_data(const string& verb) diff --git a/server/core/test/rest-api/test/auth.js b/server/core/test/rest-api/test/auth.js index b8489b905..aa0eaf783 100644 --- a/server/core/test/rest-api/test/auth.js +++ b/server/core/test/rest-api/test/auth.js @@ -6,7 +6,7 @@ function set_auth(auth, value) { .then(function(resp) { var d = JSON.parse(resp) d.data.attributes.parameters.admin_auth = value; - return request.put(auth + host + "/maxscale", { json: d }) + return request.patch(auth + host + "/maxscale", { json: d }) }) .then(function() { return request.get(auth + host + "/maxscale") diff --git a/server/core/test/rest-api/test/core.js b/server/core/test/rest-api/test/core.js index 46fa3f279..ad8a4a12a 100644 --- a/server/core/test/rest-api/test/core.js +++ b/server/core/test/rest-api/test/core.js @@ -6,7 +6,7 @@ function set_value(key, value) { .then(function(resp) { var d = JSON.parse(resp) d.data.attributes.parameters[key] = value; - return request.put(base_url + "/maxscale", { json: d }) + return request.patch(base_url + "/maxscale", { json: d }) }) .then(function() { return request.get(base_url + "/maxscale") diff --git a/server/core/test/rest-api/test/errors.js b/server/core/test/rest-api/test/errors.js index c8885a81e..a1d498048 100644 --- a/server/core/test/rest-api/test/errors.js +++ b/server/core/test/rest-api/test/errors.js @@ -7,7 +7,7 @@ describe("Errors", function() it("error on invalid PUT request", function() { - return request.put(base_url + "/servers/server1", { json: {this_is: "a test"}}) + return request.patch(base_url + "/servers/server1", { json: {this_is: "a test"}}) .should.be.rejected }) diff --git a/server/core/test/rest-api/test/http.js b/server/core/test/rest-api/test/http.js index 8287e5826..9a0104a9a 100644 --- a/server/core/test/rest-api/test/http.js +++ b/server/core/test/rest-api/test/http.js @@ -10,7 +10,7 @@ describe("HTTP Headers", function() { resp.headers.etag.should.be.equal("\"0\"") var srv = JSON.parse(resp.body) delete srv.data.relationships - return request.put(base_url + "/servers/server1", {json: srv}) + return request.patch(base_url + "/servers/server1", {json: srv}) }) .then(function() { return request.get(base_url + "/servers/server1", {resolveWithFullResponse: true}) @@ -42,7 +42,7 @@ describe("HTTP Headers", function() { } } - request.put(base_url + "/servers/server1", {json: srv}) + request.patch(base_url + "/servers/server1", {json: srv}) .then(function() { return request.get(base_url + "/servers/server1", {resolveWithFullResponse: true}) }) diff --git a/server/core/test/rest-api/test/logs.js b/server/core/test/rest-api/test/logs.js index 10bd2d4ed..2c2e24326 100644 --- a/server/core/test/rest-api/test/logs.js +++ b/server/core/test/rest-api/test/logs.js @@ -17,7 +17,7 @@ describe("Logs", function() { logs.data.attributes.parameters.throttling.suppress_ms = 1 logs.data.attributes.parameters.throttling.window_ms = 1 - return request.put(base_url + "/maxscale/logs", {json: logs}) + return request.patch(base_url + "/maxscale/logs", {json: logs}) }) .then(function(resp) { return request.get(base_url + "/maxscale/logs") diff --git a/server/core/test/rest-api/test/monitor.js b/server/core/test/rest-api/test/monitor.js index 4d28bec3e..7c4deadcc 100644 --- a/server/core/test/rest-api/test/monitor.js +++ b/server/core/test/rest-api/test/monitor.js @@ -27,7 +27,7 @@ describe("Monitor", function() { monitor.data.attributes.parameters = { monitor_interval: 1000 } - return request.put(base_url + "/monitors/" + monitor.data.id, {json:monitor}) + return request.patch(base_url + "/monitors/" + monitor.data.id, {json:monitor}) .should.be.fulfilled }); @@ -53,7 +53,7 @@ describe("Monitor Relationships", function() { .then(function(resp) { var mon = JSON.parse(resp) delete mon.data.relationships.servers - return request.put(base_url + "/monitors/MySQL-Monitor", {json: mon}) + return request.patch(base_url + "/monitors/MySQL-Monitor", {json: mon}) }) .should.be.fulfilled }); @@ -69,7 +69,7 @@ describe("Monitor Relationships", function() { {id: "server3", type: "servers"}, {id: "server4", type: "servers"}, ] - return request.put(base_url + "/monitors/" + monitor.data.id, {json: mon}) + return request.patch(base_url + "/monitors/" + monitor.data.id, {json: mon}) }) .should.be.fulfilled }); @@ -80,7 +80,7 @@ describe("Monitor Relationships", function() { .then(function(resp) { var mon = JSON.parse(resp) delete mon.data.relationships.servers - return request.put(base_url + "/monitors/" + monitor.data.id, {json: mon}) + return request.patch(base_url + "/monitors/" + monitor.data.id, {json: mon}) }) .then(function() { return request.get(base_url + "/monitors/MySQL-Monitor") @@ -93,7 +93,7 @@ describe("Monitor Relationships", function() { {id: "server3", type: "servers"}, {id: "server4", type: "servers"}, ] - return request.put(base_url + "/monitors/MySQL-Monitor", {json: mon}) + return request.patch(base_url + "/monitors/MySQL-Monitor", {json: mon}) }) .should.be.fulfilled }); diff --git a/server/core/test/rest-api/test/server.js b/server/core/test/rest-api/test/server.js index 30f4ae9d4..4e0b86360 100644 --- a/server/core/test/rest-api/test/server.js +++ b/server/core/test/rest-api/test/server.js @@ -38,7 +38,7 @@ describe("Server", function() { it("update server", function() { server.data.attributes.parameters.weight = 10 - return request.put(base_url + "/servers/" + server.data.id, { json: server}) + return request.patch(base_url + "/servers/" + server.data.id, { json: server}) .should.be.fulfilled }); @@ -69,7 +69,7 @@ describe("Server Relationships", function() { it("remove relationships", function() { delete rel_server.data["relationships"] - return request.put(base_url + "/servers/" + rel_server.data.id, {json: rel_server}) + return request.patch(base_url + "/servers/" + rel_server.data.id, {json: rel_server}) .should.be.fulfilled }); diff --git a/server/core/test/rest-api/test/service.js b/server/core/test/rest-api/test/service.js index 2aac1fdff..6f445a120 100644 --- a/server/core/test/rest-api/test/service.js +++ b/server/core/test/rest-api/test/service.js @@ -8,7 +8,7 @@ describe("Service", function() { .then(function(resp) { var svc = JSON.parse(resp) svc.data.attributes.parameters.enable_root_user = true - return request.put(base_url + "/services/RW-Split-Router", {json: svc}) + return request.patch(base_url + "/services/RW-Split-Router", {json: svc}) }) .then(function(resp) { return request.get(base_url + "/services/RW-Split-Router") @@ -25,7 +25,7 @@ describe("Service", function() { .then(function(resp) { var svc = JSON.parse(resp) delete svc.data.relationships - return request.put(base_url + "/services/RW-Split-Router", {json: svc}) + return request.patch(base_url + "/services/RW-Split-Router", {json: svc}) }) .then(function(resp) { return request.get(base_url + "/services/RW-Split-Router") @@ -51,7 +51,7 @@ describe("Service", function() { } } - return request.put(base_url + "/services/RW-Split-Router", {json: svc}) + return request.patch(base_url + "/services/RW-Split-Router", {json: svc}) }) .then(function(resp) { return request.get(base_url + "/services/RW-Split-Router")