fix: update data model for infinispan

[achmelo]

Signed-off-by: achmelo [email protected]

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Linked to #1982

Type of change

Please delete options that are not relevant.

• (fix) Bug fix (non-breaking change which fixes an issue)
• (feat) New feature (non-breaking change which adds functionality)
• (docs) Change in a documentation
• (refactor) Refactor the code
• (chore) Chore, repository cleanup, updates the dependencies.
• (BREAKING CHANGE or !) Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules

For more details about how should the code look like read the Contributing guideline

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

94.3% Coverage
0.0% Duplication

[CarsonCook]

What is the reason for using serviceId + toCreate.getKey() as the key, rather than a nested map structure like the in memory implementation? This seems to add complexity to the code in the read/delete all for service endpoints, and creates unintuitive coupling between the CRUD operations to calculate the correct key.

I think a comment explaining this might be useful in this file.

[achmelo]

Nested map structure was used in the original implementation of Infinispan storage and it turned out to be slow and error-prone in HA setup. Cache object can be mutated by another instance of Caching service when it is not directly used, e.g.

Map<String, KeyValue> serviceCache = cache.computeIfAbsent(serviceId, k -> new HashMap<>());
KeyValue entry = serviceCache.put(toCreate.getKey(), toCreate);
        cache.put(serviceId, serviceCache);

after conputeIfAbsent is returned, the cache lock is released and can be acquired by another instance. When cache.put() is called, the object could not be the same. It could be also an issue for services with too many entries. Each call for serviceId will return a large object that will not be necessarily useful.
There is a trade-off for read/delete all methods but I think that those will not be used extensively. If this will not be the case, we can adjust them.