From 23852b9e5307caa0ca08c4a403854719371cb74d Mon Sep 17 00:00:00 2001 From: Pa-Touche <47572440+Pa-Touche@users.noreply.github.com> Date: Thu, 30 Jul 2026 11:43:45 +0200 Subject: [PATCH 1/2] Feat: DataPatching: Map collection elements: Set/List elements. Supports all types supported by the ValueMapperRegistry. --- .../api/patch/mapping/ValuePatchRequest.java | 24 ++- .../sormas/backend/patch/DataPatcherImpl.java | 8 + .../backend/patch/PropertyAccessor.java | 47 ++++++ .../patch/mapping/ValueMapperRegistry.java | 12 ++ .../valuemapper/CollectionPatchMapper.java | 105 +++++++++++++ .../sormas/backend/AbstractBeanTest.java | 5 - .../backend/patch/PropertyAccessorTest.java | 112 +++++++++++++ .../mapping/ValueMapperRegistryTest.java | 63 ++++++++ .../CollectionPatchMapperTest.java | 148 ++++++++++++++++++ .../sormas/patch/DataPatcherImplTest.java | 75 +++++++++ 10 files changed, 591 insertions(+), 8 deletions(-) create mode 100644 sormas-backend/src/main/java/de/symeda/sormas/backend/patch/mapping/impl/valuemapper/CollectionPatchMapper.java create mode 100644 sormas-backend/src/test/java/de/symeda/sormas/backend/patch/mapping/impl/valuemapper/CollectionPatchMapperTest.java diff --git a/sormas-api/src/main/java/de/symeda/sormas/api/patch/mapping/ValuePatchRequest.java b/sormas-api/src/main/java/de/symeda/sormas/api/patch/mapping/ValuePatchRequest.java index db670ece62e..eb3150b8be9 100644 --- a/sormas-api/src/main/java/de/symeda/sormas/api/patch/mapping/ValuePatchRequest.java +++ b/sormas-api/src/main/java/de/symeda/sormas/api/patch/mapping/ValuePatchRequest.java @@ -19,6 +19,13 @@ public class ValuePatchRequest { @NotNull private Class targetType; + /** + * Only present when {@link #targetType} is of type: {@link java.util.Collection}. + * Gives the actual type within the collection. + */ + @Nullable + private Class collectionSubType; + /** * To be able to support I18n inputs the input languages can be passed, system locale by default. */ @@ -49,6 +56,16 @@ public ValuePatchRequest setTargetType(Class targetType) { return this; } + @Nullable + public Class getCollectionSubType() { + return collectionSubType; + } + + public ValuePatchRequest setCollectionSubType(@Nullable Class collectionSubType) { + this.collectionSubType = collectionSubType; + return this; + } + public List getInputLanguages() { return inputLanguages; } @@ -75,17 +92,18 @@ public boolean equals(Object o) { return allowFallbackValues == that.allowFallbackValues && Objects.equals(value, that.value) && Objects.equals(targetType, that.targetType) + && Objects.equals(collectionSubType, that.collectionSubType) && Objects.equals(inputLanguages, that.inputLanguages); } @Override public int hashCode() { - return Objects.hash(value, targetType, inputLanguages, allowFallbackValues); + return Objects.hash(value, targetType, collectionSubType, inputLanguages, allowFallbackValues); } @Override public String toString() { - return "ValuePatchRequest{" + "value=" + value + ", targetType=" + targetType + ", inputLanguages=" + inputLanguages + ", allowDefaultValues=" - + allowFallbackValues + '}'; + return "ValuePatchRequest{" + "value=" + value + ", targetType=" + targetType + ", collectionSubType=" + collectionSubType + + ", inputLanguages=" + inputLanguages + ", allowFallbackValues=" + allowFallbackValues + '}'; } } diff --git a/sormas-backend/src/main/java/de/symeda/sormas/backend/patch/DataPatcherImpl.java b/sormas-backend/src/main/java/de/symeda/sormas/backend/patch/DataPatcherImpl.java index 243b808d755..b104ae632dc 100644 --- a/sormas-backend/src/main/java/de/symeda/sormas/backend/patch/DataPatcherImpl.java +++ b/sormas-backend/src/main/java/de/symeda/sormas/backend/patch/DataPatcherImpl.java @@ -1,6 +1,7 @@ package de.symeda.sormas.backend.patch; import java.util.AbstractMap; +import java.util.Collection; import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; @@ -300,9 +301,16 @@ private void saveDTOsIfAppropriate(Map, AttachedEntityWra } Class targetType = nestedPropertyTypeTuple.getFirst(); + Class collectionSubType = null; + if (Collection.class.isAssignableFrom(targetType)) { + logger.info("Subtype will be computed as the targetType is a collectionType: [{}]", targetType); + collectionSubType = PropertyAccessor.getCollectionGenericType(target, relativeFieldName); + } + ValueMappingResult result = valueMapperRegistry.map( new ValuePatchRequest().setValue(untypedTargetValue) .setTargetType(targetType) + .setCollectionSubType(collectionSubType) .setInputLanguages(request.getInputLanguages()) .setAllowFallbackValues(request.isAllowFallbackValues())); diff --git a/sormas-backend/src/main/java/de/symeda/sormas/backend/patch/PropertyAccessor.java b/sormas-backend/src/main/java/de/symeda/sormas/backend/patch/PropertyAccessor.java index 3372ff8eead..d000837cfdf 100644 --- a/sormas-backend/src/main/java/de/symeda/sormas/backend/patch/PropertyAccessor.java +++ b/sormas-backend/src/main/java/de/symeda/sormas/backend/patch/PropertyAccessor.java @@ -1,6 +1,9 @@ package de.symeda.sormas.backend.patch; +import java.lang.reflect.Field; import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.ParameterizedType; +import java.lang.reflect.Type; import java.util.Optional; import javax.validation.constraints.NotNull; @@ -125,6 +128,50 @@ public static Optional getNestedProperty(final Object bean, final String } } + public static Class getCollectionGenericType(final Object bean, final String fieldName) { + if (bean == null || fieldName == null || fieldName.isEmpty()) { + return null; + } + + boolean notNestedPath = fieldName.indexOf(PATH_SEPARATOR) == -1; + + if (notNestedPath) { + return getCollectionGenericType(bean.getClass(), fieldName); + } + + String leafPath = fieldName.substring(fieldName.lastIndexOf(PATH_SEPARATOR) + 1); + + return getNestedProperty(bean, fieldName.substring(0, fieldName.lastIndexOf(PATH_SEPARATOR))) + .map(leafParent -> getCollectionGenericType(leafParent.getClass(), leafPath)) + .orElse(null); + } + + private static Class getCollectionGenericType(final Class beanClass, final String fieldName) { + Class currentClass = beanClass; + + while (currentClass != null) { + try { + Field field = currentClass.getDeclaredField(fieldName); + Type genericType = field.getGenericType(); + + if (genericType instanceof ParameterizedType) { + Type[] typeArguments = ((ParameterizedType) genericType).getActualTypeArguments(); + + if (typeArguments.length > 0 && typeArguments[0] instanceof Class) { + return (Class) typeArguments[0]; + } + } + + return null; + } catch (NoSuchFieldException e) { + currentClass = currentClass.getSuperclass(); + } + } + + logger.info("Could not get collection generic type for field [{}] on [{}]", fieldName, beanClass); + return null; + } + public static Optional setNestedProperty(final Object bean, final String name, final Object value) { try { PropertyUtils.setNestedProperty(bean, name, value); diff --git a/sormas-backend/src/main/java/de/symeda/sormas/backend/patch/mapping/ValueMapperRegistry.java b/sormas-backend/src/main/java/de/symeda/sormas/backend/patch/mapping/ValueMapperRegistry.java index 3874611d877..f61a9e6dfa8 100644 --- a/sormas-backend/src/main/java/de/symeda/sormas/backend/patch/mapping/ValueMapperRegistry.java +++ b/sormas-backend/src/main/java/de/symeda/sormas/backend/patch/mapping/ValueMapperRegistry.java @@ -1,6 +1,8 @@ package de.symeda.sormas.backend.patch.mapping; +import java.util.Collection; import java.util.List; +import java.util.Optional; import java.util.stream.Collectors; import javax.annotation.PostConstruct; @@ -54,6 +56,16 @@ public ValueMappingResult map(ValuePatchRequest request) { } if (targetType.isInstance(value)) { + if (Collection.class.isAssignableFrom(targetType)) { + // Making sure it's appropriate type + Optional anyElement = ((Collection) value).stream().findAny(); + if (anyElement.isPresent()) { + if (anyElement.get().getClass() != request.getCollectionSubType()) { + return ValueMappingResult.withCause(DataPatchFailureCause.INVALID_VALUE_TYPE); + } + } + } + return ValueMappingResult.withData((T) targetType.cast(value)); } diff --git a/sormas-backend/src/main/java/de/symeda/sormas/backend/patch/mapping/impl/valuemapper/CollectionPatchMapper.java b/sormas-backend/src/main/java/de/symeda/sormas/backend/patch/mapping/impl/valuemapper/CollectionPatchMapper.java new file mode 100644 index 00000000000..c51832ac8aa --- /dev/null +++ b/sormas-backend/src/main/java/de/symeda/sormas/backend/patch/mapping/impl/valuemapper/CollectionPatchMapper.java @@ -0,0 +1,105 @@ +package de.symeda.sormas.backend.patch.mapping.impl.valuemapper; + +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collector; +import java.util.stream.Collectors; + +import javax.enterprise.context.ApplicationScoped; + +import org.apache.commons.lang3.StringUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import de.symeda.sormas.api.patch.DataPatchFailureCause; +import de.symeda.sormas.api.patch.mapping.ValueMappingResult; +import de.symeda.sormas.api.patch.mapping.ValuePatchMapper; +import de.symeda.sormas.api.patch.mapping.ValuePatchRequest; +import de.symeda.sormas.backend.patch.mapping.ValueMapperRegistry; +import de.symeda.sormas.backend.util.InstanceProvider; + +/** + * Edge-case handling: field which is a collection of a singular values: Set/List. + * Calls the registry (where it's also contained!) to allow to map to every supported type. + */ +@ApplicationScoped +public class CollectionPatchMapper implements ValuePatchMapper { + + private static final Logger logger = LoggerFactory.getLogger(CollectionPatchMapper.class); + + public static final String SEPARATOR = ","; + + public static final Set> SUPPORTED_TYPES = Set.of(Set.class, List.class); + + public static final Map, Collector> COLLECTOR_DICTIONARY = + Map.of(Set.class, Collectors.toSet(), List.class, Collectors.toList()); + + /** + * Not injected through CDI, but lazily to avoid cycle: Registry <-> CollectionPatchMapper + */ + private ValueMapperRegistry valueMapperRegistry; + + @Override + public ValueMappingResult map(ValuePatchRequest request) { + Class collectionSubType = request.getCollectionSubType(); + if (collectionSubType == null) { + logger.warn("CollectionSubType is null for request: [{}]", request); + return ValueMappingResult.withCause(DataPatchFailureCause.TECHNICAL); + } + + Object groupedValue = request.getValue(); + + if (groupedValue.getClass() != String.class) { + logger.warn( + "CollectionSubType: [{}] only supports matching type or string, split with ',' (commas). Request: [{}]", + collectionSubType, + request); + return ValueMappingResult.withCause(DataPatchFailureCause.INVALID_VALUE_TYPE); + } + + List> mappingResults = Arrays.stream(((String) groupedValue).split(SEPARATOR)) + .filter(StringUtils::isNotBlank) + .map(String::trim) + .map(singleValue -> getValueMapperRegistry().map(buildRequestFrom(request, singleValue))) + .collect(Collectors.toList()); + + if (mappingResults.isEmpty()) { + logger.warn("The input string was empty, therefore no value can be patched within the collection, this seems off: [{}]", request); + return ValueMappingResult.withData(null); + } + + Optional> failureOpt = mappingResults.stream().filter(result -> result.getDataPatchFailureCause() != null).findAny(); + Collector appropriateCollector = COLLECTOR_DICTIONARY.get(request.getTargetType()); + + return failureOpt.> map(result -> ValueMappingResult.withCause(result.getDataPatchFailureCause())) + .orElseGet(() -> ValueMappingResult.withData((T) mappingResults.stream().map(ValueMappingResult::getData).collect(appropriateCollector))); + + } + + private static ValuePatchRequest buildRequestFrom(ValuePatchRequest request, String singleValue) { + return new ValuePatchRequest().setInputLanguages(request.getInputLanguages()) + .setValue(singleValue) + .setAllowFallbackValues(request.isAllowFallbackValues()) + .setTargetType(request.getCollectionSubType()); + } + + private ValueMapperRegistry getValueMapperRegistry() { + if (valueMapperRegistry == null) { + valueMapperRegistry = InstanceProvider.getInstanceFor(ValueMapperRegistry.class); + } + + return valueMapperRegistry; + } + + public void setValueMapperRegistry(ValueMapperRegistry valueMapperRegistry) { + this.valueMapperRegistry = valueMapperRegistry; + } + + @Override + public Set> getSupportedTypes() { + return SUPPORTED_TYPES; + } +} diff --git a/sormas-backend/src/test/java/de/symeda/sormas/backend/AbstractBeanTest.java b/sormas-backend/src/test/java/de/symeda/sormas/backend/AbstractBeanTest.java index 4eb9f9555ea..1fc629efbc8 100644 --- a/sormas-backend/src/test/java/de/symeda/sormas/backend/AbstractBeanTest.java +++ b/sormas-backend/src/test/java/de/symeda/sormas/backend/AbstractBeanTest.java @@ -101,7 +101,6 @@ import de.symeda.sormas.api.patch.DataPatcher; import de.symeda.sormas.api.patch.partial_retrieval.PartialRetriever; import de.symeda.sormas.api.person.notifier.NotifierFacade; -import de.symeda.sormas.api.referencedata.ReferenceDataValueInstanceProvider; import de.symeda.sormas.api.report.AggregateReportFacade; import de.symeda.sormas.api.report.WeeklyReportFacade; import de.symeda.sormas.api.sample.AdditionalTestFacade; @@ -1165,10 +1164,6 @@ public PartialRetriever getPartialRetriever() { return getBean(PartialRetrieverImpl.class); } - public ReferenceDataValueInstanceProvider getReferenceDataValueInstanceProvider() { - return getBean(ReferenceDataValueInstanceProviderImpl.class); - } - /** * The context of the {@link AbstractBeanTest} does not possess a proper (Initial)Context, therefore if you want to ma * diff --git a/sormas-backend/src/test/java/de/symeda/sormas/backend/patch/PropertyAccessorTest.java b/sormas-backend/src/test/java/de/symeda/sormas/backend/patch/PropertyAccessorTest.java index e919ae3c8c5..dca094df2c2 100644 --- a/sormas-backend/src/test/java/de/symeda/sormas/backend/patch/PropertyAccessorTest.java +++ b/sormas-backend/src/test/java/de/symeda/sormas/backend/patch/PropertyAccessorTest.java @@ -9,6 +9,10 @@ import org.junit.jupiter.api.Test; +import de.symeda.sormas.api.exposure.ExposureContactFactor; +import de.symeda.sormas.api.exposure.ExposureDto; +import de.symeda.sormas.api.exposure.ExposureProtectiveMeasure; +import de.symeda.sormas.api.exposure.ExposureSubSetting; import de.symeda.sormas.api.utils.Tuple; import de.symeda.sormas.api.utils.fieldvisibility.FieldVisibilityCheckers; import de.symeda.sormas.backend.AbstractUnitTest; @@ -36,6 +40,7 @@ public static class AddressBean { private AddressBean address; private String street; + private java.util.List tags; public AddressBean getAddress() { return address; @@ -52,6 +57,14 @@ public String getStreet() { public void setStreet(String street) { this.street = street; } + + public java.util.List getTags() { + return tags; + } + + public void setTags(java.util.List tags) { + this.tags = tags; + } } // ---- getNestedPropertyType ---- @@ -321,4 +334,103 @@ void setNestedProperty_nonExistentField_returnsException() { assertTrue(result.isPresent()); } + + // ---- getCollectionGenericType ---- + + @Test + void getCollectionGenericType_subSettings_returnsExposureSubSetting() { + Class result = PropertyAccessor.getCollectionGenericType(new ExposureDto(), "subSettings"); + + assertEquals(ExposureSubSetting.class, result); + } + + @Test + void getCollectionGenericType_contactFactors_returnsExposureContactFactor() { + Class result = PropertyAccessor.getCollectionGenericType(new ExposureDto(), "contactFactors"); + + assertEquals(ExposureContactFactor.class, result); + } + + @Test + void getCollectionGenericType_protectiveMeasures_returnsExposureProtectiveMeasure() { + Class result = PropertyAccessor.getCollectionGenericType(new ExposureDto(), "protectiveMeasures"); + + assertEquals(ExposureProtectiveMeasure.class, result); + } + + @Test + void getCollectionGenericType_nonExistentField_returnsNull() { + Class result = PropertyAccessor.getCollectionGenericType(new ExposureDto(), "nonExistent"); + + assertNull(result); + } + + @Test + void getCollectionGenericType_nonGenericField_returnsNull() { + // "description" is a plain String field, not a parameterized Set/List + Class result = PropertyAccessor.getCollectionGenericType(new ExposureDto(), "description"); + + assertNull(result); + } + + @Test + void getCollectionGenericType_nullBean_returnsNull() { + Class result = PropertyAccessor.getCollectionGenericType(null, "subSettings"); + + assertNull(result); + } + + @Test + void getCollectionGenericType_nullFieldName_returnsNull() { + Class result = PropertyAccessor.getCollectionGenericType(new ExposureDto(), null); + + assertNull(result); + } + + @Test + void getCollectionGenericType_inheritedField_returnsGenericType() { + // Field declared on a superclass must still be found by walking up the class hierarchy. + Class result = PropertyAccessor.getCollectionGenericType(new ChildBean(), "items"); + + assertEquals(String.class, result); + } + + @Test + void getCollectionGenericType_nestedPath_returnsGenericType() { + // PREPARE — "address.tags" has 1 dot -> triggers the nested-path branch, navigating + // to the AddressBean instance via getNestedProperty before resolving the leaf field's type. + AddressBean address = new AddressBean(); + PersonBean person = new PersonBean(); + person.setAddress(address); + + // EXECUTE + Class result = PropertyAccessor.getCollectionGenericType(person, "address.tags"); + + // CHECK + assertEquals(String.class, result); + } + + @Test + void getCollectionGenericType_nestedPath_nonExistentParent_returnsNull() { + // person.address is null, so navigating "address.tags" fails to reach a leaf parent + Class result = PropertyAccessor.getCollectionGenericType(new PersonBean(), "address.tags"); + + assertNull(result); + } + + public static class ParentBean { + + private java.util.List items; + + public java.util.List getItems() { + return items; + } + + public void setItems(java.util.List items) { + this.items = items; + } + } + + public static class ChildBean extends ParentBean { + } } diff --git a/sormas-backend/src/test/java/de/symeda/sormas/backend/patch/mapping/ValueMapperRegistryTest.java b/sormas-backend/src/test/java/de/symeda/sormas/backend/patch/mapping/ValueMapperRegistryTest.java index b43b5a8cd78..d9190c6a217 100644 --- a/sormas-backend/src/test/java/de/symeda/sormas/backend/patch/mapping/ValueMapperRegistryTest.java +++ b/sormas-backend/src/test/java/de/symeda/sormas/backend/patch/mapping/ValueMapperRegistryTest.java @@ -8,6 +8,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.util.Set; import java.util.stream.Stream; import javax.enterprise.inject.Instance; @@ -73,6 +74,68 @@ void map_valueAlreadyTargetType_returnsCastValue() { assertSame(value, result.getData()); } + // ---- map: already-typed collection values ---- + + @Test + @SuppressWarnings("rawtypes") + void map_collectionValueAlreadyTargetType_matchingSubType_returnsCastValue() { + // PREPARE + ValuePatchRequest request = new ValuePatchRequest(); + Set value = Set.of("A", "B"); + request.setValue(value).setTargetType(Set.class).setCollectionSubType(String.class); + + // EXECUTE + ValueMappingResult result = victim.map(request); + + // CHECK + assertSame(value, result.getData()); + } + + @Test + @SuppressWarnings("rawtypes") + void map_collectionValueAlreadyTargetType_mismatchingSubType_returnsInvalidValueType() { + // PREPARE — collectionSubType declares String, but the Set actually holds Integers + ValuePatchRequest request = new ValuePatchRequest(); + Set value = Set.of(1, 2); + request.setValue(value).setTargetType(Set.class).setCollectionSubType(String.class); + + // EXECUTE + ValueMappingResult result = victim.map(request); + + // CHECK + assertEquals(DataPatchFailureCause.INVALID_VALUE_TYPE, result.getDataPatchFailureCause()); + } + + @Test + @SuppressWarnings("rawtypes") + void map_collectionValueAlreadyTargetType_noCollectionSubTypeSet_returnsInvalidValueType() { + // PREPARE — collectionSubType left unset (null), so any actual element class mismatches it + ValuePatchRequest request = new ValuePatchRequest(); + Set value = Set.of("A"); + request.setValue(value).setTargetType(Set.class); + + // EXECUTE + ValueMappingResult result = victim.map(request); + + // CHECK + assertEquals(DataPatchFailureCause.INVALID_VALUE_TYPE, result.getDataPatchFailureCause()); + } + + @Test + @SuppressWarnings("rawtypes") + void map_emptyCollectionValueAlreadyTargetType_returnsCastValueWithoutSubTypeCheck() { + // PREPARE — nothing to inspect, so the sub-type check is skipped entirely + ValuePatchRequest request = new ValuePatchRequest(); + Set value = Set.of(); + request.setValue(value).setTargetType(Set.class).setCollectionSubType(String.class); + + // EXECUTE + ValueMappingResult result = victim.map(request); + + // CHECK + assertSame(value, result.getData()); + } + @Test void map_firstSupportingMapperUsed() { // PREPARE diff --git a/sormas-backend/src/test/java/de/symeda/sormas/backend/patch/mapping/impl/valuemapper/CollectionPatchMapperTest.java b/sormas-backend/src/test/java/de/symeda/sormas/backend/patch/mapping/impl/valuemapper/CollectionPatchMapperTest.java new file mode 100644 index 00000000000..69630ea3065 --- /dev/null +++ b/sormas-backend/src/test/java/de/symeda/sormas/backend/patch/mapping/impl/valuemapper/CollectionPatchMapperTest.java @@ -0,0 +1,148 @@ +package de.symeda.sormas.backend.patch.mapping.impl.valuemapper; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.when; + +import java.util.List; +import java.util.Set; + +import org.junit.jupiter.api.Test; +import org.mockito.InjectMocks; +import org.mockito.Mock; + +import de.symeda.sormas.api.patch.DataPatchFailureCause; +import de.symeda.sormas.api.patch.mapping.ValueMappingResult; +import de.symeda.sormas.api.patch.mapping.ValuePatchRequest; +import de.symeda.sormas.backend.AbstractUnitTest; +import de.symeda.sormas.backend.patch.mapping.ValueMapperRegistry; + +class CollectionPatchMapperTest extends AbstractUnitTest { + + @Mock + private ValueMapperRegistry valueMapperRegistry; + + @InjectMocks + private CollectionPatchMapper victim; + + /** + * Makes sure no type is forgotten, to be able to collect the values to the appropriate type. + */ + @Test + void supportedTypes_allHaveACollectorDictionaryEntry() { + for (Class supportedType : CollectionPatchMapper.SUPPORTED_TYPES) { + assertTrue( + CollectionPatchMapper.COLLECTOR_DICTIONARY.containsKey(supportedType), + "COLLECTOR_DICTIONARY is missing an entry for supported type: " + supportedType); + } + } + + @Test + @SuppressWarnings({ + "unchecked", + "rawtypes" }) + void map_setTarget_splitsCommaSeparatedValueAndCollectsIntoSet() { + // PREPARE — registry echoes back whatever single value it was asked to map + when(valueMapperRegistry.map(any(ValuePatchRequest.class))) + .thenAnswer(invocation -> ValueMappingResult.withData(((ValuePatchRequest) invocation.getArgument(0)).getValue())); + + ValuePatchRequest request = new ValuePatchRequest().setValue("A,B,C").setTargetType(Set.class).setCollectionSubType(String.class); + + // EXECUTE + ValueMappingResult result = victim.map(request); + + // CHECK + assertEquals(Set.of("A", "B", "C"), result.getData()); + } + + @Test + @SuppressWarnings({ + "unchecked", + "rawtypes" }) + void map_listTarget_splitsCommaSeparatedValueAndCollectsIntoListPreservingOrder() { + // PREPARE + when(valueMapperRegistry.map(any(ValuePatchRequest.class))) + .thenAnswer(invocation -> ValueMappingResult.withData(((ValuePatchRequest) invocation.getArgument(0)).getValue())); + + ValuePatchRequest request = new ValuePatchRequest().setValue("C,A,B").setTargetType(List.class).setCollectionSubType(String.class); + + // EXECUTE + ValueMappingResult result = victim.map(request); + + // CHECK + assertEquals(List.of("C", "A", "B"), result.getData()); + } + + @Test + @SuppressWarnings({ + "unchecked", + "rawtypes" }) + void map_delegatesEachSplitValueToRegistryWithCollectionSubTypeAsTargetType() { + // PREPARE + when(valueMapperRegistry.map(any(ValuePatchRequest.class))) + .thenAnswer(invocation -> ValueMappingResult.withData(((ValuePatchRequest) invocation.getArgument(0)).getTargetType())); + + ValuePatchRequest request = new ValuePatchRequest().setValue("A,B").setTargetType(Set.class).setCollectionSubType(Integer.class); + + // EXECUTE + ValueMappingResult result = victim.map(request); + + // CHECK — each delegated request was resolved against the element type, not Set.class + assertEquals(Set.of(Integer.class), result.getData()); + } + + @Test + @SuppressWarnings("rawtypes") + void map_nullCollectionSubType_returnsTechnicalFailure() { + // PREPARE + ValuePatchRequest request = new ValuePatchRequest().setValue("A,B").setTargetType(Set.class); + + // EXECUTE + ValueMappingResult result = victim.map(request); + + // CHECK + assertEquals(DataPatchFailureCause.TECHNICAL, result.getDataPatchFailureCause()); + } + + @Test + @SuppressWarnings("rawtypes") + void map_nonStringValue_returnsInvalidValueType() { + // PREPARE + ValuePatchRequest request = new ValuePatchRequest().setValue(42).setTargetType(Set.class).setCollectionSubType(String.class); + + // EXECUTE + ValueMappingResult result = victim.map(request); + + // CHECK + assertEquals(DataPatchFailureCause.INVALID_VALUE_TYPE, result.getDataPatchFailureCause()); + } + + @Test + @SuppressWarnings({ + "unchecked", + "rawtypes" }) + void map_oneElementFailsToMap_propagatesFailureCause() { + // PREPARE — "B" is rejected by the (mocked) downstream mapper + when(valueMapperRegistry.map(any(ValuePatchRequest.class))).thenAnswer(invocation -> { + Object value = ((ValuePatchRequest) invocation.getArgument(0)).getValue(); + if ("B".equals(value)) { + return ValueMappingResult.withCause(DataPatchFailureCause.NOT_PRESENT_IN_REFERENCE_DATA_LIST); + } + return ValueMappingResult.withData(value); + }); + + ValuePatchRequest request = new ValuePatchRequest().setValue("A,B,C").setTargetType(Set.class).setCollectionSubType(String.class); + + // EXECUTE + ValueMappingResult result = victim.map(request); + + // CHECK + assertEquals(DataPatchFailureCause.NOT_PRESENT_IN_REFERENCE_DATA_LIST, result.getDataPatchFailureCause()); + } + + @Test + void getSupportedTypes_containsSetAndList() { + assertEquals(Set.of(Set.class, List.class), victim.getSupportedTypes()); + } +} diff --git a/sormas-backend/src/test/java/de/symeda/sormas/patch/DataPatcherImplTest.java b/sormas-backend/src/test/java/de/symeda/sormas/patch/DataPatcherImplTest.java index ef84ff3b838..b2d3a45c395 100644 --- a/sormas-backend/src/test/java/de/symeda/sormas/patch/DataPatcherImplTest.java +++ b/sormas-backend/src/test/java/de/symeda/sormas/patch/DataPatcherImplTest.java @@ -7,12 +7,14 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.function.Supplier; import java.util.stream.Collectors; import java.util.stream.Stream; import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -36,7 +38,10 @@ import de.symeda.sormas.api.customizablefield.CustomizableFieldType; import de.symeda.sormas.api.customizablefield.CustomizableFieldValueDto; import de.symeda.sormas.api.epidata.EpiDataDto; +import de.symeda.sormas.api.exposure.ExposureContactFactor; import de.symeda.sormas.api.exposure.ExposureDto; +import de.symeda.sormas.api.exposure.ExposureProtectiveMeasure; +import de.symeda.sormas.api.exposure.ExposureSubSetting; import de.symeda.sormas.api.exposure.ExposureType; import de.symeda.sormas.api.externalmessage.survey.PatchDictionary; import de.symeda.sormas.api.externalmessage.survey.PatchField; @@ -75,11 +80,19 @@ import de.symeda.sormas.backend.customizablefield.CustomizableFieldMetadataFacadeEjb; import de.symeda.sormas.backend.customizablefield.CustomizableFieldValueFacadeEjb; import de.symeda.sormas.backend.patch.EqualValueOverrideHelper; +import de.symeda.sormas.backend.patch.mapping.ValueMapperRegistry; +import de.symeda.sormas.backend.patch.mapping.impl.valuemapper.CollectionPatchMapper; import de.symeda.sormas.backend.systemconfiguration.SystemConfigurationCategory; import de.symeda.sormas.backend.systemconfiguration.SystemConfigurationCategoryService; class DataPatcherImplTest extends AbstractBeanTest { + @BeforeEach + void setUp() { + // CDI look-up not easily achievable within this therefore setting the singleton before each test "in-case". + getBean(CollectionPatchMapper.class).setValueMapperRegistry(getBean(ValueMapperRegistry.class)); + } + @Test void patch_aliasUsage() { // PREPARE @@ -1093,6 +1106,68 @@ void patch_exposure() { () -> Assertions.assertEquals("market visit", exposures.get(0).getDescription())); } + @Test + void patch_exposure_collection_fields() { + // PREPARE + Disease disease = Disease.GIARDIASIS; + CaseDataDto originalCase = creator.createUnclassifiedCase(disease); + + // EXECUTE + DataPatchResponse response = victim().patch( + new CaseDataPatchRequest().setCaseUuid(originalCase.getUuid()) + .setPatchDictionary( + Map.of( + toFieldName(ExposureDto.I18N_PREFIX, ExposureDto.SUB_SETTINGS), + "CLOSED_POORLY_VENTILATED,,SHARED_HIGH_OCCUPANCY,EATING_AT_HOME", + toFieldName(ExposureDto.I18N_PREFIX, ExposureDto.PROTECTIVE_MEASURES), + "FACE_TO_FACE_15MIN,,WEARING_PPE,COILS", + toFieldName(ExposureDto.I18N_PREFIX, ExposureDto.CONTACT_FACTORS), + "DURATION_OF_EXPOSURE,PROXIMITY_AND_DURATION,EGG,"))); + + // CHECK + CaseDataDto actualCase = getCaseFacade().getByUuid(originalCase.getUuid()); + List exposures = actualCase.getEpiData().getExposures(); + Assertions.assertAll( + () -> Assertions.assertTrue(response.getFailures().isEmpty(), "Failures: " + response.getFailures()), + () -> Assertions.assertTrue(response.isApplied()), + () -> Assertions.assertEquals(1, exposures.size()), + () -> Assertions.assertEquals( + Set.of(ExposureSubSetting.CLOSED_POORLY_VENTILATED, ExposureSubSetting.SHARED_HIGH_OCCUPANCY, ExposureSubSetting.EATING_AT_HOME), + exposures.get(0).getSubSettings()), + () -> Assertions.assertEquals( + Set.of(ExposureProtectiveMeasure.FACE_TO_FACE_15MIN, ExposureProtectiveMeasure.WEARING_PPE, ExposureProtectiveMeasure.COILS), + exposures.get(0).getProtectiveMeasures()), + () -> Assertions.assertEquals( + Set.of(ExposureContactFactor.DURATION_OF_EXPOSURE, ExposureContactFactor.PROXIMITY_AND_DURATION, ExposureContactFactor.EGG), + exposures.get(0).getContactFactors())); + } + + @Test + void patch_exposure_collection_field_directSetValue() { + // PREPARE + Disease disease = Disease.GIARDIASIS; + CaseDataDto originalCase = creator.createUnclassifiedCase(disease); + + Set expectedSubSettings = + Set.of(ExposureSubSetting.CLOSED_POORLY_VENTILATED, ExposureSubSetting.SHARED_HIGH_OCCUPANCY, ExposureSubSetting.EATING_AT_HOME); + + // EXECUTE — the value is already a Set of the expected element type, not a comma-separated + // String: ValueMapperRegistry short-circuits on targetType.isInstance(value) and stores it + // as-is, without ever routing through CollectionPatchMapper. + DataPatchResponse response = victim().patch( + new CaseDataPatchRequest().setCaseUuid(originalCase.getUuid()) + .setPatchDictionary(Map.of(toFieldName(ExposureDto.I18N_PREFIX, ExposureDto.SUB_SETTINGS), expectedSubSettings))); + + // CHECK + CaseDataDto actualCase = getCaseFacade().getByUuid(originalCase.getUuid()); + List exposures = actualCase.getEpiData().getExposures(); + Assertions.assertAll( + () -> Assertions.assertTrue(response.getFailures().isEmpty(), "Failures: " + response.getFailures()), + () -> Assertions.assertTrue(response.isApplied()), + () -> Assertions.assertEquals(1, exposures.size()), + () -> Assertions.assertEquals(expectedSubSettings, exposures.get(0).getSubSettings())); + } + @Test void patch_multipleEntities_caseDataExposureHospitalizationSymptoms() { // This modifies many entities are triggers case update. From f14626b0e1fc958211278a9809935eb761a9515c Mon Sep 17 00:00:00 2001 From: Pa-Touche <47572440+Pa-Touche@users.noreply.github.com> Date: Thu, 30 Jul 2026 12:40:48 +0200 Subject: [PATCH 2/2] FIX: mix of types edge-case, lenient class checking instead of exact class match --- .../patch/mapping/ValueMapperRegistry.java | 18 +++++---- .../mapping/ValueMapperRegistryTest.java | 37 ++++++++++++++++++- 2 files changed, 47 insertions(+), 8 deletions(-) diff --git a/sormas-backend/src/main/java/de/symeda/sormas/backend/patch/mapping/ValueMapperRegistry.java b/sormas-backend/src/main/java/de/symeda/sormas/backend/patch/mapping/ValueMapperRegistry.java index f61a9e6dfa8..1b7b23df80e 100644 --- a/sormas-backend/src/main/java/de/symeda/sormas/backend/patch/mapping/ValueMapperRegistry.java +++ b/sormas-backend/src/main/java/de/symeda/sormas/backend/patch/mapping/ValueMapperRegistry.java @@ -2,7 +2,6 @@ import java.util.Collection; import java.util.List; -import java.util.Optional; import java.util.stream.Collectors; import javax.annotation.PostConstruct; @@ -57,12 +56,17 @@ public ValueMappingResult map(ValuePatchRequest request) { if (targetType.isInstance(value)) { if (Collection.class.isAssignableFrom(targetType)) { - // Making sure it's appropriate type - Optional anyElement = ((Collection) value).stream().findAny(); - if (anyElement.isPresent()) { - if (anyElement.get().getClass() != request.getCollectionSubType()) { - return ValueMappingResult.withCause(DataPatchFailureCause.INVALID_VALUE_TYPE); - } + + Class collectionSubType = request.getCollectionSubType(); + if (collectionSubType == null) { + logger.error("Subtype must be present, was not for: [{}]", request); + return ValueMappingResult.withCause(DataPatchFailureCause.TECHNICAL); + } + + // Making sure every element is of the appropriate type; empty collections trivially pass. + boolean allElementsMatch = ((Collection) value).stream().allMatch(collectionSubType::isInstance); + if (!allElementsMatch) { + return ValueMappingResult.withCause(DataPatchFailureCause.INVALID_VALUE_TYPE); } } diff --git a/sormas-backend/src/test/java/de/symeda/sormas/backend/patch/mapping/ValueMapperRegistryTest.java b/sormas-backend/src/test/java/de/symeda/sormas/backend/patch/mapping/ValueMapperRegistryTest.java index d9190c6a217..27004d223eb 100644 --- a/sormas-backend/src/test/java/de/symeda/sormas/backend/patch/mapping/ValueMapperRegistryTest.java +++ b/sormas-backend/src/test/java/de/symeda/sormas/backend/patch/mapping/ValueMapperRegistryTest.java @@ -8,6 +8,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.util.LinkedHashSet; import java.util.Set; import java.util.stream.Stream; @@ -118,7 +119,7 @@ void map_collectionValueAlreadyTargetType_noCollectionSubTypeSet_returnsInvalidV ValueMappingResult result = victim.map(request); // CHECK - assertEquals(DataPatchFailureCause.INVALID_VALUE_TYPE, result.getDataPatchFailureCause()); + assertEquals(DataPatchFailureCause.TECHNICAL, result.getDataPatchFailureCause()); } @Test @@ -136,6 +137,40 @@ void map_emptyCollectionValueAlreadyTargetType_returnsCastValueWithoutSubTypeChe assertSame(value, result.getData()); } + @Test + @SuppressWarnings("rawtypes") + void map_collectionValueAlreadyTargetType_subTypeElement_matchesViaIsInstance() { + // PREPARE — collectionSubType declares Number, elements are Integer: exact-class matching + // would have rejected this, isInstance correctly accepts the subtype. + ValuePatchRequest request = new ValuePatchRequest(); + Set value = Set.of(1, 2); + request.setValue(value).setTargetType(Set.class).setCollectionSubType(Number.class); + + // EXECUTE + ValueMappingResult result = victim.map(request); + + // CHECK + assertSame(value, result.getData()); + } + + @Test + @SuppressWarnings("rawtypes") + void map_collectionValueAlreadyTargetType_mismatchNotInFirstElement_stillReturnsInvalidValueType() { + // PREPARE — first element matches collectionSubType, a later one doesn't: every element must + // be checked, not just a sampled one (the previous implementation used findAny()). + ValuePatchRequest request = new ValuePatchRequest(); + LinkedHashSet value = new LinkedHashSet<>(); + value.add("A"); + value.add(42); + request.setValue(value).setTargetType(Set.class).setCollectionSubType(String.class); + + // EXECUTE + ValueMappingResult result = victim.map(request); + + // CHECK + assertEquals(DataPatchFailureCause.INVALID_VALUE_TYPE, result.getDataPatchFailureCause()); + } + @Test void map_firstSupportingMapperUsed() { // PREPARE