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.
This commit is contained in:
Markus Mäkelä 2018-08-06 16:54:38 +03:00
parent 3754008e43
commit 4fd4d6bb01
No known key found for this signature in database
GPG Key ID: 72D48FCE664F7B19
4 changed files with 51 additions and 14 deletions

View File

@ -21,6 +21,7 @@
#include <set>
#include <iterator>
#include <algorithm>
#include <vector>
#include <maxscale/atomic.h>
#include <maxscale/hk_heartbeat.h>
@ -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<const char*> 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;
}

View File

@ -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) => {

View File

@ -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
});

View File

@ -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")