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: remove junctions from query engine API [DHIS2-18041] #20008

Merged
merged 11 commits into from
Feb 25, 2025

Conversation

jbee
Copy link
Contributor

@jbee jbee commented Feb 20, 2025

Summary

Fixes special filters that would require junctions by moving their composition into the query engine.

While this refactoring removes nested junctions from the query engine API this does not remove a web API feature. Nested junctions were only used in a few special programmatic cases to build more sophisticated filters. Within the Query all filters are now just one Restriction. Sophisticated ones are expanded first within the engine making it unnecessary to model junctions on the Query or Restriction level.

To distinguish Restrictions that are simple from those that need expanding the isVirtual() method got added encoding the few conditions where this is needed.

Simplifications

  • removed Criteria, Criterion, Disjunction and Conjunction as all filters are now simple Restrictions
  • aliases are no longer in Query but extracted from the Restriction in the JPA query engine when needed
  • Typed got replaced by simply using List.of
  • Operator.collectionArgs was replaced with using Operator.args instead (there never was a need for another field as only in/not-in operator would use it and they always would only have one list of args that could use args as all other operators)
  • QueryPlanner.planQuery with persistedOnly got removed (this was never necessary as Query instances passed to a QueryEngine should always be already planned as they should come from within the QueryService and never be passed to the engine directly)
  • handle user Locale dependency for token filter within the TokenOperator class

Corrections

  • DefaultJpaQueryParser got renamed to DefaultQueryParser (wasn't specific to JPA in any way)
  • QueryEngine generic got removed (it was wrongly made generic when it was not)
  • the generic on Operator got changed from <T extends Comparable<? super T>> to <T extends Comparable<T>> (was too unspecific which forced some casts that now can be avoided)

Cleanup

  • QueryPath got moved into the schema module and is not computed by the SchemaService (as it really as an extension on Schema and Property that is only used for Query)
  • QueryService.query with ResultTransformer got removed (was unused)
  • Operator.getHibernateCriterion got removed (was unused)

@jbee jbee self-assigned this Feb 20, 2025
@jbee jbee changed the title refactor: remove query engine nested junction support [DHIS2-18041] refactor: remove query engine nested junctions [DHIS2-18041] Feb 20, 2025
@jbee jbee changed the title refactor: remove query engine nested junctions [DHIS2-18041] refactor: remove junctions query engine API [DHIS2-18041] Feb 20, 2025
@jbee jbee changed the title refactor: remove junctions query engine API [DHIS2-18041] refactor: remove junctions from query engine API [DHIS2-18041] Feb 20, 2025
@ToString
public class Query {

private final List<Restriction> filters = new ArrayList<>();

Check notice

Code scanning / CodeQL

Exposing internal representation Note

getFilters exposes the internal representation stored in field filters. The value may be modified
after this call to getFilters
.
@jbee jbee marked this pull request as ready for review February 25, 2025 10:39
Copy link
Contributor

@david-mackessy david-mackessy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lovely catch spotting this 👍

@jbee jbee merged commit 93ac4d3 into master Feb 25, 2025
17 checks passed
@jbee jbee deleted the DHIS2-18041-query-engine branch February 25, 2025 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants