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

chore: use EnrollmentService in TE service to get enrollements/events DHIS2-18541 #19723

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

teleivo
Copy link
Contributor

@teleivo teleivo commented Jan 20, 2025

  • use EnrollmentService in TE aggregate to fetch enrollments and events as well as to serve /trackedEntities/uid.
  • temporarily fetch relationship/items eagerly using mappers and do ACL on relationships in JdbcEventStore. This is needed anyway but here its used to eagerly fetch the entities we need later on when there is no hibernate session available. See Architecture challenges for more.
  • temporarily return primary key id and uid as TrackedEntityIdentifiers pair from the TE store. This is needed for now so we can pass the TE uids to the enrollment service in the enrollment aggregate. All other aggregate code uses the primary key id. We might be able to untangle the TE aggregate later on.

Next

  • use exactly the same code to serve single vs multiple TEs
  • change our code structure when it comes to relationships as described below (maybe in a separate ticket)

Architecture challenges

Our goal was that every service of an entity depends on its childs service to fetch such nested entities. This assumes an acyclic graph TE -> EN -> EV. This neglects the fact that relationships can add cycles. This is an example graph where relationships add edges causing cycles

entities dot

We cannot have cycles in our code as Spring would not know how to resolve them unless we resort to some hacks.

This was already true. Up to now it was solved by hibernate fetching relationships. Even in services/stores that use JDBC like hibernate was ultimately responsible for getting relationships

We are going to stick with hibernate being responsible for fetching relationships. The target architecture will be

architecture dot

This will mean that there is one place that is also responsible for ACL of relationships (trackerAccessManager.canRead(user, relationship). Note that this underneath checks the ACL of TE, EN, EV.

@teleivo teleivo force-pushed the DHIS2-18541-uid-enrollmentservice branch 8 times, most recently from eaf76a3 to c69131d Compare January 21, 2025 09:32
@teleivo teleivo marked this pull request as ready for review January 21, 2025 09:32
@teleivo teleivo requested a review from a team as a code owner January 21, 2025 09:32
to get enrollments on /trackedEntities/{uid}?fields=enrollment

chore: wire ids and uids through

chore: use enrollment service in TE aggregate
as these are fetched by the EN service
@teleivo teleivo force-pushed the DHIS2-18541-uid-enrollmentservice branch from a300de2 to 6aa9c8a Compare January 22, 2025 11:15
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
8 New issues
8 New Code Smells (required ≤ 0)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@Mapping(target = "createdByUserInfo")
@Mapping(target = "lastUpdatedByUserInfo")
@Mapping(target = "status")
Enrollment map(Enrollment enrollment);

Check notice

Code scanning / CodeQL

Missing Override annotation Note

This method overrides
PreheatMapper.map
; it is advisable to add an Override annotation.
@Mapping(target = "createdByUserInfo")
@Mapping(target = "lastUpdatedByUserInfo")
@Mapping(target = "geometry")
Event map(Event event);

Check notice

Code scanning / CodeQL

Missing Override annotation Note

This method overrides
PreheatMapper.map
; it is advisable to add an Override annotation.
@Mapping(target = "deleted")
@Mapping(target = "createdByUserInfo")
@Mapping(target = "lastUpdatedByUserInfo")
TrackedEntity map(TrackedEntity trackedEntity);

Check notice

Code scanning / CodeQL

Missing Override annotation Note

This method overrides
PreheatMapper.map
; it is advisable to add an Override annotation.
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