From 0b7c1031d8e85efe3550d1ffe6a57f2723512e2b Mon Sep 17 00:00:00 2001 From: Fred Deniger Date: Fri, 15 Mar 2024 17:07:53 +0100 Subject: [PATCH] Revert "RESTWS-901: Call property setters only once on map conversion (#581)" This reverts commit cbcf07f27ada8d73f2954fce7cf126987b84b5cf. --- .../webservices/rest/web/ConversionUtil.java | 4 +- .../rest/web/ConversionUtilTest.java | 114 ------------------ 2 files changed, 1 insertion(+), 117 deletions(-) diff --git a/omod-common/src/main/java/org/openmrs/module/webservices/rest/web/ConversionUtil.java b/omod-common/src/main/java/org/openmrs/module/webservices/rest/web/ConversionUtil.java index 3c98ec5e0..1d47c9336 100644 --- a/omod-common/src/main/java/org/openmrs/module/webservices/rest/web/ConversionUtil.java +++ b/omod-common/src/main/java/org/openmrs/module/webservices/rest/web/ConversionUtil.java @@ -314,7 +314,6 @@ public static Object convertMap(Map map, Class toClass) throws Con } // If the converter is a resource handler use the order of properties of its default representation - Set alreadySetProperties = new HashSet(); if (converter instanceof DelegatingResourceHandler) { DelegatingResourceHandler handler = (DelegatingResourceHandler) converter; @@ -325,14 +324,13 @@ public static Object convertMap(Map map, Class toClass) throws Con for (Map.Entry prop : resDesc.getProperties().entrySet()) { if (map.containsKey(prop.getKey()) && !RestConstants.PROPERTY_FOR_TYPE.equals(prop.getKey())) { converter.setProperty(ret, prop.getKey(), map.get(prop.getKey())); - alreadySetProperties.add(prop.getKey()); } } } } for (Map.Entry prop : map.entrySet()) { - if (RestConstants.PROPERTY_FOR_TYPE.equals(prop.getKey()) || alreadySetProperties.contains(prop.getKey())) + if (RestConstants.PROPERTY_FOR_TYPE.equals(prop.getKey())) continue; converter.setProperty(ret, prop.getKey(), prop.getValue()); } diff --git a/omod-common/src/test/java/org/openmrs/module/webservices/rest/web/ConversionUtilTest.java b/omod-common/src/test/java/org/openmrs/module/webservices/rest/web/ConversionUtilTest.java index 207e24b87..ea4dd901b 100644 --- a/omod-common/src/test/java/org/openmrs/module/webservices/rest/web/ConversionUtilTest.java +++ b/omod-common/src/test/java/org/openmrs/module/webservices/rest/web/ConversionUtilTest.java @@ -12,8 +12,6 @@ import static org.hamcrest.core.Is.is; import org.junit.Assert; import static org.junit.Assert.assertThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; import java.lang.reflect.Method; import java.lang.reflect.Type; @@ -22,25 +20,14 @@ import java.util.Arrays; import java.util.Calendar; import java.util.Date; -import java.util.HashMap; import java.util.List; import java.util.Locale; -import java.util.Map; import java.util.TimeZone; import org.apache.commons.beanutils.PropertyUtils; import org.junit.Test; -import org.mockito.Matchers; -import org.openmrs.User; import org.openmrs.api.ConceptNameType; -import org.openmrs.module.webservices.rest.web.annotation.Resource; import org.openmrs.module.webservices.rest.web.representation.Representation; -import org.openmrs.module.webservices.rest.web.resource.api.Converter; -import org.openmrs.module.webservices.rest.web.resource.impl.DelegatingResourceDescription; -import org.openmrs.module.webservices.rest.web.resource.impl.DelegatingResourceHandler; -import org.openmrs.module.webservices.rest.web.resource.impl.MetadataDelegatingCrudResource; -import org.openmrs.module.webservices.rest.web.response.ConversionException; -import org.openmrs.module.webservices.rest.web.response.ResponseException; import org.openmrs.web.test.BaseModuleWebContextSensitiveTest; public class ConversionUtilTest extends BaseModuleWebContextSensitiveTest { @@ -302,47 +289,6 @@ public void getTypeVariableClass_shouldThrowIllegalArgumentExceptionWhenTypeVari ConversionUtil.getTypeVariableClass(Temp.class, null); } - @Test - @SuppressWarnings({ "rawtypes" }) - public void setProperty_onlyCalledOnceWhenDelegatingResourceDescriptionIsPresent() { - String expectedKey = "someKey"; - String expectedValue = "someValue"; - String expectedUUID = "12345"; - - Map mapToConvert = new HashMap(); - mapToConvert.put(expectedKey, expectedValue); - mapToConvert.put(RestConstants.PROPERTY_UUID, expectedUUID); - mapToConvert.put(RestConstants.PROPERTY_FOR_TYPE, "someType"); - - Converter converter = ConversionUtil.getConverter(DummyUser.class); - Assert.assertNotNull(converter); - - DelegatingResourceHandler handler = mock(DelegatingResourceHandler.class); - DelegatingResourceDescription resDesc = mock(DelegatingResourceDescription.class); - - Map resourceDescProperties = new HashMap(); - DelegatingResourceDescription.Property mockProperty = mock(DelegatingResourceDescription.Property.class); - resourceDescProperties.put(expectedKey, mockProperty); - - DelegatingResourceDescription representationDesc = ((DelegatingResourceHandler) converter) - .getRepresentationDescription(Representation.DEFAULT); - - Map repDescProperties = new HashMap(); - repDescProperties.put(expectedKey, mockProperty); - - DummyUser expectedUser = mock(DummyUser.class); - ((DummyUserConverter) converter).addUser(expectedUUID, expectedUser); - - when(handler.getRepresentationDescription(Matchers. any())).thenReturn(resDesc); - when(resDesc.getProperties()).thenReturn(resourceDescProperties); - when(representationDesc.getProperties()).thenReturn(repDescProperties); - - DummyUser actualuser = (DummyUser) ConversionUtil.convertMap(mapToConvert, DummyUser.class); - - Assert.assertEquals(expectedUser, actualuser); - Assert.assertEquals(1, (int) ((DummyUserConverter) converter).getPropertyKeyToNbTimesSet().get(expectedKey)); - } - public abstract class BaseGenericType { private T value; @@ -402,64 +348,4 @@ public class GrandchildGenericType_Int extends ChildGenericType_Int {} public class GreatGrandchildGenericType_Int extends GrandchildGenericType_Int {} public class ChildMultiGenericType extends BaseMultiGenericType {} - - @Resource(name = "dummyUserConverter", supportedClass = DummyUser.class, supportedOpenmrsVersions = { "1.0.* - 9.*" }) - public static class DummyUserConverter extends MetadataDelegatingCrudResource { - - private final DelegatingResourceDescription mockRepDesc = mock(DelegatingResourceDescription.class); - - private final Map properties = new HashMap(); - - private final Map propertyKeyToNbTimesSet = new HashMap(); - - private final Map uniqueIdToUser = new HashMap(); - - public void addUser(String uniqueId, DummyUser user) { - uniqueIdToUser.put(uniqueId, user); - } - - @Override - public void setProperty(Object instance, String propertyName, Object value) throws ConversionException { - properties.put(propertyName, value); - Integer nbTimesCalled = propertyKeyToNbTimesSet.get(propertyName); - propertyKeyToNbTimesSet.put(propertyName, nbTimesCalled == null ? 1 : nbTimesCalled + 1); - } - - @Override - public DummyUser getByUniqueId(String uniqueId) { - return uniqueIdToUser.get(uniqueId); - } - - @Override - public DelegatingResourceDescription getRepresentationDescription(Representation rep) { - return mockRepDesc; - } - - @Override - public void purge(DummyUser delegate, RequestContext context) throws ResponseException { - // Do nothing - } - - @Override - public DummyUser newDelegate() { - return null; - } - - @Override - public DummyUser save(DummyUser delegate) { - return delegate; - } - - public Map getProperties() { - return properties; - } - - public Map getPropertyKeyToNbTimesSet() { - return propertyKeyToNbTimesSet; - } - } - - protected static class DummyUser extends User { - // No overrides needed - } }