Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: uniform in-memory and sql store implementations #3330

Merged

Conversation

ndr-brt
Copy link
Member

@ndr-brt ndr-brt commented Jul 25, 2023

What this PR changes/adds

Uniform behavior between InMemory and Sql store implementations

Why it does that

consistency

Further notes

  • when querying by an invalid field, the stores will return empty list. It wasn't possible to throw exceptions for all the cases because on postgres a json query with an invalid field does not return any error. Returning an empty result was the more comprehensive way of handling that case.
  • add the like support on InMemory implementations
  • support the query collection nested objects on InMemory implementations
  • added support for boolean criterion in sql implementation
  • avoided implementation custom tests, moved all of them in the TestBase classes
  • used JUnit's @Nested annotation in the TestBase classes because they were becoming too hard to maintain.
  • refactored ReflectionUtil
  • better to merge after feat: add pending state to ContractNegotiation and TransferProcess #3321

Linked Issue(s)

Closes #3328
Closes #3059

Please be sure to take a look at the contributing guidelines and our etiquette for pull requests.

*/
public class CriterionToAssetPredicateConverterImpl extends CriterionToPredicateConverterImpl implements CriterionToAssetPredicateConverter, CriterionToPredicateConverter {

public Object property(String key, Object object) {

Check notice

Code scanning / CodeQL

Missing Override annotation Note

This method overrides
CriterionToPredicateConverterImpl.property
; it is advisable to add an Override annotation.
@ndr-brt ndr-brt added enhancement New feature or request refactoring Cleaning up code and dependencies labels Jul 25, 2023
@codecov-commenter
Copy link

codecov-commenter commented Jul 25, 2023

Codecov Report

Patch coverage: 82.87% and project coverage change: +0.14% 🎉

Comparison is base (de80081) 71.98% compared to head (d589eed) 72.12%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3330      +/-   ##
==========================================
+ Coverage   71.98%   72.12%   +0.14%     
==========================================
  Files         810      809       -1     
  Lines       16615    16602      -13     
  Branches      967      958       -9     
==========================================
+ Hits        11960    11974      +14     
+ Misses       4252     4234      -18     
+ Partials      403      394       -9     
Files Changed Coverage Δ
...se/edc/connector/catalog/CatalogCoreExtension.java 0.00% <0.00%> (ø)
...onnector/ControlPlaneDefaultServicesExtension.java 0.00% <0.00%> (ø)
...c/web/jersey/mapper/UnexpectedExceptionMapper.java 100.00% <ø> (ø)
.../eclipse/edc/sql/translation/JsonFieldMapping.java 0.00% <0.00%> (ø)
.../connector/store/sql/assetindex/SqlAssetIndex.java 85.29% <ø> (+1.49%) ⬆️
...main/java/org/eclipse/edc/spi/query/Criterion.java 25.00% <0.00%> (-0.81%) ⬇️
...clipse/edc/sql/translation/TranslationMapping.java 86.66% <66.66%> (-0.84%) ⬇️
...defaults/storage/ReflectionBasedQueryResolver.java 84.00% <69.23%> (-16.00%) ⬇️
.../asset/CriterionToAssetPredicateConverterImpl.java 75.00% <75.00%> (ø)
...rg/eclipse/edc/util/reflection/ReflectionUtil.java 80.70% <78.26%> (+0.91%) ⬆️
... and 11 more

... and 5 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ndr-brt ndr-brt force-pushed the 3328-uniform-inmemory-sql-impl branch from 0b9f975 to d589eed Compare July 27, 2023 09:55
@ndr-brt ndr-brt merged commit edcdd4a into eclipse-edc:main Jul 31, 2023
17 checks passed
@ndr-brt ndr-brt deleted the 3328-uniform-inmemory-sql-impl branch July 31, 2023 06:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request refactoring Cleaning up code and dependencies
Projects
None yet
3 participants