PengZheng commented on code in PR #743: URL: https://github.com/apache/celix/pull/743#discussion_r1608278296
########## libs/utils/src/properties_encoding.c: ########## @@ -0,0 +1,580 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "celix_properties.h" + +#include "celix_err.h" +#include "celix_stdlib_cleanup.h" +#include "celix_utils.h" + +#include <assert.h> +#include <jansson.h> +#include <math.h> +#include <string.h> + +static celix_status_t +celix_properties_decodeValue(celix_properties_t* props, const char* key, json_t* jsonValue, int flags); + +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."); + return ENOMEM; + } + *out = json_sprintf("version<%s>", versionStr); + if (!*out) { + celix_err_push("Failed to create json string."); + return ENOMEM; + } + return CELIX_SUCCESS; +} + +static celix_status_t celix_properties_arrayElementEntryValueToJson(celix_array_list_element_type_t elType, + celix_array_list_entry_t entry, + json_t** out) { + *out = NULL; + switch (elType) { + case CELIX_ARRAY_LIST_ELEMENT_TYPE_STRING: + *out = json_string(entry.stringVal); + break; + case CELIX_ARRAY_LIST_ELEMENT_TYPE_LONG: + *out = json_integer(entry.longVal); + break; + case CELIX_ARRAY_LIST_ELEMENT_TYPE_DOUBLE: + *out = json_real(entry.doubleVal); + break; + case CELIX_ARRAY_LIST_ELEMENT_TYPE_BOOL: + *out = json_boolean(entry.boolVal); + break; + case CELIX_ARRAY_LIST_ELEMENT_TYPE_VERSION: + return celix_properties_versionToJson(entry.versionVal, out); + default: + // LCOV_EXCL_START + 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."); + return ENOMEM; + } + return CELIX_SUCCESS; +} + +static celix_status_t celix_properties_arrayEntryValueToJson(const char* key, + const celix_properties_entry_t* entry, + int flags, + json_t** out) { + *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); + return CELIX_ILLEGAL_ARGUMENT; + } + return CELIX_SUCCESS; // empty array -> treat as unset property + } + + json_auto_t* array = json_array(); + if (!array) { + celix_err_push("Failed to create json array."); + return ENOMEM; + } + + for (int i = 0; i < celix_arrayList_size(entry->typed.arrayValue); ++i) { + celix_array_list_entry_t arrayEntry = celix_arrayList_getEntry(entry->typed.arrayValue, i); + celix_array_list_element_type_t elType = celix_arrayList_getElementType(entry->typed.arrayValue); + json_t* jsonValue; + celix_status_t status = celix_properties_arrayElementEntryValueToJson(elType, arrayEntry, &jsonValue); + if (status != CELIX_SUCCESS) { + return status; + } else if (!jsonValue) { + // ignore unset values + } else { + int rc = json_array_append_new(array, jsonValue); + if (rc != 0) { + celix_err_push("Failed to append json string to array."); + return ENOMEM; + } + } + } + + *out = celix_steal_ptr(array); + return CELIX_SUCCESS; +} + +static celix_status_t +celix_properties_entryValueToJson(const char* key, const celix_properties_entry_t* entry, int flags, json_t** out) { + *out = NULL; + switch (entry->valueType) { + case CELIX_PROPERTIES_VALUE_TYPE_STRING: + *out = json_string(entry->value); + break; + case CELIX_PROPERTIES_VALUE_TYPE_LONG: + *out = json_integer(entry->typed.longValue); + break; + case CELIX_PROPERTIES_VALUE_TYPE_DOUBLE: + if (isnan(entry->typed.doubleValue) || isinf(entry->typed.doubleValue)) { + if (flags & CELIX_PROPERTIES_ENCODE_ERROR_ON_NAN_INF) { + celix_err_pushf("Invalid NaN or Inf in key '%s'.", key); + return CELIX_ILLEGAL_ARGUMENT; + } + return CELIX_SUCCESS; // ignore NaN and Inf + } + *out = json_real(entry->typed.doubleValue); + break; + case CELIX_PROPERTIES_VALUE_TYPE_BOOL: + *out = json_boolean(entry->typed.boolValue); + break; + case CELIX_PROPERTIES_VALUE_TYPE_VERSION: + return celix_properties_versionToJson(entry->typed.versionValue, out); + case CELIX_PROPERTIES_VALUE_TYPE_ARRAY_LIST: + return celix_properties_arrayEntryValueToJson(key, entry, flags, out); + default: + // LCOV_EXCL_START + celix_err_pushf("Unexpected properties entry type %d.", entry->valueType); + return CELIX_ILLEGAL_ARGUMENT; + // LCOV_EXCL_STOP + } + + if (!*out) { + celix_err_pushf("Failed to create json value for key '%s'.", key); + return ENOMEM; + } + return CELIX_SUCCESS; +} + +static celix_status_t celix_properties_addJsonValueToJson(json_t* value, const char* key, json_t* obj, int flags) { + if (!value) { + // ignore unset values + return CELIX_SUCCESS; + } + + json_t* field = json_object_get(obj, key); + if (field) { + if (flags & CELIX_PROPERTIES_ENCODE_ERROR_ON_COLLISIONS) { + celix_err_pushf("Invalid key collision. key '%s' already exists.", key); + return CELIX_ILLEGAL_ARGUMENT; + } + } + + int rc = json_object_set_new(obj, key, value); + if (rc != 0) { + celix_err_push("Failed to set json object"); + return ENOMEM; + } + return CELIX_SUCCESS; +} + +static celix_status_t celix_properties_addPropertiesEntryFlatToJson(const celix_properties_entry_t* entry, + const char* key, + json_t* root, + int flags) { + json_t* value; + celix_status_t status = celix_properties_entryValueToJson(key, entry, flags, &value); + status = CELIX_DO_IF(status, celix_properties_addJsonValueToJson(value, key, root, flags)); + return status; +} + +static celix_status_t celix_properties_addPropertiesEntryToJson(const celix_properties_entry_t* entry, + const char* key, + json_t* root, + int flags) { + json_t* jsonObj = root; + const char* fieldName = key; + const char* slash = strstr(key, "/"); + while (slash) { + char buf[64]; + char* name = celix_utils_writeOrCreateString(buf, sizeof(buf), "%.*s", (int)(slash - fieldName), fieldName); + celix_auto(celix_utils_string_guard_t) strGuard = celix_utils_stringGuard_init(buf, name); + if (!name) { + celix_err_push("Failed to create name string"); + return ENOMEM; + } + json_t* subObj = json_object_get(jsonObj, name); + if (!subObj || !json_is_object(subObj)) { Review Comment: If `subObj` is not an object, it makes no sense to try to insert key-value pair into it, which merely leads to errors. The fix still fails the following test ```C++ celix_autoptr(celix_properties_t) props2 = celix_properties_create(); // pick keys such that key1 appears before key2 when iterating over the properties celix_properties_set(props2, "a.b.c", "value1"); celix_properties_set(props2, "a.b.c.d", "value2"); CELIX_PROPERTIES_ITERATE(props2, iter) { std::cout << "ZPTest " << iter.key << std::endl; } status = celix_properties_saveToString(props2, CELIX_PROPERTIES_ENCODE_NESTED_STYLE, &output); EXPECT_EQ(CELIX_SUCCESS, status); ``` Note that the above is included in `TEST_F(PropertiesSerializationTestSuite, KeyCollision)`. Please check my fix. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@celix.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
