From d371ecb30fbf1a0cefe8fbc0be5ce3cea53f0277 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Sun, 22 Oct 2017 19:30:55 +0300 Subject: [PATCH] Only remove explicitly deleted relationships Only when a relationship is defined as a null JSON value, it should be deleted. If it is missing, it should be ignored. --- server/core/config_runtime.cc | 104 ++++++++++++++--------- server/core/test/rest-api/test/server.js | 4 +- 2 files changed, 67 insertions(+), 41 deletions(-) diff --git a/server/core/config_runtime.cc b/server/core/config_runtime.cc index 14ebcf1cc..f116c5dfb 100644 --- a/server/core/config_runtime.cc +++ b/server/core/config_runtime.cc @@ -973,51 +973,61 @@ bool runtime_destroy_monitor(MXS_MONITOR *monitor) } static bool extract_relations(json_t* json, StringSet& relations, - const char** relation_types, + const char* relation_type, bool (*relation_check)(const std::string&, const std::string&)) { bool rval = true; + json_t* arr = mxs_json_pointer(json, relation_type); - for (int i = 0; relation_types[i]; i++) + if (arr && json_is_array(arr)) { - json_t* arr = mxs_json_pointer(json, relation_types[i]); + size_t size = json_array_size(arr); - if (arr && json_is_array(arr)) + for (size_t j = 0; j < size; j++) { - size_t size = json_array_size(arr); + json_t* obj = json_array_get(arr, j); + json_t* id = json_object_get(obj, CN_ID); + json_t* type = mxs_json_pointer(obj, CN_TYPE); - for (size_t j = 0; j < size; j++) + if (id && json_is_string(id) && + type && json_is_string(type)) { - json_t* obj = json_array_get(arr, j); - json_t* id = json_object_get(obj, CN_ID); - json_t* type = mxs_json_pointer(obj, CN_TYPE); + std::string id_value = json_string_value(id); + std::string type_value = json_string_value(type); - if (id && json_is_string(id) && - type && json_is_string(type)) + if (relation_check(type_value, id_value)) { - std::string id_value = json_string_value(id); - std::string type_value = json_string_value(type); - - if (relation_check(type_value, id_value)) - { - relations.insert(id_value); - } - else - { - rval = false; - } + relations.insert(id_value); } else { rval = false; } } + else + { + rval = false; + } } } return rval; } +static inline bool is_null_relation(json_t* json, const char* relation) +{ + std::string str(relation); + size_t pos = str.rfind("/data"); + + ss_dassert(pos != std::string::npos); + str = str.substr(0, pos); + + json_t* data = mxs_json_pointer(json, relation); + json_t* base = mxs_json_pointer(json, str.c_str()); + + return (data && json_is_null(data)) || (base && json_is_null(base)); +} + static inline const char* get_string_or_null(json_t* json, const char* path) { const char* rval = NULL; @@ -1133,13 +1143,6 @@ static bool server_contains_required_fields(json_t* json) return rval; } -const char* server_relation_types[] = -{ - MXS_JSON_PTR_RELATIONSHIPS_SERVICES, - MXS_JSON_PTR_RELATIONSHIPS_MONITORS, - NULL -}; - static bool server_relation_is_valid(const std::string& type, const std::string& value) { return (type == CN_SERVICES && service_find(value.c_str())) || @@ -1289,7 +1292,8 @@ SERVER* runtime_create_server_from_json(json_t* json) StringSet relations; - if (extract_relations(json, relations, server_relation_types, server_relation_is_valid)) + if (extract_relations(json, relations, MXS_JSON_PTR_RELATIONSHIPS_SERVICES, server_relation_is_valid) && + extract_relations(json, relations, MXS_JSON_PTR_RELATIONSHIPS_MONITORS, server_relation_is_valid)) { if (runtime_create_server(name, address, port.c_str(), protocol, authenticator, authenticator_options)) { @@ -1322,12 +1326,33 @@ bool server_to_object_relations(SERVER* server, json_t* old_json, json_t* new_js return true; } - bool rval = false; + const char* server_relation_types[] = + { + MXS_JSON_PTR_RELATIONSHIPS_SERVICES, + MXS_JSON_PTR_RELATIONSHIPS_MONITORS, + NULL + }; + + bool rval = true; StringSet old_relations; StringSet new_relations; - if (extract_relations(old_json, old_relations, server_relation_types, server_relation_is_valid) && - extract_relations(new_json, new_relations, server_relation_types, server_relation_is_valid)) + for (int i = 0; server_relation_types[i]; i++) + { + // Extract only changed or deleted relationships + if (is_null_relation(new_json, server_relation_types[i]) || + mxs_json_pointer(new_json, server_relation_types[i])) + { + if (!extract_relations(new_json, new_relations, server_relation_types[i], server_relation_is_valid) || + !extract_relations(old_json, old_relations, server_relation_types[i], server_relation_is_valid)) + { + rval = false; + break; + } + } + } + + if (rval) { StringSet removed_relations; StringSet added_relations; @@ -1340,10 +1365,10 @@ bool server_to_object_relations(SERVER* server, json_t* old_json, json_t* new_js old_relations.begin(), old_relations.end(), std::inserter(added_relations, added_relations.begin())); - if (unlink_server_from_objects(server, removed_relations) && - link_server_to_objects(server, added_relations)) + if (!unlink_server_from_objects(server, removed_relations) || + !link_server_to_objects(server, added_relations)) { - rval = true; + rval = false; } } @@ -1434,7 +1459,7 @@ static bool validate_monitor_json(json_t* json) else { StringSet relations; - if (extract_relations(json, relations, object_relation_types, object_relation_is_valid)) + if (extract_relations(json, relations, MXS_JSON_PTR_RELATIONSHIPS_SERVERS, object_relation_is_valid)) { rval = true; } @@ -1517,9 +1542,10 @@ bool object_to_server_relations(const char* target, json_t* old_json, json_t* ne bool rval = false; StringSet old_relations; StringSet new_relations; + const char* object_relation = MXS_JSON_PTR_RELATIONSHIPS_SERVERS; - if (extract_relations(old_json, old_relations, object_relation_types, object_relation_is_valid) && - extract_relations(new_json, new_relations, object_relation_types, object_relation_is_valid)) + if (extract_relations(old_json, old_relations, object_relation, object_relation_is_valid) && + extract_relations(new_json, new_relations, object_relation, object_relation_is_valid)) { StringSet removed_relations; StringSet added_relations; diff --git a/server/core/test/rest-api/test/server.js b/server/core/test/rest-api/test/server.js index bf975fc6e..025cbba14 100644 --- a/server/core/test/rest-api/test/server.js +++ b/server/core/test/rest-api/test/server.js @@ -68,8 +68,8 @@ describe("Server Relationships", function() { }); it("remove relationships", function() { - delete rel_server.data.relationships["services"] - delete rel_server.data.relationships["monitors"] + 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.fulfilled });