Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -260,21 +260,22 @@ private <T extends TrackerDto> boolean hasInvalidParents(

private <T extends TrackerDto> Predicate<T> parentConditions(
List<Function<T, TrackerDto>> parents) {
final Predicate<TrackerDto> parentCondition =
parent -> isMarked(parent) || this.preheat.exists(parent);

return parents.stream()
.map(
p ->
(Predicate<T>)
t ->
parentCondition.test(
isPersistableParent(
p.apply(t))) // children of invalid parents can only be persisted under
// certain conditions
.reduce(Predicate::and)
.orElse(t -> true); // predicate always returning true for entities without parents
}

private boolean isPersistableParent(TrackerDto parent) {
return isMarked(parent) || this.preheat.exists(parent);
}

/**
* Add error for valid child entity with invalid parent as a reason. If a child is invalid that is
* enough information for a user to know why it could not be persisted.
Expand All @@ -288,7 +289,7 @@ private <T extends TrackerDto> void addErrorsForChildren(
List<Error> errors =
parents.stream()
.map(p -> p.apply(entity))
.filter(this::isNotValid) // remove valid parents
.filter(p -> !isPersistableParent(p))
.map(p -> error(ValidationCode.E5000, entity, p))
.toList();
this.result.errors.addAll(errors);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,32 @@ void testCreateAndUpdateValidEventOfInvalidEnrollmentCannotBeCreatedIfEnrollment
persistable, EVENT, "Qck4PQ7TMun", E5000, "because enrollment `t1zaUjKgT3p`"));
}

@Test
void testCreateAndUpdateValidEventOfValidEnrollmentWithInvalidTrackedEntityCannotBeCreated() {
Setup setup =
new Setup.Builder()
.trackedEntity("xK7H53f4Hc2")
.isNotValid()
.enrollment("t1zaUjKgT3p")
.event("Qck4PQ7TMun")
.build();

PersistablesFilter.Result persistable =
filter(setup.bundle, setup.invalidEntities, TrackerImportStrategy.CREATE_AND_UPDATE);

assertAll(
() -> assertIsEmpty(persistable.get(TrackedEntity.class)),
() -> assertIsEmpty(persistable.get(Enrollment.class)),
() -> assertIsEmpty(persistable.get(TrackerEvent.class)),
() -> assertIsEmpty(persistable.get(SingleEvent.class)),
() ->
assertHasError(
persistable, ENROLLMENT, "t1zaUjKgT3p", E5000, "trackedEntity `xK7H53f4Hc2`"),
() ->
assertHasError(
persistable, EVENT, "Qck4PQ7TMun", E5000, "because enrollment `t1zaUjKgT3p`"));
}

@Test
void testCreateAndUpdateInvalidEventOfValidEnrollmentCannotBePersisted() {
Setup setup =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@
import static org.hisp.dhis.tracker.imports.TrackerImportStrategy.DELETE;
import static org.hisp.dhis.webapi.controller.tracker.export.MappingErrors.ensureNoMappingErrors;
import static org.hisp.dhis.webapi.controller.tracker.sync.TrackerSyncReportUtils.alreadyDeletedOrSucceededUids;
import static org.hisp.dhis.webapi.controller.tracker.sync.TrackerSyncReportUtils.blockingFailedUids;
import static org.hisp.dhis.webapi.controller.tracker.sync.TrackerSyncReportUtils.blockingFailedItems;
import static org.hisp.dhis.webapi.controller.tracker.sync.TrackerSyncReportUtils.failedItems;
import static org.hisp.dhis.webapi.controller.tracker.sync.TrackerSyncReportUtils.failedUids;
import static org.hisp.dhis.webapi.controller.tracker.sync.TrackerSyncReportUtils.formatFailedUids;
import static org.hisp.dhis.webapi.controller.tracker.sync.TrackerSyncReportUtils.sendTrackerRequest;
Expand Down Expand Up @@ -72,6 +73,7 @@
import org.hisp.dhis.tracker.TrackerType;
import org.hisp.dhis.tracker.imports.report.ImportReport;
import org.hisp.dhis.webapi.controller.tracker.export.MappingErrors;
import org.hisp.dhis.webapi.controller.tracker.sync.TrackerSyncReportUtils.FailedItem;
import org.hisp.dhis.webapi.controller.tracker.view.Relationship;
import org.springframework.web.client.RestTemplate;

Expand Down Expand Up @@ -101,6 +103,12 @@
abstract class BaseDataSynchronizationWithPaging<V, D extends SoftDeletableEntity>
implements DataSynchronizationWithPaging {

/**
* Synthetic error code used in place of a real one when an entity is excluded from sync only
* because one of its children failed, not because of any error of its own.
*/
static final String CHILD_FAILED_ERROR_CODE = "CHILD_FAILED";

private final RenderService renderService;
private final RestTemplate restTemplate;
private final SystemSettingsService systemSettingsService;
Expand Down Expand Up @@ -379,20 +387,20 @@ private DeleteSyncResult syncDeleted(
// An entity whose own delete came back "already deleted" achieved its goal, so it is treated
// as synced here too.
Set<UID> syncedTopLevelUids = alreadyDeletedOrSucceededUids(report, getTrackerType());
Set<UID> failedTopLevelUids = blockingFailedUids(report, getTrackerType());
List<FailedItem> failedTopLevelItems = blockingFailedItems(report, getTrackerType());

Set<UID> blockingFailedChildUids = new HashSet<>();
StringBuilder childSummary = new StringBuilder();
for (Map.Entry<TrackerType, Integer> entry : nested.deletedCountByType().entrySet()) {
TrackerType childType = entry.getKey();
int total = entry.getValue();
Set<UID> syncedChild = alreadyDeletedOrSucceededUids(report, childType);
Set<UID> failedChild = blockingFailedUids(report, childType);
blockingFailedChildUids.addAll(failedChild);
List<FailedItem> failedChildItems = blockingFailedItems(report, childType);
failedChildItems.forEach(item -> blockingFailedChildUids.add(item.uid()));
childSummary.append(
format(
", %s=%d/%d synced%s",
childType, syncedChild.size(), total, formatFailedUids(failedChild)));
childType, syncedChild.size(), total, formatFailedUids(failedChildItems)));
}

log.info(
Expand All @@ -401,7 +409,7 @@ private DeleteSyncResult syncDeleted(
getTrackerType(),
syncedTopLevelUids.size(),
deletedTopLevelDtos.size(),
formatFailedUids(failedTopLevelUids),
formatFailedUids(failedTopLevelItems),
childSummary);

return new DeleteSyncResult(syncedTopLevelUids, blockingFailedChildUids);
Expand All @@ -426,22 +434,38 @@ private Set<UID> syncActive(List<V> entities, SystemInstance instance, SystemSet
Set<UID> succeededTopLevelUids = successfullyProcessedUids(report, getTrackerType());
Set<UID> syncedUids = filterByFailedChildren(succeededTopLevelUids, entities, report);

Set<UID> failedTopLevelUids =
entities.stream()
.map(this::getUid)
.filter(uid -> !syncedUids.contains(uid))
.collect(Collectors.toCollection(HashSet::new));

log.info(
"{} create/update sync: {}/{} synced{}",
getEntityName(),
syncedUids.size(),
entities.size(),
formatFailedUids(failedTopLevelUids));
formatFailedUids(explainUnsyncedEntities(succeededTopLevelUids, syncedUids, report)));

return syncedUids;
}

/**
* Adds the default error code {@link #CHILD_FAILED_ERROR_CODE} to all top entities that are
* successfully processed but are not synchronized and don't have an error code yet.
*
* @return the list of (entity, errorCode) pairs
*/
List<FailedItem> explainUnsyncedEntities(
Set<UID> succeededTopLevelUids, Set<UID> syncedUids, ImportReport report) {
List<FailedItem> failures = new ArrayList<>(failedItems(report, getTrackerType()));
Set<UID> explainedUids =
failures.stream().map(FailedItem::uid).collect(Collectors.toCollection(HashSet::new));

for (UID uid : succeededTopLevelUids) {
if (!syncedUids.contains(uid) && !explainedUids.contains(uid)) {
explainedUids.add(uid);
failures.add(new FailedItem(uid, CHILD_FAILED_ERROR_CODE));
}
}

return failures;
}

private Set<UID> filterByFailedChildren(
Set<UID> succeededTopLevelUids, List<V> candidates, ImportReport report) {
if (succeededTopLevelUids.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,12 @@
import static org.hisp.dhis.tracker.imports.validation.ValidationCode.E4017;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -110,8 +115,105 @@ private static RequestCallback createRequestCallback(
};
}

static String formatFailedUids(Set<UID> failedUids) {
return failedUids.isEmpty() ? "" : format(" (failed: %s)", failedUids);
record FailedItem(UID uid, String errorCode) {}

private static final int MAX_UIDS_PER_REASON = 5;

/** Formats failures grouped by error code, so a large batch of failures stays readable */
static String formatFailedUids(List<FailedItem> failed) {
if (failed.isEmpty()) {
return "";
}

Map<String, List<UID>> uidsByErrorCode =
failed.stream()
.collect(
Collectors.groupingBy(
FailedItem::errorCode,
LinkedHashMap::new,
Collectors.collectingAndThen(
Collectors.mapping(
FailedItem::uid, Collectors.toCollection(LinkedHashSet::new)),
ArrayList::new)));

String reasons =
uidsByErrorCode.entrySet().stream()
.sorted(
Comparator.<Map.Entry<String, List<UID>>>comparingInt(e -> e.getValue().size())
.reversed())
.map(e -> formatReasonGroup(e.getKey(), e.getValue()))
.collect(Collectors.joining(", "));

return format(" (failed: %s)", reasons);
}

private static String formatReasonGroup(String errorCode, List<UID> uids) {
List<UID> shown =
uids.size() > MAX_UIDS_PER_REASON ? uids.subList(0, MAX_UIDS_PER_REASON) : uids;
String uidList = shown.stream().map(UID::getValue).collect(Collectors.joining(", "));
String more =
uids.size() > MAX_UIDS_PER_REASON
? format(", +%d more", uids.size() - MAX_UIDS_PER_REASON)
: "";
return format("%s x%d [%s%s]", errorCode, uids.size(), uidList, more);
}

/** Like {@link #failedUids}, but keeps the error code behind each failure for logging. */
static List<FailedItem> failedItems(ImportReport report, TrackerType type) {
List<FailedItem> failed = new ArrayList<>();

if (report.getValidationReport() != null) {
report.getValidationReport().getErrors().stream()
.filter(e -> type.name().equals(e.getTrackerType()))
.forEach(e -> failed.add(new FailedItem(e.getUid(), e.getErrorCode())));
}

if (report.getPersistenceReport() != null) {
TrackerTypeReport typeReport = report.getPersistenceReport().getTypeReportMap().get(type);
if (typeReport != null) {
typeReport
.getEntityReport()
.forEach(
entity ->
entity
.getErrorReports()
.forEach(
e -> failed.add(new FailedItem(entity.getUid(), e.getErrorCode()))));
}
}

return failed;
}

/**
* Like {@link #blockingFailedUids}, but keeps the error code behind each failure for logging.
* Only meaningful against a DELETE report.
*/
static List<FailedItem> blockingFailedItems(ImportReport report, TrackerType type) {
List<FailedItem> blocking = new ArrayList<>();

if (report.getValidationReport() != null) {
report.getValidationReport().getErrors().stream()
.filter(e -> type.name().equals(e.getTrackerType()))
.filter(e -> !ALREADY_DELETED_CODES.contains(e.getErrorCode()))
.forEach(e -> blocking.add(new FailedItem(e.getUid(), e.getErrorCode())));
}

if (report.getPersistenceReport() != null) {
TrackerTypeReport typeReport = report.getPersistenceReport().getTypeReportMap().get(type);
if (typeReport != null) {
typeReport
.getEntityReport()
.forEach(
entity ->
entity.getErrorReports().stream()
.filter(e -> !ALREADY_DELETED_CODES.contains(e.getErrorCode()))
.forEach(
e -> blocking.add(new FailedItem(entity.getUid(), e.getErrorCode()))));
}
}

return blocking;
}

static Set<UID> successfullyProcessedUids(ImportReport report, TrackerType type) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,28 @@ void shouldNotStampSyncTimestampWhenActiveChildRelationshipFailedRemotely() thro
verify(trackedEntityService, never()).updateTrackedEntitiesSyncTimestamp(any(), any());
}

@Test
void shouldExplainExclusionWhenEntityItselfSucceededButChildFailed() {
UID teUid = UID.of("TrackedEnt1");
UID relationshipUid = UID.of("Rel00000001");
ImportReport report =
reportWith(
successEntity(TrackerType.TRACKED_ENTITY, teUid),
failedEntity(TrackerType.RELATIONSHIP, relationshipUid, "E4009", "validation failed"));

// teUid succeeded at its own type (present in succeededTopLevelUids) but is absent from
// syncedUids, i.e. filterByFailedChildren actually excluded it over the failed relationship —
// a verified child failure, not a guess.
List<TrackerSyncReportUtils.FailedItem> failures =
service.explainUnsyncedEntities(Set.of(teUid), Set.of(), report);

assertEquals(
List.of(
new TrackerSyncReportUtils.FailedItem(
teUid, BaseDataSynchronizationWithPaging.CHILD_FAILED_ERROR_CODE)),
failures);
}

@Test
void shouldNotStampSyncTimestampWhenDeletedRelationshipFailedForRealReason() throws Exception {
UID teUid = UID.of("TrackedEnt1");
Expand Down
Loading
Loading