Skip to content

Commit

Permalink
feat: Only allow duplicate Category Option Combos for merge [DHIS2-18…
Browse files Browse the repository at this point in the history
…838] (#19857)

* feat: Start new validation for COC merge [DHIS2-18838]

* feat: Add validation to restrict merges [DHIS2-18838]

* WIP handling duplicate coc refs natively

* feat: Int tests working for duplicate COC

* feat: Update e2e tests

* feat: Add unit tests for duplicate logic

* feat: Update removal technique for CO and CC. No SQL required.

* feat: clean up and javadoc

* feat: Amend error code
  • Loading branch information
david-mackessy authored Feb 6, 2025
1 parent 82caaf2 commit 2d0333b
Show file tree
Hide file tree
Showing 13 changed files with 699 additions and 1,012 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,25 +27,13 @@
*/
package org.hisp.dhis.category;

import java.util.Collection;
import java.util.List;
import javax.annotation.Nonnull;
import org.hisp.dhis.common.DataDimensionType;
import org.hisp.dhis.common.IdentifiableObjectStore;
import org.hisp.dhis.common.UID;

/**
* @author Lars Helge Overland
*/
public interface CategoryComboStore extends IdentifiableObjectStore<CategoryCombo> {
List<CategoryCombo> getCategoryCombosByDimensionType(DataDimensionType dataDimensionType);

/**
* Retrieve all {@link CategoryCombo}s with {@link CategoryOptionCombo} {@link UID}s
*
* @param uids {@link CategoryOptionCombo} {@link UID}s
* @return {@link CategoryCombo}s with references to {@link CategoryOptionCombo} {@link UID}s
* passed in
*/
List<CategoryCombo> getByCategoryOptionCombo(@Nonnull Collection<UID> uids);
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,8 @@
*/
package org.hisp.dhis.category;

import java.util.Collection;
import java.util.List;
import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
import org.hisp.dhis.common.IdentifiableObjectStore;
import org.hisp.dhis.common.UID;
import org.hisp.dhis.user.UserDetails;
Expand All @@ -49,13 +47,4 @@ public interface CategoryOptionStore extends IdentifiableObjectStore<CategoryOpt
List<CategoryOption> getCategoryOptions(Category category);

List<CategoryOption> getDataWriteCategoryOptions(Category category, UserDetails userDetails);

/**
* Retrieve all {@link CategoryOption}s with {@link CategoryOptionCombo} {@link UID}s
*
* @param uids {@link CategoryOptionCombo} {@link UID}s
* @return {@link CategoryOption}s with references to {@link CategoryOptionCombo} {@link UID}s
* passed in
*/
List<CategoryOption> getByCategoryOptionCombo(@Nonnull Collection<UID> uids);
}
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,10 @@ public enum ErrorCode {
E1533("{0} {1} does not exist: `{2}`"),
E1534("dataMergeStrategy field must be specified. With value `DISCARD` or `LAST_UPDATED`"),

/* CategoryOptionCombo merge */
E1540(
"CategoryOptionCombos must be duplicates (same cat combo, same cat options, different UID) in order to merge"),

/* DataElement merge */
E1550("All source ValueTypes must match target ValueType: `{0}`. Other ValueTypes found: `{1}`"),
E1551(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,29 @@ public MergeRequest validate(@Nonnull MergeParams params, @Nonnull MergeReport m
if (params.getDataMergeStrategy() == null) {
mergeReport.addErrorMessage(new ErrorMessage(ErrorCode.E1534));
}

if (mergeReport.hasErrorMessages()) return request;

// only allow merge if COCs are duplicate
List<CategoryOptionCombo> sources =
categoryService.getCategoryOptionCombosByUid(request.getSources());
CategoryOptionCombo target =
categoryService.getCategoryOptionCombo(request.getTarget().getValue());
if (!catOptCombosAreDuplicates(sources, target)) {
mergeReport.addErrorMessage(new ErrorMessage(ErrorCode.E1540));
}
return request;
}

protected static boolean catOptCombosAreDuplicates(
List<CategoryOptionCombo> sources, CategoryOptionCombo target) {
boolean allSourcesEqualTarget = sources.stream().allMatch(source -> source.equals(target));
boolean allSourceUidsAreDifferentThanTarget =
sources.stream().noneMatch(source -> source.getUid().equals(target.getUid()));

return allSourcesEqualTarget && allSourceUidsAreDifferentThanTarget;
}

@Override
public MergeReport merge(@Nonnull MergeRequest request, @Nonnull MergeReport mergeReport) {
log.info("Performing CategoryOptionCombo merge");
Expand Down Expand Up @@ -105,17 +125,17 @@ public MergeReport merge(@Nonnull MergeRequest request, @Nonnull MergeReport mer
private void handleDeleteSources(List<CategoryOptionCombo> sources, MergeReport mergeReport) {
log.info("Deleting source CategoryOptionCombos");
for (CategoryOptionCombo source : sources) {
mergeReport.addDeletedSource(source.getUid());
categoryService.deleteCategoryOptionCombo(source);
mergeReport.addDeletedSource(source.getUid());
}
}

@PostConstruct
private void initMergeHandlers() {
metadataMergeHandlers =
List.of(
metadataMergeHandler::handleCategoryOptions,
metadataMergeHandler::handleCategoryCombos,
(sources, target) -> metadataMergeHandler.handleCategoryOptions(sources),
(sources, target) -> metadataMergeHandler.handleCategoryCombos(),
metadataMergeHandler::handlePredictors,
metadataMergeHandler::handleDataElementOperands,
metadataMergeHandler::handleMinMaxDataElements,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@

import java.util.List;
import java.util.stream.Collectors;
import javax.annotation.Nonnull;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.hisp.dhis.category.CategoryCombo;
import org.hisp.dhis.category.CategoryComboStore;
import org.hisp.dhis.category.CategoryComboDeletionHandler;
import org.hisp.dhis.category.CategoryOption;
import org.hisp.dhis.category.CategoryOptionCombo;
import org.hisp.dhis.category.CategoryOptionStore;
import org.hisp.dhis.common.BaseIdentifiableObject;
import org.hisp.dhis.common.UID;
import org.hisp.dhis.datadimensionitem.DataDimensionItemStore;
Expand All @@ -61,8 +61,6 @@
@RequiredArgsConstructor
public class MetadataCategoryOptionComboMergeHandler {

private final CategoryOptionStore categoryOptionStore;
private final CategoryComboStore categoryComboStore;
private final DataElementOperandStore dataElementOperandStore;
private final DataDimensionItemStore dataDimensionItemStore;
private final MinMaxDataElementStore minMaxDataElementStore;
Expand All @@ -72,41 +70,23 @@ public class MetadataCategoryOptionComboMergeHandler {
private final ExpressionStore expressionStore;

/**
* Remove sources from {@link CategoryOption} and add target to {@link CategoryOption}
* Remove all {@link CategoryOption}s from source {@link CategoryOptionCombo}s
*
* @param sources to be removed
* @param target to add
*/
public void handleCategoryOptions(List<CategoryOptionCombo> sources, CategoryOptionCombo target) {
log.info("Merging source category options");
List<CategoryOption> categoryOptions =
categoryOptionStore.getByCategoryOptionCombo(
UID.of(sources.stream().map(BaseIdentifiableObject::getUid).toList()));

categoryOptions.forEach(
co -> {
co.addCategoryOptionCombo(target);
co.removeCategoryOptionCombos(sources);
});
public void handleCategoryOptions(@Nonnull List<CategoryOptionCombo> sources) {
for (CategoryOptionCombo coc : sources) coc.removeAllCategoryOptions();
log.info("Removed all category option references for source category option combos");
}

/**
* Remove sources from {@link CategoryCombo} and add target to {@link CategoryCombo}
*
* @param sources to be removed
* @param target to add
* Although nothing is done but log in this method, it is worth having to expose how the {@link
* CategoryCombo}s refs are being handled and also show that this has been thought about. This
* helps keep it up front and not hidden. The easiest way to remove this relationship is to let
* the {@link CategoryComboDeletionHandler} look after it.
*/
public void handleCategoryCombos(List<CategoryOptionCombo> sources, CategoryOptionCombo target) {
log.info("Merging source category combos");
List<CategoryCombo> categoryCombos =
categoryComboStore.getByCategoryOptionCombo(
UID.of(sources.stream().map(BaseIdentifiableObject::getUid).toList()));

categoryCombos.forEach(
cc -> {
cc.addCategoryOptionCombo(target);
cc.removeCategoryOptionCombos(sources);
});
public void handleCategoryCombos() {
log.info("Category combo references will be removed when the category option combo is deleted");
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,186 @@
/*
* Copyright (c) 2004-2025, University of Oslo
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
* Redistributions of source code must retain the above copyright notice, this
* list of conditions and the following disclaimer.
*
* Redistributions in binary form must reproduce the above copyright notice,
* this list of conditions and the following disclaimer in the documentation
* and/or other materials provided with the distribution.
* Neither the name of the HISP project nor the names of its contributors may
* be used to endorse or promote products derived from this software without
* specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
* WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
* DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
* ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
* (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
* LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
* ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/
package org.hisp.dhis.merge.category.optioncombo;

import static org.hisp.dhis.merge.category.optioncombo.CategoryOptionComboMergeService.catOptCombosAreDuplicates;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.util.List;
import java.util.Set;
import org.hisp.dhis.category.CategoryCombo;
import org.hisp.dhis.category.CategoryOption;
import org.hisp.dhis.category.CategoryOptionCombo;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInstance;
import org.junit.jupiter.api.TestInstance.Lifecycle;

@TestInstance(Lifecycle.PER_CLASS)
class CategoryOptionComboMergeServiceTest {

private CategoryCombo cc1;
private CategoryCombo cc2;
private CategoryOption co1;
private CategoryOption co2;
private CategoryOption co3;
private CategoryOptionCombo coc1;
private CategoryOptionCombo coc2;
private CategoryOptionCombo coc3;

@BeforeAll
public void setup() {
cc1 = new CategoryCombo();
cc1.setName("cc1");
cc1.setUid("UIDcatcom01");
cc1.setCode("code1");

cc2 = new CategoryCombo();
cc2.setName("cc2");
cc2.setUid("UIDcatcom02");
cc2.setCode("code2");

co1 = new CategoryOption();
co1.setName("co1");
co1.setUid("UIDcatopt01");
co1.setCode("co1");
co1.setShortName("co1");
co1.setDescription("co1");

co2 = new CategoryOption();
co2.setName("co2");
co2.setUid("UIDcatopt02");
co2.setCode("co2");
co2.setShortName("co2");
co2.setDescription("co2");

co3 = new CategoryOption();
co3.setName("co3");
co3.setUid("UIDcatopt03");
co3.setCode("co3");
co3.setShortName("co3");
co3.setDescription("co3");

coc1 = new CategoryOptionCombo();
coc1.setName("coc1");
coc1.setUid("UIDcoc00001");

coc2 = new CategoryOptionCombo();
coc2.setName("coc2");
coc2.setUid("UIDcoc00002");

coc3 = new CategoryOptionCombo();
coc3.setName("coc3");
coc3.setUid("UIDcoc00003");
}

@Test
@DisplayName("COC with same CC and COs are detected as duplicates")
void sameCcSameCoTest() {
coc1.setCategoryCombo(cc1);
coc2.setCategoryCombo(cc1);
coc3.setCategoryCombo(cc1);
coc1.setCategoryOptions(Set.of(co1, co2));
coc2.setCategoryOptions(Set.of(co1, co2));
coc3.setCategoryOptions(Set.of(co1, co2));

assertTrue(catOptCombosAreDuplicates(List.of(coc1, coc2), coc3));
}

@Test
@DisplayName("COC with same CC and different number of COs are not detected as duplicates")
void sameCcDiffCoTest() {
coc1.setCategoryCombo(cc1);
coc2.setCategoryCombo(cc1);
coc3.setCategoryCombo(cc1);
coc1.setCategoryOptions(Set.of(co1, co2));
coc2.setCategoryOptions(Set.of(co1));
coc3.setCategoryOptions(Set.of(co1, co2));

assertFalse(catOptCombosAreDuplicates(List.of(coc1, coc2), coc3));
}

@Test
@DisplayName("COC with different CC and same COs are not detected as duplicates")
void diffCcSameCoTest() {
coc1.setCategoryCombo(cc1);
coc2.setCategoryCombo(cc2);
coc3.setCategoryCombo(cc1);
coc1.setCategoryOptions(Set.of(co1, co2));
coc2.setCategoryOptions(Set.of(co1, co2));
coc3.setCategoryOptions(Set.of(co1, co2));

assertFalse(catOptCombosAreDuplicates(List.of(coc1, coc2), coc3));
}

@Test
@DisplayName("COC with different CC and different COs are not detected as duplicates")
void diffCcDiffCoTest() {
coc1.setCategoryCombo(cc1);
coc2.setCategoryCombo(cc2);
coc3.setCategoryCombo(cc1);
coc1.setCategoryOptions(Set.of(co2));
coc2.setCategoryOptions(Set.of(co1, co2));
coc3.setCategoryOptions(Set.of(co1));

assertFalse(catOptCombosAreDuplicates(List.of(coc1, coc2), coc3));
}

@Test
@DisplayName("COC with different UIDs, same CC and same COs are detected as duplicates")
void diffUidsCcCoTest() {
coc1.setCategoryCombo(cc1);
coc1.setUid("diff");
coc2.setCategoryCombo(cc1);
coc2.setUid("other");
coc3.setCategoryCombo(cc1);
coc3.setUid("more");
coc1.setCategoryOptions(Set.of(co1, co2));
coc2.setCategoryOptions(Set.of(co1, co2));
coc3.setCategoryOptions(Set.of(co1, co2));

assertTrue(catOptCombosAreDuplicates(List.of(coc1, coc2), coc3));
}

@Test
@DisplayName("COC with same UIDs, same CC and same COs are not detected as duplicates")
void sameUidsCcCoTest() {
coc1.setCategoryCombo(cc1);
coc1.setUid("same");
coc2.setCategoryCombo(cc1);
coc2.setUid("same");
coc3.setCategoryCombo(cc1);
coc3.setUid("same");
coc1.setCategoryOptions(Set.of(co1, co2));
coc2.setCategoryOptions(Set.of(co1, co2));
coc3.setCategoryOptions(Set.of(co1, co2));

assertFalse(catOptCombosAreDuplicates(List.of(coc1, coc2), coc3));
}
}
Loading

0 comments on commit 2d0333b

Please sign in to comment.