From b6908f2f8112d1b0f17eeb2144ba9a4d3aabcce2 Mon Sep 17 00:00:00 2001 From: Kishore Nallan Date: Sat, 30 Apr 2022 19:19:25 +0530 Subject: [PATCH] Add test for missing fields property in schema change. --- include/collection_manager.h | 2 -- src/collection.cpp | 2 +- src/collection_manager.cpp | 18 ------------------ test/collection_schema_change_test.cpp | 9 +++++++++ 4 files changed, 10 insertions(+), 21 deletions(-) diff --git a/include/collection_manager.h b/include/collection_manager.h index c77a067c..c0e1dbd0 100644 --- a/include/collection_manager.h +++ b/include/collection_manager.h @@ -152,8 +152,6 @@ public: const std::vector& symbols_to_index = {}, const std::vector& token_separators = {}); - Option update_collection(const std::string& name, nlohmann::json& req_json); - locked_resource_view_t get_collection(const std::string & collection_name) const; locked_resource_view_t get_collection_with_id(uint32_t collection_id) const; diff --git a/src/collection.cpp b/src/collection.cpp index 21700d3d..dae7da13 100644 --- a/src/collection.cpp +++ b/src/collection.cpp @@ -2716,7 +2716,7 @@ Option Collection::validate_alter_payload(nlohmann::json& schema_changes, const std::string err_msg = "The `fields` value should be an array of objects containing " "the field `name` and other properties."; - if(!schema_changes["fields"].is_array() || schema_changes["fields"].empty()) { + if(!schema_changes.contains("fields") || !schema_changes["fields"].is_array() || schema_changes["fields"].empty()) { return Option(400, err_msg); } diff --git a/src/collection_manager.cpp b/src/collection_manager.cpp index ecfdd8f9..2adbc7b2 100644 --- a/src/collection_manager.cpp +++ b/src/collection_manager.cpp @@ -1244,21 +1244,3 @@ Option CollectionManager::delete_preset(const string& preset_name) { preset_configs.erase(preset_name); return Option(true); } - -Option CollectionManager::update_collection(const std::string& name, nlohmann::json& req_json) { - if(!req_json["fields"].is_array() || req_json["fields"].empty()) { - return Option(400, "The `fields` value should be an array of objects containing " - "the field properties."); - } - - auto collection = get_collection(name); - if(collection == nullptr) { - return Option(404, "Not found."); - } - - // Supported operations: - // - Adding a new field - // - Dropping a field - - return collection->alter(req_json); -} diff --git a/test/collection_schema_change_test.cpp b/test/collection_schema_change_test.cpp index bbebe994..aa584a68 100644 --- a/test/collection_schema_change_test.cpp +++ b/test/collection_schema_change_test.cpp @@ -443,6 +443,15 @@ TEST_F(CollectionSchemaChangeTest, AlterValidations) { ASSERT_EQ("Field `desc` has been declared in the schema, but is not found in the documents already present " "in the collection. If you still want to add this field, set it as `optional: true`.", alter_op.error()); + // 7. schema JSON missing "fields" property + schema_changes = R"({ + "foo": "bar" + })"_json; + alter_op = coll1->alter(schema_changes); + ASSERT_FALSE(alter_op.ok()); + ASSERT_EQ("The `fields` value should be an array of objects containing the field `name` " + "and other properties.", alter_op.error()); + collectionManager.drop_collection("coll1"); }