PengZheng commented on code in PR #721: URL: https://github.com/apache/celix/pull/721#discussion_r1472848600
########## libs/utils/src/filter.c: ########## @@ -801,10 +880,10 @@ bool celix_filter_match(const celix_filter_t* filter, const celix_properties_t* return !childResult; } - // substring, equal, greater, greaterEqual, less, lessEqual, approx done with matchPropertyEntry + // present, substring, equal, greater, greaterEqual, less, lessEqual, approx done with matchPropertyEntry Review Comment: Present is on line 852. ########## libs/utils/src/filter.c: ########## @@ -673,32 +705,81 @@ static bool celix_filter_matchSubString(const celix_filter_t* filter, const celi if (final) { const char* found = strstr(currentValue, final); - if (!found || found + celix_utils_strlen(final) != entry->value + strLen) { + if (!found || found + celix_utils_strlen(final) != value + strLen) { return false; } } return true; } +static bool celix_filter_matchSubString(const celix_filter_t* filter, const celix_properties_entry_t* entry) { + if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_STRING_ARRAY) { + for (int i = 0; i < celix_arrayList_size(entry->typed.arrayValue); i++) { + const char* substr = celix_arrayList_getString(entry->typed.arrayValue, i); + if (celix_filter_matchSubStringForValue(filter, substr)) { + return true; + } + return false; + } + } + return celix_filter_matchSubStringForValue(filter, entry->value); +} + +static bool celix_filter_matchApprox(const celix_filter_t* filter, const celix_properties_entry_t* entry) { + if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_STRING_ARRAY) { + for (int i = 0; i < celix_arrayList_size(entry->typed.arrayValue); i++) { + const char* substr = celix_arrayList_getString(entry->typed.arrayValue, i); + if (strcasestr(substr, filter->value) != NULL) { + return true; + } + } + return false; + } + return strcasestr(entry->value, filter->value) != NULL; +} + static bool celix_filter_matchPropertyEntry(const celix_filter_t* filter, const celix_properties_entry_t* entry) { - switch (filter->operand) { - case CELIX_FILTER_OPERAND_SUBSTRING: + if (filter->operand == CELIX_FILTER_OPERAND_SUBSTRING) { return celix_filter_matchSubString(filter, entry); - case CELIX_FILTER_OPERAND_APPROX: - return strcasecmp(entry->value, filter->value) == 0; - case CELIX_FILTER_OPERAND_EQUAL: - return celix_filter_compareAttributeValue(filter, entry) == 0; - case CELIX_FILTER_OPERAND_GREATER: - return celix_filter_compareAttributeValue(filter, entry) > 0; - case CELIX_FILTER_OPERAND_GREATEREQUAL: - return celix_filter_compareAttributeValue(filter, entry) >= 0; - case CELIX_FILTER_OPERAND_LESS: - return celix_filter_compareAttributeValue(filter, entry) < 0; - default: - assert(filter->operand == CELIX_FILTER_OPERAND_LESSEQUAL); - return celix_filter_compareAttributeValue(filter, entry) <= 0; + } else if (filter->operand == CELIX_FILTER_OPERAND_APPROX) { + return celix_filter_matchApprox(filter, entry); + } + + assert(filter->operand == CELIX_FILTER_OPERAND_EQUAL || filter->operand == CELIX_FILTER_OPERAND_GREATER || + filter->operand == CELIX_FILTER_OPERAND_LESS || filter->operand == CELIX_FILTER_OPERAND_GREATEREQUAL || + filter->operand == CELIX_FILTER_OPERAND_LESSEQUAL); + + + //match for array types + if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_LONG_ARRAY && filter->internal->convertedToLong) { + return celix_utils_matchLongArrays(filter->operand, entry->typed.arrayValue, filter->internal->longValue); + } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_DOUBLE_ARRAY && filter->internal->convertedToDouble) { + return celix_utils_matchDoubleArrays(filter->operand, entry->typed.arrayValue, filter->internal->doubleValue); + } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_BOOL_ARRAY && filter->internal->convertedToBool) { + return celix_utils_matchBoolArrays(filter->operand, entry->typed.arrayValue, filter->internal->boolValue); + } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_VERSION_ARRAY && filter->internal->convertedToVersion) { + return celix_utils_matchVersionArrays(filter->operand, entry->typed.arrayValue, filter->internal->versionValue); + } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_STRING_ARRAY) { + return celix_utils_matchStringArrays(filter->operand, entry->typed.arrayValue, filter->value); + } + + //regular compare -> match + int cmp; + if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_LONG && filter->internal->convertedToLong) { + cmp = celix_filter_cmpLong(entry->typed.longValue, filter->internal->longValue); + } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_DOUBLE && filter->internal->convertedToDouble) { + cmp = celix_filter_cmpDouble(entry->typed.doubleValue, filter->internal->doubleValue); + } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_BOOL && filter->internal->convertedToBool) { + cmp = celix_filter_cmpBool(entry->typed.boolValue, filter->internal->boolValue); + } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_VERSION && filter->internal->convertedToVersion) { + cmp = celix_version_compareTo(entry->typed.versionValue, filter->internal->versionValue); + } else { + // type string or property type and converted filter attribute value do not match -> + // fallback on string compare + cmp = strcmp(entry->value, filter->value); } + return celix_utils_convertCompareToBool(filter->operand, cmp); Review Comment: Consider the example given in Doxygen: > Example: The filter "(key>20)" and a property set with a long value 3 for key "key", will match and the same filter but with a property set which has a string value "3" for key "key", will not match. For a property set that has value "3" for key "key", it will match because as a string "3" > "20". On the other hand, A property set has value 3 for key "key" will NOT match since 3 < 20. Just contrary to what's said in documentation. ########## libs/utils/src/filter.c: ########## @@ -515,35 +515,38 @@ static bool celix_filter_hasFilterChildren(celix_filter_t* filter) { filter->operand == CELIX_FILTER_OPERAND_NOT; } +static void celix_filter_destroyInternal(celix_filter_internal_t* internal) { + if (internal) { + celix_version_destroy(internal->versionValue); + free(internal); + } +} + +CELIX_DEFINE_AUTOPTR_CLEANUP_FUNC(celix_filter_internal_t, celix_filter_destroyInternal) + /** * Compiles the filter, so that the attribute values are converted to the typed values if possible. */ static celix_status_t celix_filter_compile(celix_filter_t* filter) { if (celix_filter_isCompareOperand(filter->operand)) { - filter->internal = calloc(1, sizeof(*filter->internal)); - if (filter->internal == NULL) { - celix_err_push("Filter Error: Failed to allocate memory."); - return CELIX_ENOMEM; + celix_autoptr(celix_filter_internal_t) internal = calloc(1, sizeof(*internal)); + if (!internal) { + return ENOMEM; } - do { - filter->internal->longValue = - celix_utils_convertStringToLong(filter->value, 0, &filter->internal->convertedToLong); - if (filter->internal->convertedToLong) { - break; - } - filter->internal->doubleValue = - celix_utils_convertStringToDouble(filter->value, 0.0, &filter->internal->convertedToDouble); - if (filter->internal->convertedToDouble) { - break; - } - filter->internal->boolValue = - celix_utils_convertStringToBool(filter->value, false, &filter->internal->convertedToBool); - if (filter->internal->convertedToBool) { - break; - } - filter->internal->versionValue = - celix_utils_convertStringToVersion(filter->value, NULL, &filter->internal->convertedToVersion); - } while(false); + internal->longValue = + celix_utils_convertStringToLong(filter->value, 0, &internal->convertedToLong); + internal->doubleValue = + celix_utils_convertStringToDouble(filter->value, 0.0, &internal->convertedToDouble); + internal->boolValue = + celix_utils_convertStringToBool(filter->value, false, &internal->convertedToBool); + + celix_status_t convertStatus = celix_utils_convertStringToVersion(filter->value, NULL, &internal->versionValue); Review Comment: Without typing information provided by filter creator or some central property registry, I agree trying all types rather than stopping at the first success is correct. -- 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