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 0ceaeb8f0f7eddf0e05b156e6c61f583db07ea4b Author: Pepijn Noltes <[email protected]> AuthorDate: Sat Apr 13 19:59:35 2024 +0200 gh-685: Add additional prop encoding test based on test cov --- .../utils/gtest/src/PropertiesEncodingTestSuite.cc | 132 +++++++++++++++++++-- libs/utils/src/properties_encoding.c | 86 +++++++------- 2 files changed, 161 insertions(+), 57 deletions(-) diff --git a/libs/utils/gtest/src/PropertiesEncodingTestSuite.cc b/libs/utils/gtest/src/PropertiesEncodingTestSuite.cc index 84ed5d0a..b854e183 100644 --- a/libs/utils/gtest/src/PropertiesEncodingTestSuite.cc +++ b/libs/utils/gtest/src/PropertiesEncodingTestSuite.cc @@ -76,7 +76,7 @@ TEST_F(PropertiesSerializationTestSuite, SavePropertiesWithSingleValuesTest) { EXPECT_NE(nullptr, strstr(buf, R"("key3":3)")) << "JSON: " << buf; EXPECT_NE(nullptr, strstr(buf, R"("key4":4.0)")) << "JSON: " << buf; EXPECT_NE(nullptr, strstr(buf, R"("key5":true)")) << "JSON: " << buf; - EXPECT_NE(nullptr, strstr(buf, R"("key6":"celix_version<1.2.3.qualifier>")")) << "JSON: " << buf; + EXPECT_NE(nullptr, strstr(buf, R"("key6":"version<1.2.3.qualifier>")")) << "JSON: " << buf; //And the buf is a valid JSON object json_error_t error; @@ -150,7 +150,7 @@ TEST_F(PropertiesSerializationTestSuite, SavePropertiesWithArrayListsTest) { EXPECT_NE(nullptr, strstr(buf, R"("key2":[1,2])")) << "JSON: " << buf; EXPECT_NE(nullptr, strstr(buf, R"("key3":[1.0,2.0])")) << "JSON: " << buf; EXPECT_NE(nullptr, strstr(buf, R"("key4":[true,false])")) << "JSON: " << buf; - EXPECT_NE(nullptr, strstr(buf, R"("key5":["celix_version<1.2.3.qualifier>","celix_version<4.5.6.qualifier>"])")) + EXPECT_NE(nullptr, strstr(buf, R"("key5":["version<1.2.3.qualifier>","version<4.5.6.qualifier>"])")) << "JSON: " << buf; // And the buf is a valid JSON object @@ -260,9 +260,7 @@ TEST_F(PropertiesSerializationTestSuite, SavePropertiesWithKeyNamesWithSlashesTe //Given a properties set with key names with slashes celix_autoptr(celix_properties_t) props = celix_properties_create(); celix_properties_set(props, "a/key/name/with/slashes", "value1"); - //TODO test separately celix_properties_set(props, "/", "value2"); celix_properties_set(props, "/keyThatStartsWithSlash", "value3"); - //TODO test separately celix_properties_set(props, "//keyThatStartsWithDoubleSlashes", "value4"); celix_properties_set(props, "keyThatEndsWithSlash/", "value5"); celix_properties_set(props, "keyThatEndsWithDoubleSlashes//", "value6"); celix_properties_set(props, "key//With//Double//Slashes", "value7"); @@ -397,6 +395,24 @@ TEST_F(PropertiesSerializationTestSuite, SavePropertiesWithPrettyPrintTest) { EXPECT_STREQ(expected, output); } +TEST_F(PropertiesSerializationTestSuite, SaveWithInvalidStreamTest) { + celix_autoptr(celix_properties_t) properties = celix_properties_create(); + celix_properties_set(properties, "key", "value"); + + // Saving properties with invalid stream will fail + auto status = celix_properties_save(properties, "/non-existing/no/rights/file.json", 0); + EXPECT_EQ(CELIX_FILE_IO_EXCEPTION, status); + EXPECT_EQ(1, celix_err_getErrorCount()); + + auto* readStream = fopen("/dev/null", "r"); + status = celix_properties_saveToStream(properties, readStream, 0); + EXPECT_EQ(CELIX_FILE_IO_EXCEPTION, status); + EXPECT_EQ(2, celix_err_getErrorCount()); + fclose(readStream); + + celix_err_printErrors(stderr, "Error: ", "\n"); +} + TEST_F(PropertiesSerializationTestSuite, LoadEmptyPropertiesTest) { //Given an empty JSON object const char* json = "{}"; @@ -420,7 +436,7 @@ TEST_F(PropertiesSerializationTestSuite, LoadPropertiesWithSingleValuesTest) { "longKey":42, "doubleKey":2.0, "boolKey":true, - "versionKey":"celix_version<1.2.3.qualifier>" + "versionKey":"version<1.2.3.qualifier>" })"; //And a stream with the JSON object @@ -450,7 +466,7 @@ TEST_F(PropertiesSerializationTestSuite, LoadPropertiesWithArrayListsTest) { "intArr":[1,2], "realArr":[1.0,2.0], "boolArr":[true,false], - "versionArr":["celix_version<1.2.3.qualifier>","celix_version<4.5.6.qualifier>"], + "versionArr":["version<1.2.3.qualifier>","version<4.5.6.qualifier>"], "mixedRealAndIntArr1":[1,2.0,2,3.0], "mixedRealAndIntArr2":[1.0,2,2.0,3] })"; @@ -714,6 +730,7 @@ TEST_F(PropertiesSerializationTestSuite, LoadPropertiesEscapedSlashesTest) { TEST_F(PropertiesSerializationTestSuite, LoadPropertiesWithAndWithoutStrictFlagTest) { auto invalidInputs = { R"({"mixedArr":["string", true]})", // Mixed array gives error on strict + R"({"mixedVersionAndStringArr":["version<1.2.3.qualifier>","2.3.4"]})", // 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 @@ -728,7 +745,6 @@ TEST_F(PropertiesSerializationTestSuite, LoadPropertiesWithAndWithoutStrictFlagT //When loading the properties from the stream with an empty flags celix_autoptr(celix_properties_t) props = nullptr; auto status = celix_properties_loadFromStream(stream, 0, &props); - celix_err_printErrors(stderr, "Error: ", "\n"); //Then decoding succeeds, because strict is disabled ASSERT_EQ(CELIX_SUCCESS, status); @@ -792,8 +808,100 @@ TEST_F(PropertiesSerializationTestSuite, LoadPropertiesWithSlashesInTheKeysTest) EXPECT_STREQ("value6", celix_properties_getString(props, "object/key//With//Double//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 error injection tests and wrappers for jansson functions -//TODO test load and save with filename -//TODO check all the celix_err messages for consistency +TEST_F(PropertiesSerializationTestSuite, LoadPropertiesWithInvalidVersionsTest) { + // Given a JSON object with an invalid version string (<, > not allowed in qualifier) + const auto* jsonInput = R"( {"key":"version<1.2.3.<qualifier>>"} )"; + + // When loading the properties from the stream + celix_autoptr(celix_properties_t) props = nullptr; + auto status = celix_properties_loadFromString2(jsonInput, 0, &props); + + // Then loading fails + ASSERT_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"); + + // Given a JSON object with an invalid version strings, that are not recognized as versions + jsonInput = + R"( {"key1":"version<1.2.3", "key2":"version1.2.3>", "key3":"ver<1.2.3>}", "key4":"celix_version<1.2.3>"} )"; + + // When loading the properties from the stream + status = celix_properties_loadFromString2(jsonInput, 0, &props); + + // Then loading succeeds + ASSERT_EQ(CELIX_SUCCESS, status); + + // But the values are not recognized as versions + EXPECT_NE(CELIX_PROPERTIES_VALUE_TYPE_VERSION, celix_properties_getType(props, "key1")); + EXPECT_NE(CELIX_PROPERTIES_VALUE_TYPE_VERSION, celix_properties_getType(props, "key2")); + EXPECT_NE(CELIX_PROPERTIES_VALUE_TYPE_VERSION, celix_properties_getType(props, "key3")); + EXPECT_NE(CELIX_PROPERTIES_VALUE_TYPE_VERSION, celix_properties_getType(props, "key4")); +} + +TEST_F(PropertiesSerializationTestSuite, LoadWithInvalidStreamTest) { + celix_properties_t* dummyProps = nullptr; + + // Loading properties with invalid stream will fail + auto status = celix_properties_load2("non_existing_file.json", 0, &dummyProps); + EXPECT_EQ(CELIX_FILE_IO_EXCEPTION, status); + EXPECT_EQ(1, celix_err_getErrorCount()); + + char* buf = nullptr; + size_t size = 0; + FILE* stream = open_memstream(&buf, &size); // empty stream + status = celix_properties_loadFromStream(stream, 0, &dummyProps); + EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, status); + EXPECT_EQ(2, celix_err_getErrorCount()); + + fclose(stream); + free(buf); + celix_err_printErrors(stderr, "Error: ", "\n"); +} + +TEST_F(PropertiesSerializationTestSuite, SaveAndLoadFlatProperties) { + // Given a properties object with all possible types (but no empty arrays) + celix_autoptr(celix_properties_t) props = celix_properties_create(); + celix_properties_set(props, "strKey", "strValue"); + celix_properties_setLong(props, "longKey", 42); + celix_properties_setDouble(props, "doubleKey", 2.0); + celix_properties_setBool(props, "boolKey", true); + celix_properties_setVersion(props, "versionKey", celix_version_create(1, 2, 3, "qualifier")); + auto* strArr = celix_arrayList_createStringArray(); + celix_arrayList_addString(strArr, "value1"); + celix_arrayList_addString(strArr, "value2"); + auto* longArr = celix_arrayList_createLongArray(); + celix_arrayList_addLong(longArr, 1); + celix_arrayList_addLong(longArr, 2); + celix_properties_assignArrayList(props, "longArr", longArr); + auto* doubleArr = celix_arrayList_createDoubleArray(); + celix_arrayList_addDouble(doubleArr, 1.0); + celix_arrayList_addDouble(doubleArr, 2.0); + celix_properties_assignArrayList(props, "doubleArr", doubleArr); + auto* boolArr = celix_arrayList_createBoolArray(); + celix_arrayList_addBool(boolArr, true); + celix_arrayList_addBool(boolArr, false); + celix_properties_assignArrayList(props, "boolArr", boolArr); + auto* versionArr = celix_arrayList_createVersionArray(); + celix_arrayList_addVersion(versionArr, celix_version_create(1, 2, 3, "qualifier")); + celix_arrayList_addVersion(versionArr, celix_version_create(4, 5, 6, "qualifier")); + celix_properties_assignArrayList(props, "versionArr", versionArr); + + // When saving the properties to a properties_test.json file + const char* filename = "properties_test.json"; + auto status = celix_properties_save(props, filename, 0); + + // Then saving succeeds + ASSERT_EQ(CELIX_SUCCESS, status); + + // When loading the properties from the properties_test.json file + celix_autoptr(celix_properties_t) loadedProps = nullptr; + status = celix_properties_load2(filename, 0, &loadedProps); + + // Then loading succeeds + ASSERT_EQ(CELIX_SUCCESS, status); + + // And the loaded properties are equal to the original properties + EXPECT_TRUE(celix_properties_equals(props, loadedProps)); +} diff --git a/libs/utils/src/properties_encoding.c b/libs/utils/src/properties_encoding.c index e1ad99b0..087590f7 100644 --- a/libs/utils/src/properties_encoding.c +++ b/libs/utils/src/properties_encoding.c @@ -34,12 +34,12 @@ celix_properties_decodeValue(celix_properties_t* props, const char* key, json_t* static celix_status_t celix_properties_versionToJson(const celix_version_t* version, json_t** out) { celix_autofree char* versionStr = celix_version_toString(version); if (!versionStr) { - celix_err_push("Failed to create version string"); + celix_err_push("Failed to create version string."); return CELIX_ENOMEM; } - *out = json_sprintf("celix_version<%s>", versionStr); + *out = json_sprintf("version<%s>", versionStr); if (!*out) { - celix_err_push("Failed to create json string"); + celix_err_push("Failed to create json string."); return CELIX_ENOMEM; } return CELIX_SUCCESS; @@ -66,12 +66,12 @@ static celix_status_t celix_properties_arrayElementEntryValueToJson(celix_array_ return celix_properties_versionToJson(entry.versionVal, out); default: // LCOV_EXCL_START - celix_err_pushf("Invalid array list element type %d", elType); + celix_err_pushf("Invalid array list element type %d.", elType); return CELIX_ILLEGAL_ARGUMENT; // LCOV_EXCL_STOP } if (!*out) { - celix_err_push("Failed to create json value"); + celix_err_push("Failed to create json value."); return CELIX_ENOMEM; } return CELIX_SUCCESS; @@ -84,7 +84,7 @@ static celix_status_t celix_properties_arrayEntryValueToJson(const char* key, *out = NULL; if (celix_arrayList_size(entry->typed.arrayValue) == 0) { if (flags & CELIX_PROPERTIES_ENCODE_ERROR_ON_EMPTY_ARRAYS) { - celix_err_pushf("Invalid empty array for key %s", key); + celix_err_pushf("Invalid empty array for key %s.", key); return CELIX_ILLEGAL_ARGUMENT; } return CELIX_SUCCESS; // empty array -> treat as unset property @@ -92,7 +92,7 @@ static celix_status_t celix_properties_arrayEntryValueToJson(const char* key, json_t* array = json_array(); if (!array) { - celix_err_push("Failed to create json array"); + celix_err_push("Failed to create json array."); return CELIX_ENOMEM; } @@ -109,7 +109,7 @@ static celix_status_t celix_properties_arrayEntryValueToJson(const char* key, } else { int rc = json_array_append_new(array, jsonValue); if (rc != 0) { - celix_err_push("Failed to append json string to array"); + celix_err_push("Failed to append json string to array."); json_decref(array); return CELIX_ENOMEM; } @@ -271,8 +271,8 @@ celix_status_t celix_properties_saveToStream(const celix_properties_t* propertie int rc = json_dumpf(root, stream, jsonFlags); json_decref(root); if (rc != 0) { - celix_err_push("Failed to dump json object"); - return CELIX_ENOMEM; + celix_err_push("Failed to dump json object to stream."); + return CELIX_FILE_IO_EXCEPTION; } return CELIX_SUCCESS; } @@ -280,7 +280,7 @@ celix_status_t celix_properties_saveToStream(const celix_properties_t* propertie celix_status_t celix_properties_save(const celix_properties_t* properties, const char* filename, int encodeFlags) { FILE* stream = fopen(filename, "w"); if (!stream) { - celix_err_pushf("Failed to open file %s", filename); + celix_err_pushf("Failed to open file %s.", filename); return CELIX_FILE_IO_EXCEPTION; } celix_status_t status = celix_properties_saveToStream(properties, stream, encodeFlags); @@ -294,7 +294,7 @@ celix_status_t celix_properties_saveToString(const celix_properties_t* propertie size_t size = 0; FILE* stream = open_memstream(&buffer, &size); if (!stream) { - celix_err_push("Failed to open memstream"); + celix_err_push("Failed to open memstream."); return CELIX_FILE_IO_EXCEPTION; } @@ -306,26 +306,26 @@ celix_status_t celix_properties_saveToString(const celix_properties_t* propertie return status; } -static celix_version_t* celix_properties_parseVersion(const char* value) { - // precondition: value is a valid version string (14 chars prefix and 1 char suffix) - celix_version_t* version = NULL; +static celix_status_t celix_properties_parseVersion(const char* value, celix_version_t** out) { + // precondition: value is a valid version string (8 chars prefix and 1 char suffix) + *out = NULL;; char buf[32]; - char* corrected = celix_utils_writeOrCreateString(buf, sizeof(buf), "%.*s", (int)strlen(value) - 15, value + 14); + char* corrected = celix_utils_writeOrCreateString(buf, sizeof(buf), "%.*s", (int)strlen(value) - 9, value + 8); + celix_auto(celix_utils_string_guard_t) guard = celix_utils_stringGuard_init(buf, corrected); if (!corrected) { - celix_err_push("Failed to create corrected version string"); - return NULL; + celix_err_push("Failed to create corrected version string."); + return ENOMEM; } - celix_status_t status = celix_version_parse(corrected, &version); - celix_utils_freeStringIfNotEqual(buf, corrected); + celix_status_t status = celix_version_parse(corrected, out); if (status != CELIX_SUCCESS) { - celix_err_push("Failed to parse version string"); - return NULL; + celix_err_push("Failed to parse version string."); + return status; } - return version; + return CELIX_SUCCESS; } static bool celix_properties_isVersionString(const char* value) { - return strncmp(value, "celix_version<", 14) == 0 && value[strlen(value) - 1] == '>'; + return strncmp(value, "version<", 8) == 0 && value[strlen(value) - 1] == '>'; } /** @@ -401,7 +401,7 @@ celix_properties_decodeArray(celix_properties_t* props, const char* key, const j celix_status_t status = celix_properties_determineArrayType(jsonArray, &elType); 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("Invalid mixed or null array for key '%s': %s", key, arrStr); + celix_err_pushf("Invalid mixed or null array for key '%s': %s.", key, arrStr); return status; } else if (status != CELIX_SUCCESS) { //ignore mixed types @@ -432,16 +432,14 @@ celix_properties_decodeArray(celix_properties_t* props, const char* key, const j status = celix_arrayList_addBool(array, json_boolean_value(value)); break; case CELIX_ARRAY_LIST_ELEMENT_TYPE_VERSION: { - celix_version_t* v = celix_properties_parseVersion(json_string_value(value)); - if (!v) { - return CELIX_ILLEGAL_ARGUMENT; - } - status = celix_arrayList_addVersion(array, v); + celix_version_t* v; + status = celix_properties_parseVersion(json_string_value(value), &v); + status = CELIX_DO_IF(status, celix_arrayList_addVersion(array, v)); break; } default: // LCOV_EXCL_START - celix_err_pushf("Invalid array list element type %d for key %s", elType, key); + celix_err_pushf("Invalid array list element type %d for key %s.", elType, key); return CELIX_ILLEGAL_ARGUMENT; // LCOV_EXCL_STOP } @@ -456,7 +454,7 @@ 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"); + celix_err_push("Key cannot be empty."); return CELIX_ILLEGAL_ARGUMENT; } return CELIX_SUCCESS; // ignore empty keys. @@ -470,11 +468,9 @@ celix_properties_decodeValue(celix_properties_t* props, const char* key, json_t* celix_status_t status = CELIX_SUCCESS; if (json_is_string(jsonValue) && celix_properties_isVersionString(json_string_value(jsonValue))) { - celix_version_t* version = celix_properties_parseVersion(json_string_value(jsonValue)); - if (!version) { - return CELIX_ILLEGAL_ARGUMENT; - } - status = celix_properties_setVersion(props, key, version); + celix_version_t* version; + status = celix_properties_parseVersion(json_string_value(jsonValue), &version); + status = CELIX_DO_IF(status, celix_properties_setVersion(props, key, version)); } else if (json_is_string(jsonValue)) { status = celix_properties_setString(props, key, json_string_value(jsonValue)); } else if (json_is_integer(jsonValue)) { @@ -491,7 +487,7 @@ celix_properties_decodeValue(celix_properties_t* props, const char* key, json_t* char* combinedKey = celix_utils_writeOrCreateString(buf, sizeof(buf), "%s/%s", key, fieldName); celix_auto(celix_utils_string_guard_t) strGuard = celix_utils_stringGuard_init(buf, combinedKey); if (!combinedKey) { - celix_err_push("Failed to create sub key"); + celix_err_push("Failed to create sub key."); return CELIX_ENOMEM; } status = celix_properties_decodeValue(props, combinedKey, fieldValue, flags); @@ -502,7 +498,7 @@ celix_properties_decodeValue(celix_properties_t* props, const char* key, json_t* return CELIX_SUCCESS; } else if (json_is_array(jsonValue) && json_array_size(jsonValue) == 0) { if (flags & CELIX_PROPERTIES_DECODE_ERROR_ON_EMPTY_ARRAYS) { - celix_err_pushf("Invalid empty array for key '%s'", key); + celix_err_pushf("Invalid empty array for key '%s'.", key); return CELIX_ILLEGAL_ARGUMENT; } // ignore empty arrays @@ -511,14 +507,14 @@ celix_properties_decodeValue(celix_properties_t* props, const char* key, json_t* status = celix_properties_decodeArray(props, key, jsonValue, flags); } else if (json_is_null(jsonValue)) { if (flags & CELIX_PROPERTIES_DECODE_ERROR_ON_NULL_VALUES) { - celix_err_pushf("Invalid null value for key '%s'", key); + celix_err_pushf("Invalid null value for key '%s'.", key); return CELIX_ILLEGAL_ARGUMENT; } // ignore null values return CELIX_SUCCESS; } else { // LCOV_EXCL_START - celix_err_pushf("Invalid json value type for key '%s'", key); + celix_err_pushf("Invalid json value type for key '%s'.", key); return CELIX_ILLEGAL_ARGUMENT; // LCOV_EXCL_STOP } @@ -527,7 +523,7 @@ celix_properties_decodeValue(celix_properties_t* props, const char* key, json_t* 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"); + celix_err_push("Expected json object."); return CELIX_ILLEGAL_ARGUMENT; } @@ -557,7 +553,7 @@ celix_status_t celix_properties_loadFromStream(FILE* stream, int decodeFlags, ce } json_t* root = json_loadf(stream, jsonFlags, &jsonError); if (!root) { - celix_err_pushf("Failed to parse json: %s", jsonError.text); + celix_err_pushf("Failed to parse json: %s.", jsonError.text); return CELIX_ILLEGAL_ARGUMENT; } return celix_properties_decodeFromJson(root, decodeFlags, out); @@ -566,7 +562,7 @@ celix_status_t celix_properties_loadFromStream(FILE* stream, int decodeFlags, ce celix_status_t celix_properties_load2(const char* filename, int decodeFlags, celix_properties_t** out) { FILE* stream = fopen(filename, "r"); if (!stream) { - celix_err_pushf("Failed to open file %s", filename); + celix_err_pushf("Failed to open file %s.", filename); return CELIX_FILE_IO_EXCEPTION; } celix_status_t status = celix_properties_loadFromStream(stream, decodeFlags, out); @@ -577,7 +573,7 @@ celix_status_t celix_properties_load2(const char* filename, int decodeFlags, cel celix_status_t celix_properties_loadFromString2(const char* input, int decodeFlags, celix_properties_t** out) { FILE* stream = fmemopen((void*)input, strlen(input), "r"); if (!stream) { - celix_err_push("Failed to open memstream"); + celix_err_push("Failed to open memstream."); return CELIX_FILE_IO_EXCEPTION; } celix_status_t status = celix_properties_loadFromStream(stream, decodeFlags, out);
