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

Reply via email to