This is an automated email from the ASF dual-hosted git repository. pnoltes pushed a commit to branch feature/parse_filter_attribute_types in repository https://gitbox.apache.org/repos/asf/celix.git
commit 71c6a9149d577c9d59eb5bacdf273bdf4c2622cb Author: Pepijn Noltes <[email protected]> AuthorDate: Fri Mar 31 12:46:54 2023 +0200 Refactor filter match so that filter attributes use compare with long, double or version if needed. --- libs/utils/gtest/src/CxxFilterTestSuite.cc | 56 +++++++- libs/utils/include/celix_filter.h | 7 +- libs/utils/src/filter.c | 199 +++++++++++++++++++++++++++-- 3 files changed, 248 insertions(+), 14 deletions(-) diff --git a/libs/utils/gtest/src/CxxFilterTestSuite.cc b/libs/utils/gtest/src/CxxFilterTestSuite.cc index cfcbebc6..0389d4b6 100644 --- a/libs/utils/gtest/src/CxxFilterTestSuite.cc +++ b/libs/utils/gtest/src/CxxFilterTestSuite.cc @@ -115,4 +115,58 @@ TEST_F(CxxFilterTestSuite, HasMandatoryNegatedPresenceAttribute) { EXPECT_FALSE(filter2.hasMandatoryNegatedPresenceAttribute("key1")); EXPECT_TRUE(filter2.hasMandatoryNegatedPresenceAttribute("key2")); EXPECT_FALSE(filter2.hasMandatoryNegatedPresenceAttribute("key3")); -} \ No newline at end of file +} + +TEST_F(CxxFilterTestSuite, FilterForLongAttribute) { + celix::Filter filter1{"(key1>=10)"}; + EXPECT_TRUE(filter1.match({{"key1", "12"}})); + EXPECT_TRUE(filter1.match({{"key1", "11.1"}})); + EXPECT_TRUE(filter1.match({{"key1", "10"}})); + EXPECT_FALSE(filter1.match({{"key1", "2"}})); + EXPECT_FALSE(filter1.match({{"key1", "1"}})); + EXPECT_FALSE(filter1.match({{"key1", "0.1"}})); + + celix::Filter filter2{"(key1>20)"}; + EXPECT_TRUE(filter2.match({{"key1", "21"}})); + EXPECT_TRUE(filter2.match({{"key1", "20.1"}})); + EXPECT_FALSE(filter2.match({{"key1", "20"}})); + EXPECT_FALSE(filter2.match({{"key1", "3"}})); + EXPECT_FALSE(filter2.match({{"key1", "2"}})); + EXPECT_FALSE(filter2.match({{"key1", "0.1"}})); +} + +TEST_F(CxxFilterTestSuite, FilterForDoubleAttribute) { + celix::Filter filter1{"(key1>=10.5)"}; + EXPECT_TRUE(filter1.match({{"key1", "12"}})); + EXPECT_TRUE(filter1.match({{"key1", "11.1"}})); + EXPECT_TRUE(filter1.match({{"key1", "10.5"}})); + EXPECT_FALSE(filter1.match({{"key1", "2"}})); + EXPECT_FALSE(filter1.match({{"key1", "1"}})); + EXPECT_FALSE(filter1.match({{"key1", "0.1"}})); + + celix::Filter filter2{"(key1>20.5)"}; + EXPECT_TRUE(filter2.match({{"key1", "21"}})); + EXPECT_TRUE(filter2.match({{"key1", "20.7"}})); + EXPECT_FALSE(filter2.match({{"key1", "20.5"}})); + EXPECT_FALSE(filter2.match({{"key1", "3"}})); + EXPECT_FALSE(filter2.match({{"key1", "2"}})); + EXPECT_FALSE(filter2.match({{"key1", "0.1"}})); +} + +TEST_F(CxxFilterTestSuite, FilterForVersionAttribute) { + celix::Filter filter1{"(key1>=1.2.3.qualifier)"}; + EXPECT_TRUE(filter1.match({{"key1", "1.2.3.qualifier"}})); + EXPECT_TRUE(filter1.match({{"key1", "2"}})); + EXPECT_TRUE(filter1.match({{"key1", "2.0.0"}})); + EXPECT_TRUE(filter1.match({{"key1", "1.2.4"}})); + EXPECT_FALSE(filter1.match({{"key1", "1.2.2"}})); + EXPECT_FALSE(filter1.match({{"key1", "1.0.0"}})); + EXPECT_FALSE(filter1.match({{"key1", "0.1"}})); + + celix::Filter filter2{"(key1>2.3.4)"}; + EXPECT_TRUE(filter2.match({{"key1", "3"}})); + EXPECT_TRUE(filter2.match({{"key1", "3.0.0"}})); + EXPECT_TRUE(filter2.match({{"key1", "2.3.5"}})); + EXPECT_FALSE(filter2.match({{"key1", "2.3.4"}})); + EXPECT_FALSE(filter2.match({{"key1", "0.0.3"}})); +} diff --git a/libs/utils/include/celix_filter.h b/libs/utils/include/celix_filter.h index b64c5806..aed8bd1f 100644 --- a/libs/utils/include/celix_filter.h +++ b/libs/utils/include/celix_filter.h @@ -42,6 +42,9 @@ typedef enum celix_filter_operand_enum CELIX_FILTER_OPERAND_NOT, } celix_filter_operand_t; + +typedef struct celix_filter_internal celix_filter_internal_t; //opaque struct for internal use only + typedef struct celix_filter_struct celix_filter_t; struct celix_filter_struct { @@ -53,9 +56,9 @@ struct celix_filter_struct { //type is celix_filter_t* for AND, OR and NOT operator and char* for SUBSTRING //for other operands children is NULL celix_array_list_t *children; -}; - + celix_filter_internal_t* internal; //for internal use only +}; celix_filter_t* celix_filter_create(const char *filterStr); diff --git a/libs/utils/src/filter.c b/libs/utils/src/filter.c index 9ffda8a8..bf5ee47e 100644 --- a/libs/utils/src/filter.c +++ b/libs/utils/src/filter.c @@ -27,6 +27,76 @@ #include "celix_filter.h" #include "filter.h" #include "celix_errno.h" +#include "celix_version.h" + + +struct celix_filter_internal { + bool convertedToLong; + long longValue; + bool convertedToDouble; + double doubleValue; + bool convertedToVersion; + celix_version_t *versionValue; +}; + +//TODO replace with usage of celix_convert_utils.h if gh-476 is merged +static double celix_utils_convertStringToDouble(const char* val, double defaultValue, bool* converted) { + double result = defaultValue; + if (converted != NULL) { + *converted = false; + } + if (val != NULL) { + char *endptr; + double d = strtod(val, &endptr); + if (endptr != val && *endptr == '\0') { + result = d; + if (converted) { + *converted = true; + } + } + } + return result; +} + +//TODO replace with usage of celix_convert_utils.h if gh-476 is merged +static long celix_utils_convertStringToLong(const char* val, long defaultValue, bool* converted) { + long result = defaultValue; + if (converted != NULL) { + *converted = false; + } + if (val != NULL) { + char *endptr; + long l = strtol(val, &endptr, 10); + if (endptr != val && *endptr == '\0') { + result = l; + if (converted) { + *converted = true; + } + } + } + return result; +} + +//TODO replace with usage of celix_convert_utils.h if gh-476 is merged +static celix_version_t* celix_utils_convertStringToVersion(const char* val, const celix_version_t* defaultValue, bool* converted) { + celix_version_t *result = NULL; + if (val != NULL) { + //check if string has two dots ('.'), and only try to create string if it has two dots + char *firstDot = strchr(val, '.'); + char *lastDot = strrchr(val, '.'); + if (firstDot != NULL && lastDot != NULL && firstDot != lastDot) { + result = celix_version_createVersionFromString(val); + } + } + if (converted) { + *converted = result != NULL; + } + if (result == NULL && defaultValue != NULL) { + result = celix_version_copy(defaultValue); + } + return result; +} + static void filter_skipWhiteSpace(char* filterString, int* pos); static celix_filter_t * filter_parseFilter(char* filterString, int* pos); @@ -55,7 +125,7 @@ void filter_destroy(celix_filter_t * filter) { return celix_filter_destroy(filter); } -static celix_filter_t * filter_parseFilter(char * filterString, int * pos) { +static celix_filter_t * filter_parseFilter(char* filterString, int* pos) { celix_filter_t * filter; filter_skipWhiteSpace(filterString, pos); if (filterString[*pos] == '\0') { @@ -230,7 +300,7 @@ static celix_filter_t * filter_parseItem(char * filterString, int * pos) { *pos += 2; filter_skipWhiteSpace(filterString, pos); if (filterString[*pos] == ')') { - celix_filter_t * filter = (celix_filter_t *) calloc(1, sizeof(*filter)); + filter = (celix_filter_t *) calloc(1, sizeof(*filter)); filter->operand = CELIX_FILTER_OPERAND_PRESENT; filter->attribute = attr; filter->value = NULL; @@ -407,6 +477,49 @@ static celix_array_list_t* filter_parseSubstring(char * filterString, int * pos) return operands; } +static bool celix_filter_isCompareOperand(celix_filter_operand_t operand) { + return operand == CELIX_FILTER_OPERAND_EQUAL || + operand == CELIX_FILTER_OPERAND_GREATER || + operand == CELIX_FILTER_OPERAND_LESS || + operand == CELIX_FILTER_OPERAND_GREATEREQUAL || + operand == CELIX_FILTER_OPERAND_LESSEQUAL; +} + + +static bool celix_filter_hasFilterChildren(celix_filter_t* filter) { + return filter->operand == CELIX_FILTER_OPERAND_AND || + filter->operand == CELIX_FILTER_OPERAND_OR || + filter->operand == CELIX_FILTER_OPERAND_NOT; +} + +/** + * 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 = malloc(sizeof(*filter->internal)); + if (filter->internal == NULL) { + return CELIX_ENOMEM; + } else { + filter->internal->longValue = celix_utils_convertStringToLong(filter->value, 0, &filter->internal->convertedToLong); + filter->internal->doubleValue = celix_utils_convertStringToDouble(filter->value, 0.0, &filter->internal->convertedToDouble); + filter->internal->versionValue = celix_utils_convertStringToVersion(filter->value, NULL, &filter->internal->convertedToVersion); + } + } + + if (celix_filter_hasFilterChildren(filter)) { + for (int i = 0; i < celix_arrayList_size(filter->children); i++) { + celix_filter_t* child = celix_arrayList_get(filter->children, i); + celix_status_t status = celix_filter_compile(child); + if (status != CELIX_SUCCESS) { + return status; + } + } + } + + return CELIX_SUCCESS; +} + celix_status_t filter_match(celix_filter_t * filter, celix_properties_t *properties, bool *out) { bool result = celix_filter_match(filter, properties); if (out != NULL) { @@ -415,6 +528,62 @@ celix_status_t filter_match(celix_filter_t * filter, celix_properties_t *propert return CELIX_SUCCESS; } +static bool celix_filter_propertyValueHasAtLeastTwoDots(const char* propertyValue) { + int count = 0; + for (int i = 0; i < strlen(propertyValue); i++) { + if (propertyValue[i] == '.') { + count += 1; + if (count >= 2) { + break; + } + } + } + return count >= 2; +} + +static int celix_filter_compareAttributeValue(const celix_filter_t* filter, const char* propertyValue) { + if (!filter->internal->convertedToLong && !filter->internal->convertedToDouble && !filter->internal->convertedToVersion) { + return strcmp(propertyValue, filter->value); + } + + if (filter->internal->convertedToLong) { + bool propertyValueIsLong = false; + long value = celix_utils_convertStringToLong(propertyValue, 0, &propertyValueIsLong); + if (propertyValueIsLong) { + return (int) (value - filter->internal->longValue); + } + } + + if (filter->internal->convertedToDouble) { + bool propertyValueIsDouble = false; + double value = celix_utils_convertStringToDouble(propertyValue, 0.0, &propertyValueIsDouble); + if (propertyValueIsDouble) { + if (value < filter->internal->doubleValue) { + return -1; + } else if (value > filter->internal->doubleValue) { + return 1; + } else { + return 0; + } + } + } + + //note: only supporting version compare if the property value has at least two dots, this to prevent unnecessary + //malloc and free of version objects. + if (filter->internal->convertedToVersion && celix_filter_propertyValueHasAtLeastTwoDots(propertyValue)) { + bool propertyValueIsVersion = false; + celix_version_t *value = celix_utils_convertStringToVersion(propertyValue, NULL, &propertyValueIsVersion); + if (propertyValueIsVersion) { + int cmp = celix_version_compareTo(value, filter->internal->versionValue); + celix_version_destroy(value); + return cmp; + } + } + + //fallback on string compare + return strcmp(propertyValue, filter->value); +} + static celix_status_t filter_compare(const celix_filter_t* filter, const char *propertyValue, bool *out) { celix_status_t status = CELIX_SUCCESS; bool result = false; @@ -479,25 +648,28 @@ static celix_status_t filter_compare(const celix_filter_t* filter, const char *p *out = true; return CELIX_SUCCESS; } - case CELIX_FILTER_OPERAND_APPROX: //TODO: Implement strcmp with ignorecase and ignorespaces + case CELIX_FILTER_OPERAND_APPROX: { + *out = strcasestr(propertyValue, filter->value) != NULL; + return CELIX_SUCCESS; + } case CELIX_FILTER_OPERAND_EQUAL: { - *out = (strcmp(propertyValue, filter->value) == 0); + *out = (celix_filter_compareAttributeValue(filter, propertyValue) == 0); return CELIX_SUCCESS; } case CELIX_FILTER_OPERAND_GREATER: { - *out = (strcmp(propertyValue, filter->value) > 0); + *out = (celix_filter_compareAttributeValue(filter, propertyValue) > 0); return CELIX_SUCCESS; } case CELIX_FILTER_OPERAND_GREATEREQUAL: { - *out = (strcmp(propertyValue, filter->value) >= 0); + *out = (celix_filter_compareAttributeValue(filter, propertyValue) >= 0); return CELIX_SUCCESS; } case CELIX_FILTER_OPERAND_LESS: { - *out = (strcmp(propertyValue, filter->value) < 0); + *out = (celix_filter_compareAttributeValue(filter, propertyValue) < 0); return CELIX_SUCCESS; } case CELIX_FILTER_OPERAND_LESSEQUAL: { - *out = (strcmp(propertyValue, filter->value) <= 0); + *out = (celix_filter_compareAttributeValue(filter, propertyValue) <= 0); return CELIX_SUCCESS; } case CELIX_FILTER_OPERAND_AND: @@ -508,7 +680,7 @@ static celix_status_t filter_compare(const celix_filter_t* filter, const char *p /* no break */ } - if (status == CELIX_SUCCESS && out != NULL) { + if (out != NULL) { *out = result; } return status; @@ -554,6 +726,7 @@ celix_filter_t* celix_filter_create(const char *filterString) { free(filterStr); } else { filter->filterStr = filterStr; + celix_filter_compile(filter); } return filter; @@ -561,7 +734,7 @@ celix_filter_t* celix_filter_create(const char *filterString) { void celix_filter_destroy(celix_filter_t *filter) { if (filter != NULL) { - if(filter->children != NULL){ + if(filter->children != NULL) { if (filter->operand == CELIX_FILTER_OPERAND_SUBSTRING) { int size = celix_arrayList_size(filter->children); int i = 0; @@ -576,7 +749,7 @@ void celix_filter_destroy(celix_filter_t *filter) { int i = 0; for (i = 0; i < size; i++) { celix_filter_t *f = celix_arrayList_get(filter->children, i); - filter_destroy(f); + celix_filter_destroy(f); } celix_arrayList_destroy(filter->children); filter->children = NULL; @@ -590,6 +763,10 @@ void celix_filter_destroy(celix_filter_t *filter) { filter->attribute = NULL; free((char*)filter->filterStr); filter->filterStr = NULL; + if (filter->internal != NULL) { + celix_version_destroy(filter->internal->versionValue); + free(filter->internal); + } free(filter); } }
