Skip to content

Commit

Permalink
chore: simplify TrackedEntityService.getTrackedEntity(UID, ...) logic (…
Browse files Browse the repository at this point in the history 
…#19633)

We have two methods for getting a single TE. They were implemented
independently with convoluted logic. Let one reuse the other. Separate
fetching, ACL, mapping to make the flow easier to follow. Move the TETAV
logic in one place. This is the only way we can ensure the behavior is
the same across different code paths.
  • Loading branch information
@teleivo
teleivo authored Jan 13, 2025
1 parent ca8a28b commit 132a631
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 115 deletions.
133 changes: 42 additions & 91 deletions ...src/main/java/org/hisp/dhis/tracker/export/trackedentity/DefaultTrackedEntityService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
import static org.hisp.dhis.user.CurrentUserUtil.getCurrentUsername;

import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -211,129 +210,71 @@ private static TrackedEntityAttribute getAttribute(
() -> new NotFoundException(TrackedEntityAttribute.class, attribute.getValue()));
}

@Nonnull
@Override
public TrackedEntity getTrackedEntity(@Nonnull UID uid)
throws NotFoundException, ForbiddenException {
UserDetails currentUser = getCurrentUserDetails();
TrackedEntity trackedEntity =
mapTrackedEntity(
getTrackedEntity(uid, currentUser),
TrackedEntityParams.FALSE,
currentUser,
null,
false);
mapTrackedEntityTypeAttributes(trackedEntity);
return trackedEntity;
return getTrackedEntity(uid, null, TrackedEntityParams.FALSE);
}

@Nonnull
@Override
public TrackedEntity getTrackedEntity(
@Nonnull UID trackedEntityUid,
@CheckForNull UID programIdentifier,
@Nonnull TrackedEntityParams params)
throws NotFoundException, ForbiddenException {
Program program = null;

if (programIdentifier != null) {
program = programService.getProgram(programIdentifier.getValue());
if (program == null) {
throw new NotFoundException(Program.class, programIdentifier);
}
}

TrackedEntity trackedEntity;
if (program != null) {
trackedEntity = getTrackedEntity(trackedEntityUid.getValue(), program, params);

if (params.isIncludeProgramOwners()) {
Set<TrackedEntityProgramOwner> filteredProgramOwners =
trackedEntity.getProgramOwners().stream()
.filter(te -> te.getProgram().getUid().equals(programIdentifier.getValue()))
.collect(Collectors.toSet());
trackedEntity.setProgramOwners(filteredProgramOwners);
}
} else {
UserDetails userDetails = getCurrentUserDetails();

trackedEntity =
mapTrackedEntity(
getTrackedEntity(trackedEntityUid, userDetails), params, userDetails, null, false);

mapTrackedEntityTypeAttributes(trackedEntity);
}
UserDetails userDetails = getCurrentUserDetails();
TrackedEntity trackedEntity = getTrackedEntity(trackedEntityUid, userDetails, program);
trackedEntity = mapTrackedEntity(trackedEntity, params, userDetails, program, false);
return trackedEntity;
}

/**
* Gets a tracked entity based on the program and org unit ownership
* Gets a tracked entity based on the program and org unit ownership.
*
* @return the TE object if found and accessible by the current user
* @throws NotFoundException if uid does not exist
* @throws ForbiddenException if TE owner is not in user's scope or not enough sharing access
*/
private TrackedEntity getTrackedEntity(String uid, Program program, TrackedEntityParams params)
throws NotFoundException, ForbiddenException {
TrackedEntity trackedEntity = trackedEntityStore.getByUid(uid);
trackedEntityAuditService.addTrackedEntityAudit(trackedEntity, getCurrentUsername(), READ);
if (trackedEntity == null) {
throw new NotFoundException(TrackedEntity.class, uid);
}

UserDetails userDetails = getCurrentUserDetails();
List<String> errors =
trackerAccessManager.canReadProgramAndTrackedEntityType(
userDetails, trackedEntity, program);
if (!errors.isEmpty()) {
throw new ForbiddenException(errors.toString());
}

String error =
trackerAccessManager.canAccessProgramOwner(userDetails, trackedEntity, program, false);
if (error != null) {
throw new ForbiddenException(error);
}

return mapTrackedEntity(trackedEntity, params, userDetails, program, false);
}

/**
* Gets the requested tracked entity if the user owns at least one TE/program pair, or has access
* to the TE registering org unit, in case it doesn't own any.
*
* @return the TE object if found and accessible by the user
* @throws NotFoundException if TE does not exist
* @throws ForbiddenException if TE is not accessible
*/
private TrackedEntity getTrackedEntity(UID uid, UserDetails userDetails)
private TrackedEntity getTrackedEntity(UID uid, UserDetails userDetails, Program program)
throws NotFoundException, ForbiddenException {
TrackedEntity trackedEntity = trackedEntityStore.getByUid(uid.getValue());
trackedEntityAuditService.addTrackedEntityAudit(trackedEntity, getCurrentUsername(), READ);
if (trackedEntity == null) {
throw new NotFoundException(TrackedEntity.class, uid);
}

if (!trackerAccessManager.canRead(userDetails, trackedEntity).isEmpty()) {
throw new ForbiddenException(TrackedEntity.class, uid);
if (program != null) {
List<String> errors =
trackerAccessManager.canReadProgramAndTrackedEntityType(
userDetails, trackedEntity, program);
if (!errors.isEmpty()) {
throw new ForbiddenException(errors.toString());
}

String error =
trackerAccessManager.canAccessProgramOwner(userDetails, trackedEntity, program, false);
if (error != null) {
throw new ForbiddenException(error);
}
} else {
if (!trackerAccessManager.canRead(userDetails, trackedEntity).isEmpty()) {
throw new ForbiddenException(TrackedEntity.class, uid);
}
}

return trackedEntity;
}

private void mapTrackedEntityTypeAttributes(TrackedEntity trackedEntity) {
TrackedEntityType trackedEntityType = trackedEntity.getTrackedEntityType();
if (trackedEntityType != null) {
Set<String> tetAttributes =
trackedEntityType.getTrackedEntityAttributes().stream()
.map(TrackedEntityAttribute::getUid)
.collect(Collectors.toSet());
Set<TrackedEntityAttributeValue> tetAttributeValues =
trackedEntity.getTrackedEntityAttributeValues().stream()
.filter(att -> tetAttributes.contains(att.getAttribute().getUid()))
.collect(Collectors.toCollection(LinkedHashSet::new));
trackedEntity.setTrackedEntityAttributeValues(tetAttributeValues);
}
}

private TrackedEntity mapTrackedEntity(
TrackedEntity trackedEntity,
TrackedEntityParams params,
Expand Down Expand Up @@ -364,7 +305,7 @@ private TrackedEntity mapTrackedEntity(
result.setEnrollments(getEnrollments(trackedEntity, user, includeDeleted, program));
}
if (params.isIncludeProgramOwners()) {
result.setProgramOwners(trackedEntity.getProgramOwners());
result.setProgramOwners(getTrackedEntityProgramOwners(trackedEntity, program));
}

result.setTrackedEntityAttributeValues(getTrackedEntityAttributeValues(trackedEntity, program));
Expand Down Expand Up @@ -408,22 +349,31 @@ private Set<Enrollment> getEnrollments(
.collect(Collectors.toSet());
}

private static Set<TrackedEntityProgramOwner> getTrackedEntityProgramOwners(
TrackedEntity trackedEntity, Program program) {
if (program == null) {
return trackedEntity.getProgramOwners();
}

return trackedEntity.getProgramOwners().stream()
.filter(te -> te.getProgram().getUid().equals(program.getUid()))
.collect(Collectors.toSet());
}

private Set<TrackedEntityAttributeValue> getTrackedEntityAttributeValues(
TrackedEntity trackedEntity, Program program) {
Set<String> readableAttributes =
Set<String> teas = // tracked entity type attributes
trackedEntity.getTrackedEntityType().getTrackedEntityAttributes().stream()
.map(IdentifiableObject::getUid)
.collect(Collectors.toSet());

if (program != null) {
readableAttributes.addAll(
if (program != null) { // add program tracked entity attributes
teas.addAll(
program.getTrackedEntityAttributes().stream()
.map(IdentifiableObject::getUid)
.collect(Collectors.toSet()));
}

return trackedEntity.getTrackedEntityAttributeValues().stream()
.filter(av -> readableAttributes.contains(av.getAttribute().getUid()))
.filter(av -> teas.contains(av.getAttribute().getUid()))
.collect(Collectors.toSet());
}

Expand Down Expand Up @@ -488,6 +438,7 @@ private RelationshipItem getTrackedEntityInRelationshipItem(
return relationshipItem;
}

@Nonnull
@Override
public List<TrackedEntity> getTrackedEntities(
@Nonnull TrackedEntityOperationParams operationParams)
Expand Down
12 changes: 7 additions & 5 deletions ...racker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,16 +57,18 @@ FileResourceStream getFileResourceImage(
* relationships are not included. Use {@link #getTrackedEntity(UID, UID, TrackedEntityParams)}
* instead to also get the relationships, enrollments and program attributes.
*/
TrackedEntity getTrackedEntity(UID uid)
@Nonnull
TrackedEntity getTrackedEntity(@Nonnull UID uid)
throws NotFoundException, ForbiddenException, BadRequestException;

/**
* Get the tracked entity matching given {@code UID} under the privileges of the currently
* authenticated user. If @param programIdentifier is defined, program attributes for such program
* are included, otherwise only TETAs are included. It will include enrollments, relationships,
* attributes and ownerships as defined in @param params
* authenticated user. If {@code program} is defined, program attributes for such program are
* included, otherwise only TETAs are included. It will include enrollments, relationships,
* attributes and ownerships as defined in {@code params}.
*/
TrackedEntity getTrackedEntity(UID uid, UID programIdentifier, TrackedEntityParams params)
@Nonnull
TrackedEntity getTrackedEntity(@Nonnull UID uid, UID program, @Nonnull TrackedEntityParams params)
throws NotFoundException, ForbiddenException, BadRequestException;

/** Get all tracked entities matching given params. */
Expand Down
18 changes: 18 additions & 0 deletions dhis-2/dhis-support/dhis-support-test/src/main/java/org/hisp/dhis/test/utils/Assertions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,24 @@ public static <E> void assertContainsOnly(Collection<E> expected, Collection<E>
assertContainsOnly(expected, actual, "assertContainsOnly found mismatch");
}

/**
* Asserts that the given collection contains exactly the given items in any order. Collections
* will be mapped by {@code map} before passing it to {@link #assertContainsOnly(Collection,
* Collection)}.
*
* @param expected the expected items.
* @param actual the actual collection.
* @param map map the items of expected and actual collections to the type that will be used for
* comparison
*/
public static <T, R> void assertContainsOnly(
Collection<T> expected, Collection<T> actual, Function<T, R> map) {
assertContainsOnly(
expected.stream().map(map).toList(),
actual.stream().map(map).toList(),
"assertContainsOnly found mismatch");
}

/**
* Asserts that the given collection contains exactly the given items in any order.
*
Expand Down
33 changes: 14 additions & 19 deletions ...on/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,11 +143,11 @@ class TrackedEntityServiceTest extends PostgresIntegrationTestBase {

private TrackedEntityAttribute teaC;

private TrackedEntityAttributeValue trackedEntityAttributeValueA;
private TrackedEntityAttributeValue tetavA;

private TrackedEntityAttributeValue trackedEntityAttributeValueB;
private TrackedEntityAttributeValue tetavB;

private TrackedEntityAttributeValue trackedEntityAttributeValueC;
private TrackedEntityAttributeValue pteavC;

private TrackedEntityType trackedEntityTypeA;

Expand Down Expand Up @@ -324,17 +324,13 @@ void setUp() {
new ProgramTrackedEntityAttribute(programB, teaE)));
manager.update(programB);

trackedEntityAttributeValueA = new TrackedEntityAttributeValue(teaA, trackedEntityA, "A");
trackedEntityAttributeValueB = new TrackedEntityAttributeValue(teaB, trackedEntityA, "B");
trackedEntityAttributeValueC = new TrackedEntityAttributeValue(teaC, trackedEntityA, "C");
tetavA = new TrackedEntityAttributeValue(teaA, trackedEntityA, "A");
tetavB = new TrackedEntityAttributeValue(teaB, trackedEntityA, "B");
pteavC = new TrackedEntityAttributeValue(teaC, trackedEntityA, "C");

trackedEntityA = createTrackedEntity(orgUnitA);
trackedEntityA.setTrackedEntityType(trackedEntityTypeA);
trackedEntityA.setTrackedEntityAttributeValues(
Set.of(
trackedEntityAttributeValueA,
trackedEntityAttributeValueB,
trackedEntityAttributeValueC));
trackedEntityA.setTrackedEntityAttributeValues(Set.of(tetavA, tetavB, pteavC));
manager.save(trackedEntityA, false);

trackedEntityChildA = createTrackedEntity(orgUnitChildA);
Expand Down Expand Up @@ -2015,23 +2011,22 @@ void shouldReturnProgramAttributesWhenSingleTERequestedAndProgramSpecified()
UID.of(trackedEntityA), UID.of(programA), TrackedEntityParams.TRUE);

assertContainsOnly(
Set.of(
trackedEntityAttributeValueA,
trackedEntityAttributeValueB,
trackedEntityAttributeValueC),
trackedEntity.getTrackedEntityAttributeValues());
Set.of(tetavA, tetavB, pteavC),
trackedEntity.getTrackedEntityAttributeValues(),
TrackedEntityAttributeValue::getValue);
}

@Test
void shouldReturnTrackedEntityTypeAttributesWhenSingleTERequestedAndNoProgramSpecified()
void shouldReturnTrackedEntityTypeAttributesOnlyWhenSingleTERequestedAndNoProgramSpecified()
throws ForbiddenException, NotFoundException, BadRequestException {
TrackedEntity trackedEntity =
trackedEntityService.getTrackedEntity(
UID.of(trackedEntityA), null, TrackedEntityParams.TRUE);

assertContainsOnly(
Set.of(trackedEntityAttributeValueA, trackedEntityAttributeValueB),
trackedEntity.getTrackedEntityAttributeValues());
Set.of(tetavA, tetavB),
trackedEntity.getTrackedEntityAttributeValues(),
TrackedEntityAttributeValue::getValue);
}

@Test
Expand Down

0 comments on commit 132a631

Please sign in to comment.