PengZheng commented on code in PR #692:
URL: https://github.com/apache/celix/pull/692#discussion_r1413567641
##########
libs/utils/include/celix_filter.h:
##########
@@ -44,63 +89,121 @@ typedef enum celix_filter_operand_enum
CELIX_FILTER_OPERAND_NOT,
} celix_filter_operand_t;
+/**
+ * @brief Internal opaque struct for internal use only.
+ */
+typedef struct celix_filter_internal celix_filter_internal_t; // opaque struct
for internal use only
-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 {
- celix_filter_operand_t operand;
- const char *attribute; //NULL for operands AND, OR ot NOT
- const char *value; //NULL for operands AND, OR or NOT NOT
- const char *filterStr;
+/**
+ * @brief The Apache Celix filter struct.
+ */
+typedef struct celix_filter_struct {
+ celix_filter_operand_t operand; /**< The filter operand. */
+ const char* attribute; /**< The filter attribute; NULL for
operands `AND`, `OR` or `NOT`. */
+ const char* value; /**< The filter value; NULL for operands
`AND`, `OR` or `NOT`. */
+ const char* filterStr; /**< The filter string representation. */
- //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_array_list_t* children; /**< The filter children; only valid if the
operand is not `AND`, `OR` or `NOT` else
Review Comment:
It seems that they can form something like typed union.
##########
documents/services.md:
##########
@@ -37,6 +37,45 @@ Also note that for Celix - in contrast with Java OSGi - it
is only possible to r
per service registration in the Apache Celix Framework. This restriction was
added because C does not
(natively) supports multiple interfaces (struct with function pointers) on a
single object/pointer.
+## Service Properties
+As mentioned, a service is always registered with a set of properties
(metadata).
+The service properties are represented as a `celix_properties_t` for C and as
a `celix::Properties` for C++.
+Service properties are used to classify and select services.
+
+Apache Celix properties store key/value pairs and the key is always a string,
but the value can be a string,
+a long, a double, a bool or `celix_version_t` (`celix::Version` for C++). Next
to the actual typed value a property
+entry always stores a string representation of the value.
+
+The following C functions can be used to create and manipulate properties:
+- `celix_properties_create` - Create a new properties object.
+- `celix_properties_destroy` - Destroy a properties object.
+- `celix_properties_set` - Set a string property value.
+- `celix_properties_setLong` - Set a long property value.
+- `celix_properties_setDouble` - Set a double property value.
+- `celix_properties_setBool` - Set a bool property value.
+- `celix_properties_setVersion` - Set a version property value.
+- `celix_properties_get` - Get a string or string representation of the
property value.
+- `celix_properties_getAsLong` - Get a long property value or string value
parsed as long.
+- `celix_properties_getAsDouble` - Get a double property value or string value
parsed as double.
+- `celix_properties_getAsBool` - Get a bool property value or string value
parsed as bool.
+- `celix_properties_getAsVersion` - Get a version property value or string
value parsed as version.
Review Comment:
`celix_properties_getAsVersion` will incur extra memory allocation. Instead,
we may advertise `celix_properties_getVersion` and make sure it is used in all
efficiency-sensitive scenarios.
##########
libs/utils/include/celix_filter.h:
##########
@@ -44,63 +89,121 @@ typedef enum celix_filter_operand_enum
CELIX_FILTER_OPERAND_NOT,
} celix_filter_operand_t;
+/**
+ * @brief Internal opaque struct for internal use only.
+ */
+typedef struct celix_filter_internal celix_filter_internal_t; // opaque struct
for internal use only
-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 {
- celix_filter_operand_t operand;
- const char *attribute; //NULL for operands AND, OR ot NOT
- const char *value; //NULL for operands AND, OR or NOT NOT
- const char *filterStr;
+/**
+ * @brief The Apache Celix filter struct.
+ */
+typedef struct celix_filter_struct {
+ celix_filter_operand_t operand; /**< The filter operand. */
+ const char* attribute; /**< The filter attribute; NULL for
operands `AND`, `OR` or `NOT`. */
+ const char* value; /**< The filter value; NULL for operands
`AND`, `OR` or `NOT`. */
+ const char* filterStr; /**< The filter string representation. */
- //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_array_list_t* children; /**< The filter children; only valid if the
operand is not `AND`, `OR` or `NOT` else
+ the value is NULL. */
- celix_filter_internal_t* internal; //for internal use only
-};
+ celix_filter_internal_t* internal; /**< Internal use only. */
+} celix_filter_t;
-CELIX_UTILS_EXPORT celix_filter_t* celix_filter_create(const char *filterStr);
+/**
+ * @brief Create a filter based on the provided filter string.
+ *
+ * If the return status is NULL, an error message is logged to celix_err.
+ *
+ * @param filterStr The filter string. If NULL or "" a match-all "(|)" filter
is created.
+ * @return The created filter or NULL if the filter string is invalid.
+ */
+CELIX_UTILS_EXPORT celix_filter_t* celix_filter_create(const char* filterStr);
-CELIX_UTILS_EXPORT void celix_filter_destroy(celix_filter_t *filter);
+/**
+ * @brief Destroy the provided filter. Ignores NULL values.
+ */
+CELIX_UTILS_EXPORT void celix_filter_destroy(celix_filter_t* filter);
+/**
+ * @brief Add support for `celix_autoptr`.
+ */
CELIX_DEFINE_AUTOPTR_CLEANUP_FUNC(celix_filter_t, celix_filter_destroy)
-CELIX_UTILS_EXPORT bool celix_filter_match(const celix_filter_t *filter, const
celix_properties_t* props);
+/**
+ * @brief Check whether the provided filter matches the provided properties.
+ * @param[in] filter The filter.
+ * @param[in] props The properties.
+ * @return True if the filter matches the properties, false otherwise. If
filter is NULL always returns true and
+ * if props is NULL, the result is the same as if an empty properties
set was provided.
+ */
+CELIX_UTILS_EXPORT bool celix_filter_match(const celix_filter_t* filter, const
celix_properties_t* props);
-CELIX_UTILS_EXPORT bool celix_filter_matchFilter(const celix_filter_t
*filter1, const celix_filter_t *filter2);
+/**
+ * @brief Check whether the 2 filters are equal.
+ *
+ * Note that the equals is done based on the parsed filter string and not on
the provided filter strings.
+ * So the filters parsed from the strings "(key=value)" and " (key=value)" are
equal.
+ *
+ * @param[in] filter1 The first filter.
+ * @param[in] filter2 The second filter.
+ * @return True if the filters are equal, false otherwise.
+ */
+CELIX_UTILS_EXPORT bool celix_filter_equals(const celix_filter_t* filter1,
const celix_filter_t* filter2);
-CELIX_UTILS_EXPORT const char* celix_filter_getFilterString(const
celix_filter_t *filter);
+/**
+ * @brief Get the filter string representation.
+ *
+ * @param[in] filter The filter.
+ * @return The filter string representation. The returned string is owned by
the filter, must not be freed and is only
+ * valid as long as the filter is valid.
+ */
+CELIX_UTILS_EXPORT const char* celix_filter_getFilterString(const
celix_filter_t* filter);
/**
- * Find the filter attribute.
- * @return The attribute value or NULL if the attribute is not found.
+ * @brief Find the filter attribute.
+ *
+ * @param[in] filter The filter.
+ * @param[in] attribute The attribute to find.
+ * @return The found attribute value or NULL if the attribute is not found.
The returned string is owned by the filter,
+ * must not be freed and is only valid as long as the filter is valid.
*/
-CELIX_UTILS_EXPORT const char* celix_filter_findAttribute(const celix_filter_t
*filter, const char *attribute);
+CELIX_UTILS_EXPORT const char* celix_filter_findAttribute(const
celix_filter_t* filter, const char* attribute);
/**
- * Check whether the filter indicates the mandatory presence of an attribute
with a specific value for the provided attribute key.
+ * @brief Check whether the filter indicates the mandatory presence of an
attribute with a specific value for the
+ * provided attribute key.
*
* Example:
* using this function for attribute key "key1" on filter "(key1=value1)"
yields true.
* using this function for attribute key "key1" on filter "(!(key1=value1))"
yields false.
* using this function for attribute key "key1" on filter "(key1>=value1)"
yields false.
* using this function for attribute key "key1" on filter
"(|(key1=value1)(key2=value2))" yields false.
-*/
-CELIX_UTILS_EXPORT bool celix_filter_hasMandatoryEqualsValueAttribute(const
celix_filter_t *filter, const char *attribute);
+ *
+ * @param[in] filter The filter.
+ * @param[in] attribute The attribute to check.
+ * @return True if the filter indicates the mandatory presence of an attribute
with a specific value for the provided
+ * attribute key, false otherwise.
+ */
+CELIX_UTILS_EXPORT bool celix_filter_hasMandatoryEqualsValueAttribute(const
celix_filter_t* filter,
+ const
char* attribute);
/**
- * Chek whether the filter indicates the mandatory absence of an attribute,
regardless of its value, for the provided attribute key.
+ * @brief Check whether the filter indicates the mandatory absence of an
attribute, regardless of its value, for the
+ * provided attribute key.
*
* example:
* using this function for attribute key "key1" on the filter "(!(key1=*))"
yields true.
* using this function for attribute key "key1" on the filter "(key1=*)
yields false.
* using this function for attribute key "key1" on the filter "(key1=value)"
yields false.
* using this function for attribute key "key1" on the filter
"(|(!(key1=*))(key2=value2))" yields false.
+ *
+ * @param[in] filter The filter.
+ * @param[in] attribute The attribute to check.
+ * @return True if the filter indicates the mandatory absence of an attribute,
regardless of its value, for the provided
+ * attribute key, false otherwise.
*/
-CELIX_UTILS_EXPORT bool
celix_filter_hasMandatoryNegatedPresenceAttribute(const celix_filter_t *filter,
const char *attribute);
+CELIX_UTILS_EXPORT bool
celix_filter_hasMandatoryNegatedPresenceAttribute(const celix_filter_t* filter,
Review Comment:
This also suffers from lack of well-defined semantics.
##########
libs/utils/include/celix_filter.h:
##########
@@ -17,20 +17,65 @@
* under the License.
*/
+/**
+ * @file celix_filter.h
+ * @brief Header file for the Celix Filter API.
+ *
+ * An Apache Celix filter is a based on LDAP filters, for more information
about LDAP filters see:
+ * - https://tools.ietf.org/html/rfc4515
+ * - https://tools.ietf.org/html/rfc1960
+ *
+ * In short an Apache Celix filter is a tree of filter nodes, parsed from a
filter string.
+ * Each node has an operand, an optional attribute string and an optional
value string.
+ * The operand can be one of the following:
+ * - `=`: equal
+ * - `=*`: present
+ * - `~=`: approx
+ * - `>=`: greater
+ * - `<=`: less
+ * - `>=`: greater equal
+ * - `<=`: less equal
+ * - `=`: substring if string contains a `*`.
+ * - `!`: not
+ * - `&`: and
+ * - `|`: or
+ *
+ The filter string is a list of filter node strings wrapped in parenthesis to
group filter nodes.
+ * The following are valid filter strings:
+ * - `(key=value)` # Matches if properties contains a entry with key "key" and
value "value".
+ * - `(key>=2)`# Matches if properties contains a entry with key "key" and
value greater or equal to 2.
+ * - `(key=*)` # Matches if properties contains a entry with key "key".
+ * - `(!(key=value))` # Matches if properties does not contain a entry with
key "key" and value "value".
+ * - `(&(key1=value1)(key2=value2))` # Matches if properties contains a entry
with key "key1" and value "value1" and
+ * # a entry with key "key2" and value
"value2".
+ * - `(|(key1=value1)(key2=value2))` # Matches if properties contains a entry
with key "key1" and value "value1" or
+ * # a entry with key "key2" and value
"value2".
+ *
+ * Apache Celix filters can be used to match a set of Apache Celix properties
and such Apache Celix filters should be
+ * used together with a set of properties.
+ *
+ * Internally attribute values will be parsed to a long, double, boolean and
Apache Celix version if
+ * possible during creation. And these types attribute values will be used in
the to-be-matched property value,
Review Comment:
Typo? "these typed attribute values" or "these types"?
##########
libs/utils/include/celix_filter.h:
##########
@@ -44,63 +89,121 @@ typedef enum celix_filter_operand_enum
CELIX_FILTER_OPERAND_NOT,
} celix_filter_operand_t;
+/**
+ * @brief Internal opaque struct for internal use only.
+ */
+typedef struct celix_filter_internal celix_filter_internal_t; // opaque struct
for internal use only
-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 {
- celix_filter_operand_t operand;
- const char *attribute; //NULL for operands AND, OR ot NOT
- const char *value; //NULL for operands AND, OR or NOT NOT
- const char *filterStr;
+/**
+ * @brief The Apache Celix filter struct.
+ */
+typedef struct celix_filter_struct {
+ celix_filter_operand_t operand; /**< The filter operand. */
+ const char* attribute; /**< The filter attribute; NULL for
operands `AND`, `OR` or `NOT`. */
+ const char* value; /**< The filter value; NULL for operands
`AND`, `OR` or `NOT`. */
+ const char* filterStr; /**< The filter string representation. */
- //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_array_list_t* children; /**< The filter children; only valid if the
operand is not `AND`, `OR` or `NOT` else
+ the value is NULL. */
- celix_filter_internal_t* internal; //for internal use only
-};
+ celix_filter_internal_t* internal; /**< Internal use only. */
+} celix_filter_t;
-CELIX_UTILS_EXPORT celix_filter_t* celix_filter_create(const char *filterStr);
+/**
+ * @brief Create a filter based on the provided filter string.
+ *
+ * If the return status is NULL, an error message is logged to celix_err.
+ *
+ * @param filterStr The filter string. If NULL or "" a match-all "(|)" filter
is created.
+ * @return The created filter or NULL if the filter string is invalid.
+ */
+CELIX_UTILS_EXPORT celix_filter_t* celix_filter_create(const char* filterStr);
-CELIX_UTILS_EXPORT void celix_filter_destroy(celix_filter_t *filter);
+/**
+ * @brief Destroy the provided filter. Ignores NULL values.
+ */
+CELIX_UTILS_EXPORT void celix_filter_destroy(celix_filter_t* filter);
+/**
+ * @brief Add support for `celix_autoptr`.
+ */
CELIX_DEFINE_AUTOPTR_CLEANUP_FUNC(celix_filter_t, celix_filter_destroy)
-CELIX_UTILS_EXPORT bool celix_filter_match(const celix_filter_t *filter, const
celix_properties_t* props);
+/**
+ * @brief Check whether the provided filter matches the provided properties.
+ * @param[in] filter The filter.
+ * @param[in] props The properties.
+ * @return True if the filter matches the properties, false otherwise. If
filter is NULL always returns true and
+ * if props is NULL, the result is the same as if an empty properties
set was provided.
+ */
+CELIX_UTILS_EXPORT bool celix_filter_match(const celix_filter_t* filter, const
celix_properties_t* props);
-CELIX_UTILS_EXPORT bool celix_filter_matchFilter(const celix_filter_t
*filter1, const celix_filter_t *filter2);
+/**
+ * @brief Check whether the 2 filters are equal.
+ *
+ * Note that the equals is done based on the parsed filter string and not on
the provided filter strings.
+ * So the filters parsed from the strings "(key=value)" and " (key=value)" are
equal.
+ *
+ * @param[in] filter1 The first filter.
+ * @param[in] filter2 The second filter.
+ * @return True if the filters are equal, false otherwise.
+ */
+CELIX_UTILS_EXPORT bool celix_filter_equals(const celix_filter_t* filter1,
const celix_filter_t* filter2);
-CELIX_UTILS_EXPORT const char* celix_filter_getFilterString(const
celix_filter_t *filter);
+/**
+ * @brief Get the filter string representation.
+ *
+ * @param[in] filter The filter.
+ * @return The filter string representation. The returned string is owned by
the filter, must not be freed and is only
+ * valid as long as the filter is valid.
+ */
+CELIX_UTILS_EXPORT const char* celix_filter_getFilterString(const
celix_filter_t* filter);
/**
- * Find the filter attribute.
- * @return The attribute value or NULL if the attribute is not found.
+ * @brief Find the filter attribute.
+ *
+ * @param[in] filter The filter.
+ * @param[in] attribute The attribute to find.
+ * @return The found attribute value or NULL if the attribute is not found.
The returned string is owned by the filter,
+ * must not be freed and is only valid as long as the filter is valid.
*/
-CELIX_UTILS_EXPORT const char* celix_filter_findAttribute(const celix_filter_t
*filter, const char *attribute);
+CELIX_UTILS_EXPORT const char* celix_filter_findAttribute(const
celix_filter_t* filter, const char* attribute);
/**
- * Check whether the filter indicates the mandatory presence of an attribute
with a specific value for the provided attribute key.
+ * @brief Check whether the filter indicates the mandatory presence of an
attribute with a specific value for the
+ * provided attribute key.
*
* Example:
* using this function for attribute key "key1" on filter "(key1=value1)"
yields true.
* using this function for attribute key "key1" on filter "(!(key1=value1))"
yields false.
* using this function for attribute key "key1" on filter "(key1>=value1)"
yields false.
* using this function for attribute key "key1" on filter
"(|(key1=value1)(key2=value2))" yields false.
-*/
-CELIX_UTILS_EXPORT bool celix_filter_hasMandatoryEqualsValueAttribute(const
celix_filter_t *filter, const char *attribute);
+ *
+ * @param[in] filter The filter.
+ * @param[in] attribute The attribute to check.
+ * @return True if the filter indicates the mandatory presence of an attribute
with a specific value for the provided
+ * attribute key, false otherwise.
+ */
+CELIX_UTILS_EXPORT bool celix_filter_hasMandatoryEqualsValueAttribute(const
celix_filter_t* filter,
Review Comment:
In `TEST_F(CxxFilterTestSuite, HasMandatoryEqualsValueAttribute)`, I found
the following confusing:
```C++
celix::Filter filter2{"(&(key1=value)(!(&(key=value)(!(key3=value)))))"};
EXPECT_TRUE(filter2.hasMandatoryEqualsValueAttribute("key1"));
EXPECT_FALSE(filter2.hasMandatoryEqualsValueAttribute("key2"));
EXPECT_TRUE(filter2.hasMandatoryEqualsValueAttribute("key3"));
celix::Properties props{};
props.set("key1", "value");
EXPECT_TRUE(filter2.match(props));
```
If "key3" is mandatory, how could `filter2` matches `props` without "key3".
The literal meaning of `hasMandatoryEqualsValueAttribute` seems to be that
`filter2` should not match any properties that don't have "key3" or have "key3"
with a value other than "value". If my understanding is correct, then this
feature seems highly non-trivial to implement. For example, some complicated
filters might always evaluate to be true/false. Without ability to do such
deduction, the semantics of this interface is not well-defined.
I'd like to consult the original author @jermus67 for more information.
--
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
