Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

merging values passed in path params with deserialized values #953

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions vraptor-core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@
</exclusion>
</exclusions>
</dependency>

<dependency>
<groupId>commons-beanutils</groupId>
<artifactId>commons-beanutils</artifactId>
<version>1.9.2</version>
</dependency>
<!-- /mandatory -->


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -51,20 +53,22 @@ public class DeserializingObserver {

private final Deserializers deserializers;
private final Container container;
private final ObjectMerger objectMerger;

private static final Logger logger = getLogger(DeserializingObserver.class);

/**
* @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,
Expand Down Expand Up @@ -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);
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -59,24 +60,26 @@ public class ParametersInstantiator {
private final Validator validator;
private final MutableRequest request;
private final FlashScope flash;
private final ObjectMerger objectMerger;

private final List<Message> errors = new ArrayList<>();

/**
* @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) {
Expand All @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do you need this code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually it isn't. I just did this to make it explicit. What do you think?

}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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 objects
*
* @author Nykolas Lima
*/
public class ObjectMergeException extends VRaptorException {

private static final long serialVersionUID = 7057844184695106988L;

public ObjectMergeException(String msg, Throwable e) {
super(msg, e);
}

}
Original file line number Diff line number Diff line change
@@ -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);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -37,14 +39,17 @@
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;


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;
Expand All @@ -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));
}


Expand All @@ -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
Expand Down Expand Up @@ -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;
}
}

}
Loading