From b82ae1cc70a0d5aa0ac4e70fdab1f9f8ecabd197 Mon Sep 17 00:00:00 2001 From: teleivo Date: Tue, 26 Nov 2024 13:31:12 +0100 Subject: [PATCH] fix: /events?dataElementIdScheme!=UID for shared data elements --- .../tracker/export/event/JdbcEventStore.java | 11 ++-- .../org/hisp/dhis/test/utils/Assertions.java | 15 ++++- .../export/IdSchemeExportControllerTest.java | 61 +++++++++++++++++++ 3 files changed, 81 insertions(+), 6 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/JdbcEventStore.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/JdbcEventStore.java index 69210072ff1..8de7b2d18e6 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/JdbcEventStore.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/JdbcEventStore.java @@ -288,7 +288,8 @@ private List fetchEvents(EventQueryParams queryParams, PageParams pagePar mapSqlParameterSource, resultSet -> { Set notes = new HashSet<>(); - Set dataElementUids = new HashSet<>(); + // data elements per event + Map> dataElementUids = new HashMap<>(); while (resultSet.next()) { if (resultSet.getString(COLUMN_EVENT_UID) == null) { @@ -305,6 +306,7 @@ private List fetchEvents(EventQueryParams queryParams, PageParams pagePar event.setId(resultSet.getLong(COLUMN_EVENT_ID)); event.setUid(eventUid); eventsByUid.put(eventUid, event); + dataElementUids.put(eventUid, new HashSet<>()); TrackedEntity te = new TrackedEntity(); te.setUid(resultSet.getString(COLUMN_TRACKEDENTITY_UID)); @@ -437,11 +439,11 @@ private List fetchEvents(EventQueryParams queryParams, PageParams pagePar // data value per data element. The same data element can be in the result set // multiple times if the event also has notes. String dataElementUid = resultSet.getString("de_uid"); - if (!dataElementUids.contains(dataElementUid)) { + if (!dataElementUids.get(eventUid).contains(dataElementUid)) { EventDataValue eventDataValue = parseEventDataValue(dataElementIdScheme, resultSet); if (eventDataValue != null) { event.getEventDataValues().add(eventDataValue); - dataElementUids.add(dataElementUid); + dataElementUids.get(eventUid).add(dataElementUid); } } } @@ -536,8 +538,7 @@ private String getDataElementIdentifier( case CODE: return resultSet.getString("de_code"); case NAME: - return resultSet.getString("de_uid"); - // return resultSet.getString("de_name"); + return resultSet.getString("de_name"); case ATTRIBUTE: String attributeValuesString = resultSet.getString("de_attributevalues"); if (StringUtils.isEmpty(attributeValuesString)) { diff --git a/dhis-2/dhis-support/dhis-support-test/src/main/java/org/hisp/dhis/test/utils/Assertions.java b/dhis-2/dhis-support/dhis-support-test/src/main/java/org/hisp/dhis/test/utils/Assertions.java index 869fe537d80..043f4389d68 100644 --- a/dhis-2/dhis-support/dhis-support-test/src/main/java/org/hisp/dhis/test/utils/Assertions.java +++ b/dhis-2/dhis-support/dhis-support-test/src/main/java/org/hisp/dhis/test/utils/Assertions.java @@ -66,6 +66,19 @@ private Assertions() { * @param actual the actual collection. */ public static void assertContainsOnly(Collection expected, Collection actual) { + assertContainsOnly(expected, actual, "assertContainsOnly found mismatch"); + } + + /** + * Asserts that the given collection contains exactly the given items in any order. + * + * @param the type. + * @param expected the expected items. + * @param actual the actual collection. + * @param heading the assertAll heading + */ + public static void assertContainsOnly( + Collection expected, Collection actual, String heading) { assertNotNull( actual, () -> String.format("Expected collection to contain %s, got null instead", expected)); @@ -73,7 +86,7 @@ public static void assertContainsOnly(Collection expected, Collection List missing = CollectionUtils.difference(expected, actual); List extra = CollectionUtils.difference(actual, expected); assertAll( - "assertContainsOnly found mismatch", + heading, () -> assertTrue( missing.isEmpty(), () -> String.format("Expected %s to be in %s", missing, actual)), diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/export/IdSchemeExportControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/export/IdSchemeExportControllerTest.java index 34b73b00454..fc17683ff3b 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/export/IdSchemeExportControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/export/IdSchemeExportControllerTest.java @@ -50,6 +50,7 @@ import org.hisp.dhis.attribute.Attribute; import org.hisp.dhis.common.IdentifiableObject; import org.hisp.dhis.common.IdentifiableObjectManager; +import org.hisp.dhis.common.collection.CollectionUtils; import org.hisp.dhis.dataelement.DataElement; import org.hisp.dhis.dxf2.metadata.objectbundle.ObjectBundle; import org.hisp.dhis.dxf2.metadata.objectbundle.ObjectBundleMode; @@ -57,8 +58,10 @@ import org.hisp.dhis.dxf2.metadata.objectbundle.ObjectBundleService; import org.hisp.dhis.dxf2.metadata.objectbundle.ObjectBundleValidationService; import org.hisp.dhis.dxf2.metadata.objectbundle.feedback.ObjectBundleValidationReport; +import org.hisp.dhis.eventdatavalue.EventDataValue; import org.hisp.dhis.http.HttpStatus; import org.hisp.dhis.importexport.ImportStrategy; +import org.hisp.dhis.jsontree.JsonList; import org.hisp.dhis.jsontree.JsonObject; import org.hisp.dhis.program.Event; import org.hisp.dhis.render.RenderFormat; @@ -282,6 +285,35 @@ void shouldExportEventUsingNonUIDDataElementIdSchemeEvenIfItHasNoDataValues() { assertEquals("jxgFyJEMUPf", actual.getEvent()); } + @Test + void shouldExportEventsUsingNonUIDDataElementIdScheme() { + Event event1 = get(Event.class, "QRYjLTiJTrA"); + Event event2 = get(Event.class, "kWjSezkXHVp"); + assertNotEmpty( + CollectionUtils.intersection( + event1.getEventDataValues().stream() + .map(EventDataValue::getDataElement) + .collect(Collectors.toSet()), + event2.getEventDataValues().stream() + .map(EventDataValue::getDataElement) + .collect(Collectors.toSet())), + "test expects both events to have at least one data value for the same data element"); + + JsonList jsonEvents = + GET("/tracker/events?events=QRYjLTiJTrA,kWjSezkXHVp&fields=event,dataValues&dataElementIdScheme=NAME") + .content(HttpStatus.OK) + .getList("events", JsonEvent.class); + + Map events = + jsonEvents.stream().collect(Collectors.toMap(JsonEvent::getEvent, Function.identity())); + assertContainsOnly(List.of(event1.getUid(), event2.getUid()), events.keySet()); + + TrackerIdSchemeParam idSchemeParam = TrackerIdSchemeParam.NAME; + assertAll( + () -> assertDataValues(events.get("QRYjLTiJTrA"), event1, idSchemeParam), + () -> assertDataValues(events.get("kWjSezkXHVp"), event2, idSchemeParam)); + } + @ParameterizedTest @ValueSource(strings = {"/{id}?", "?events={id}&paging=true&", "?events={id}&paging=false&"}) void shouldReportMetadataWhichDoesNotHaveAnIdentifierForGivenIdScheme(String urlPortion) { @@ -350,6 +382,35 @@ private static void assertIdScheme( field, idSchemeParam)); } + private void assertDataValues( + JsonEvent actual, Event expected, TrackerIdSchemeParam idSchemeParam) { + String field = "dataValues"; + List expectedDataElement = + expected.getEventDataValues().stream() + .map(dv -> idSchemeParam.getIdentifier(get(DataElement.class, dv.getDataElement()))) + .toList(); + assertNotEmpty( + expectedDataElement, + String.format( + "metadata corresponding to field \"%s\" has no value in test data for" + + " idScheme '%s'", + field, idSchemeParam)); + assertTrue( + actual.has(field), + () -> + String.format( + "field \"%s\" is not in response %s for idScheme '%s'", + field, actual, idSchemeParam)); + List actualDataElement = + actual + .getList(field, JsonObject.class) + .toList(el -> el.getString("dataElement").string("")); + assertContainsOnly( + expectedDataElement, + actualDataElement, + "mismatch in data elements of event " + expected.getUid()); + } + private T get(Class type, String uid) { T t = manager.get(type, uid); assertNotNull(