This is an automated email from the ASF dual-hosted git repository. pnoltes pushed a commit to branch feature/685-properties-json-serialization in repository https://gitbox.apache.org/repos/asf/celix.git
commit 72c785dfc706630c87322604b4363542ed9c0f5f Author: Pepijn Noltes <[email protected]> AuthorDate: Tue Apr 9 23:16:02 2024 +0200 gh-685: Add initial setup for properties decode flags --- .../utils/gtest/src/PropertiesEncodingTestSuite.cc | 197 +++++++++++++++------ libs/utils/gtest/src/PropertiesTestSuite.cc | 7 + libs/utils/include/celix_properties.h | 14 +- libs/utils/src/properties_encoding.c | 81 ++++++--- 4 files changed, 217 insertions(+), 82 deletions(-) diff --git a/libs/utils/gtest/src/PropertiesEncodingTestSuite.cc b/libs/utils/gtest/src/PropertiesEncodingTestSuite.cc index 4aa2469d..d346f5f6 100644 --- a/libs/utils/gtest/src/PropertiesEncodingTestSuite.cc +++ b/libs/utils/gtest/src/PropertiesEncodingTestSuite.cc @@ -252,6 +252,49 @@ TEST_F(PropertiesSerializationTestSuite, EncodeJPathKeysWithCollisionTest) { json_decref(root); } + +//TODO check desired behaviour, currently every "/" leads to a new object (except if an collision occurs) +//TEST_F(PropertiesSerializationTestSuite, EncodePropertiesWithSpecialKeyNamesTest) { +// //Given a properties set with special key names (slashes) +// celix_autoptr(celix_properties_t) props = celix_properties_create(); +// celix_properties_set(props, "/", "value1"); +// celix_properties_set(props, "keyThatEndsWithSlash/", "value2"); +// celix_properties_set(props, "key//With//Double//Slash", "value3"); +// celix_properties_set(props, "object/", "value5"); +// celix_properties_set(props, "object//", "value4"); +// celix_properties_set(props, "object/keyThatEndsWithSlash/", "value6"); +// celix_properties_set(props, "object/key//With//Double//Slash", "value7"); +// +// //And an in-memory stream +// celix_autofree char* buf = nullptr; +// size_t bufLen = 0; +// FILE* stream = open_memstream(&buf, &bufLen); +// +// //When encoding the properties to the stream +// auto status = celix_properties_encodeToStream(props, stream, 0); +// ASSERT_EQ(CELIX_SUCCESS, status); +// +// std::cout << buf << std::endl; +// +// //Then the stream contains the JSON representation snippets of the properties +// fclose(stream); +// EXPECT_NE(nullptr, strstr(buf, R"("/":"value1")")) << "JSON: " << buf; +// EXPECT_NE(nullptr, strstr(buf, R"("keyThatEndsWithSlash/":"value2")")) << "JSON: " << buf; +// EXPECT_NE(nullptr, strstr(buf, R"("key//With//Double//Slash":"value3")")) << "JSON: " << buf; +// EXPECT_NE(nullptr, strstr(buf, R"("object/":"value5")")) << "JSON: " << buf; +// EXPECT_NE(nullptr, strstr(buf, R"("/":"value5")")) << "JSON: " << buf; //child of object +// EXPECT_NE(nullptr, strstr(buf, R"("keyThatEndsWithSlash/":"value6")")) << "JSON: " << buf; //child of object +// EXPECT_NE(nullptr, strstr(buf, R"("key//With//Double//Slash":"value7")")) << "JSON: " << buf; //child of object +// +// +// //And the buf is a valid JSON object +// json_error_t error; +// json_t* root = json_loads(buf, 0, &error); +// EXPECT_NE(nullptr, root) << "Unexpected JSON error: " << error.text; +// json_decref(root); +//} + + TEST_F(PropertiesSerializationTestSuite, DecodeEmptyPropertiesTest) { //Given an empty JSON object const char* json = "{}"; @@ -392,8 +435,6 @@ TEST_F(PropertiesSerializationTestSuite, DecodePropertiesWithInvalidInputTest) { R"({)", // invalid JSON (caught by jansson) R"([])", // unsupported JSON (top level array not supported) R"(42)", // invalid JSON (caught by jansson) - R"({"mixedArr":["string", true]})", // Mixed array, not supported - R"({"key1":null})", // Null value, not supported }; for (auto& invalidInput: invalidInputs) { //Given an invalid JSON object @@ -477,7 +518,9 @@ TEST_F(PropertiesSerializationTestSuite, DecodePropertiesWithNestedObjectsAndJPa "key1":true } }, - "object1/object2/key1":6 + "object1/object2/key1":6, + "key2":2, + "key2":3 })"; // And a stream with the JSON object @@ -487,7 +530,22 @@ TEST_F(PropertiesSerializationTestSuite, DecodePropertiesWithNestedObjectsAndJPa celix_autoptr(celix_properties_t) props = nullptr; auto status = celix_properties_decodeFromStream(stream, 0, &props); - // Then loading fails + // Then loading succeeds + ASSERT_EQ(CELIX_SUCCESS, status); + + // And the properties object contains the last values of the jpath keys + EXPECT_EQ(2, celix_properties_size(props)); + EXPECT_EQ(6, celix_properties_getLong(props, "object1/object2/key1", 0)); + EXPECT_EQ(3, celix_properties_getLong(props, "key2", 0)); + + // When the stream is reset + fseek(stream, 0, SEEK_SET); + + // And decoding the properties from the stream using a flog that does not allow collisions + celix_autoptr(celix_properties_t) props2 = nullptr; + status = celix_properties_decodeFromStream(stream, CELIX_PROPERTIES_DECODE_ERROR_ON_DUPLICATES, &props2); + + // Then loading fails, because of a duplicate key EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, status); // And at least one error message is added to celix_err @@ -495,51 +553,88 @@ TEST_F(PropertiesSerializationTestSuite, DecodePropertiesWithNestedObjectsAndJPa celix_err_printErrors(stderr, "Error: ", "\n"); } -//TODO -//TEST_F(PropertiesSerializationTestSuite, DecodePropertiesWithStrictEnabledDisabledTest) { -// auto invalidInputs = { -// R"({"mixedArr":["string", true]})", // Mixed array gives error on strict -// R"({"key1":null})", // Null value gives error on strict -// R"({"":"value"})", // "" key gives error on strict -// R"({"emptyArr":[]})", // Empty array gives error on strict -// R"({"key1":"val1", "key1:"val2"})", // Duplicate key gives error on strict -// }; -// -// for (auto& invalidInput: invalidInputs) { -// //Given an invalid JSON object -// FILE* stream = fmemopen((void*)invalidInput, strlen(invalidInput), "r"); -// -// //When decoding the properties from the stream with an empty flags -// celix_autoptr(celix_properties_t) props = nullptr; -// auto status = celix_properties_decodeFromStream(stream, 0, &props); -// -// //Then decoding succeeds, because strict is disabled -// ASSERT_EQ(CELIX_SUCCESS, status); -// EXPECT_GE(celix_err_getErrorCount(), 0); -// -// //But the properties object is empty, because the invalid input is ignored -// EXPECT_EQ(0, celix_properties_size(props)); -// -// fclose(stream); -// } -// -// for (auto& invalidInput: invalidInputs) { -// //Given an invalid JSON object -// FILE* stream = fmemopen((void*)invalidInput, strlen(invalidInput), "r"); -// -// //When decoding the properties from the stream with a strict flag -// celix_autoptr(celix_properties_t) props = nullptr; -// auto status = celix_properties_decodeFromStream(stream, CELIX_PROPERTIES_DECODE_STRICT, &props); -// -// //Then decoding fails -// EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, status); -// -// //And at least one error message is added to celix_err -// EXPECT_GE(celix_err_getErrorCount(), 1); -// celix_err_printErrors(stderr, "Error: ", "\n"); -// -// fclose(stream); -// } -//} +TEST_F(PropertiesSerializationTestSuite, DecodePropertiesWithStrictEnabledDisabledTest) { + auto invalidInputs = { + R"({"mixedArr":["string", true]})", // Mixed array gives error on strict + R"({"key1":null})", // Null value gives error on strict + R"({"":"value"})", // "" key gives error on strict + R"({"emptyArr":[]})", // Empty array gives error on strict + R"({"key1":"val1", "key1":"val2"})",// Duplicate key gives error on strict + R"({"nullArr":[null,null]})", // Array with null values gives error on strict + }; + + for (auto& invalidInput: invalidInputs) { + //Given an invalid JSON object + FILE* stream = fmemopen((void*)invalidInput, strlen(invalidInput), "r"); + + //When decoding the properties from the stream with an empty flags + celix_autoptr(celix_properties_t) props = nullptr; + auto status = celix_properties_decodeFromStream(stream, 0, &props); + celix_err_printErrors(stderr, "Error: ", "\n"); + + //Then decoding succeeds, because strict is disabled + ASSERT_EQ(CELIX_SUCCESS, status); + EXPECT_GE(celix_err_getErrorCount(), 0); + + //But the properties size is 0 or 1, because the all invalid inputs are ignored, except the duplicate key + auto size = celix_properties_size(props); + EXPECT_TRUE(size == 0 || size == 1); + + fclose(stream); + } + + for (auto& invalidInput: invalidInputs) { + //Given an invalid JSON object + FILE* stream = fmemopen((void*)invalidInput, strlen(invalidInput), "r"); + + //When decoding the properties from the stream with a strict flag + celix_autoptr(celix_properties_t) props = nullptr; + auto status = celix_properties_decodeFromStream(stream, CELIX_PROPERTIES_DECODE_STRICT, &props); + + //Then decoding fails + EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, status); + + //And at least one error message is added to celix_err + EXPECT_GE(celix_err_getErrorCount(), 1); + celix_err_printErrors(stderr, "Error: ", "\n"); + + fclose(stream); + } +} + +TEST_F(PropertiesSerializationTestSuite, DecodePropertiesWithSpecialKeyNamesTest) { + // Given a complex JSON object + const char* jsonInput = R"({ + "/": "value1", + "keyThatEndsWithSlash/": "value2", + "key//With//Double//Slash": "value3", + "object": { + "/": "value4", + "keyThatEndsWithSlash/": "value5", + "key//With//Double//Slash": "value6" + } + })"; + + // And a stream with the JSON object + FILE* stream = fmemopen((void*)jsonInput, strlen(jsonInput), "r"); + + // When decoding the properties from the stream + celix_autoptr(celix_properties_t) props = nullptr; + auto status = celix_properties_decodeFromStream(stream, 0, &props); + celix_err_printErrors(stderr, "Error: ", "\n"); + ASSERT_EQ(CELIX_SUCCESS, status); + + // Then the properties object contains the nested objects + EXPECT_EQ(6, celix_properties_size(props)); + EXPECT_STREQ("value1", celix_properties_getString(props, "/")); + EXPECT_STREQ("value2", celix_properties_getString(props, "keyThatEndsWithSlash/")); + EXPECT_STREQ("value3", celix_properties_getString(props, "key//With//Double//Slash")); + EXPECT_STREQ("value4", celix_properties_getString(props, "object//")); + EXPECT_STREQ("value5", celix_properties_getString(props, "object/keyThatEndsWithSlash/")); + EXPECT_STREQ("value6", celix_properties_getString(props, "object/key//With//Double//Slash")); +} -//TODO test with key starting and ending with slash +//TODO test with invalid version string +//TODO is there a strict option needed for version (e.g. not parseable as version handle as string) +//TODO test encoding flags +//TODO error injection tests and wrappers for jansson functions \ No newline at end of file diff --git a/libs/utils/gtest/src/PropertiesTestSuite.cc b/libs/utils/gtest/src/PropertiesTestSuite.cc index 9c1825b5..e1b9391b 100644 --- a/libs/utils/gtest/src/PropertiesTestSuite.cc +++ b/libs/utils/gtest/src/PropertiesTestSuite.cc @@ -959,3 +959,10 @@ TEST_F(PropertiesTestSuite, SetArrayListWithIllegalArgumentsTest) { //And when an NULL key is used, a ILLEGAL_ARGUMENT error is returned EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, celix_properties_setArrayList(props, nullptr, list)); } + +TEST_F(PropertiesTestSuite, EmptyStringKeyTest) { + celix_autoptr(celix_properties_t) props = celix_properties_create(); + celix_properties_set(props, "", "value"); // "" is a valid key (nullptr is not) + EXPECT_EQ(1, celix_properties_size(props)); + EXPECT_STREQ("value", celix_properties_getString(props, "")); +} diff --git a/libs/utils/include/celix_properties.h b/libs/utils/include/celix_properties.h index e501071c..b82ad9ad 100644 --- a/libs/utils/include/celix_properties.h +++ b/libs/utils/include/celix_properties.h @@ -180,9 +180,16 @@ CELIX_UTILS_EXPORT celix_status_t celix_properties_store(celix_properties_t* pro const char* file, const char* header); +//TODO document the encode flags #define CELIX_PROPERTIES_ENCODE_PRETTY 0x01 #define CELIX_PROPERTIES_ENCODE_SORT_KEYS 0x02 +//TODO doc. Not encode does not reset the stream position. +CELIX_UTILS_EXPORT celix_status_t celix_properties_encodeToStream(const celix_properties_t* properties, + FILE* stream, + int encodeFlags); + +//TODO document the decode flags #define CELIX_PROPERTIES_DECODE_ERROR_ON_DUPLICATES 0x01 #define CELIX_PROPERTIES_DECODE_ERROR_ON_NULL_VALUES 0x02 #define CELIX_PROPERTIES_DECODE_ERROR_ON_EMPTY_ARRAYS 0x04 @@ -193,12 +200,7 @@ CELIX_UTILS_EXPORT celix_status_t celix_properties_store(celix_properties_t* pro CELIX_PROPERTIES_DECODE_ERROR_ON_EMPTY_ARRAYS | CELIX_PROPERTIES_DECODE_ERROR_ON_EMPTY_KEYS | \ CELIX_PROPERTIES_DECODE_ERROR_ON_MIXED_ARRAYS) -//TODO doc -CELIX_UTILS_EXPORT celix_status_t celix_properties_encodeToStream(const celix_properties_t* properties, - FILE* stream, - int encodeFlags); - -//TODO doc +//TODO doc. Note decode does not reset the stream position. CELIX_UTILS_EXPORT celix_status_t celix_properties_decodeFromStream(FILE* stream, int decodeFlags, celix_properties_t** out); diff --git a/libs/utils/src/properties_encoding.c b/libs/utils/src/properties_encoding.c index a75c9f57..ba30b670 100644 --- a/libs/utils/src/properties_encoding.c +++ b/libs/utils/src/properties_encoding.c @@ -28,7 +28,8 @@ #include <math.h> #include <string.h> -static celix_status_t celix_properties_loadValue(celix_properties_t* props, const char* key, json_t* jsonValue); +static celix_status_t +celix_properties_decodeValue(celix_properties_t* props, const char* key, json_t* jsonValue, int flags); // TODO make jansson wrapper header for auto cleanup, wrap json_object_set_new and json_dumpf for error injection @@ -225,14 +226,19 @@ celix_status_t celix_properties_encodeToStream(const celix_properties_t* propert } } - int rc = - json_dumpf(root, - stream, - JSON_COMPACT); // TODO make celix properties flags for COMPACT and INDENT and maybe other json flags + size_t jsonFlags = JSON_COMPACT; + if (encodeFlags & CELIX_PROPERTIES_ENCODE_PRETTY) { + jsonFlags = JSON_INDENT(2); + } + if (encodeFlags & CELIX_PROPERTIES_ENCODE_SORT_KEYS) { + jsonFlags |= JSON_SORT_KEYS; + } + + int rc = json_dumpf(root, stream, jsonFlags); json_decref(root); if (rc != 0) { celix_err_push("Failed to dump json object"); - return CELIX_ENOMEM; // TODO improve error + return CELIX_ENOMEM; } return CELIX_SUCCESS; } @@ -296,11 +302,9 @@ static celix_status_t celix_properties_determineArrayType(const json_t* jsonArra // mixed real and integer, ok continue; } else if (type != json_typeof(value)) { - celix_err_push("Mixed types in array"); return CELIX_ILLEGAL_ARGUMENT; } else if (versionType) { if (json_typeof(value) != JSON_STRING || !celix_properties_isVersionString(json_string_value(value))) { - celix_err_push("Mixed version and non-version strings in array"); return CELIX_ILLEGAL_ARGUMENT; } } @@ -320,19 +324,25 @@ static celix_status_t celix_properties_determineArrayType(const json_t* jsonArra case JSON_FALSE: *out = CELIX_ARRAY_LIST_ELEMENT_TYPE_BOOL; break; + case JSON_NULL: default: - celix_err_pushf("Unexpected json array type %d", type); - return CELIX_ILLEGAL_ARGUMENT; + return CELIX_ILLEGAL_ARGUMENT; // TODO Add test for this case and maybe return a different error code and log error } return CELIX_SUCCESS; } -static celix_status_t celix_properties_loadArray(celix_properties_t* props, const char* key, const json_t* jsonArray) { +static celix_status_t +celix_properties_decodeArray(celix_properties_t* props, const char* key, const json_t* jsonArray, int flags) { celix_array_list_element_type_t elType; celix_status_t status = celix_properties_determineArrayType(jsonArray, &elType); - if (status != CELIX_SUCCESS) { + if (status != CELIX_SUCCESS && (flags & CELIX_PROPERTIES_DECODE_ERROR_ON_MIXED_ARRAYS)) { + celix_autofree char* arrStr = json_dumps(jsonArray, JSON_ENCODE_ANY); + celix_err_pushf("Unsupported mixed or null array for key '%s': %s", key, arrStr); return status; + } else if (status != CELIX_SUCCESS) { + //ignore mixed types + return CELIX_SUCCESS; } celix_array_list_create_options_t opts = CELIX_EMPTY_ARRAY_LIST_CREATE_OPTIONS; @@ -368,7 +378,7 @@ static celix_status_t celix_properties_loadArray(celix_properties_t* props, cons } default: // LCOV_EXCL_START - celix_err_pushf("Unexpected array list element type %d", elType); + celix_err_pushf("Unexpected array list element type %d for key %s", elType, key); return CELIX_ILLEGAL_ARGUMENT; // LCOV_EXCL_STOP } @@ -379,8 +389,17 @@ static celix_status_t celix_properties_loadArray(celix_properties_t* props, cons return celix_properties_assignArrayList(props, key, celix_steal_ptr(array)); } -static celix_status_t celix_properties_loadValue(celix_properties_t* props, const char* key, json_t* jsonValue) { - if (celix_properties_hasKey(props, key)) { +static celix_status_t +celix_properties_decodeValue(celix_properties_t* props, const char* key, json_t* jsonValue, int flags) { + if (strncmp(key, "", 1) == 0) { + if (flags & CELIX_PROPERTIES_DECODE_ERROR_ON_EMPTY_KEYS) { + celix_err_push("Key cannot be empty"); + return CELIX_ILLEGAL_ARGUMENT; + } + return CELIX_SUCCESS; // ignore empty keys. + } + + if (celix_properties_hasKey(props, key) && (flags & CELIX_PROPERTIES_DECODE_ERROR_ON_DUPLICATES)) { celix_err_pushf("Key `%s` already exists.", key); return CELIX_ILLEGAL_ARGUMENT; } @@ -410,20 +429,28 @@ static celix_status_t celix_properties_loadValue(celix_properties_t* props, cons celix_err_push("Failed to create sub key"); return CELIX_ENOMEM; } - status = celix_properties_loadValue(props, subKey, fieldValue); + status = celix_properties_decodeValue(props, subKey, fieldValue, flags); if (status != CELIX_SUCCESS) { return status; } } return CELIX_SUCCESS; } else if (json_is_array(jsonValue) && json_array_size(jsonValue) == 0) { - // empty array -> treat as unset property. silently ignore + if (flags & CELIX_PROPERTIES_DECODE_ERROR_ON_EMPTY_ARRAYS) { + celix_err_pushf("Unexpected empty array for key `%s`", key); + return CELIX_ILLEGAL_ARGUMENT; + } + // ignore empty arrays return CELIX_SUCCESS; } else if (json_is_array(jsonValue)) { - status = celix_properties_loadArray(props, key, jsonValue); + status = celix_properties_decodeArray(props, key, jsonValue, flags); } else if (json_is_null(jsonValue)) { - celix_err_pushf("Unexpected null value for key `%s`", key); - return CELIX_ILLEGAL_ARGUMENT; + if (flags & CELIX_PROPERTIES_DECODE_ERROR_ON_NULL_VALUES) { + celix_err_pushf("Unexpected null value for key `%s`", key); + return CELIX_ILLEGAL_ARGUMENT; + } + // ignore null values + return CELIX_SUCCESS; } else { // LCOV_EXCL_START celix_err_pushf("Unexpected json value type for key `%s`", key); @@ -433,7 +460,7 @@ static celix_status_t celix_properties_loadValue(celix_properties_t* props, cons return status; } -static celix_status_t celix_properties_decodeFromJson(json_t* obj, celix_properties_t** out) { +static celix_status_t celix_properties_decodeFromJson(json_t* obj, int flags, celix_properties_t** out) { if (!json_is_object(obj)) { celix_err_push("Expected json object"); return CELIX_ILLEGAL_ARGUMENT; @@ -447,7 +474,7 @@ static celix_status_t celix_properties_decodeFromJson(json_t* obj, celix_propert const char* key; json_t* value; json_object_foreach(obj, key, value) { - celix_status_t status = celix_properties_loadValue(props, key, value); + celix_status_t status = celix_properties_decodeValue(props, key, value, flags); if (status != CELIX_SUCCESS) { return status; } @@ -459,11 +486,15 @@ static celix_status_t celix_properties_decodeFromJson(json_t* obj, celix_propert celix_status_t celix_properties_decodeFromStream(FILE* stream, int decodeFlags, celix_properties_t** out) { json_error_t jsonError; - json_t* root = json_loadf(stream, 0, &jsonError); + size_t jsonFlags = 0; + if (decodeFlags & CELIX_PROPERTIES_DECODE_ERROR_ON_DUPLICATES) { + jsonFlags = JSON_REJECT_DUPLICATES; + } + json_t* root = json_loadf(stream, jsonFlags, &jsonError); if (!root) { celix_err_pushf("Failed to parse json: %s", jsonError.text); return CELIX_ILLEGAL_ARGUMENT; } - return celix_properties_decodeFromJson(root, out); -} \ No newline at end of file + return celix_properties_decodeFromJson(root, decodeFlags, out); +}
