From 8dffe41b8c63f30820786afdd056183516989859 Mon Sep 17 00:00:00 2001 From: Nykolas Lima Date: Wed, 4 Mar 2015 14:44:22 -0300 Subject: [PATCH 1/2] merging values passed in path params with deserialized values --- vraptor-core/pom.xml | 6 ++ .../observer/DeserializingObserver.java | 21 +++++- .../observer/ParametersInstantiator.java | 17 +++-- .../vraptor/util/ObjectMergeException.java | 33 +++++++++ .../com/caelum/vraptor/util/ObjectMerger.java | 52 ++++++++++++++ ...alizingAndInstantiatorIntegrationTest.java | 7 +- .../observer/DeserializingObserverTest.java | 67 ++++++++++++++++++- .../observer/ParametersInstantiatorTest.java | 53 ++++++++++++++- 8 files changed, 245 insertions(+), 11 deletions(-) create mode 100644 vraptor-core/src/main/java/br/com/caelum/vraptor/util/ObjectMergeException.java create mode 100644 vraptor-core/src/main/java/br/com/caelum/vraptor/util/ObjectMerger.java diff --git a/vraptor-core/pom.xml b/vraptor-core/pom.xml index 21baf07cb..6fc6f27e6 100644 --- a/vraptor-core/pom.xml +++ b/vraptor-core/pom.xml @@ -57,6 +57,12 @@ + + + commons-beanutils + commons-beanutils + 1.9.2 + diff --git a/vraptor-core/src/main/java/br/com/caelum/vraptor/observer/DeserializingObserver.java b/vraptor-core/src/main/java/br/com/caelum/vraptor/observer/DeserializingObserver.java index dd961750d..0eb2c9be2 100644 --- a/vraptor-core/src/main/java/br/com/caelum/vraptor/observer/DeserializingObserver.java +++ b/vraptor-core/src/main/java/br/com/caelum/vraptor/observer/DeserializingObserver.java @@ -33,9 +33,11 @@ import br.com.caelum.vraptor.core.MethodInfo; import br.com.caelum.vraptor.events.InterceptorsReady; import br.com.caelum.vraptor.events.MethodReady; +import br.com.caelum.vraptor.http.ValuedParameter; import br.com.caelum.vraptor.ioc.Container; import br.com.caelum.vraptor.serialization.Deserializer; import br.com.caelum.vraptor.serialization.Deserializers; +import br.com.caelum.vraptor.util.ObjectMerger; import br.com.caelum.vraptor.view.Status; /** @@ -51,6 +53,7 @@ public class DeserializingObserver { private final Deserializers deserializers; private final Container container; + private final ObjectMerger objectMerger; private static final Logger logger = getLogger(DeserializingObserver.class); @@ -58,13 +61,14 @@ public class DeserializingObserver { * @deprecated CDI eyes only */ protected DeserializingObserver() { - this(null, null); + this(null, null, null); } @Inject - public DeserializingObserver(Deserializers deserializers, Container container) { + public DeserializingObserver(Deserializers deserializers, Container container, ObjectMerger objectMerger) { this.deserializers = deserializers; this.container = container; + this.objectMerger = objectMerger; } public void deserializes(@Observes InterceptorsReady event, HttpServletRequest request, @@ -96,9 +100,20 @@ public void deserializes(@Observes InterceptorsReady event, HttpServletRequest r Object[] deserialized = deserializer.deserialize(request.getInputStream(), method); logger.debug("Deserialized parameters for {} are {} ", method, deserialized); + ValuedParameter[] valuedParameters = methodInfo.getValuedParameters(); + for (int i = 0; i < deserialized.length; i++) { if (deserialized[i] != null) { - methodInfo.setParameter(i, deserialized[i]); + ValuedParameter valuedParameter = valuedParameters[i]; + Object existingValue = valuedParameter.getValue(); + Object deserializedValue = deserialized[i]; + + if (existingValue == null) { + methodInfo.setParameter(i, deserializedValue); + } else { + Object mergedValue = objectMerger.merge(deserializedValue, existingValue); + methodInfo.setParameter(i, mergedValue); + } } } } diff --git a/vraptor-core/src/main/java/br/com/caelum/vraptor/observer/ParametersInstantiator.java b/vraptor-core/src/main/java/br/com/caelum/vraptor/observer/ParametersInstantiator.java index 07e8737ac..bc28449d1 100644 --- a/vraptor-core/src/main/java/br/com/caelum/vraptor/observer/ParametersInstantiator.java +++ b/vraptor-core/src/main/java/br/com/caelum/vraptor/observer/ParametersInstantiator.java @@ -38,6 +38,7 @@ import br.com.caelum.vraptor.http.Parameter; import br.com.caelum.vraptor.http.ParametersProvider; import br.com.caelum.vraptor.http.ValuedParameter; +import br.com.caelum.vraptor.util.ObjectMerger; import br.com.caelum.vraptor.validator.Message; import br.com.caelum.vraptor.validator.Validator; import br.com.caelum.vraptor.view.FlashScope; @@ -59,6 +60,7 @@ public class ParametersInstantiator { private final Validator validator; private final MutableRequest request; private final FlashScope flash; + private final ObjectMerger objectMerger; private final List errors = new ArrayList<>(); @@ -66,17 +68,18 @@ public class ParametersInstantiator { * @deprecated CDI eyes only */ protected ParametersInstantiator() { - this(null, null, null, null, null); + this(null, null, null, null, null, null); } @Inject public ParametersInstantiator(ParametersProvider provider, MethodInfo methodInfo, Validator validator, - MutableRequest request, FlashScope flash) { + MutableRequest request, FlashScope flash, ObjectMerger objectMerger) { this.provider = provider; this.methodInfo = methodInfo; this.validator = validator; this.request = request; this.flash = flash; + this.objectMerger = objectMerger; } public void instantiate(@Observes InterceptorsReady event) { @@ -101,8 +104,14 @@ public void instantiate(@Observes InterceptorsReady event) { valuedParameters[i].setValue(request.getHeader(headerParam.value())); } else { ValuedParameter valuedParameter = valuedParameters[i]; - if (valuedParameter.getValue() == null) { - valuedParameter.setValue(values[i]); + Object existingValue = valuedParameter.getValue(); + Object value = values[i]; + + if (existingValue == null) { + valuedParameter.setValue(value); + } else { + Object mergedValue = objectMerger.merge(existingValue, value); + valuedParameter.setValue(mergedValue); } } } diff --git a/vraptor-core/src/main/java/br/com/caelum/vraptor/util/ObjectMergeException.java b/vraptor-core/src/main/java/br/com/caelum/vraptor/util/ObjectMergeException.java new file mode 100644 index 000000000..dc08045f7 --- /dev/null +++ b/vraptor-core/src/main/java/br/com/caelum/vraptor/util/ObjectMergeException.java @@ -0,0 +1,33 @@ +/*** + * Copyright (c) 2009 Caelum - www.caelum.com.br/opensource All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package br.com.caelum.vraptor.util; + +import br.com.caelum.vraptor.VRaptorException; + +/** + * This exception is thrown when we have some problem trying to merge to objects + * + * @author Nykolas Lima + */ +public class ObjectMergeException extends VRaptorException { + + private static final long serialVersionUID = 7057844184695106988L; + + public ObjectMergeException(String msg, Throwable e) { + super(msg, e); + } + +} diff --git a/vraptor-core/src/main/java/br/com/caelum/vraptor/util/ObjectMerger.java b/vraptor-core/src/main/java/br/com/caelum/vraptor/util/ObjectMerger.java new file mode 100644 index 000000000..268eb05c9 --- /dev/null +++ b/vraptor-core/src/main/java/br/com/caelum/vraptor/util/ObjectMerger.java @@ -0,0 +1,52 @@ +/*** + * Copyright (c) 2009 Caelum - www.caelum.com.br/opensource All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package br.com.caelum.vraptor.util; + +import java.lang.reflect.InvocationTargetException; + +import javax.enterprise.context.RequestScoped; + +import org.apache.commons.beanutils.BeanUtilsBean; + +/** + * This classes is used to merge 2 objects ignoring Null values. + * + * It's used by our parameter's resolvers + * + * @author Nykolas Lima + */ +@RequestScoped +public class ObjectMerger { + + public Object merge(Object dest, Object from) { + try { + new BeanUtilsBean() { + @Override + public void copyProperty(Object dest, String name, Object value) throws IllegalAccessException, InvocationTargetException { + //do not override null values + if(value != null) { + super.copyProperty(dest, name, value); + } + } + }.copyProperties(dest, from); + + return dest; + } catch (IllegalAccessException | InvocationTargetException e) { + throw new ObjectMergeException(String.format("Error trying to merge values from %s", dest.getClass().getName()), e); + } + } + +} diff --git a/vraptor-core/src/test/java/br/com/caelum/vraptor/observer/DeserializingAndInstantiatorIntegrationTest.java b/vraptor-core/src/test/java/br/com/caelum/vraptor/observer/DeserializingAndInstantiatorIntegrationTest.java index b06fd50fc..ba7248c4b 100644 --- a/vraptor-core/src/test/java/br/com/caelum/vraptor/observer/DeserializingAndInstantiatorIntegrationTest.java +++ b/vraptor-core/src/test/java/br/com/caelum/vraptor/observer/DeserializingAndInstantiatorIntegrationTest.java @@ -20,6 +20,7 @@ import br.com.caelum.vraptor.ioc.Container; import br.com.caelum.vraptor.serialization.Deserializer; import br.com.caelum.vraptor.serialization.Deserializers; +import br.com.caelum.vraptor.util.ObjectMerger; import br.com.caelum.vraptor.validator.Message; import br.com.caelum.vraptor.validator.Validator; import br.com.caelum.vraptor.view.FlashScope; @@ -36,6 +37,7 @@ public class DeserializingAndInstantiatorIntegrationTest { @Mock private Container container; @Mock private Status status; @Mock private Deserializer deserializer; + private ObjectMerger objectMerger; private MethodInfo methodInfo; private ParametersInstantiator instantiator; @@ -50,9 +52,10 @@ public void setUp() throws Exception { when(deserializers.deserializerFor("application/xml", container)).thenReturn(deserializer); when(request.getContentType()).thenReturn("application/xml"); + objectMerger = new ObjectMerger(); methodInfo = new MethodInfo(new ParanamerNameProvider()); - instantiator = new ParametersInstantiator(provider, methodInfo, validator, request, flash); - deserializing = new DeserializingObserver(deserializers, container); + instantiator = new ParametersInstantiator(provider, methodInfo, validator, request, flash, objectMerger); + deserializing = new DeserializingObserver(deserializers, container, objectMerger); controllerMethod = new DefaultControllerMethod(null, DeserializingObserverTest .DummyResource.class.getDeclaredMethod("consumeXml", String.class, String.class)); diff --git a/vraptor-core/src/test/java/br/com/caelum/vraptor/observer/DeserializingObserverTest.java b/vraptor-core/src/test/java/br/com/caelum/vraptor/observer/DeserializingObserverTest.java index deb4ae5eb..c69693516 100644 --- a/vraptor-core/src/test/java/br/com/caelum/vraptor/observer/DeserializingObserverTest.java +++ b/vraptor-core/src/test/java/br/com/caelum/vraptor/observer/DeserializingObserverTest.java @@ -21,6 +21,8 @@ import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; +import java.io.IOException; + import javax.servlet.http.HttpServletRequest; import org.junit.Before; @@ -37,6 +39,7 @@ import br.com.caelum.vraptor.ioc.Container; import br.com.caelum.vraptor.serialization.Deserializer; import br.com.caelum.vraptor.serialization.Deserializers; +import br.com.caelum.vraptor.util.ObjectMerger; import br.com.caelum.vraptor.view.Status; @@ -44,7 +47,9 @@ public class DeserializingObserverTest { private DeserializingObserver observer; private ControllerMethod consumeXml; private ControllerMethod doesntConsume; + private ControllerMethod consumeClientXml; private MethodInfo methodInfo; + private ObjectMerger objectMerger; @Mock private HttpServletRequest request; @Mock Deserializers deserializers; @@ -57,9 +62,12 @@ public void setUp() throws Exception { methodInfo = new MethodInfo(new ParanamerNameProvider()); - observer = new DeserializingObserver(deserializers, container); + objectMerger = new ObjectMerger(); + + observer = new DeserializingObserver(deserializers, container, objectMerger); consumeXml = new DefaultControllerMethod(null, DummyResource.class.getDeclaredMethod("consumeXml", String.class, String.class)); doesntConsume = new DefaultControllerMethod(null, DummyResource.class.getDeclaredMethod("doesntConsume")); + consumeClientXml = new DefaultControllerMethod(null, DummyResource.class.getDeclaredMethod("addClient", Client.class)); } @@ -71,6 +79,9 @@ public void consumeXml(String a, String b) {} public void consumesAnything(String a, String b) {} public void doesntConsume() {} + + @Consumes("application/xml") + public void addClient(Client client) {} } @Test @@ -178,4 +189,58 @@ public void willSetOnlyNonNullParameters() throws Exception { assertEquals(methodInfo.getValuedParameters()[0].getValue(), "original1"); assertEquals(methodInfo.getValuedParameters()[1].getValue(), "deserialized"); } + + @Test + public void shouldOverrideDeserializedAttributeWithAlreadyInstantiatedValue() throws IOException { + final Deserializer deserializer = mock(Deserializer.class); + methodInfo.setControllerMethod(consumeClientXml); + + Client alreadInstantiatedClient = new Client(2, null, 18); + methodInfo.setParameter(0, alreadInstantiatedClient); + + Client deserializedClient = new Client(1, "nykolas", null); + + when(request.getContentType()).thenReturn("application/xml"); + when(deserializer.deserialize(null, consumeXml)).thenReturn(new Object[] {deserializedClient}); + + when(deserializers.deserializerFor("application/xml", container)).thenReturn(deserializer); + observer.deserializes(new InterceptorsReady(consumeXml), request, methodInfo, status); + + Client mergedClient = (Client) methodInfo.getValuedParameters()[0].getValue(); + assertEquals(2, mergedClient.getId().intValue()); + assertEquals("nykolas", mergedClient.getName()); + assertEquals(18, mergedClient.getAge().intValue()); + } + + public class Client { + public Integer id; + public String name; + public Integer age; + + public Client(Integer id, String nome, Integer age) { + this.id = id; + this.name = nome; + this.age = age; + } + + public Integer getId() { + return id; + } + public String getName() { + return name; + } + public Integer getAge() { + return age; + } + public void setId(Integer id) { + this.id = id; + } + public void setName(String name) { + this.name = name; + } + public void setAge(Integer age) { + this.age = age; + } + } + } diff --git a/vraptor-core/src/test/java/br/com/caelum/vraptor/observer/ParametersInstantiatorTest.java b/vraptor-core/src/test/java/br/com/caelum/vraptor/observer/ParametersInstantiatorTest.java index ca66bc4df..318813c92 100644 --- a/vraptor-core/src/test/java/br/com/caelum/vraptor/observer/ParametersInstantiatorTest.java +++ b/vraptor-core/src/test/java/br/com/caelum/vraptor/observer/ParametersInstantiatorTest.java @@ -50,6 +50,7 @@ import br.com.caelum.vraptor.http.MutableRequest; import br.com.caelum.vraptor.http.ParametersProvider; import br.com.caelum.vraptor.http.ParanamerNameProvider; +import br.com.caelum.vraptor.util.ObjectMerger; import br.com.caelum.vraptor.validator.Message; import br.com.caelum.vraptor.validator.SimpleMessage; import br.com.caelum.vraptor.validator.Validator; @@ -66,24 +67,28 @@ public class ParametersInstantiatorTest { private @Mock ResourceBundle bundle; private @Mock MutableRequest request; private @Mock FlashScope flash; + private ObjectMerger objectMerger; private List errors ; private ParametersInstantiator instantiator; private ControllerMethod method; private ControllerMethod otherMethod; + private ControllerMethod addClientMethod; @Before @SuppressWarnings("unchecked") public void setup() throws Exception { MockitoAnnotations.initMocks(this); when(request.getParameterNames()).thenReturn(Collections. emptyEnumeration()); + objectMerger = new ObjectMerger(); - this.instantiator = new ParametersInstantiator(parametersProvider, methodInfo, validator, request, flash); + this.instantiator = new ParametersInstantiator(parametersProvider, methodInfo, validator, request, flash, objectMerger); this.errors = (List) new Mirror().on(instantiator).get().field("errors"); this.method = DefaultControllerMethod.instanceFor(Component.class, Component.class.getDeclaredMethod("method")); this.otherMethod = DefaultControllerMethod.instanceFor(Component.class, Component.class.getDeclaredMethod("otherMethod", int.class)); + this.addClientMethod = DefaultControllerMethod.instanceFor(Component.class, Component.class.getDeclaredMethod("addClient", Client.class)); } class Component { @@ -91,6 +96,8 @@ void method() { } void otherMethod(int oneParam){ } + void addClient(Client client) { + } } class HeaderParamComponent{ @@ -218,6 +225,50 @@ public void shouldNotAddHeaderInformationToRequestWhenHeaderParamAnnotationIsNot verify(validator).addAll(Collections.emptyList()); } + @Test + public void shouldOverrideParameterAttributeAlreadyInstantiatedAndDoNotOverrideIfValueIsNull() { + methodInfo.setControllerMethod(addClientMethod); + + Client client = new Client(1, "nykolas"); + methodInfo.setParameter(0, client); + + Client pathParamClient = new Client(2, null); + Object[] pathParamValues = new Object[] { pathParamClient }; + + when(parametersProvider.getParametersFor(addClientMethod, errors)).thenReturn(pathParamValues); + + instantiator.instantiate(new InterceptorsReady(otherMethod)); + + verify(validator).addAll(Collections. emptyList()); + + Client instantiatedClient = (Client) methodInfo.getValuedParameters()[0].getValue(); + assertEquals(2, instantiatedClient.getId().intValue()); + assertEquals("nykolas", instantiatedClient.getName()); + } + + public class Client { + public Integer id; + public String name; + + public Client(Integer id, String nome) { + this.id = id; + this.name = nome; + } + + public Integer getId() { + return id; + } + public String getName() { + return name; + } + public void setId(Integer id) { + this.id = id; + } + public void setName(String name) { + this.name = name; + } + } + private Answer addErrorsToListAndReturn(final T value, final String... messages) { return new Answer() { @Override From a2ceb45459bd03cb65102e71b0ea8cd548729722 Mon Sep 17 00:00:00 2001 From: Nykolas Lima Date: Wed, 4 Mar 2015 15:04:13 -0300 Subject: [PATCH 2/2] typo --- .../java/br/com/caelum/vraptor/util/ObjectMergeException.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vraptor-core/src/main/java/br/com/caelum/vraptor/util/ObjectMergeException.java b/vraptor-core/src/main/java/br/com/caelum/vraptor/util/ObjectMergeException.java index dc08045f7..c62d03303 100644 --- a/vraptor-core/src/main/java/br/com/caelum/vraptor/util/ObjectMergeException.java +++ b/vraptor-core/src/main/java/br/com/caelum/vraptor/util/ObjectMergeException.java @@ -18,7 +18,7 @@ import br.com.caelum.vraptor.VRaptorException; /** - * This exception is thrown when we have some problem trying to merge to objects + * This exception is thrown when we have some problem trying to merge objects * * @author Nykolas Lima */