From 7d62ab58553b2b7eb906053320eb7db4ab33c81a Mon Sep 17 00:00:00 2001 From: Enrico Colasante Date: Tue, 6 Oct 2026 14:34:27 +0200 Subject: [PATCH] fix: Remove N+1 query when validating unique attributes [DHIS2-22157] (#25440) --- .../TrackedEntityAttributeValueService.java | 31 +- .../TrackedEntityAttributeValueStore.java | 34 +-- .../UniqueAttributeValueMatch.java | 43 +++ ...ultTrackedEntityAttributeValueService.java | 23 +- ...rnateTrackedEntityAttributeValueStore.java | 151 +++++++--- .../imports/preheat/TrackerPreheat.java | 36 ++- .../imports/preheat/UniqueAttributeValue.java | 25 +- .../supplier/UniqueAttributesSupplier.java | 163 +++++++---- .../validator/AttributeValidator.java | 25 +- .../imports/preheat/TrackerPreheatTest.java | 37 +++ .../supplier/UniqueAttributeSupplierTest.java | 271 +++++++++++++++--- .../trackedentity/AttributeValidatorTest.java | 127 +++++++- .../UniqueAttributeValueLookupTest.java | 214 ++++++++++++++ .../TeTaEncryptionValidationTest.java | 43 +++ .../TrackedEntityImportValidationTest.java | 32 +++ 15 files changed, 1053 insertions(+), 202 deletions(-) create mode 100644 dhis-2/dhis-api/src/main/java/org/hisp/dhis/trackedentityattributevalue/UniqueAttributeValueMatch.java create mode 100644 dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/trackedentityattributevalue/UniqueAttributeValueLookupTest.java diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/trackedentityattributevalue/TrackedEntityAttributeValueService.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/trackedentityattributevalue/TrackedEntityAttributeValueService.java index 4b247d0a63b6..3bd186dbc098 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/trackedentityattributevalue/TrackedEntityAttributeValueService.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/trackedentityattributevalue/TrackedEntityAttributeValueService.java @@ -29,6 +29,8 @@ import java.util.List; import java.util.Map; +import java.util.Set; +import javax.annotation.Nonnull; import org.hisp.dhis.trackedentity.TrackedEntity; import org.hisp.dhis.trackedentity.TrackedEntityAttribute; import org.hisp.dhis.user.User; @@ -103,12 +105,29 @@ List getTrackedEntityAttributeValues( int getCountOfAssignedTrackedEntityAttributeValues(TrackedEntityAttribute attribute); /** - * Retrieve a list of {@link TrackedEntityAttributeValue} that matches the values and the tea - * present in uniqueAttributes + * Finds the stored values of a unique attribute that match any of the given values, in any org + * unit. Values are compared case-insensitively. Use it for attributes unique across the whole + * system. * - * @param uniqueAttributes A map that links a list of values to a TEA - * @return TrackedEntityAttributeValue list + * @param attribute the unique attribute + * @param values the values to look for + * @return the matching values, empty if none + */ + @Nonnull + List getUniqueAttributeValues( + @Nonnull TrackedEntityAttribute attribute, @Nonnull Set values); + + /** + * Finds the stored values of an org unit scoped unique attribute that match one of the given (org + * unit, value) pairs exactly: a value only matches when stored for a tracked entity in the org + * unit it is paired with. Values are compared case-insensitively. + * + * @param attribute the unique attribute, unique within an org unit + * @param valuesByOrgUnitId the values to look for, keyed by the id of the org unit they are + * unique in + * @return the matching values, empty if none */ - List getUniqueAttributeByValues( - Map> uniqueAttributes); + @Nonnull + List getUniqueAttributeValues( + @Nonnull TrackedEntityAttribute attribute, @Nonnull Map> valuesByOrgUnitId); } diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/trackedentityattributevalue/TrackedEntityAttributeValueStore.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/trackedentityattributevalue/TrackedEntityAttributeValueStore.java index 5feb263e9115..245a3404f961 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/trackedentityattributevalue/TrackedEntityAttributeValueStore.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/trackedentityattributevalue/TrackedEntityAttributeValueStore.java @@ -27,10 +27,10 @@ */ package org.hisp.dhis.trackedentityattributevalue; -import java.util.Collection; import java.util.List; +import java.util.Map; +import java.util.Set; import org.hisp.dhis.common.GenericStore; -import org.hisp.dhis.program.Program; import org.hisp.dhis.trackedentity.TrackedEntity; import org.hisp.dhis.trackedentity.TrackedEntityAttribute; @@ -82,33 +82,11 @@ public interface TrackedEntityAttributeValueStore */ List get(TrackedEntityAttribute attribute); - /** - * Gets a list of {@link TrackedEntityAttributeValue} that matches the parameters - * - * @param attribute {@link TrackedEntityAttribute} to get value for - * @param values List of literal values - * @return list of {@link TrackedEntityAttributeValue} - */ - List get( - TrackedEntityAttribute attribute, Collection values); + List getUniqueAttributeValues( + TrackedEntityAttribute attribute, Set values); - /** - * Gets a list of {@link TrackedEntityAttributeValue} that matches the parameters - * - * @param attribute {@link TrackedEntityAttribute} to get value for - * @param value literal value to find within the specified {@link TrackedEntityAttribute} - * @return list of {@link TrackedEntityAttributeValue} - */ - List get(TrackedEntityAttribute attribute, String value); - - /** - * Retrieve attribute values of an instance by a program. - * - * @param instance the TrackedEntity - * @param program the Program. - * @return TrackedEntityAttributeValue list - */ - List get(TrackedEntity instance, Program program); + List getUniqueAttributeValues( + TrackedEntityAttribute attribute, Map> valuesByOrgUnitId); /** * Return the number of assigned {@link TrackedEntityAttributeValue}s to the {@link diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/trackedentityattributevalue/UniqueAttributeValueMatch.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/trackedentityattributevalue/UniqueAttributeValueMatch.java new file mode 100644 index 000000000000..e7196451ad63 --- /dev/null +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/trackedentityattributevalue/UniqueAttributeValueMatch.java @@ -0,0 +1,43 @@ +/* + * Copyright (c) 2004-2022, 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.trackedentityattributevalue; + +import javax.annotation.CheckForNull; +import javax.annotation.Nonnull; +import org.hisp.dhis.common.UID; + +/** + * A stored value of a unique tracked entity attribute that matches a value being imported. + * + * @param trackedEntity the tracked entity owning the value + * @param value the stored value, as persisted (not normalized to lower case) + * @param orgUnitId the id of the org unit of the tracked entity owning the value, only set for an + * attribute unique within an org unit + */ +public record UniqueAttributeValueMatch( + @Nonnull UID trackedEntity, @Nonnull String value, @CheckForNull Long orgUnitId) {} diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/trackedentityattributevalue/DefaultTrackedEntityAttributeValueService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/trackedentityattributevalue/DefaultTrackedEntityAttributeValueService.java index 42d0f6627d85..2f6523027473 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/trackedentityattributevalue/DefaultTrackedEntityAttributeValueService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/trackedentityattributevalue/DefaultTrackedEntityAttributeValueService.java @@ -31,7 +31,7 @@ import java.util.List; import java.util.Map; -import java.util.stream.Collectors; +import java.util.Set; import lombok.RequiredArgsConstructor; import org.apache.commons.lang3.StringUtils; import org.hisp.dhis.changelog.ChangeLogType; @@ -217,10 +217,21 @@ private boolean addFileValue(TrackedEntityAttributeValue value) { @Override @Transactional(readOnly = true) - public List getUniqueAttributeByValues( - Map> uniqueAttributes) { - return uniqueAttributes.entrySet().stream() - .flatMap(entry -> this.attributeValueStore.get(entry.getKey(), entry.getValue()).stream()) - .collect(Collectors.toList()); + public List getUniqueAttributeValues( + TrackedEntityAttribute attribute, Set values) { + if (values.isEmpty()) { + return List.of(); + } + return attributeValueStore.getUniqueAttributeValues(attribute, values); + } + + @Override + @Transactional(readOnly = true) + public List getUniqueAttributeValues( + TrackedEntityAttribute attribute, Map> valuesByOrgUnitId) { + if (valuesByOrgUnitId.isEmpty()) { + return List.of(); + } + return attributeValueStore.getUniqueAttributeValues(attribute, valuesByOrgUnitId); } } diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/trackedentityattributevalue/hibernate/HibernateTrackedEntityAttributeValueStore.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/trackedentityattributevalue/hibernate/HibernateTrackedEntityAttributeValueStore.java index 796783c71e8d..7a49e2b22af4 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/trackedentityattributevalue/hibernate/HibernateTrackedEntityAttributeValueStore.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/trackedentityattributevalue/hibernate/HibernateTrackedEntityAttributeValueStore.java @@ -27,18 +27,23 @@ */ package org.hisp.dhis.trackedentityattributevalue.hibernate; -import java.util.Collection; +import com.google.common.collect.Lists; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; import java.util.List; -import java.util.stream.Collectors; +import java.util.Map; +import java.util.Set; import javax.persistence.EntityManager; import org.apache.commons.lang3.StringUtils; import org.hibernate.query.Query; +import org.hisp.dhis.common.UID; import org.hisp.dhis.hibernate.HibernateGenericStore; -import org.hisp.dhis.program.Program; import org.hisp.dhis.trackedentity.TrackedEntity; import org.hisp.dhis.trackedentity.TrackedEntityAttribute; import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValue; import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValueStore; +import org.hisp.dhis.trackedentityattributevalue.UniqueAttributeValueMatch; import org.springframework.context.ApplicationEventPublisher; import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.stereotype.Repository; @@ -50,6 +55,25 @@ public class HibernateTrackedEntityAttributeValueStore extends HibernateGenericStore implements TrackedEntityAttributeValueStore { + /** + * Upper bound of values (or org unit/value pairs) bound in one query. A scoped query binds up to + * three parameters per pair, which keeps it below the Postgres limit of 32767 bind parameters. + */ + private static final int MAX_VALUES_PER_QUERY = 10_000; + + /** + * Selects only the columns the uniqueness validation needs. Selecting entities instead would load + * one tracked entity per matching row (the association from {@code TrackedEntityAttributeValue} + * is eager) and the users referenced by each org unit. + */ + private static final String SELECT_UNIQUE_VALUES = + """ + select te.uid, v.plainValue, te.organisationUnit.id + from TrackedEntityAttributeValue v + join v.trackedEntity te + where v.attribute = :attribute + """; + public HibernateTrackedEntityAttributeValueStore( EntityManager entityManager, JdbcTemplate jdbcTemplate, ApplicationEventPublisher publisher) { super(entityManager, jdbcTemplate, publisher, TrackedEntityAttributeValue.class, false); @@ -106,44 +130,103 @@ public List get(TrackedEntityAttribute attribute) { return getList(typedQuery); } + /** + * Finds the stored values of {@code attribute} matching any of {@code values} (case-insensitive), + * in any org unit. + */ @Override - public List get( - TrackedEntityAttribute attribute, Collection values) { - String query = - " from TrackedEntityAttributeValue v where v.attribute =:attribute and lower(v.plainValue) in :values"; - - Query typedQuery = - getQuery(query) - .setParameter("attribute", attribute) - .setParameter( - "values", values.stream().map(StringUtils::lowerCase).collect(Collectors.toList())); - - return getList(typedQuery); + public List getUniqueAttributeValues( + TrackedEntityAttribute attribute, Set values) { + List matches = new ArrayList<>(); + for (List partition : Lists.partition(lowerCase(values), MAX_VALUES_PER_QUERY)) { + String hql = SELECT_UNIQUE_VALUES + " and lower(v.plainValue) in (:values)"; + getSession() + .createQuery(hql, Object[].class) + .setParameter("attribute", attribute) + .setParameterList("values", partition) + .list() + .forEach( + row -> + matches.add( + new UniqueAttributeValueMatch( + UID.of((String) row[0]), (String) row[1], (Long) row[2]))); + } + return matches; } + /** + * Finds the stored values of {@code attribute} matching exactly one of the (org unit, value) + * pairs in {@code valuesByOrgUnitId} (value compared case-insensitively). + * + *

The value and org unit {@code in} filters let Postgres use the value or the org unit index, + * whichever is more selective. The key filter on {@code lower(value)|orgUnitId} then drops the + * cross product combinations that were not requested, e.g. a value only sent for org unit A that + * exists in org unit B. Postgres hashes the key list, so it does not slow the query down. + */ @Override - public List get(TrackedEntityAttribute attribute, String value) { - String query = - " from TrackedEntityAttributeValue v where v.attribute =:attribute and lower(v.plainValue) like :value"; - - Query typedQuery = - getQuery(query) - .setParameter("attribute", attribute) - .setParameter("value", StringUtils.lowerCase(value)); - - return getList(typedQuery); + public List getUniqueAttributeValues( + TrackedEntityAttribute attribute, Map> valuesByOrgUnitId) { + List matches = new ArrayList<>(); + for (Map> partition : partition(valuesByOrgUnitId)) { + Set values = new HashSet<>(); + List keys = new ArrayList<>(); + for (Map.Entry> entry : partition.entrySet()) { + for (String value : lowerCase(entry.getValue())) { + values.add(value); + keys.add(value + "|" + entry.getKey()); + } + } + + String hql = + SELECT_UNIQUE_VALUES + + """ + and lower(v.plainValue) in (:values) + and te.organisationUnit.id in (:orgUnitIds) + and concat(lower(v.plainValue), '|', str(te.organisationUnit.id)) in (:keys) + """; + getSession() + .createQuery(hql, Object[].class) + .setParameter("attribute", attribute) + .setParameterList("values", values) + .setParameterList("orgUnitIds", partition.keySet()) + .setParameterList("keys", keys) + .list() + .forEach( + row -> + matches.add( + new UniqueAttributeValueMatch( + UID.of((String) row[0]), (String) row[1], (Long) row[2]))); + } + return matches; } - @Override - public List get(TrackedEntity trackedEntity, Program program) { - String query = - " from TrackedEntityAttributeValue v where v.trackedEntity =:trackedEntity and v.attribute.program =:program"; - - Query typedQuery = getQuery(query); - typedQuery.setParameter("trackedEntity", trackedEntity); - typedQuery.setParameter("program", program); + /** + * Splits the pairs into chunks of at most {@link #MAX_VALUES_PER_QUERY} pairs, so every query + * stays below the Postgres bind parameter limit (values + org units + keys per query). + */ + private static List>> partition(Map> valuesByOrgUnitId) { + List>> partitions = new ArrayList<>(); + Map> current = new HashMap<>(); + int pairs = 0; + for (Map.Entry> entry : valuesByOrgUnitId.entrySet()) { + for (String value : entry.getValue()) { + if (pairs == MAX_VALUES_PER_QUERY) { + partitions.add(current); + current = new HashMap<>(); + pairs = 0; + } + current.computeIfAbsent(entry.getKey(), k -> new HashSet<>()).add(value); + pairs++; + } + } + if (!current.isEmpty()) { + partitions.add(current); + } + return partitions; + } - return getList(typedQuery); + private static List lowerCase(Set values) { + return values.stream().map(StringUtils::lowerCase).distinct().toList(); } @Override diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheat.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheat.java index ca7d6efdceee..561355d2b807 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheat.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheat.java @@ -293,11 +293,10 @@ public MetadataIdentifier getCategoryOptionComboIdentifier( private final Map users = Maps.newHashMap(); /** - * A list of all unique attribute values that are both present in the payload and in the database. - * This is going to be used to validate the uniqueness of attribute values in the Validation - * phase. + * {@link UniqueAttributeValue}s grouped by {@link #caseInsensitiveKey(String)} of their value, so + * the validation only compares a value against the few entries sharing it instead of all of them. */ - @Getter @Setter private List uniqueAttributeValues = Lists.newArrayList(); + private Map> uniqueAttributeValuesByValue = Map.of(); /** A list of all Enrollment UID having at least one Event that is not deleted. */ @Getter @Setter @@ -646,6 +645,35 @@ public Optional getUserByUid(String uid) { return this.users.values().stream().filter(u -> Objects.equals(uid, u.getUid())).findAny(); } + public void setUniqueAttributeValues(List uniqueAttributeValues) { + this.uniqueAttributeValuesByValue = + uniqueAttributeValues.stream() + .collect(Collectors.groupingBy(v -> caseInsensitiveKey(v.value()))); + } + + /** + * Returns the unique attribute values whose value is equal, ignoring case, to {@code value}. The + * entries can belong to any unique attribute and org unit. + */ + @Nonnull + public List getUniqueAttributeValues(@Nonnull String value) { + return uniqueAttributeValuesByValue.getOrDefault(caseInsensitiveKey(value), List.of()); + } + + /** + * Folds each code point the same way {@link String#equalsIgnoreCase(String)} compares them (upper + * case, then lower case), so two values have the same key exactly when they are equal ignoring + * case. Lower casing alone would not do, e.g. for the Greek final sigma. + */ + public static String caseInsensitiveKey(@Nonnull String value) { + StringBuilder key = new StringBuilder(value.length()); + value + .codePoints() + .map(cp -> Character.toLowerCase(Character.toUpperCase(cp))) + .forEach(key::appendCodePoint); + return key.toString(); + } + public OrganisationUnit getOrganisationUnit(MetadataIdentifier id) { return get(OrganisationUnit.class, id); } diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/preheat/UniqueAttributeValue.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/preheat/UniqueAttributeValue.java index c56d4055f39f..8c65bde631e4 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/preheat/UniqueAttributeValue.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/preheat/UniqueAttributeValue.java @@ -27,28 +27,17 @@ */ package org.hisp.dhis.tracker.imports.preheat; -import lombok.AllArgsConstructor; -import lombok.EqualsAndHashCode; -import lombok.Getter; import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValue; import org.hisp.dhis.tracker.imports.domain.MetadataIdentifier; /** - * Simple pojo used to store the data for {@link TrackedEntityAttributeValue} that are needed in the - * Validation phase + * Simple record used to store the data for {@link TrackedEntityAttributeValue} that are needed in + * the Validation phase * + * @param orgUnit org unit of the tracked entity owning the value. Null when it is unknown (e.g. an + * enrollment of a tracked entity that does not exist) and for values from the DB of an + * attribute unique in the whole system, where the validation does not use it * @author Enrico Colasante */ -@Getter -@AllArgsConstructor -@EqualsAndHashCode -public class UniqueAttributeValue { - - private String teUid; - - private MetadataIdentifier attribute; - - private String value; - - private MetadataIdentifier orgUnit; -} +public record UniqueAttributeValue( + String te, MetadataIdentifier attribute, String value, MetadataIdentifier orgUnit) {} diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/preheat/supplier/UniqueAttributesSupplier.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/preheat/supplier/UniqueAttributesSupplier.java index 6ab6cca6668b..8ceabf5f7826 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/preheat/supplier/UniqueAttributesSupplier.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/preheat/supplier/UniqueAttributesSupplier.java @@ -28,28 +28,27 @@ package org.hisp.dhis.tracker.imports.preheat.supplier; import static java.util.stream.Collectors.groupingBy; -import static java.util.stream.Collectors.mapping; import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toMap; import static java.util.stream.Collectors.toSet; -import com.google.common.collect.Lists; import com.google.common.collect.Sets; -import java.util.Collection; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Set; -import java.util.TreeMap; import java.util.function.Function; import java.util.stream.Stream; import javax.annotation.Nonnull; import lombok.RequiredArgsConstructor; +import org.hisp.dhis.organisationunit.OrganisationUnit; import org.hisp.dhis.trackedentity.TrackedEntity; import org.hisp.dhis.trackedentity.TrackedEntityAttribute; import org.hisp.dhis.trackedentity.TrackedEntityAttributeService; -import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValue; import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValueService; +import org.hisp.dhis.trackedentityattributevalue.UniqueAttributeValueMatch; import org.hisp.dhis.tracker.imports.TrackerIdSchemeParams; import org.hisp.dhis.tracker.imports.domain.Attribute; import org.hisp.dhis.tracker.imports.domain.MetadataIdentifier; @@ -86,9 +85,7 @@ public void preheatAdd(TrackerObjects trackerObjects, TrackerPreheat preheat) { List uniqueAttributeValuesFromDB = getAlreadyPresentInDbUniqueValues( - preheat.getIdSchemes(), - allUniqueAttributesByTrackedEntity, - uniqueTrackedEntityAttributes); + preheat, allUniqueAttributesByTrackedEntity, uniqueTrackedEntityAttributes); List uniqueAttributeValues = Stream.concat( @@ -184,77 +181,127 @@ private Map> })); } + /** + * Finds the unique values sent for more than one tracked entity in the payload. Values are + * compared ignoring case, like the validation does, so the case folded value is the one added to + * the preheat. + */ private List getDuplicatedUniqueValuesInPayload( Map> allAttributes) { - - // TE grouped by attribute and value. - // Two attributes with the same value and uid are considered equals - TreeMap> - teByAttributeValue = - new TreeMap<>( - (o1, o2) -> { - if (o1.getValue().equalsIgnoreCase(o2.getValue()) - && Objects.equals(o1.getAttribute(), o2.getAttribute())) { - return 0; - } - return 1; - }); + Map> + trackedEntitiesByAttributeValue = new HashMap<>(); for (Map.Entry> entry : allAttributes.entrySet()) { for (Attribute attribute : entry.getValue()) { - if (!teByAttributeValue.containsKey(attribute)) { - teByAttributeValue.put(attribute, Lists.newArrayList()); - } - teByAttributeValue.get(attribute).add(entry.getKey()); + trackedEntitiesByAttributeValue + .computeIfAbsent( + new AttributeValueKey( + attribute.getAttribute(), + TrackerPreheat.caseInsensitiveKey(attribute.getValue())), + k -> new HashSet<>()) + .add(entry.getKey()); } } - // If an attribute and value appear in 2 or more tracked entities - // it means that is not unique - return teByAttributeValue.entrySet().stream() + return trackedEntitiesByAttributeValue.entrySet().stream() .filter(e -> e.getValue().size() > 1) - .flatMap(av -> buildUniqueAttributeValues(av.getKey(), av.getValue()).stream()) - .collect(toList()); + .flatMap( + e -> + e.getValue().stream() + .map( + te -> + new UniqueAttributeValue( + te.getUid(), + e.getKey().attribute(), + e.getKey().value(), + te.getOrgUnit()))) + .toList(); } - private Collection buildUniqueAttributeValues( - Attribute attribute, List teList) { - return teList.stream() - .map( - te -> - new UniqueAttributeValue( - te.getUid(), attribute.getAttribute(), attribute.getValue(), te.getOrgUnit())) - .collect(toList()); - } + private record AttributeValueKey(MetadataIdentifier attribute, String value) {} + /** + * Finds the values in the DB that collide with the unique values in the payload. Values of an + * attribute unique within an org unit are only looked up in the org unit of the tracked entity + * they are sent for, as the validation only compares them within that org unit. Values of an + * attribute unique in the whole system get no org unit, as the validation does not use it. + */ private List getAlreadyPresentInDbUniqueValues( - TrackerIdSchemeParams idSchemes, + TrackerPreheat preheat, Map> allAttributesByTrackedEntity, List uniqueTrackedEntityAttributes) { + Map> valuesByAttribute = new HashMap<>(); + Map>> valuesByOrgUnitIdByAttribute = + new HashMap<>(); + Map orgUnitsById = new HashMap<>(); - Map> uniqueValuesByAttribute = - allAttributesByTrackedEntity.values().stream() - .flatMap(Collection::stream) - .distinct() - .collect( - groupingBy( - a -> extractAttribute(a.getAttribute(), uniqueTrackedEntityAttributes), - mapping(Attribute::getValue, toList()))); + for (Map.Entry> entry : + allAttributesByTrackedEntity.entrySet()) { + for (Attribute attribute : entry.getValue()) { + TrackedEntityAttribute tea = + extractAttribute(attribute.getAttribute(), uniqueTrackedEntityAttributes); + if (tea != null && !tea.getOrgUnitScopeNullSafe()) { + valuesByAttribute.computeIfAbsent(tea, k -> new HashSet<>()).add(attribute.getValue()); + continue; + } - List uniqueAttributeAlreadyPresentInDB = - trackedEntityAttributeValueService.getUniqueAttributeByValues(uniqueValuesByAttribute); + // an org unit that cannot be resolved can't match any stored value in the validation + OrganisationUnit orgUnit = preheat.getOrganisationUnit(entry.getKey().getOrgUnit()); + if (orgUnit != null) { + orgUnitsById.put(orgUnit.getId(), orgUnit); + valuesByOrgUnitIdByAttribute + .computeIfAbsent(tea, k -> new HashMap<>()) + .computeIfAbsent(orgUnit.getId(), k -> new HashSet<>()) + .add(attribute.getValue()); + } + } + } - return uniqueAttributeAlreadyPresentInDB.stream() + TrackerIdSchemeParams idSchemes = preheat.getIdSchemes(); + List uniqueAttributeValues = new ArrayList<>(); + valuesByAttribute.forEach( + (attribute, values) -> + uniqueAttributeValues.addAll( + toUniqueAttributeValues( + idSchemes, + attribute, + orgUnitsById, + trackedEntityAttributeValueService.getUniqueAttributeValues( + attribute, values)))); + valuesByOrgUnitIdByAttribute.forEach( + (attribute, valuesByOrgUnitId) -> + uniqueAttributeValues.addAll( + toUniqueAttributeValues( + idSchemes, + attribute, + orgUnitsById, + trackedEntityAttributeValueService.getUniqueAttributeValues( + attribute, valuesByOrgUnitId)))); + return uniqueAttributeValues; + } + + private static List toUniqueAttributeValues( + TrackerIdSchemeParams idSchemes, + TrackedEntityAttribute attribute, + Map orgUnitsById, + List matches) { + MetadataIdentifier attributeIdentifier = idSchemes.toMetadataIdentifier(attribute); + // only org units of org unit scoped lookups are known. The org unit of a value unique in the + // whole system is not needed by the validation, and resolving it would need a query + boolean orgUnitScoped = attribute.getOrgUnitScopeNullSafe(); + return matches.stream() .map( - av -> + match -> new UniqueAttributeValue( - av.getTrackedEntity().getUid(), - idSchemes.toMetadataIdentifier(av.getAttribute()), - av.getValue(), - idSchemes.toMetadataIdentifier(av.getTrackedEntity().getOrganisationUnit()))) - .collect(toList()); + match.trackedEntity().getValue(), + attributeIdentifier, + match.value(), + orgUnitScoped + ? idSchemes.toMetadataIdentifier(orgUnitsById.get(match.orgUnitId())) + : null)) + .toList(); } private TrackedEntityAttribute extractAttribute( diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/AttributeValidator.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/AttributeValidator.java index b978caa92a9a..856fef6589ac 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/AttributeValidator.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/AttributeValidator.java @@ -33,7 +33,6 @@ import static org.hisp.dhis.tracker.imports.validation.validator.TrackerImporterAssertErrors.TRACKED_ENTITY_ATTRIBUTE_CANT_BE_NULL; import static org.hisp.dhis.tracker.imports.validation.validator.TrackerImporterAssertErrors.TRACKED_ENTITY_ATTRIBUTE_VALUE_CANT_BE_NULL; -import java.util.List; import java.util.Objects; import org.hisp.dhis.encryption.EncryptionStatus; import org.hisp.dhis.external.conf.DhisConfigurationProvider; @@ -94,20 +93,20 @@ protected void validateAttributeUniqueness( if (Boolean.FALSE.equals(trackedEntityAttribute.isUnique())) return; - List uniqueAttributeValues = preheat.getUniqueAttributeValues(); - - for (UniqueAttributeValue uniqueAttributeValue : uniqueAttributeValues) { - boolean isTeaUniqueInOrgUnitScope = - !trackedEntityAttribute.getOrgunitScope() - || uniqueAttributeValue.getOrgUnit().isEqualTo(organisationUnit); - - boolean isTheSameTea = uniqueAttributeValue.getAttribute().isEqualTo(trackedEntityAttribute); - boolean hasTheSameValue = value.equalsIgnoreCase(uniqueAttributeValue.getValue()); - boolean isNotSameTei = + for (UniqueAttributeValue uniqueAttributeValue : preheat.getUniqueAttributeValues(value)) { + boolean isTheSameTea = uniqueAttributeValue.attribute().isEqualTo(trackedEntityAttribute); + boolean hasTheSameValue = value.equalsIgnoreCase(uniqueAttributeValue.value()); + boolean isNotSameTe = trackedEntity == null - || !Objects.equals(trackedEntity.getUid(), uniqueAttributeValue.getTeUid()); + || !Objects.equals(trackedEntity.getUid(), uniqueAttributeValue.te()); - if (isTeaUniqueInOrgUnitScope && isTheSameTea && hasTheSameValue && isNotSameTei) { + // the org unit is only compared for entries of this attribute, as values of an attribute + // unique in the whole system have none + if (isTheSameTea + && hasTheSameValue + && isNotSameTe + && (!trackedEntityAttribute.getOrgunitScope() + || uniqueAttributeValue.orgUnit().isEqualTo(organisationUnit))) { reporter.addError(dto, ValidationCode.E1064, value, trackedEntityAttribute); return; } diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatTest.java b/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatTest.java index eec662fd685b..458dda65851b 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatTest.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/imports/preheat/TrackerPreheatTest.java @@ -96,6 +96,43 @@ void shouldReturnTrueOnlyForTheExactOptionSetAndCodePairAdded() { assertFalse(preheat.isValidOptionCode(2L, "CODE")); } + @Test + void shouldReturnUniqueAttributeValuesEqualToValueIgnoringCase() { + UniqueAttributeValue upper = uniqueAttributeValue("ABC"); + UniqueAttributeValue lower = uniqueAttributeValue("abc"); + UniqueAttributeValue other = uniqueAttributeValue("abd"); + preheat.setUniqueAttributeValues(List.of(upper, lower, other)); + + assertEquals(List.of(upper, lower), preheat.getUniqueAttributeValues("aBc")); + assertEquals(List.of(other), preheat.getUniqueAttributeValues("ABD")); + } + + @Test + void shouldReturnUniqueAttributeValuesEqualIgnoringCaseWhenLowerCaseDiffers() { + // final and non-final sigma lower case differently but are equal ignoring case + UniqueAttributeValue finalSigma = uniqueAttributeValue("σας"); + preheat.setUniqueAttributeValues(List.of(finalSigma)); + + assertTrue("σασ".equalsIgnoreCase("σας")); + assertEquals(List.of(finalSigma), preheat.getUniqueAttributeValues("σασ")); + } + + @Test + void shouldReturnNoUniqueAttributeValuesWhenNoneHasTheValue() { + preheat.setUniqueAttributeValues(List.of(uniqueAttributeValue("abc"))); + + assertEquals(List.of(), preheat.getUniqueAttributeValues("abcd")); + assertEquals(List.of(), new TrackerPreheat().getUniqueAttributeValues("abc")); + } + + private static UniqueAttributeValue uniqueAttributeValue(String value) { + return new UniqueAttributeValue( + CodeGenerator.generateUid(), + MetadataIdentifier.ofUid(CodeGenerator.generateUid()), + value, + MetadataIdentifier.ofUid(CodeGenerator.generateUid())); + } + @Test void shouldReturnFalseForOptionSetThatWasNeverResolved() { assertFalse(preheat.isOptionSetResolved(1L)); diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/imports/preheat/supplier/UniqueAttributeSupplierTest.java b/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/imports/preheat/supplier/UniqueAttributeSupplierTest.java index bcbfc8831da1..5562d02919fc 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/imports/preheat/supplier/UniqueAttributeSupplierTest.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/imports/preheat/supplier/UniqueAttributeSupplierTest.java @@ -29,13 +29,18 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.hasSize; -import static org.hamcrest.Matchers.is; +import static org.hisp.dhis.utils.Assertions.assertContainsOnly; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import com.google.common.collect.Lists; +import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Set; import org.hisp.dhis.DhisConvenienceTest; import org.hisp.dhis.attribute.Attribute; import org.hisp.dhis.attribute.AttributeValue; @@ -48,11 +53,14 @@ import org.hisp.dhis.trackedentity.TrackedEntity; import org.hisp.dhis.trackedentity.TrackedEntityAttribute; import org.hisp.dhis.trackedentity.TrackedEntityAttributeService; -import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValue; import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValueService; +import org.hisp.dhis.trackedentityattributevalue.UniqueAttributeValueMatch; +import org.hisp.dhis.tracker.imports.TrackerIdSchemeParam; +import org.hisp.dhis.tracker.imports.TrackerIdSchemeParams; import org.hisp.dhis.tracker.imports.domain.MetadataIdentifier; import org.hisp.dhis.tracker.imports.domain.TrackerObjects; import org.hisp.dhis.tracker.imports.preheat.TrackerPreheat; +import org.hisp.dhis.tracker.imports.preheat.UniqueAttributeValue; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -88,14 +96,14 @@ class UniqueAttributeSupplierTest extends DhisConvenienceTest { private Enrollment enrollment; - private TrackedEntityAttributeValue trackedEntityAttributeValue; + private OrganisationUnit orgUnit; @BeforeEach void setUp() { params = TrackerObjects.builder().build(); preheat = new TrackerPreheat(); uniqueAttribute = createTrackedEntityAttribute('A', ValueType.TEXT); - OrganisationUnit orgUnit = createOrganisationUnit('A'); + orgUnit = createOrganisationUnit('A'); Program program = createProgram('A'); Attribute attribute = createAttribute('A'); AttributeValue attributeValue = createAttributeValue(attribute, UNIQUE_VALUE); @@ -104,8 +112,6 @@ void setUp() { trackedEntity.setAttributeValues(Collections.singleton(attributeValue)); enrollment = createEnrollment(program, trackedEntity, orgUnit); enrollment.setAttributeValues(Collections.singleton(attributeValue)); - trackedEntityAttributeValue = - createTrackedEntityAttributeValue('A', trackedEntity, uniqueAttribute); } @Test @@ -115,7 +121,7 @@ void verifySupplierWhenNoUniqueAttributeIsPresentInTheSystem() { this.supplier.preheatAdd(params, preheat); - assertThat(preheat.getUniqueAttributeValues(), hasSize(0)); + assertThat(preheat.getUniqueAttributeValues(UNIQUE_VALUE), hasSize(0)); } @Test @@ -130,7 +136,7 @@ void verifySupplierWhenTeAndEnrollmentHaveTheSameUniqueAttribute() { this.supplier.preheatAdd(importParams, preheat); - assertThat(preheat.getUniqueAttributeValues(), hasSize(0)); + assertThat(preheat.getUniqueAttributeValues(UNIQUE_VALUE), hasSize(0)); } @Test @@ -144,7 +150,7 @@ void verifySupplierWhenTwoTesHaveAttributeWithSameUniqueValue() { this.supplier.preheatAdd(importParams, preheat); - assertThat(preheat.getUniqueAttributeValues(), hasSize(2)); + assertThat(preheat.getUniqueAttributeValues(UNIQUE_VALUE), hasSize(2)); } @Test @@ -159,7 +165,7 @@ void verifySupplierWhenTeAndEnrollmentFromAnotherTeHaveAttributeWithSameUniqueVa this.supplier.preheatAdd(importParams, preheat); - assertThat(preheat.getUniqueAttributeValues(), hasSize(2)); + assertThat(preheat.getUniqueAttributeValues(UNIQUE_VALUE), hasSize(2)); } @Test @@ -171,7 +177,7 @@ void shouldNotFlagAsDuplicateWhenUniqueAttributeValueIsNullForBothTrackedEntitie this.supplier.preheatAdd(importParams, preheat); - assertThat(preheat.getUniqueAttributeValues(), hasSize(0)); + assertThat(preheat.getUniqueAttributeValues(UNIQUE_VALUE), hasSize(0)); } @Test @@ -182,26 +188,61 @@ void shouldFlagAsDuplicateWhenUniqueAttributeValuesHaveDifferentCasing() { TrackerObjects.builder() .trackedEntities( List.of( - trackedEntityWithAttributeValue(UID.of(TE_UID), UNIQUE_VALUE.toUpperCase()), - trackedEntityWithAttributeValue( - UID.of(ANOTHER_TE_UID), UNIQUE_VALUE.toLowerCase()))) + trackedEntityWithAttributeValue(TE_UID, UNIQUE_VALUE.toUpperCase()), + trackedEntityWithAttributeValue(ANOTHER_TE_UID, UNIQUE_VALUE.toLowerCase()))) .build(); this.supplier.preheatAdd(importParams, preheat); - assertThat(preheat.getUniqueAttributeValues(), hasSize(2)); + assertThat(preheat.getUniqueAttributeValues(UNIQUE_VALUE), hasSize(2)); + } + + @Test + void shouldFlagAsDuplicateWhenPayloadHasManyTrackedEntitiesWithDistinctUniqueValues() { + when(trackedEntityAttributeService.getAllUniqueTrackedEntityAttributes()) + .thenReturn(Collections.singletonList(uniqueAttribute)); + List trackedEntities = + new ArrayList<>(sameUniqueAttributeTrackedEntities(UNIQUE_VALUE)); + for (int i = 0; i < 20; i++) { + trackedEntities.add( + trackedEntityWithAttributeValue(CodeGenerator.generateUid(), "value " + i)); + } + TrackerObjects importParams = TrackerObjects.builder().trackedEntities(trackedEntities).build(); + + this.supplier.preheatAdd(importParams, preheat); + + assertContainsOnly( + List.of( + new UniqueAttributeValue( + TE_UID, MetadataIdentifier.ofUid(uniqueAttribute), UNIQUE_VALUE, null), + new UniqueAttributeValue( + ANOTHER_TE_UID, MetadataIdentifier.ofUid(uniqueAttribute), UNIQUE_VALUE, null)), + preheat.getUniqueAttributeValues(UNIQUE_VALUE)); + } + + @Test + void shouldNotFlagAsDuplicateWhenTeAndItsEnrollmentHaveUniqueValueWithDifferentCasing() { + when(trackedEntityAttributeService.getAllUniqueTrackedEntityAttributes()) + .thenReturn(Collections.singletonList(uniqueAttribute)); + TrackerObjects importParams = + TrackerObjects.builder() + .trackedEntities( + List.of(trackedEntityWithAttributeValue(TE_UID, UNIQUE_VALUE.toUpperCase()))) + .enrollments(Collections.singletonList(enrollment(TE_UID))) + .build(); + + this.supplier.preheatAdd(importParams, preheat); + + assertThat(preheat.getUniqueAttributeValues(UNIQUE_VALUE), hasSize(0)); } @Test void verifySupplierWhenTeinPayloadAndDBHaveTheSameUniqueAttribute() { when(trackedEntityAttributeService.getAllUniqueTrackedEntityAttributes()) .thenReturn(Collections.singletonList(uniqueAttribute)); - Map> trackedEntityAttributeListMap = - Map.of(uniqueAttribute, List.of(UNIQUE_VALUE)); - List attributeValues = List.of(trackedEntityAttributeValue); - when(trackedEntityAttributeValueService.getUniqueAttributeByValues( - trackedEntityAttributeListMap)) - .thenReturn(attributeValues); + when(trackedEntityAttributeValueService.getUniqueAttributeValues( + uniqueAttribute, Set.of(UNIQUE_VALUE))) + .thenReturn(List.of(new UniqueAttributeValueMatch(UID.of(TE_UID), UNIQUE_VALUE, null))); TrackerObjects importParams = TrackerObjects.builder() .trackedEntities(Collections.singletonList(trackedEntity())) @@ -209,19 +250,147 @@ void verifySupplierWhenTeinPayloadAndDBHaveTheSameUniqueAttribute() { this.supplier.preheatAdd(importParams, preheat); - assertThat(preheat.getUniqueAttributeValues(), hasSize(1)); + assertThat(preheat.getUniqueAttributeValues(UNIQUE_VALUE), hasSize(1)); } @Test void verifySupplierWhenTeinPayloadAndAnotherTeInDBHaveTheSameUniqueAttribute() { when(trackedEntityAttributeService.getAllUniqueTrackedEntityAttributes()) .thenReturn(Collections.singletonList(uniqueAttribute)); - Map> trackedEntityAttributeListMap = - Map.of(uniqueAttribute, List.of(UNIQUE_VALUE)); - List attributeValues = List.of(trackedEntityAttributeValue); - when(trackedEntityAttributeValueService.getUniqueAttributeByValues( - trackedEntityAttributeListMap)) - .thenReturn(attributeValues); + when(trackedEntityAttributeValueService.getUniqueAttributeValues( + uniqueAttribute, Set.of(UNIQUE_VALUE))) + .thenReturn(List.of(new UniqueAttributeValueMatch(UID.of(TE_UID), UNIQUE_VALUE, null))); + TrackerObjects importParams = + TrackerObjects.builder() + .trackedEntities(Collections.singletonList(anotherTrackedEntity())) + .build(); + + this.supplier.preheatAdd(importParams, preheat); + + assertThat(preheat.getUniqueAttributeValues(UNIQUE_VALUE), hasSize(1)); + assertEquals(TE_UID, preheat.getUniqueAttributeValues(UNIQUE_VALUE).get(0).te()); + } + + @Test + void shouldMapOrgUnitScopedValuesFoundInDbUsingTheImportIdSchemes() { + TrackedEntityAttribute scopedAttribute = scopedUniqueAttribute(); + orgUnit.setId(1); + orgUnit.setCode("OU_CODE"); + preheat.setIdSchemes( + TrackerIdSchemeParams.builder().orgUnitIdScheme(TrackerIdSchemeParam.CODE).build()); + preheat.put(TrackerIdSchemeParam.CODE, orgUnit); + when(trackedEntityAttributeService.getAllUniqueTrackedEntityAttributes()) + .thenReturn(List.of(scopedAttribute)); + when(trackedEntityAttributeValueService.getUniqueAttributeValues( + scopedAttribute, Map.of(1L, Set.of(UNIQUE_VALUE)))) + .thenReturn(List.of(new UniqueAttributeValueMatch(UID.of(TE_UID), "Unique Value", 1L))); + TrackerObjects importParams = + TrackerObjects.builder() + .trackedEntities( + List.of( + org.hisp.dhis.tracker.imports.domain.TrackedEntity.builder() + .trackedEntity(ANOTHER_TE_UID) + .orgUnit(MetadataIdentifier.ofCode("OU_CODE")) + .attributes(List.of(value(scopedAttribute, UNIQUE_VALUE))) + .build())) + .build(); + + this.supplier.preheatAdd(importParams, preheat); + + assertEquals( + List.of( + new UniqueAttributeValue( + TE_UID, + MetadataIdentifier.ofUid(scopedAttribute), + "Unique Value", + MetadataIdentifier.ofCode("OU_CODE"))), + preheat.getUniqueAttributeValues("Unique Value")); + } + + @Test + void shouldLookUpOrgUnitScopedValuesOnlyInTheOrgUnitOfTheTrackedEntitiesSendingThem() { + TrackedEntityAttribute scopedAttribute = scopedUniqueAttribute(); + OrganisationUnit orgUnit1 = orgUnitInPreheat('1', 1); + OrganisationUnit orgUnit2 = orgUnitInPreheat('2', 2); + OrganisationUnit orgUnit3 = orgUnitInPreheat('3', 3); + when(trackedEntityAttributeService.getAllUniqueTrackedEntityAttributes()) + .thenReturn(List.of(uniqueAttribute, scopedAttribute)); + TrackerObjects importParams = + TrackerObjects.builder() + .trackedEntities( + List.of( + trackedEntity( + orgUnit1, value(uniqueAttribute, "g1"), value(scopedAttribute, "s1")), + trackedEntity( + orgUnit1, value(uniqueAttribute, "g2"), value(scopedAttribute, "s2")), + trackedEntity( + orgUnit1, value(uniqueAttribute, "g3"), value(scopedAttribute, "s3")), + // same scoped value as in orgUnit1, which is allowed in another org unit + trackedEntity( + orgUnit2, value(uniqueAttribute, "g4"), value(scopedAttribute, "s1")), + // no scoped value, so orgUnit3 must not be part of the scoped lookup + trackedEntity(orgUnit3, value(uniqueAttribute, "g5")))) + .build(); + + this.supplier.preheatAdd(importParams, preheat); + + verify(trackedEntityAttributeValueService) + .getUniqueAttributeValues(uniqueAttribute, Set.of("g1", "g2", "g3", "g4", "g5")); + verify(trackedEntityAttributeValueService) + .getUniqueAttributeValues( + scopedAttribute, + Map.of(orgUnit1.getId(), Set.of("s1", "s2", "s3"), orgUnit2.getId(), Set.of("s1"))); + } + + @Test + void shouldLookUpOrgUnitScopedValueOfEnrollmentInTheOrgUnitOfItsTrackedEntityInDb() { + TrackedEntityAttribute scopedAttribute = scopedUniqueAttribute(); + OrganisationUnit dbOrgUnit = orgUnitInPreheat('D', 4); + trackedEntity.setOrganisationUnit(dbOrgUnit); + preheat.putTrackedEntities(List.of(trackedEntity)); + when(trackedEntityAttributeService.getAllUniqueTrackedEntityAttributes()) + .thenReturn(List.of(scopedAttribute)); + TrackerObjects importParams = + TrackerObjects.builder() + .enrollments( + List.of( + org.hisp.dhis.tracker.imports.domain.Enrollment.builder() + .enrollment(CodeGenerator.generateUid()) + .trackedEntity(TE_UID) + .attributes(List.of(value(scopedAttribute, "s1"))) + .build())) + .build(); + + this.supplier.preheatAdd(importParams, preheat); + + verify(trackedEntityAttributeValueService) + .getUniqueAttributeValues(scopedAttribute, Map.of(dbOrgUnit.getId(), Set.of("s1"))); + } + + @Test + void shouldNotLookUpOrgUnitScopedValueWhenOrgUnitCannotBeResolved() { + TrackedEntityAttribute scopedAttribute = scopedUniqueAttribute(); + OrganisationUnit notInPreheat = createOrganisationUnit('N'); + when(trackedEntityAttributeService.getAllUniqueTrackedEntityAttributes()) + .thenReturn(List.of(scopedAttribute)); + TrackerObjects importParams = + TrackerObjects.builder() + .trackedEntities(List.of(trackedEntity(notInPreheat, value(scopedAttribute, "s1")))) + .build(); + + this.supplier.preheatAdd(importParams, preheat); + + verifyNoInteractions(trackedEntityAttributeValueService); + assertThat(preheat.getUniqueAttributeValues("s1"), hasSize(0)); + } + + @Test + void shouldAddValuesFoundInDbWithoutOrgUnitWhenAttributeIsUniqueInTheSystem() { + when(trackedEntityAttributeService.getAllUniqueTrackedEntityAttributes()) + .thenReturn(Collections.singletonList(uniqueAttribute)); + when(trackedEntityAttributeValueService.getUniqueAttributeValues( + uniqueAttribute, Set.of(UNIQUE_VALUE))) + .thenReturn(List.of(new UniqueAttributeValueMatch(UID.of(TE_UID), UNIQUE_VALUE, 42L))); TrackerObjects importParams = TrackerObjects.builder() .trackedEntities(Collections.singletonList(anotherTrackedEntity())) @@ -229,21 +398,55 @@ void verifySupplierWhenTeinPayloadAndAnotherTeInDBHaveTheSameUniqueAttribute() { this.supplier.preheatAdd(importParams, preheat); - assertThat(preheat.getUniqueAttributeValues(), hasSize(1)); - assertThat(preheat.getUniqueAttributeValues().get(0).getTeUid(), is(TE_UID)); + assertEquals( + List.of( + new UniqueAttributeValue( + TE_UID, MetadataIdentifier.ofUid(uniqueAttribute), UNIQUE_VALUE, null)), + preheat.getUniqueAttributeValues(UNIQUE_VALUE)); + } + + private TrackedEntityAttribute scopedUniqueAttribute() { + TrackedEntityAttribute attribute = createTrackedEntityAttribute('S', ValueType.TEXT); + attribute.setUnique(true); + attribute.setOrgunitScope(true); + return attribute; + } + + private OrganisationUnit orgUnitInPreheat(char uniqueChar, long id) { + OrganisationUnit ou = createOrganisationUnit(uniqueChar); + ou.setId(id); + preheat.put(TrackerIdSchemeParam.UID, ou); + return ou; + } + + private static org.hisp.dhis.tracker.imports.domain.TrackedEntity trackedEntity( + OrganisationUnit orgUnit, org.hisp.dhis.tracker.imports.domain.Attribute... attributes) { + return org.hisp.dhis.tracker.imports.domain.TrackedEntity.builder() + .trackedEntity(CodeGenerator.generateUid()) + .orgUnit(MetadataIdentifier.ofUid(orgUnit)) + .attributes(List.of(attributes)) + .build(); + } + + private static org.hisp.dhis.tracker.imports.domain.Attribute value( + TrackedEntityAttribute attribute, String value) { + return org.hisp.dhis.tracker.imports.domain.Attribute.builder() + .attribute(MetadataIdentifier.ofUid(attribute)) + .value(value) + .build(); } private List sameUniqueAttributeTrackedEntities(String value) { return Lists.newArrayList( - trackedEntityWithAttributeValue(UID.of(TE_UID), value), - trackedEntityWithAttributeValue(UID.of(ANOTHER_TE_UID), value)); + trackedEntityWithAttributeValue(TE_UID, value), + trackedEntityWithAttributeValue(ANOTHER_TE_UID, value)); } private org.hisp.dhis.tracker.imports.domain.TrackedEntity trackedEntityWithAttributeValue( - UID teUid, String value) { + String teUid, String value) { return org.hisp.dhis.tracker.imports.domain.TrackedEntity.builder() - .trackedEntity(teUid.getValue()) + .trackedEntity(teUid) .attributes(Collections.singletonList(attributeWithValue(value))) .build(); } diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/trackedentity/AttributeValidatorTest.java b/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/trackedentity/AttributeValidatorTest.java index 41de6dc40326..ba92c396d74e 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/trackedentity/AttributeValidatorTest.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/trackedentity/AttributeValidatorTest.java @@ -47,6 +47,7 @@ import org.hisp.dhis.external.conf.DhisConfigurationProvider; import org.hisp.dhis.fileresource.FileResource; import org.hisp.dhis.option.OptionSet; +import org.hisp.dhis.organisationunit.OrganisationUnit; import org.hisp.dhis.trackedentity.TrackedEntityAttribute; import org.hisp.dhis.trackedentity.TrackedEntityType; import org.hisp.dhis.trackedentity.TrackedEntityTypeAttribute; @@ -81,6 +82,8 @@ class AttributeValidatorTest { private static final String MANDATORY_ATTRIBUTE_UID = "cFr94jf03AA"; + private static final MetadataIdentifier ORG_UNIT_ID = MetadataIdentifier.ofUid("orgUnitUid1"); + @InjectMocks private AttributeValidator validator; @Mock private TrackerPreheat preheat; @@ -399,7 +402,7 @@ void shouldFailValidationWhenUniqueAttributeHasSameValueWithDifferentCase() { .thenReturn(trackedEntityAttribute); when(preheat.getTrackedEntityType((MetadataIdentifier) any())) .thenReturn(new TrackedEntityType()); - when(preheat.getUniqueAttributeValues()) + when(preheat.getUniqueAttributeValues("abc")) .thenReturn( List.of( new UniqueAttributeValue( @@ -425,6 +428,128 @@ void shouldFailValidationWhenUniqueAttributeHasSameValueWithDifferentCase() { assertHasError(reporter, trackedEntity, ValidationCode.E1064); } + @Test + void shouldFailValidationWhenOrgUnitScopedValueIsStoredForAnotherTrackedEntityInSameOrgUnit() { + TrackedEntity trackedEntity = trackedEntityWithScopedValue("abc"); + + validator.validate( + reporter, + bundle, + withStoredScopedValue( + trackedEntity, + new UniqueAttributeValue( + CodeGenerator.generateUid(), MetadataIdentifier.ofUid("uid"), "ABC", ORG_UNIT_ID))); + + assertHasError(reporter, trackedEntity, ValidationCode.E1064); + } + + @Test + void shouldPassValidationWhenOrgUnitScopedValueIsStoredInAnotherOrgUnit() { + TrackedEntity trackedEntity = trackedEntityWithScopedValue("abc"); + + validator.validate( + reporter, + bundle, + withStoredScopedValue( + trackedEntity, + new UniqueAttributeValue( + CodeGenerator.generateUid(), + MetadataIdentifier.ofUid("uid"), + "abc", + MetadataIdentifier.ofUid("otherOrgUni")))); + + assertIsEmpty(reporter.getErrors()); + } + + @Test + void shouldPassValidationWhenOrgUnitScopedValueIsStoredForTheSameTrackedEntity() { + TrackedEntity trackedEntity = trackedEntityWithScopedValue("abc"); + org.hisp.dhis.trackedentity.TrackedEntity storedTrackedEntity = + new org.hisp.dhis.trackedentity.TrackedEntity(); + storedTrackedEntity.setUid(trackedEntity.getUid()); + when(preheat.getTrackedEntity(trackedEntity.getUid())).thenReturn(storedTrackedEntity); + + validator.validate( + reporter, + bundle, + withStoredScopedValue( + trackedEntity, + new UniqueAttributeValue( + trackedEntity.getUid(), MetadataIdentifier.ofUid("uid"), "abc", ORG_UNIT_ID))); + + assertIsEmpty(reporter.getErrors()); + } + + @Test + void shouldPassValidationWhenSameValueIsStoredForAnotherAttributeInSameOrgUnit() { + TrackedEntity trackedEntity = trackedEntityWithScopedValue("abc"); + + validator.validate( + reporter, + bundle, + withStoredScopedValue( + trackedEntity, + new UniqueAttributeValue( + CodeGenerator.generateUid(), + MetadataIdentifier.ofUid("otherUid"), + "abc", + ORG_UNIT_ID))); + + assertIsEmpty(reporter.getErrors()); + } + + @Test + void shouldPassValidationWhenSameValueIsStoredForAnAttributeUniqueInTheSystem() { + TrackedEntity trackedEntity = trackedEntityWithScopedValue("abc"); + + // values from the DB of an attribute unique in the whole system have no org unit + validator.validate( + reporter, + bundle, + withStoredScopedValue( + trackedEntity, + new UniqueAttributeValue( + CodeGenerator.generateUid(), MetadataIdentifier.ofUid("globalUid"), "abc", null))); + + assertIsEmpty(reporter.getErrors()); + } + + /** A tracked entity in {@link #ORG_UNIT_ID} sending a value of an org unit scoped attribute. */ + private TrackedEntity trackedEntityWithScopedValue(String value) { + TrackedEntityAttribute trackedEntityAttribute = new TrackedEntityAttribute(); + trackedEntityAttribute.setUid("uid"); + trackedEntityAttribute.setValueType(ValueType.TEXT); + trackedEntityAttribute.setUnique(true); + trackedEntityAttribute.setOrgunitScope(true); + OrganisationUnit orgUnit = new OrganisationUnit(); + orgUnit.setUid(ORG_UNIT_ID.getIdentifier()); + + when(preheat.getTrackedEntityAttribute((MetadataIdentifier) any())) + .thenReturn(trackedEntityAttribute); + when(preheat.getTrackedEntityType((MetadataIdentifier) any())) + .thenReturn(new TrackedEntityType()); + when(preheat.getOrganisationUnit(ORG_UNIT_ID)).thenReturn(orgUnit); + + return TrackedEntity.builder() + .trackedEntity(CodeGenerator.generateUid()) + .orgUnit(ORG_UNIT_ID) + .attributes( + List.of( + Attribute.builder() + .attribute(MetadataIdentifier.ofUid("uid")) + .value(value) + .build())) + .trackedEntityType(MetadataIdentifier.ofUid("trackedEntityType")) + .build(); + } + + private TrackedEntity withStoredScopedValue( + TrackedEntity trackedEntity, UniqueAttributeValue uniqueAttributeValue) { + when(preheat.getUniqueAttributeValues(trackedEntity.getAttributes().get(0).getValue())) + .thenReturn(List.of(uniqueAttributeValue)); + return trackedEntity; + } + @Test void shouldFailEncryptionStatus() { TrackedEntityAttribute trackedEntityAttribute = new TrackedEntityAttribute(); diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/trackedentityattributevalue/UniqueAttributeValueLookupTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/trackedentityattributevalue/UniqueAttributeValueLookupTest.java new file mode 100644 index 000000000000..2109454eb937 --- /dev/null +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/trackedentityattributevalue/UniqueAttributeValueLookupTest.java @@ -0,0 +1,214 @@ +/* + * Copyright (c) 2004-2022, 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.trackedentityattributevalue; + +import static org.hisp.dhis.utils.Assertions.assertContainsOnly; +import static org.hisp.dhis.utils.Assertions.assertIsEmpty; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.stream.IntStream; +import org.hisp.dhis.common.IdentifiableObjectManager; +import org.hisp.dhis.common.UID; +import org.hisp.dhis.organisationunit.OrganisationUnit; +import org.hisp.dhis.test.config.QueryCountDataSourceProxy; +import org.hisp.dhis.test.integration.TransactionalIntegrationTest; +import org.hisp.dhis.trackedentity.TrackedEntity; +import org.hisp.dhis.trackedentity.TrackedEntityAttribute; +import org.hisp.dhis.trackedentity.TrackedEntityType; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.test.context.ContextConfiguration; + +/** + * Tests the lookup of stored unique attribute values done during the tracker import preheat, see + * {@link TrackedEntityAttributeValueService#getUniqueAttributeValues}. + */ +@ContextConfiguration(classes = {QueryCountDataSourceProxy.class}) +class UniqueAttributeValueLookupTest extends TransactionalIntegrationTest { + + @Autowired private TrackedEntityAttributeValueService attributeValueService; + + @Autowired private IdentifiableObjectManager manager; + + private OrganisationUnit orgUnitA; + + private OrganisationUnit orgUnitB; + + private TrackedEntityType trackedEntityType; + + private TrackedEntityAttribute globalAttribute; + + private TrackedEntityAttribute scopedAttribute; + + @BeforeEach + void setUp() { + orgUnitA = createOrganisationUnit('A'); + orgUnitB = createOrganisationUnit('B'); + manager.save(orgUnitA); + manager.save(orgUnitB); + + trackedEntityType = createTrackedEntityType('A'); + manager.save(trackedEntityType); + + globalAttribute = createTrackedEntityAttribute('G'); + globalAttribute.setUnique(true); + manager.save(globalAttribute); + + scopedAttribute = createTrackedEntityAttribute('S'); + scopedAttribute.setUnique(true); + scopedAttribute.setOrgunitScope(true); + manager.save(scopedAttribute); + } + + @Test + void shouldFindValuesInAnyOrgUnitIgnoringCaseWhenAttributeIsUniqueInTheSystem() { + TrackedEntity trackedEntityA = trackedEntityWithValue(orgUnitA, globalAttribute, "Value1"); + TrackedEntity trackedEntityB = trackedEntityWithValue(orgUnitB, globalAttribute, "value2"); + trackedEntityWithValue(orgUnitB, globalAttribute, "value3"); + + List matches = + attributeValueService.getUniqueAttributeValues( + globalAttribute, Set.of("VALUE1", "value2", "not stored")); + + assertContainsOnly( + List.of( + new UniqueAttributeValueMatch(UID.of(trackedEntityA), "Value1", orgUnitA.getId()), + new UniqueAttributeValueMatch(UID.of(trackedEntityB), "value2", orgUnitB.getId())), + matches); + } + + @Test + void shouldFindOnlyRequestedOrgUnitValuePairsWhenAttributeIsUniqueInOrgUnit() { + TrackedEntity trackedEntityA1 = trackedEntityWithValue(orgUnitA, scopedAttribute, "Value1"); + // value1 is only requested for orgUnitA, so this one is not a match + trackedEntityWithValue(orgUnitB, scopedAttribute, "value1"); + TrackedEntity trackedEntityB2 = trackedEntityWithValue(orgUnitB, scopedAttribute, "value2"); + // value2 is only requested for orgUnitB, so this one is not a match + trackedEntityWithValue(orgUnitA, scopedAttribute, "value2"); + + List matches = + attributeValueService.getUniqueAttributeValues( + scopedAttribute, + Map.of(orgUnitA.getId(), Set.of("value1"), orgUnitB.getId(), Set.of("VALUE2"))); + + assertContainsOnly( + List.of( + new UniqueAttributeValueMatch(UID.of(trackedEntityA1), "Value1", orgUnitA.getId()), + new UniqueAttributeValueMatch(UID.of(trackedEntityB2), "value2", orgUnitB.getId())), + matches); + } + + @Test + void shouldNotFindValuesOfAnotherAttribute() { + trackedEntityWithValue(orgUnitA, globalAttribute, "value1"); + + assertIsEmpty( + attributeValueService.getUniqueAttributeValues( + scopedAttribute, Map.of(orgUnitA.getId(), Set.of("value1")))); + } + + @Test + void shouldReturnEmptyWhenThereAreNoValuesToLookUp() { + trackedEntityWithValue(orgUnitA, globalAttribute, "value1"); + + assertIsEmpty(attributeValueService.getUniqueAttributeValues(globalAttribute, Set.of())); + assertIsEmpty(attributeValueService.getUniqueAttributeValues(scopedAttribute, Map.of())); + } + + @Test + void shouldFindAllValuesWhenLookupIsSplitAcrossSeveralQueries() { + TrackedEntity globalMatch = trackedEntityWithValue(orgUnitA, globalAttribute, "value1"); + TrackedEntity scopedMatch = trackedEntityWithValue(orgUnitB, scopedAttribute, "value1"); + + Set values = new HashSet<>(Set.of("value1")); + IntStream.range(0, 20_000).forEach(i -> values.add("not stored " + i)); + // org unit ids that do not exist, pushing the real pair past the first query + Map> valuesByOrgUnitId = new HashMap<>(); + IntStream.range(0, 20_000) + .forEach(i -> valuesByOrgUnitId.put(-1L - i, Set.of("value1", "value2"))); + // more values than one query takes, all in the same org unit + Set orgUnitBValues = new HashSet<>(Set.of("value1")); + IntStream.range(0, 20_000).forEach(i -> orgUnitBValues.add("not stored " + i)); + valuesByOrgUnitId.put(orgUnitB.getId(), orgUnitBValues); + + assertContainsOnly( + List.of(new UniqueAttributeValueMatch(UID.of(globalMatch), "value1", orgUnitA.getId())), + attributeValueService.getUniqueAttributeValues(globalAttribute, values)); + assertContainsOnly( + List.of(new UniqueAttributeValueMatch(UID.of(scopedMatch), "value1", orgUnitB.getId())), + attributeValueService.getUniqueAttributeValues(scopedAttribute, valuesByOrgUnitId)); + } + + @Test + void shouldRunOneQueryRegardlessOfTheNumberOfMatchingTrackedEntities() { + Set globalValues = new HashSet<>(); + for (int i = 0; i < 10; i++) { + trackedEntityWithValue(orgUnitA, globalAttribute, "global" + i); + trackedEntityWithValue(i % 2 == 0 ? orgUnitA : orgUnitB, scopedAttribute, "scoped" + i); + globalValues.add("global" + i); + } + Map> scopedValues = + Map.of( + orgUnitA.getId(), Set.of("scoped0", "scoped2", "scoped4", "scoped6", "scoped8"), + orgUnitB.getId(), Set.of("scoped1", "scoped3", "scoped5", "scoped7", "scoped9")); + // start from an empty session, as a tracked entity already in the session is not loaded again + manager.flush(); + manager.clear(); + + QueryCountDataSourceProxy.clearCapturedSql(); + List globalMatches = + attributeValueService.getUniqueAttributeValues(globalAttribute, globalValues); + long globalQueries = QueryCountDataSourceProxy.countCapturedSqlMatching("select"); + + QueryCountDataSourceProxy.clearCapturedSql(); + List scopedMatches = + attributeValueService.getUniqueAttributeValues(scopedAttribute, scopedValues); + long scopedQueries = QueryCountDataSourceProxy.countCapturedSqlMatching("select"); + + assertEquals(10, globalMatches.size()); + assertEquals(10, scopedMatches.size()); + assertEquals(1, globalQueries, "expected a single query, not one per matching tracked entity"); + assertEquals(1, scopedQueries, "expected a single query, not one per matching tracked entity"); + } + + private TrackedEntity trackedEntityWithValue( + OrganisationUnit orgUnit, TrackedEntityAttribute attribute, String value) { + TrackedEntity trackedEntity = createTrackedEntity(orgUnit); + trackedEntity.setTrackedEntityType(trackedEntityType); + manager.save(trackedEntity); + attributeValueService.addTrackedEntityAttributeValue( + new TrackedEntityAttributeValue(attribute, trackedEntity, value)); + return trackedEntity; + } +} diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/validation/TeTaEncryptionValidationTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/validation/TeTaEncryptionValidationTest.java index becef1371782..3dc95051579f 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/validation/TeTaEncryptionValidationTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/validation/TeTaEncryptionValidationTest.java @@ -27,6 +27,7 @@ */ package org.hisp.dhis.tracker.imports.validation; +import static org.hisp.dhis.tracker.Assertions.assertHasErrors; import static org.hisp.dhis.tracker.Assertions.assertHasOnlyErrors; import static org.hisp.dhis.tracker.Assertions.assertNoErrors; @@ -35,6 +36,7 @@ import org.hisp.dhis.tracker.imports.TrackerImportParams; import org.hisp.dhis.tracker.imports.TrackerImportService; import org.hisp.dhis.tracker.imports.TrackerImportStrategy; +import org.hisp.dhis.tracker.imports.domain.MetadataIdentifier; import org.hisp.dhis.tracker.imports.domain.TrackerObjects; import org.hisp.dhis.tracker.imports.report.ImportReport; import org.hisp.dhis.user.UserService; @@ -70,6 +72,47 @@ void testUniqueFailInOrgUnit() throws IOException { assertNoErrors(importReport); } + @Test + void shouldFailWhenOrgUnitScopedUniqueValueExistsForAnotherTrackedEntityInTheSameOrgUnit() + throws IOException { + TrackerImportParams params = TrackerImportParams.builder().build(); + TrackerObjects trackerObjects = + fromJson("tracker/validations/te-program_with_tea_unique_data_in_country.json"); + assertNoErrors(trackerImportService.importTracker(params, trackerObjects)); + + // another tracked entity with the same value, moved to the org unit of the first one + trackerObjects = fromJson("tracker/validations/te-program_with_tea_unique_data_in_region.json"); + trackerObjects.getTrackedEntities().get(0).setOrgUnit(MetadataIdentifier.ofUid("cNEZTkdAvmg")); + + ImportReport importReport = trackerImportService.importTracker(params, trackerObjects); + + assertHasOnlyErrors(importReport, ValidationCode.E1064); + } + + @Test + void shouldRejectNewTrackedEntitiesWithTheSameOrgUnitScopedValueInTheSameOrgUnit() + throws IOException { + TrackerObjects trackerObjects = countryAndRegionTrackedEntitiesWithTheSameValue(); + trackerObjects.getTrackedEntities().get(1).setOrgUnit(MetadataIdentifier.ofUid("cNEZTkdAvmg")); + + ImportReport importReport = + trackerImportService.importTracker(TrackerImportParams.builder().build(), trackerObjects); + + assertHasErrors(importReport, 2, ValidationCode.E1064); + } + + /** Two new tracked entities, in the country and in the region, with the same value 321. */ + private TrackerObjects countryAndRegionTrackedEntitiesWithTheSameValue() throws IOException { + TrackerObjects trackerObjects = + fromJson("tracker/validations/te-program_with_tea_unique_data_in_country.json"); + trackerObjects + .getTrackedEntities() + .addAll( + fromJson("tracker/validations/te-program_with_tea_unique_data_in_region.json") + .getTrackedEntities()); + return trackerObjects; + } + @Test void testUniqueFail() throws IOException { TrackerObjects trackerObjects = diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/validation/TrackedEntityImportValidationTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/validation/TrackedEntityImportValidationTest.java index 151493072a64..c2eff943a525 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/validation/TrackedEntityImportValidationTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/validation/TrackedEntityImportValidationTest.java @@ -41,6 +41,8 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import java.io.IOException; +import java.util.List; +import org.hisp.dhis.common.CodeGenerator; import org.hisp.dhis.feedback.ForbiddenException; import org.hisp.dhis.organisationunit.OrganisationUnit; import org.hisp.dhis.program.Program; @@ -52,6 +54,7 @@ import org.hisp.dhis.tracker.imports.TrackerImportParams; import org.hisp.dhis.tracker.imports.TrackerImportService; import org.hisp.dhis.tracker.imports.TrackerImportStrategy; +import org.hisp.dhis.tracker.imports.domain.Attribute; import org.hisp.dhis.tracker.imports.domain.TrackerObjects; import org.hisp.dhis.tracker.imports.report.ImportReport; import org.hisp.dhis.user.User; @@ -109,6 +112,35 @@ void failValidationWhenTrackedEntityAttributesHaveSameUniqueValues() throws IOEx assertHasErrors(importReport, 2, ValidationCode.E1064); } + @Test + void failValidationWhenTrackedEntityAttributesHaveSameUniqueValuesAmongManyTrackedEntities() + throws IOException { + TrackerImportParams params = TrackerImportParams.builder().build(); + TrackerObjects trackerObjects = fromJson("tracker/validations/te-with_unique_attributes.json"); + org.hisp.dhis.tracker.imports.domain.TrackedEntity duplicate = + trackerObjects.getTrackedEntities().get(0); + for (int i = 0; i < 20; i++) { + trackerObjects + .getTrackedEntities() + .add( + org.hisp.dhis.tracker.imports.domain.TrackedEntity.builder() + .trackedEntity(CodeGenerator.generateUid()) + .trackedEntityType(duplicate.getTrackedEntityType()) + .orgUnit(duplicate.getOrgUnit()) + .attributes( + List.of( + Attribute.builder() + .attribute(duplicate.getAttributes().get(0).getAttribute()) + .value("uniqueName" + i) + .build())) + .build()); + } + + ImportReport importReport = trackerImportService.importTracker(params, trackerObjects); + + assertHasErrors(importReport, 2, ValidationCode.E1064); + } + @Test void testTeValidationOkAll() throws IOException { TrackerObjects trackerObjects = fromJson("tracker/validations/te-data_with_different_ou.json");