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 1d47c9336..3c98ec5e0 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,6 +314,7 @@ 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; @@ -324,13 +325,14 @@ 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())) + if (RestConstants.PROPERTY_FOR_TYPE.equals(prop.getKey()) || alreadySetProperties.contains(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 ea4dd901b..207e24b87 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,6 +12,8 @@ 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; @@ -20,14 +22,25 @@ 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 { @@ -289,6 +302,47 @@ 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; @@ -348,4 +402,64 @@ 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 + } }