From 4fd4d6bb01a600191918cf83a79f5d01cd9e65bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Mon, 6 Aug 2018 16:54:38 +0300 Subject: [PATCH] MXS-1999: Fix null relationship handling The data of the relationship should be set to null to delete the relationship. Conceptually it is different from setting it to an empty relationship but in practice both lead to the same end result. Updated tests to use correct relationships and added test cases for detection of invalid relationships. --- server/core/config_runtime.cc | 27 +++++++++++++++++++++-- server/core/test/rest-api/test/monitor.js | 21 ++++++++++++------ server/core/test/rest-api/test/server.js | 13 ++++++++--- server/core/test/rest-api/test/service.js | 4 ++-- 4 files changed, 51 insertions(+), 14 deletions(-) diff --git a/server/core/config_runtime.cc b/server/core/config_runtime.cc index 83c2df2ed..ac0c1e627 100644 --- a/server/core/config_runtime.cc +++ b/server/core/config_runtime.cc @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -1132,6 +1133,28 @@ static bool is_valid_resource_body(json_t* json) runtime_error("No '%s' field defined", MXS_JSON_PTR_DATA); rval = false; } + else + { + // Check that the relationship JSON is well-formed + const std::vector relations = + { + MXS_JSON_PTR_RELATIONSHIPS "/servers", + MXS_JSON_PTR_RELATIONSHIPS "/services", + MXS_JSON_PTR_RELATIONSHIPS "/monitors", + MXS_JSON_PTR_RELATIONSHIPS "/filters", + }; + + for (auto it = relations.begin(); it != relations.end(); it++) + { + json_t* j = mxs_json_pointer(json, *it); + + if (j && !json_is_object(j)) + { + runtime_error("Relationship '%s' is not an object",*it); + rval = false; + } + } + } return rval; } @@ -1468,9 +1491,9 @@ static bool is_valid_relationship_body(json_t* json) runtime_error("Field '%s' is not defined", MXS_JSON_PTR_DATA); rval = false; } - else if (!json_is_array(obj)) + else if (!json_is_array(obj) && !json_is_null(obj)) { - runtime_error("Field '%s' is not an array", MXS_JSON_PTR_DATA); + runtime_error("Field '%s' is not an array or null", MXS_JSON_PTR_DATA); rval = false; } diff --git a/server/core/test/rest-api/test/monitor.js b/server/core/test/rest-api/test/monitor.js index 893b36b38..4c3b41909 100644 --- a/server/core/test/rest-api/test/monitor.js +++ b/server/core/test/rest-api/test/monitor.js @@ -47,11 +47,18 @@ describe("Monitor Relationships", function() { .should.be.fulfilled }) + it("remove with malformed relationships", function() { + var mon = {data: {relationships: {servers: null}}} + return request.patch(base_url + "/monitors/MariaDB-Monitor", {json: mon}) + .should.be.rejected + .then(() => request.get(base_url + "/monitors/MariaDB-Monitor", { json: true })) + .then((res) => { + res.data.relationships.should.have.keys("servers") + }) + }); + it("remove relationships from old monitor", function() { - var mon = { data: { - relationships: { - servers: null - }}} + var mon = {data: {relationships: {servers: {data: null}}}} return request.patch(base_url + "/monitors/MariaDB-Monitor", {json: mon}) .then(() => request.get(base_url + "/monitors/MariaDB-Monitor", { json: true })) .then((res) => { @@ -79,7 +86,7 @@ describe("Monitor Relationships", function() { }); it("move relationships back to old monitor", function() { - var mon = {data: {relationships: {servers: null}}} + var mon = {data: {relationships: {servers: {data: null}}}} return request.patch(base_url + "/monitors/" + monitor.data.id, {json: mon}) .then(() => request.get(base_url + "/monitors/" + monitor.data.id, { json: true })) .then((res) => { @@ -125,7 +132,7 @@ describe("Monitor Relationships", function() { }); it("bad request body with `relationships` endpoint should be rejected", function() { - return request.patch(base_url + "/monitors/" + monitor.data.id + "/relationships/servers", {json: {data: null}}) + return request.patch(base_url + "/monitors/" + monitor.data.id + "/relationships/servers", {json: {servers: null}}) .should.be.rejected }) @@ -137,7 +144,7 @@ describe("Monitor Relationships", function() { { id: "server4", type: "servers" } ]} - return request.patch(base_url + "/monitors/" + monitor.data.id + "/relationships/servers", {json: {data: []}}) + return request.patch(base_url + "/monitors/" + monitor.data.id + "/relationships/servers", {json: {data: null}}) .then(() => request.patch(base_url + "/monitors/MariaDB-Monitor/relationships/servers", {json: old})) .then(() => request.get(base_url + "/monitors/MariaDB-Monitor", { json: true })) .then((res) => { diff --git a/server/core/test/rest-api/test/server.js b/server/core/test/rest-api/test/server.js index dcc7671f7..5ed501358 100644 --- a/server/core/test/rest-api/test/server.js +++ b/server/core/test/rest-api/test/server.js @@ -80,13 +80,13 @@ describe("Server Relationships", function() { }); it("bad request body with `relationships` endpoint should be rejected", function() { - var body = {data: null} + var body = {monitors: null} return request.patch(base_url + "/servers/" + rel_server.data.id + "/relationships/monitors", { json: body }) .should.be.rejected }); it("remove relationships with `relationships` endpoint", function() { - var body = {data: []} + var body = {data: null} return request.patch(base_url + "/servers/" + rel_server.data.id + "/relationships/monitors", { json: body }) .then(() => request.get(base_url + "/servers/" + rel_server.data.id, {json: true})) .then((res) => { @@ -96,9 +96,16 @@ describe("Server Relationships", function() { }) }); - it("remove relationships", function() { + it("remove with malformed relationships", function() { rel_server.data.relationships["services"] = null rel_server.data.relationships["monitors"] = null + return request.patch(base_url + "/servers/" + rel_server.data.id, {json: rel_server}) + .should.be.rejected + }); + + it("remove relationships", function() { + rel_server.data.relationships["services"] = {data: null} + rel_server.data.relationships["monitors"] = {data: null} 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 61a8fb4d4..7df8de961 100644 --- a/server/core/test/rest-api/test/service.js +++ b/server/core/test/rest-api/test/service.js @@ -64,12 +64,12 @@ describe("Service", function() { }); it("bad request body with `relationships` endpoint should be rejected", function() { - return request.patch(base_url + "/services/RW-Split-Router/relationships/servers", {json: {data: null}}) + return request.patch(base_url + "/services/RW-Split-Router/relationships/servers", {json: {servers: null}}) .should.be.rejected }) it("remove service relationship via `relationships` endpoint", function() { - return request.patch(base_url + "/services/RW-Split-Router/relationships/servers", { json: {data: []}}) + return request.patch(base_url + "/services/RW-Split-Router/relationships/servers", { json: {data: null}}) .then(() => request.get(base_url + "/services/RW-Split-Router", { json: true })) .then((res) => { res.data.relationships.should.not.have.keys("servers")