Feature/674 improve properties #721

pnoltes · 2024-01-22T21:41:43Z

This PR add support for setting and getting string, bool, long, double and a version array list with properties.

Also:

Add support for only getting a property value if it is stored with a specific type (getLong, getDouble, etc)
Refactor filter to use the underlying property type when matching. This prevent the needed for memory allocation during filter match. The changed behavior should also more align with Properites and Filter matching in OSGi
Add logic so that a service registration will use the correct type for service.id, service.bundleid and service.version and if needed convert service.ranking and service.version properties (if set before a service registration call) to the correct types.
Add Properties / Filter intro documentation
Improve test coverage for Filter and Properties

Refactoring the Properties store and load to include properties value type information will be done in a follow up PR.

# Conflicts: # libs/utils/src/array_list.c

PengZheng · 2024-01-23T01:50:17Z

~~I noticed Codecov has stopped working for 3 days in a row.~~ Fixed.

(cherry picked from commit 9b4501d)

pnoltes · 2024-03-24T16:00:26Z

This PR has been updated so that celix array list types are used, which minimizes the celix properties functions needed for array list handling.
This PR can be reviewed again.

After this, I will dive into properties (JSON) serialization.

PengZheng

I have reviewed the API parts, and leave some further remarks.
To make this PR easier to follow, I revised all previous remarks to close resolved ones.

Note that there are still remarks that need fix/discussion.

bundles/remote_services/rsa_utils/include/celix_rsa_utils.h

bundles/remote_services/rsa_utils/CMakeLists.txt

bundles/remote_services/rsa_utils/include/celix_rsa_utils.h

libs/utils/include/celix_properties.h

PengZheng · 2024-03-25T07:06:17Z

libs/utils/src/properties.c

+    } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_LONG_ARRAY) {
+        entry->value = celix_utils_longArrayListToString(entry->typed.arrayValue);
+    } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_DOUBLE_ARRAY) {
+        entry->value = celix_utils_doubleArrayListToString(entry->typed.arrayValue);


WIll a single celix_arrayListToString be enough?

And we'll have a single StringToArrayList.

A single StringToArrayList implies that we should detect the types in the string. This is currently not implemented. I most worry about versions (1.2.3 is a valid version, but also 1.2).

This also triggered me. If we want a properties to (de)serializer to JSON. Do we also want to support / JSON mixed arrays. So something like: [true, 1, 2.3, "string", "1.2.3.qualified"] ?

Do we also want to support / JSON mixed arrays. So something like: [true, 1, 2.3, "string", "1.2.3.qualified"] ?

As a dynamically typed language, JS can support heterogeneous arrays.
As a statically typed language, Java supports heterogeneous arrays via "the mother of all" Object.
Here we may need a generic "entry".

I am still a a bit worried about JSON serialization and especially empty ([]) arrays, but I will see how we can tackle this a properties JSON serialization pull request.

I am still a a bit worried about JSON serialization and especially empty ([]) arrays.

Yeah, this is an annoying case.

libs/utils/include/celix_array_list.h

libs/utils/include/celix_properties.h

# Conflicts: # CHANGES.md

…ix_err.

…cation. When compiling a filter, it will try to parse arbitrary string as a version. The string length may exceed 64 bytes and thus will trigger unnecessary dynamic memory allocation.

…dle CELIX_ILLEGAL_ARGUMENT of addEntry callback.

…st case.

…ests for filter. Attributes were ignored when performing equality test between substring filters.

It also replaces celix_utils_stringEquals with cheaper celix_utils_isStringNullOrEmpty.

… ptr return

PengZheng · 2024-04-01T14:26:32Z

All issues have been addressed! 🚀
Please wait for a little last minute cleanup to be merged, which will happen tomorrow.

…nctions, add more tests.

PengZheng

LGTM

Long awaited typing support of celix_properties_t finally arrives, together with improved filter. Really exciting improvement.

PengZheng · 2024-04-01T13:19:49Z

libs/utils/src/array_list.c

@@ -28,6 +28,14 @@
 #include "celix_utils.h"
 #include "celix_version.h"

+#define STRING_VALUE_UNDEFINED_EL_TYPE "CELIX_ARRAY_LIST_ELEMENT_TYPE_UNDEFINED"


I think "Undefined"/"undefined" will have better readability than "CELIX_ARRAY_LIST_ELEMENT_TYPE_UNDEFINED" as console output.

PengZheng · 2024-04-01T13:35:38Z

libs/utils/src/properties.c

+    } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_LONG_ARRAY) {
+        entry->value = celix_utils_longArrayListToString(entry->typed.arrayValue);
+    } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_DOUBLE_ARRAY) {
+        entry->value = celix_utils_doubleArrayListToString(entry->typed.arrayValue);


Do we also want to support / JSON mixed arrays. So something like: [true, 1, 2.3, "string", "1.2.3.qualified"] ?

As a dynamically typed language, JS can support heterogeneous arrays.
As a statically typed language, Java supports heterogeneous arrays via "the mother of all" Object.
Here we may need a generic "entry".

PengZheng · 2024-04-02T13:31:21Z

libs/utils/src/properties.c

+    } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_LONG_ARRAY) {
+        entry->value = celix_utils_longArrayListToString(entry->typed.arrayValue);
+    } else if (entry->valueType == CELIX_PROPERTIES_VALUE_TYPE_DOUBLE_ARRAY) {
+        entry->value = celix_utils_doubleArrayListToString(entry->typed.arrayValue);


I am still a a bit worried about JSON serialization and especially empty ([]) arrays.

Yeah, this is an annoying case.

pnoltes added 27 commits January 2, 2024 18:01

Refactor properties get version non-owning func names

ebfbb7e

Add initial long array list support for properties

2acc482

Merge branch 'master' into feature/674-improve-properties

9c0a9c6

# Conflicts: # libs/utils/src/array_list.c

Add missing convert utils doxygen and fix unit tests

6843859

Fix convert utils ei tests

f41a0f6

Add get/setString support for array list

c4a601f

Add str, long, double, bool and version array support to properties

a8038ec

Add support for filter match with an array property set entry

968cc8a

Update C++ Properties for vector support

5963dbc

Improve C/C++ properties code documentation

9a51102

Add filter/properties type header with only a typedef

4a4e53d

Add rsa_utils static lib

301064f

Fix RSA import due to changed properties behaviour

e5b99c1

Add addition properties unit tests

45b1210

Add missing linking of rsa utils lib

03df164

Add automatic conversion of ranking/version props for ctx

a555df9

Fix memleak in celix_properties_setVersions

2173103

Fix parse error in celix_version_parse

abb6456

Add getString / getAsString to celix properties

ccd4b35

Fix memleak in properties

d55c411

Improve references handling in rsa_shm_impl.c

82246c1

Add properties and filter introduction documentation

7047b64

Merge branch 'master' into feature/674-improve-properties

e448d00

Add additional properties ei tests

dbfb90d

Fix memleak in properties setStrings and setVersions

e2373df

Update NOTICE (2019->2024)

234ccc0

Update CHANGES.md

9601947

Update codecov-action.

4841034

(cherry picked from commit 9b4501d)

PengZheng self-requested a review January 23, 2024 05:37

pnoltes requested a review from PengZheng March 24, 2024 16:00

PengZheng reviewed Mar 25, 2024

View reviewed changes

PengZheng and others added 19 commits March 25, 2024 16:09

Remove duplicated declarations.

f385447

Fix minor typo.

04432b3

Merge branch 'master' into feature/674-improve-properties

70d0ae0

# Conflicts: # CHANGES.md

gh-674: Combine typed array list to string utils functions

9133f17

Fix the documentation error and add a corresponding test case.

f57bb1b

gh-674: Add missing documentation about return value and usage of cel…

38f90c2

…ix_err.

gh-674: Fix documentation and add test_package for rsa_utils.

bc8d954

gh-674: Optimize celix_version_parse to avoid unnecessary memory allo…

f830c2a

…cation. When compiling a filter, it will try to parse arbitrary string as a version. The string length may exceed 64 bytes and thus will trigger unnecessary dynamic memory allocation.

gh-674: Fix broken hash and compare functions of version.

06f1493

gh-674: Fix broken version range test cases.

fc556fa

gh-674: Optimize memstream usage to reduce dynamic allocation and han…

be178de

…dle CELIX_ILLEGAL_ARGUMENT of addEntry callback.

gh-674: Add more tests for celix_convert_utils.

81ee259

gh-674: Update celix_rsa_utils documentation and add corresponding te…

e082a65

…st case.

gh-674: Add more tests for array_list.

8e96817

gh-674: Fix broken equality test for substring filters and add more t…

eb2cf8a

…ests for filter. Attributes were ignored when performing equality test between substring filters.

gh-674: Fix broken substring match.

4d94de2

It also replaces celix_utils_stringEquals with cheaper celix_utils_isStringNullOrEmpty.

gh-674: Refactor array list type for properties

be6ca79

gh-674: Remove defaultValue arg from props get array functions

3989ce1

gh-674: Remove defaultValue arg from properties get* functions with a…

10473c5

… ptr return

PengZheng self-requested a review April 1, 2024 14:20

PengZheng added 2 commits April 2, 2024 21:29

gh-674: Add missing array list support to several celix_properties fu…

3437c04

…nctions, add more tests.

gh-674: Add missing test for celix_properties_setEntry.

d38f0b1

PengZheng approved these changes Apr 2, 2024

View reviewed changes

gh-674: Rename array el type enum to string values

6a9713e

pnoltes merged commit cb38185 into master Apr 2, 2024
32 checks passed

pnoltes deleted the feature/674-improve-properties branch April 2, 2024 20:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/674 improve properties #721

Feature/674 improve properties #721

pnoltes commented Jan 22, 2024

PengZheng commented Jan 23, 2024 •

edited

Loading

pnoltes commented Mar 24, 2024

PengZheng left a comment •

edited

Loading

PengZheng Mar 25, 2024

PengZheng Mar 25, 2024

pnoltes Mar 25, 2024

PengZheng Apr 1, 2024

pnoltes Apr 1, 2024

PengZheng Apr 2, 2024

PengZheng commented Apr 1, 2024

PengZheng left a comment

PengZheng Apr 1, 2024

PengZheng Apr 1, 2024

PengZheng Apr 2, 2024

Feature/674 improve properties #721

Feature/674 improve properties #721

Conversation

pnoltes commented Jan 22, 2024

PengZheng commented Jan 23, 2024 • edited Loading

pnoltes commented Mar 24, 2024

PengZheng left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PengZheng commented Apr 1, 2024

PengZheng left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PengZheng commented Jan 23, 2024 •

edited

Loading

PengZheng left a comment •

edited

Loading