From 22d0070b9bdb37ff158e8306d96c5671579927f4 Mon Sep 17 00:00:00 2001 From: David Mackessy Date: Fri, 10 Jan 2025 10:01:50 +0000 Subject: [PATCH] feat: Add store tests for all sql execution paths [DHIS2-18321] --- .../hisp/dhis/datavalue/DataValueStore.java | 20 ++ .../hibernate/HibernateDataValueStore.java | 20 -- .../dhis/datavalue/DataValueStoreTest.java | 244 +++++++++++++++--- 3 files changed, 231 insertions(+), 53 deletions(-) diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/datavalue/DataValueStore.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/datavalue/DataValueStore.java index b9f19d7c250..74433a5c45f 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/datavalue/DataValueStore.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/datavalue/DataValueStore.java @@ -252,6 +252,26 @@ DataValue getDataValue( void mergeDataValuesWithCategoryOptionCombos( @Nonnull CategoryOptionCombo target, @Nonnull Collection sources); + /** + * SQL for handling merging {@link DataValue}s. There may be multiple potential {@link DataValue} + * duplicates. Duplicate {@link DataValue}s with the latest {@link DataValue#lastUpdated} values + * are kept, the rest are deleted. Only one of these entries can exist due to the composite key + * constraint.
+ * The 3 execution paths are: + * + *

1. If the source {@link DataValue} is not a duplicate, it simply gets its {@link + * DataValue#attributeOptionCombo} updated to that of the target. + * + *

2. If the source {@link DataValue} is a duplicate and has an earlier {@link + * DataValue#lastUpdated} value, it is deleted. + * + *

3. If the source {@link DataValue} is a duplicate and has a later {@link + * DataValue#lastUpdated} value, the target {@link DataValue} is deleted. The source is kept and + * has its {@link DataValue#attributeOptionCombo} updated to that of the target. + * + * @param target target {@link CategoryOptionCombo} + * @param sources source {@link CategoryOptionCombo}s + */ void mergeDataValuesWithAttributeOptionCombos( @Nonnull CategoryOptionCombo target, @Nonnull Collection sources); } diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/datavalue/hibernate/HibernateDataValueStore.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/datavalue/hibernate/HibernateDataValueStore.java index 463957efddb..071d9c69c58 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/datavalue/hibernate/HibernateDataValueStore.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/datavalue/hibernate/HibernateDataValueStore.java @@ -424,26 +424,6 @@ -- loop through each record with a source CategoryOptionCombo jdbcTemplate.update(plpgsql); } - /** - * SQL for handling merging {@link DataValue}s. There may be multiple potential {@link DataValue} - * duplicates. Duplicate {@link DataValue}s with the latest {@link DataValue#lastUpdated} values - * are kept, the rest are deleted. Only one of these entries can exist due to the composite key - * constraint.
- * The 3 execution paths are: - * - *

1. If the source {@link DataValue} is not a duplicate, it simply gets its {@link - * DataValue#attributeOptionCombo} updated to that of the target. - * - *

2. If the source {@link DataValue} is a duplicate and has an earlier {@link - * DataValue#lastUpdated} value, it is deleted. - * - *

3. If the source {@link DataValue} is a duplicate and has a later {@link - * DataValue#lastUpdated} value, the target {@link DataValue} is deleted. The source is kept and - * has its {@link DataValue#attributeOptionCombo} updated to that of the target. - * - * @param target target {@link CategoryOptionCombo} - * @param sources source {@link CategoryOptionCombo}s - */ @Override public void mergeDataValuesWithAttributeOptionCombos( @Nonnull CategoryOptionCombo target, @Nonnull Collection sources) { diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/datavalue/DataValueStoreTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/datavalue/DataValueStoreTest.java index 110a4ec3d61..2a2db46c84f 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/datavalue/DataValueStoreTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/datavalue/DataValueStoreTest.java @@ -227,8 +227,8 @@ void getDataValuesByAoc() { @Test @DisplayName( - "Merging duplicate DataValues (cat opt combos) leaves only the last updated value remaining") - void mergeDvWithDuplicates() { + "Merging duplicate DataValues (cat opt combos) leaves only the last updated (source) value remaining") + void mergeDvWithDuplicatesKeepSource() { // given TestCategoryMetadata categoryMetadata = setupCategoryMetadata(); @@ -270,60 +270,238 @@ void mergeDvWithDuplicates() { dv4.setSource(ou); dv4.setLastUpdated(DateUtils.parseDate("2024-11-02")); - dataValueStore.addDataValue(dv1); - dataValueStore.addDataValue(dv2); - dataValueStore.addDataValue(dv3); - dataValueStore.addDataValue(dv4); + addDataValues(dv1, dv2, dv3, dv4); // check pre merge state List preMergeState = dataValueStore.getAllDataValuesByDataElement(List.of(de)); assertEquals(4, preMergeState.size(), "there should be 4 data values"); - assertTrue( - preMergeState.stream() - .map(dv -> dv.getCategoryOptionCombo().getId()) - .collect(Collectors.toSet()) - .containsAll( - List.of( - categoryMetadata.coc1().getId(), - categoryMetadata.coc2().getId(), - categoryMetadata.coc3().getId(), - categoryMetadata.coc4().getId())), - "All data values have different category option combos"); + checkCocIdsPresent( + preMergeState, + List.of( + categoryMetadata.coc1().getId(), + categoryMetadata.coc2().getId(), + categoryMetadata.coc3().getId(), + categoryMetadata.coc4().getId())); - entityManager.flush(); + // when + mergeDataValues( + categoryMetadata.coc3(), List.of(categoryMetadata.coc1(), categoryMetadata.coc2())); + + // then + List postMergeState = dataValueStore.getAllDataValuesByDataElement(List.of(de)); + + assertEquals(2, postMergeState.size(), "there should be 2 data values"); + checkCocIdsPresent( + preMergeState, List.of(categoryMetadata.coc3().getId(), categoryMetadata.coc4().getId())); + + checkDataValuesPresent( + postMergeState, List.of("dv test 2 - last updated", "dv test 4, untouched")); + + checkDatesPresent( + postMergeState, + List.of(DateUtils.parseDate("2025-01-08"), DateUtils.parseDate("2024-11-02"))); + } + + @Test + @DisplayName( + "Merging duplicate DataValues (cat opt combos) leaves only the last updated (target) value remaining") + void mergeDvWithDuplicatesKeepTarget() { + // given + TestCategoryMetadata categoryMetadata = setupCategoryMetadata(); + + Period p1 = createPeriod(DateUtils.getDate(2024, 1, 1), DateUtils.getDate(2023, 2, 1)); + + DataElement de = createDataElement('z'); + manager.persist(de); + + OrganisationUnit ou = createOrganisationUnit("org u 1"); + manager.persist(ou); + + // data values with same period, org unit, data element and attr opt combo + // which will be identified as duplicates during merging + DataValue dv1 = createDataValue('1', p1, "dv test 1"); + dv1.setCategoryOptionCombo(categoryMetadata.coc1()); + dv1.setAttributeOptionCombo(categoryMetadata.coc4()); + dv1.setDataElement(de); + dv1.setSource(ou); + dv1.setLastUpdated(DateUtils.parseDate("2024-12-01")); + + DataValue dv2 = createDataValue('2', p1, "dv test 2"); + dv2.setCategoryOptionCombo(categoryMetadata.coc2()); + dv2.setAttributeOptionCombo(categoryMetadata.coc4()); + dv2.setDataElement(de); + dv2.setSource(ou); + dv2.setLastUpdated(DateUtils.parseDate("2025-01-02")); + + DataValue dv3 = createDataValue('3', p1, "dv test 3 - last updated"); + dv3.setCategoryOptionCombo(categoryMetadata.coc3()); + dv3.setAttributeOptionCombo(categoryMetadata.coc4()); + dv3.setDataElement(de); + dv3.setSource(ou); + dv3.setLastUpdated(DateUtils.parseDate("2025-01-06")); + + DataValue dv4 = createDataValue('4', p1, "dv test 4, untouched"); + dv4.setCategoryOptionCombo(categoryMetadata.coc4()); + dv4.setAttributeOptionCombo(categoryMetadata.coc4()); + dv4.setDataElement(de); + dv4.setSource(ou); + dv4.setLastUpdated(DateUtils.parseDate("2024-11-02")); + + addDataValues(dv1, dv2, dv3, dv4); + + // check pre merge state + List preMergeState = dataValueStore.getAllDataValuesByDataElement(List.of(de)); + + assertEquals(4, preMergeState.size(), "there should be 4 data values"); + checkCocIdsPresent( + preMergeState, + List.of( + categoryMetadata.coc1().getId(), + categoryMetadata.coc2().getId(), + categoryMetadata.coc3().getId(), + categoryMetadata.coc4().getId())); // when - dataValueStore.mergeDataValuesWithCategoryOptionCombos( + mergeDataValues( categoryMetadata.coc3(), List.of(categoryMetadata.coc1(), categoryMetadata.coc2())); - entityManager.flush(); - entityManager.clear(); // then List postMergeState = dataValueStore.getAllDataValuesByDataElement(List.of(de)); assertEquals(2, postMergeState.size(), "there should be 2 data values"); + checkCocIdsPresent( + postMergeState, List.of(categoryMetadata.coc3().getId(), categoryMetadata.coc4().getId())); + + checkDataValuesPresent( + postMergeState, List.of("dv test 3 - last updated", "dv test 4, untouched")); + + checkDatesPresent( + postMergeState, + List.of(DateUtils.parseDate("2025-01-06"), DateUtils.parseDate("2024-11-02"))); + } + + @Test + @DisplayName( + "Merging non-duplicate DataValues (cat opt combos) updates the cat opt combo value only") + void mergeDvWithNoDuplicates() { + // given + TestCategoryMetadata categoryMetadata = setupCategoryMetadata(); + + Period p1 = createPeriod(DateUtils.getDate(2024, 1, 1), DateUtils.getDate(2023, 2, 1)); + Period p2 = createPeriod(DateUtils.getDate(2024, 2, 1), DateUtils.getDate(2023, 3, 1)); + Period p3 = createPeriod(DateUtils.getDate(2024, 3, 1), DateUtils.getDate(2023, 4, 1)); + Period p4 = createPeriod(DateUtils.getDate(2024, 4, 1), DateUtils.getDate(2023, 5, 1)); + + DataElement de = createDataElement('z'); + manager.persist(de); + + OrganisationUnit ou = createOrganisationUnit("org u 1"); + manager.persist(ou); + + // data values with different period, so no duplicates detected during merging + DataValue dv1 = createDataValue('1', p1, "dv test 1"); + dv1.setCategoryOptionCombo(categoryMetadata.coc1()); + dv1.setAttributeOptionCombo(categoryMetadata.coc4()); + dv1.setDataElement(de); + dv1.setSource(ou); + dv1.setLastUpdated(DateUtils.parseDate("2024-12-01")); + + DataValue dv2 = createDataValue('2', p2, "dv test 2 - last updated"); + dv2.setCategoryOptionCombo(categoryMetadata.coc2()); + dv2.setAttributeOptionCombo(categoryMetadata.coc4()); + dv2.setDataElement(de); + dv2.setSource(ou); + dv2.setLastUpdated(DateUtils.parseDate("2025-01-08")); + + DataValue dv3 = createDataValue('3', p3, "dv test 3"); + dv3.setCategoryOptionCombo(categoryMetadata.coc3()); + dv3.setAttributeOptionCombo(categoryMetadata.coc4()); + dv3.setDataElement(de); + dv3.setSource(ou); + dv3.setLastUpdated(DateUtils.parseDate("2024-12-06")); + + DataValue dv4 = createDataValue('4', p4, "dv test 4, untouched"); + dv4.setCategoryOptionCombo(categoryMetadata.coc4()); + dv4.setAttributeOptionCombo(categoryMetadata.coc4()); + dv4.setDataElement(de); + dv4.setSource(ou); + dv4.setLastUpdated(DateUtils.parseDate("2024-11-02")); + + addDataValues(dv1, dv2, dv3, dv4); + + // check pre merge state + List preMergeState = dataValueStore.getAllDataValuesByDataElement(List.of(de)); + + assertEquals(4, preMergeState.size(), "there should be 4 data values"); + checkCocIdsPresent( + preMergeState, + List.of( + categoryMetadata.coc1().getId(), + categoryMetadata.coc2().getId(), + categoryMetadata.coc3().getId(), + categoryMetadata.coc4().getId())); + + // when + mergeDataValues( + categoryMetadata.coc3(), List.of(categoryMetadata.coc1(), categoryMetadata.coc2())); + + // then + List postMergeState = dataValueStore.getAllDataValuesByDataElement(List.of(de)); + + assertEquals(4, postMergeState.size(), "there should still be 4 data values"); + checkCocIdsPresent( + postMergeState, List.of(categoryMetadata.coc3().getId(), categoryMetadata.coc4().getId())); + + checkDataValuesPresent( + postMergeState, + List.of("dv test 1", "dv test 2 - last updated", "dv test 3", "dv test 4, untouched")); + + checkDatesPresent( + postMergeState, + List.of( + DateUtils.parseDate("2025-01-08"), + DateUtils.parseDate("2024-11-02"), + DateUtils.parseDate("2024-12-01"), + DateUtils.parseDate("2024-12-06"))); + } + + private void checkDatesPresent(List dataValues, List dates) { assertTrue( - postMergeState.stream() - .map(dv -> dv.getCategoryOptionCombo().getId()) + dataValues.stream() + .map(DataValue::getLastUpdated) .collect(Collectors.toSet()) - .containsAll(List.of(categoryMetadata.coc3().getId(), categoryMetadata.coc4().getId())), - "Only 2 expected cat opt combos should be present"); + .containsAll(dates), + "Expected dates should be present"); + } + private void checkDataValuesPresent(List dataValues, List values) { assertTrue( - postMergeState.stream() + dataValues.stream() .map(DataValue::getValue) .collect(Collectors.toSet()) - .containsAll(List.of("dv test 2 - last updated", "dv test 4, untouched")), - "Only latest DataValue and untouched DataValue should be present"); + .containsAll(values), + "Expected DataValues should be present"); + } + private void checkCocIdsPresent(List dataValues, List cocIds) { assertTrue( - postMergeState.stream() - .map(DataValue::getLastUpdated) + dataValues.stream() + .map(dv -> dv.getCategoryOptionCombo().getId()) .collect(Collectors.toSet()) - .containsAll( - List.of(DateUtils.parseDate("2025-01-08"), DateUtils.parseDate("2024-11-02"))), - "Only latest lastUpdated value and untouched lastUpdated should exist"); + .containsAll(cocIds), + "Data values have expected category option combos"); + } + + private void mergeDataValues(CategoryOptionCombo target, List sources) { + dataValueStore.mergeDataValuesWithCategoryOptionCombos(target, sources); + entityManager.flush(); + entityManager.clear(); + } + + private void addDataValues(DataValue... dvs) { + for (DataValue dv : dvs) dataValueStore.addDataValue(dv); + entityManager.flush(); } private DataValue createDataValue(char uniqueChar, Period period, String value) {