Skip to content

Commit

Permalink
gh-674: Fix broken equality test for substring filters and add more t…
Browse files Browse the repository at this point in the history
…ests for filter.

Attributes were ignored when performing equality test between substring filters.
  • Loading branch information
PengZheng committed Mar 31, 2024
1 parent 8e96817 commit eb2cf8a
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 18 deletions.
7 changes: 7 additions & 0 deletions libs/utils/gtest/src/FilterErrorInjectionTestSuite.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "celix_filter.h"
#include "celix_utils.h"
#include "celix_err.h"
#include "celix_version.h"

#include "celix_array_list_ei.h"
#include "celix_utils_ei.h"
Expand Down Expand Up @@ -247,3 +248,9 @@ TEST_F(FilterErrorInjectionTestSuite, ErrorFputcTest) {
filter = celix_filter_create(filterStr);
EXPECT_EQ(nullptr, filter);
}

TEST_F(FilterErrorInjectionTestSuite, ErrorWithVersionConversion) {
celix_ei_expect_calloc((void*)celix_version_create, 0, nullptr);
celix_autoptr(celix_filter_t) filter = celix_filter_create("(&(versions>=2.0.0)(versions<=2.0.0))");
EXPECT_EQ(nullptr, filter);
}
21 changes: 21 additions & 0 deletions libs/utils/gtest/src/FilterTestSuite.cc
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,18 @@ TEST_F(FilterTestSuite, MissingClosingBracketsCreateTest) {
filter = celix_filter_create(str);
ASSERT_TRUE(filter == nullptr);
free(str);

// test missing closing brackets in value
str = celix_utils_strdup("(&(test_attr1=attr1)(|(test_attr2=attr2)(test_attr3~=attr3");
filter = celix_filter_create(str);
ASSERT_TRUE(filter == nullptr);
free(str);

// test missing closing brackets in value
str = celix_utils_strdup("(&(test_attr1=attr1)(|(test_attr2=attr2)(test_attr3<4");
filter = celix_filter_create(str);
ASSERT_TRUE(filter == nullptr);
free(str);
}

TEST_F(FilterTestSuite, InvalidClosingBracketsCreateTest) {
Expand Down Expand Up @@ -404,6 +416,7 @@ TEST_F(FilterTestSuite, GetStringTest) {

// cleanup
celix_filter_destroy(filter);
ASSERT_EQ(nullptr, celix_filter_getFilterString(nullptr));
}

TEST_F(FilterTestSuite, FilterEqualsTest) {
Expand All @@ -424,8 +437,12 @@ TEST_F(FilterTestSuite, FilterEqualsTest) {
celix_autoptr(celix_filter_t) f7 = celix_filter_create("(test_attr1=*test*)");
celix_autoptr(celix_filter_t) f8 = celix_filter_create("(test_attr1=*test*)");
celix_autoptr(celix_filter_t) f9 = celix_filter_create("(test_attr1=*test)");
celix_autoptr(celix_filter_t) f10 = celix_filter_create("(test_attr1=*tst*)");
celix_autoptr(celix_filter_t) f11 = celix_filter_create("(test_attr2=*test*)");
EXPECT_TRUE(celix_filter_equals(f7, f8));
EXPECT_FALSE(celix_filter_equals(f7, f9));
EXPECT_FALSE(celix_filter_equals(f7, f10));
EXPECT_FALSE(celix_filter_equals(f7, f11));
}

TEST_F(FilterTestSuite, AutoCleanupTest) {
Expand Down Expand Up @@ -482,6 +499,10 @@ TEST_F(FilterTestSuite, TypedPropertiesAndFilterTest) {

celix_autoptr(celix_filter_t) filter5 = celix_filter_create("(strBool=true)");
EXPECT_TRUE(celix_filter_match(filter5, props));

celix_autoptr(celix_filter_t) filter6 =
celix_filter_create("(bool2<true)");
EXPECT_TRUE(celix_filter_match(filter6, props));
}


Expand Down
27 changes: 9 additions & 18 deletions libs/utils/src/filter.c
Original file line number Diff line number Diff line change
Expand Up @@ -298,10 +298,12 @@ static celix_filter_t* celix_filter_parseItem(const char* filterString, int* pos
}
break;
}
//LCOV_EXCL_START
default: {
celix_err_pushf("Filter Error: Invalid operand char `%c`", op);
return NULL;
}
//LCOV_EXCL_STOP
}
return celix_steal_ptr(filter);
}
Expand Down Expand Up @@ -609,11 +611,11 @@ static bool celix_utils_convertCompareToBool(enum celix_filter_operand_enum op,
return cmp < 0;
case CELIX_FILTER_OPERAND_LESSEQUAL:
return cmp <= 0;
//LCOV_EXCL_START
default:
//LCOV_EXCL_START
assert(false);
return false;
//LCOV_EXCL_STOP
//LCOV_EXCL_STOP
}
}

Expand Down Expand Up @@ -923,6 +925,10 @@ bool celix_filter_equals(const celix_filter_t* filter1, const celix_filter_t* fi
return false;
}

if (!celix_utils_stringEquals(filter1->attribute, filter2->attribute)) {
return false;
}

if (filter1->operand == CELIX_FILTER_OPERAND_SUBSTRING) {
assert(filter1->children != NULL);
assert(filter2->children != NULL);
Expand All @@ -942,22 +948,7 @@ bool celix_filter_equals(const celix_filter_t* filter1, const celix_filter_t* fi
return false;
}

// compare attr and value
bool attrSame = false;
bool valSame = false;
if (filter1->attribute == NULL && filter2->attribute == NULL) {
attrSame = true;
} else if (filter1->attribute != NULL && filter2->attribute != NULL) {
attrSame = celix_utils_stringEquals(filter1->attribute, filter2->attribute);
}

if (filter1->value == NULL && filter2->value == NULL) {
valSame = true;
} else if (filter1->value != NULL && filter2->value != NULL) {
valSame = celix_utils_stringEquals(filter1->value, filter2->value);
}

return attrSame && valSame;
return celix_utils_stringEquals(filter1->value, filter2->value);
}

const char* celix_filter_getFilterString(const celix_filter_t* filter) {
Expand Down

0 comments on commit eb2cf8a

Please sign in to comment.