Skip to content
This repository was archived by the owner on Nov 7, 2019. It is now read-only.

Fix for MalformedParametersException #33 #34

Merged
merged 2 commits into from
Apr 4, 2016
Merged
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
10 changes: 9 additions & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ introspection of method/constructor parameter names, without having to add expli
<!-- Generate PackageVersion.java into this directory. -->
<packageVersion.dir>com/fasterxml/jackson/module/paramnames</packageVersion.dir>
<packageVersion.package>${project.groupId}.paramnames</packageVersion.package>
<assertj-core.version>3.3.0</assertj-core.version>

<assertj-core.version>3.4.0</assertj-core.version>
<mockito-core.version>1.10.19</mockito-core.version>
</properties>

<dependencies>
Expand All @@ -58,6 +60,12 @@ introspection of method/constructor parameter names, without having to add expli
<version>${assertj-core.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<version>${mockito-core.version}</version>
Copy link
Member

Choose a reason for hiding this comment

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

I think this needs scope of test too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch, fixed. :)

<scope>test</scope>
</dependency>
</dependencies>

<build>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package com.fasterxml.jackson.module.paramnames;

import java.lang.reflect.Executable;
import java.lang.reflect.Parameter;

class ParameterExtractor {

public Parameter[] getParameters(Executable executable) {
return executable.getParameters();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
import com.fasterxml.jackson.databind.AnnotationIntrospector;
import com.fasterxml.jackson.databind.introspect.*;

import java.lang.reflect.*;
import java.lang.reflect.MalformedParametersException;
import java.lang.reflect.Parameter;

/**
* Introspector that uses parameter name information provided by the Java Reflection API additions in Java 8 to
Expand All @@ -14,15 +15,16 @@
* @see AnnotationIntrospector
* @see Parameter
*/
class ParameterNamesAnnotationIntrospector extends NopAnnotationIntrospector
{
class ParameterNamesAnnotationIntrospector extends NopAnnotationIntrospector {
private static final long serialVersionUID = 1L;

private final JsonCreator.Mode creatorBinding;
private final ParameterExtractor parameterExtractor;

ParameterNamesAnnotationIntrospector(JsonCreator.Mode creatorBinding) {
ParameterNamesAnnotationIntrospector(JsonCreator.Mode creatorBinding, ParameterExtractor parameterExtractor) {
Copy link
Member

Choose a reason for hiding this comment

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

For backwards compatibility, would it make sense to keep old one to pass new one with default of ParameterExtractor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ParameterNamesAnnotationIntrospector isn't part of the module public API since the class is package private. Users that do override this class do so, from the issues reported so far, only for a hot fix, so I'm against keeping the old constructor.

Copy link
Member

Choose a reason for hiding this comment

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

@lpandzic ah ok. Makes sense.


this.creatorBinding = creatorBinding;
this.parameterExtractor = parameterExtractor;
}

@Override
Expand All @@ -44,26 +46,28 @@ public JsonCreator.Mode findCreatorBinding(Annotated a) {
return creatorBinding;
}

/**
* Returns the parameter name, or {@code null} if it could not be determined.
*
* @param annotatedParameter containing constructor or method from which {@link Parameter} can be extracted
*
* @return name or {@code null} if parameter could not be determined
*/
private String findParameterName(AnnotatedParameter annotatedParameter) {

AnnotatedWithParams owner = annotatedParameter.getOwner();
Parameter[] params;

if (owner instanceof AnnotatedConstructor) {
params = ((AnnotatedConstructor) owner).getAnnotated().getParameters();
} else if (owner instanceof AnnotatedMethod) {
params = ((AnnotatedMethod) owner).getAnnotated().getParameters();
} else {
try {
params = getParameters(annotatedParameter.getOwner());
} catch (MalformedParametersException e) {
return null;
}

Parameter p = params[annotatedParameter.getIndex()];
return p.isNamePresent() ? p.getName() : null;
}

private Parameter[] getParameters(AnnotatedWithParams owner) {
if (owner instanceof AnnotatedConstructor) {
return parameterExtractor.getParameters(((AnnotatedConstructor) owner).getAnnotated());
}

if (owner instanceof AnnotatedMethod) {
return parameterExtractor.getParameters(((AnnotatedMethod) owner).getAnnotated());
}

return null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public ParameterNamesModule() {
@Override
public void setupModule(SetupContext context) {
super.setupModule(context);
context.insertAnnotationIntrospector(new ParameterNamesAnnotationIntrospector(creatorBinding));
context.insertAnnotationIntrospector(new ParameterNamesAnnotationIntrospector(creatorBinding, new ParameterExtractor()));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,47 +4,86 @@
import com.fasterxml.jackson.databind.introspect.AnnotatedConstructor;
import com.fasterxml.jackson.databind.introspect.AnnotatedMethod;
import com.fasterxml.jackson.databind.introspect.AnnotatedParameter;

import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.BDDMockito;
import org.mockito.Mock;
import org.mockito.runners.MockitoJUnitRunner;

import java.lang.reflect.Constructor;
import java.lang.reflect.MalformedParametersException;
import java.lang.reflect.Method;
import java.lang.reflect.Parameter;

import static org.junit.Assert.assertEquals;
import static org.assertj.core.api.BDDAssertions.then;
import static org.mockito.BDDMockito.given;
import static org.mockito.Matchers.any;

/**
* @author Lovro Pandzic
*/
@RunWith(MockitoJUnitRunner.class)
public class ParameterNamesAnnotationIntrospectorTest {

private final ParameterNamesAnnotationIntrospector PN_AI = new ParameterNamesAnnotationIntrospector(JsonCreator.Mode.DEFAULT);
@Mock
private ParameterExtractor parameterExtractor;

private ParameterNamesAnnotationIntrospector introspector;

@Before
public void setUp() throws Exception {
introspector = new ParameterNamesAnnotationIntrospector(JsonCreator.Mode.DEFAULT, parameterExtractor);
}

@Test
public void shouldFindParameterNameFromConstructorForLegalIndex() throws Exception {
Constructor<?> ctor = ImmutableBean.class.getConstructor(String.class, Integer.class);

assertEquals("name", ctor.getParameters()[0].getName());

AnnotatedConstructor owner = new AnnotatedConstructor(null, ctor, null, null);
// given
Constructor<?> givenConstructor = ImmutableBean.class.getConstructor(String.class, Integer.class);
Parameter[] givenParameters = givenConstructor.getParameters();
AnnotatedConstructor owner = new AnnotatedConstructor(null, givenConstructor, null, null);
AnnotatedParameter annotatedParameter = new AnnotatedParameter(owner, null, null, 0);

String propertyName = PN_AI.findImplicitPropertyName(annotatedParameter);
given(parameterExtractor.getParameters(any())).willReturn(givenParameters);

// when
String actual = introspector.findImplicitPropertyName(annotatedParameter);

assertEquals("name", propertyName);
then(actual).isEqualTo("name");
BDDMockito.then(parameterExtractor).should().getParameters(givenConstructor);
}

@Test
public void shouldFindParameterNameFromMethodForLegalIndex() throws Exception {

Method method = ImmutableBeanWithStaticFactory.class.getMethod("of", String.class, Integer.class);
// given
Method givenMethod = ImmutableBeanWithStaticFactory.class.getMethod("of", String.class, Integer.class);
Parameter[] givenParameters = givenMethod.getParameters();
AnnotatedMethod owner = new AnnotatedMethod(null, givenMethod, null, null);
AnnotatedParameter annotatedParameter = new AnnotatedParameter(owner, null, null, 0);
given(parameterExtractor.getParameters(any())).willReturn(givenParameters);

// when
String actual = introspector.findImplicitPropertyName(annotatedParameter);

// then
then(actual).isEqualTo("name");
BDDMockito.then(parameterExtractor).should().getParameters(givenMethod);
}

@Test
public void shouldReturnNullForMalformedParametersException() throws Exception {

assertEquals("name", method.getParameters()[0].getName());

AnnotatedMethod owner = new AnnotatedMethod(null, method, null, null);
// given
Constructor<?> givenConstructor = ImmutableBean.class.getConstructor(String.class, Integer.class);
AnnotatedConstructor owner = new AnnotatedConstructor(null, givenConstructor, null, null);
AnnotatedParameter annotatedParameter = new AnnotatedParameter(owner, null, null, 0);
given(parameterExtractor.getParameters(any())).willThrow(new MalformedParametersException());

String propertyName = PN_AI.findImplicitPropertyName(annotatedParameter);
// when
String actual = introspector.findImplicitPropertyName(annotatedParameter);

assertEquals("name", propertyName);
then(actual).isNull();
BDDMockito.then(parameterExtractor).should().getParameters(givenConstructor);
}
}