PengZheng commented on code in PR #692:
URL: https://github.com/apache/celix/pull/692#discussion_r1421708787


##########
libs/utils/src/filter.c:
##########
@@ -463,367 +555,327 @@ static celix_status_t 
celix_filter_compile(celix_filter_t* filter) {
     return CELIX_SUCCESS;
 }
 
-celix_status_t filter_match(celix_filter_t * filter, celix_properties_t 
*properties, bool *out) {
+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) {
         *out = result;
     }
     return CELIX_SUCCESS;
 }
 
-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);
+static int celix_filter_cmpLong(long a, long b) {
+    if (a < b) {
+        return -1;
+    } else if (a > b) {
+        return 1;
+    } else {
+        return 0;
     }
+}
 
-    if (filter->internal->convertedToLong) {
-        bool propertyValueIsLong = false;
-        long value = celix_utils_convertStringToLong(propertyValue, 0, 
&propertyValueIsLong);
-        if (propertyValueIsLong) {
-            if (value < filter->internal->longValue)
-                return -1;
-            else if (value > filter->internal->longValue)
-                return 1;
-            else
-                return 0;
-        }
-    }
-
-    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;
-            }
-        }
+static int celix_filter_cmpDouble(double a, double b) {
+    if (a < b) {
+        return -1;
+    } else if (a > b) {
+        return 1;
+    } else {
+        return 0;
+    }
+}
+
+static int celix_filter_cmpBool(bool a, bool b) {
+    if (a == b) {
+        return 0;
+    } else if (a) {
+        return 1;
+    } else {
+        return -1;
+    }
+}
+
+static int celix_filter_compareAttributeValue(const celix_filter_t* filter, 
const celix_properties_entry_t* entry) {
+    // not converted, fallback on string compare
+    if (!filter->internal->convertedToLong && 
!filter->internal->convertedToDouble &&
+        !filter->internal->convertedToBool && 
!filter->internal->convertedToVersion) {
+        return strcmp(entry->value, filter->value);
     }
 
-    if (filter->internal->convertedToVersion) {
-        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);
+    // compare typed values
+    if (filter->internal->convertedToLong && entry->valueType == 
CELIX_PROPERTIES_VALUE_TYPE_LONG) {
+        return celix_filter_cmpLong(entry->typed.longValue, 
filter->internal->longValue);
+    } else if (filter->internal->convertedToDouble && entry->valueType == 
CELIX_PROPERTIES_VALUE_TYPE_DOUBLE) {
+        return celix_filter_cmpDouble(entry->typed.doubleValue, 
filter->internal->doubleValue);
+    } else if (filter->internal->convertedToBool && entry->valueType == 
CELIX_PROPERTIES_VALUE_TYPE_BOOL) {
+        return celix_filter_cmpBool(entry->typed.boolValue, 
filter->internal->boolValue);
+    } else if (filter->internal->convertedToVersion && entry->valueType == 
CELIX_PROPERTIES_VALUE_TYPE_VERSION) {
+        return celix_version_compareTo(entry->typed.versionValue, 
filter->internal->versionValue);
+    }
+
+    // check if the property string value can be converted to the filter value 
type
+    bool propertyConverted;
+    if (filter->internal->convertedToLong) {
+        long val = celix_utils_convertStringToLong(entry->value, 0, 
&propertyConverted);
+        if (propertyConverted) {
+            return celix_filter_cmpLong(val, filter->internal->longValue);
+        }
+    } else if (filter->internal->convertedToDouble) {
+        double val = celix_utils_convertStringToDouble(entry->value, 0.0, 
&propertyConverted);
+        if (propertyConverted) {
+            return celix_filter_cmpDouble(val, filter->internal->doubleValue);
+        }
+    } else if (filter->internal->convertedToBool) {
+        bool val = celix_utils_convertStringToBool(entry->value, false, 
&propertyConverted);
+        if (propertyConverted) {
+            return celix_filter_cmpBool(val, filter->internal->boolValue);
+        }
+    } else if (filter->internal->convertedToVersion) {
+        celix_version_t* val = 
celix_utils_convertStringToVersion(entry->value, NULL, &propertyConverted);
+        if (propertyConverted) {
+            int cmp = celix_version_compareTo(val, 
filter->internal->versionValue);
+            celix_version_destroy(val);
             return cmp;
         }
     }
 
-    //fallback on string compare
-    return strcmp(propertyValue, filter->value);
+    // fallback on string compare
+    return strcmp(entry->value, 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;
+static bool celix_filter_matchSubString(const celix_filter_t* filter, const 
celix_properties_entry_t* entry) {
+    assert(filter->children && celix_arrayList_size(filter->children) >= 2);
 
-    if (filter == NULL || propertyValue == NULL) {
-        *out = false;
-        return status;
-    }
+    size_t strLen = celix_utils_strlen(entry->value);
+    const char* initial = celix_arrayList_get(filter->children, 0);
+    const char* final = celix_arrayList_get(filter->children, 
celix_arrayList_size(filter->children) - 1);
 
-    switch (filter->operand) {
-        case CELIX_FILTER_OPERAND_SUBSTRING: {
-            int pos = 0;
-            int size = celix_arrayList_size(filter->children);
-            for (int i = 0; i < size; i++) {
-                char * substr = (char *) celix_arrayList_get(filter->children, 
i);
-
-                if (i + 1 < size) {
-                    if (substr == NULL) {
-                        unsigned int index;
-                        char * substr2 = (char *) 
celix_arrayList_get(filter->children, i + 1);
-                        if (substr2 == NULL) {
-                            continue;
-                        }
-                        index = strcspn(propertyValue+pos, substr2);
-                        if (index == strlen(propertyValue+pos)) {
-                            *out = false;
-                            return CELIX_SUCCESS;
-                        }
-
-                        pos = index + strlen(substr2);
-                        if (i + 2 < size) {
-                            i++;
-                        }
-                    } else {
-                        unsigned int len = strlen(substr);
-                        char * region = (char *)calloc(1, len+1);
-                        strncpy(region, propertyValue+pos, len);
-                        region[len]    = '\0';
-                        if (strcmp(region, substr) == 0) {
-                            pos += len;
-                        } else {
-                            free(region);
-                            *out = false;
-                            return CELIX_SUCCESS;
-                        }
-                        free(region);
-                    }
-                } else {
-                    unsigned int len;
-                    int begin;
+    const char* currentValue = entry->value;
 
-                    if (substr == NULL) {
-                        *out = true;
-                        return CELIX_SUCCESS;
-                    }
-                    len = strlen(substr);
-                    begin = strlen(propertyValue)-len;
-                    *out = (strcmp(propertyValue+begin, substr) == 0);
-                    return CELIX_SUCCESS;
-                }
-            }
-            *out = true;
-            return CELIX_SUCCESS;
-        }
-        case CELIX_FILTER_OPERAND_APPROX: {
-            *out = strcasecmp(propertyValue, filter->value) == 0;
-            return CELIX_SUCCESS;
-        }
-        case CELIX_FILTER_OPERAND_EQUAL: {
-            *out = (celix_filter_compareAttributeValue(filter, propertyValue) 
== 0);
-            return CELIX_SUCCESS;
-        }
-        case CELIX_FILTER_OPERAND_GREATER: {
-            *out = (celix_filter_compareAttributeValue(filter, propertyValue) 
> 0);
-            return CELIX_SUCCESS;
-        }
-        case CELIX_FILTER_OPERAND_GREATEREQUAL: {
-            *out = (celix_filter_compareAttributeValue(filter, propertyValue) 
>= 0);
-            return CELIX_SUCCESS;
-        }
-        case CELIX_FILTER_OPERAND_LESS: {
-            *out = (celix_filter_compareAttributeValue(filter, propertyValue) 
< 0);
-            return CELIX_SUCCESS;
+    if (initial) {
+        const char* found = strstr(entry->value, initial);
+        currentValue = found + celix_utils_strlen(initial);
+        if (!found || found != entry->value) {
+            return false;
         }
-        case CELIX_FILTER_OPERAND_LESSEQUAL: {
-            *out = (celix_filter_compareAttributeValue(filter, propertyValue) 
<= 0);
-            return CELIX_SUCCESS;
+    }
+
+    for (int i = 1; i < celix_arrayList_size(filter->children) - 1; i++) {
+        const char* substr = celix_arrayList_get(filter->children, i);
+        const char* found = strstr(currentValue, substr);
+        if (!found || found <= currentValue) {

Review Comment:
   ```suggestion
           if (!found) {
   ```
   
   I found the following test case in Apache Felix
   
   ```Java
           pieces = SimpleFilter.parseSubstring("*foo");
           assertTrue("Should match!", SimpleFilter.compareSubstring(pieces, 
"foo"));
   ```
   
   I modified `TEST_F(FilterTestSuite, SubStringTest)` to add the following 
failing test:
   
   ```C++
   TEST_F(FilterTestSuite, SubStringTest) {
       celix_autoptr(celix_properties_t) props = celix_properties_create();
       celix_properties_set(props, "test", "John Bob Doe");
       celix_properties_set(props, "test2", "*ValueWithStar");
   
       celix_autoptr(celix_filter_t) filter13 = 
celix_filter_create("(test=*John*)");
       EXPECT_TRUE(celix_filter_match(filter13, props));
   }
   ```



-- 
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

Reply via email to