Fix [JACKSON-712], inability to use Injectables with delegation constructor
diff --git a/release-notes/VERSION b/release-notes/VERSION
index aa80d46..cc4bfc7 100644
--- a/release-notes/VERSION
+++ b/release-notes/VERSION
@@ -8,6 +8,8 @@
Fixes:
+* [JACKSON-712]: Delegating @JsonCreator did not work with Injectable values
+
Improvements
* [JACKSON-546]: Indicate end-of-input with JsonMappingException instead
diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java b/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java
index 3e208ef..56ba51d 100644
--- a/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java
+++ b/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java
@@ -772,9 +772,7 @@
// [JACKSON-541] improved handling a bit so:
// 2 or more args; all params must have name annotations
// ... or @JacksonInject (or equivalent)
- // But if it was auto-detected and there's no annotations, keep silent (was not meant to be a creator?)
- boolean annotationFound = false;
- /* [JACKSON-712] One more possibility; can have 1 or more injectables, and
+ /* [JACKSON-711] One more possibility; can have 1 or more injectables, and
* exactly one non-annotated parameter: if so, it's still delegating.
*/
AnnotatedParameter nonAnnotatedParam = null;
@@ -802,17 +800,12 @@
if ((namedCount + injectCount) == argCount) {
creators.addPropertyCreator(ctor, properties);
} else if ((namedCount == 0) && ((injectCount + 1) == argCount)) {
- // secondary: all but one injectable, one un-annotated (un-named)
- // [JACKSON-712] SHOULD support; but we won't yet (tricky to do, not impossible)
- throw new IllegalArgumentException("Delegated constructor with Injectables not yet supported (see [JACKSON-712]) for "
- +ctor);
+ // [712] secondary: all but one injectable, one un-annotated (un-named)
+ creators.addDelegatingCreator(ctor, properties);
} else { // otherwise, epic fail
throw new IllegalArgumentException("Argument #"+nonAnnotatedParam.getIndex()+" of constructor "+ctor+" has no property name annotation; must have name when multiple-paramater constructor annotated as Creator");
}
}
- if (annotationFound) {
- creators.addPropertyCreator(ctor, properties);
- }
}
}
@@ -864,7 +857,7 @@
// Delegating Creator ok iff it has @JsonCreator (etc)
if (isCreator) {
- creators.addDelegatingCreator(ctor);
+ creators.addDelegatingCreator(ctor, null);
return true;
}
return false;
@@ -902,18 +895,38 @@
}
}
// 1 or more args; all params must have name annotations
+ AnnotatedParameter nonAnnotatedParam = null;
CreatorProperty[] properties = new CreatorProperty[argCount];
+ int namedCount = 0;
+ int injectCount = 0;
for (int i = 0; i < argCount; ++i) {
AnnotatedParameter param = factory.getParameter(i);
String name = intr.findDeserializationName(param);
- Object injectableId = intr.findInjectableValueId(param);
- // At this point, name annotation is NOT optional
- if ((name == null || name.length() == 0) && (injectableId == null)) {
- throw new IllegalArgumentException("Argument #"+i+" of factory method "+factory+" has no property name annotation; must have when multiple-paramater static method annotated as Creator");
+ Object injectId = intr.findInjectableValueId(param);
+ if (name != null && name.length() > 0) {
+ ++namedCount;
+ properties[i] = constructCreatorProperty(config, beanDesc, name, i, param, injectId);
+ } else if (injectId != null) {
+ ++injectCount;
+ properties[i] = constructCreatorProperty(config, beanDesc, name, i, param, injectId);
+ } else if (nonAnnotatedParam == null) {
+ nonAnnotatedParam = param;
}
- properties[i] = constructCreatorProperty(config, beanDesc, name, i, param, injectableId);
}
- creators.addPropertyCreator(factory, properties);
+
+ // Ok: if named or injectable, we have more work to do
+ if (isCreator || namedCount > 0 || injectCount > 0) {
+ // simple case; everything covered:
+ if ((namedCount + injectCount) == argCount) {
+ creators.addPropertyCreator(factory, properties);
+ } else if ((namedCount == 0) && ((injectCount + 1) == argCount)) {
+ // [712] secondary: all but one injectable, one un-annotated (un-named)
+ creators.addDelegatingCreator(factory, properties);
+ } else { // otherwise, epic fail
+ throw new IllegalArgumentException("Argument #"+nonAnnotatedParam.getIndex()
+ +" of factory method "+factory+" has no property name annotation; must have name when multiple-paramater constructor annotated as Creator");
+ }
+ }
}
}
@@ -956,7 +969,7 @@
return true;
}
if (intr.hasCreatorAnnotation(factory)) {
- creators.addDelegatingCreator(factory);
+ creators.addDelegatingCreator(factory, null);
return true;
}
return false;
diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/CreatorProperty.java b/src/main/java/com/fasterxml/jackson/databind/deser/CreatorProperty.java
index 3ff06f0..434dbfd 100644
--- a/src/main/java/com/fasterxml/jackson/databind/deser/CreatorProperty.java
+++ b/src/main/java/com/fasterxml/jackson/databind/deser/CreatorProperty.java
@@ -20,7 +20,7 @@
* It is not a full-featured implementation in that its set method
* should never be called -- instead, value must separately passed.
*<p>
- * Note on injectable values (1.9): unlike with other mutators, where
+ * Note on injectable values: unlike with other mutators, where
* deserializer and injecting are separate, here we deal the two as related
* things. This is necessary to add proper priority, as well as to simplify
* coordination.
@@ -150,4 +150,7 @@
public Object getInjectableValueId() {
return _injectableValueId;
}
+
+ @Override
+ public String toString() { return "[creator property, name '"+getName()+"'; inject id '"+_injectableValueId+"']"; }
}
diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/impl/CreatorCollector.java b/src/main/java/com/fasterxml/jackson/databind/deser/impl/CreatorCollector.java
index 7575cf0..b26fa3e 100644
--- a/src/main/java/com/fasterxml/jackson/databind/deser/impl/CreatorCollector.java
+++ b/src/main/java/com/fasterxml/jackson/databind/deser/impl/CreatorCollector.java
@@ -30,6 +30,8 @@
protected AnnotatedWithParams _doubleCreator, _booleanCreator;
protected AnnotatedWithParams _delegateCreator;
+ // when there are injectable values along with delegate:
+ protected CreatorProperty[] _delegateArgs;
protected AnnotatedWithParams _propertyBasedCreator;
protected CreatorProperty[] _propertyBasedArgs = null;
@@ -55,12 +57,22 @@
if (_delegateCreator == null) {
delegateType = null;
} else {
+ // need to find type...
+ int ix = 0;
+ if (_delegateArgs != null) {
+ for (int i = 0, len = _delegateArgs.length; i < len; ++i) {
+ if (_delegateArgs[i] == null) { // marker for delegate itself
+ ix = i;
+ break;
+ }
+ }
+ }
TypeBindings bindings = _beanDesc.bindingsForBeanType();
- delegateType = bindings.resolveType(_delegateCreator.getParameterType(0));
+ delegateType = bindings.resolveType(_delegateCreator.getParameterType(ix));
}
inst.configureFromObjectSettings(_defaultConstructor,
- _delegateCreator, delegateType,
+ _delegateCreator, delegateType, _delegateArgs,
_propertyBasedCreator, _propertyBasedArgs);
inst.configureFromStringCreator(_stringCreator);
inst.configureFromIntCreator(_intCreator);
@@ -96,8 +108,11 @@
_booleanCreator = verifyNonDup(creator, _booleanCreator, "boolean");
}
- public void addDelegatingCreator(AnnotatedWithParams creator) {
+ public void addDelegatingCreator(AnnotatedWithParams creator,
+ CreatorProperty[] injectables)
+ {
_delegateCreator = verifyNonDup(creator, _delegateCreator, "delegate");
+ _delegateArgs = injectables;
}
public void addPropertyCreator(AnnotatedWithParams creator, CreatorProperty[] properties)
diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/std/StdValueInstantiator.java b/src/main/java/com/fasterxml/jackson/databind/deser/std/StdValueInstantiator.java
index bcfa247..ff71bc3 100644
--- a/src/main/java/com/fasterxml/jackson/databind/deser/std/StdValueInstantiator.java
+++ b/src/main/java/com/fasterxml/jackson/databind/deser/std/StdValueInstantiator.java
@@ -14,10 +14,9 @@
/**
- * Basic {@link ValueInstantiator} implementation, which only
- * supports use of default constructor. Sub-types can add
- * support for alternate construction methods, such as using
- * argument-taking constructors or static factory methods.
+ * Default {@link ValueInstantiator} implementation, which supports
+ * Creator methods that can be indicated by standard Jackson
+ * annotations.
*/
public class StdValueInstantiator
extends ValueInstantiator
@@ -43,13 +42,14 @@
// // // With-args (property-based) construction
- protected CreatorProperty[] _constructorArguments;
protected AnnotatedWithParams _withArgsCreator;
+ protected CreatorProperty[] _constructorArguments;
// // // Delegate construction
protected JavaType _delegateType;
protected AnnotatedWithParams _delegateCreator;
+ protected CreatorProperty[] _delegateArguments;
// // // Scalar construction
@@ -95,6 +95,7 @@
_delegateType = src._delegateType;
_delegateCreator = src._delegateCreator;
+ _delegateArguments = src._delegateArguments;
_fromStringCreator = src._fromStringCreator;
_fromIntCreator = src._fromIntCreator;
@@ -109,12 +110,13 @@
* three), and clear other properties
*/
public void configureFromObjectSettings(AnnotatedWithParams defaultCreator,
- AnnotatedWithParams delegateCreator, JavaType delegateType,
+ AnnotatedWithParams delegateCreator, JavaType delegateType, CreatorProperty[] delegateArgs,
AnnotatedWithParams withArgsCreator, CreatorProperty[] constructorArgs)
{
_defaultCreator = defaultCreator;
_delegateCreator = delegateCreator;
_delegateType = delegateType;
+ _delegateArguments = delegateArgs;
_withArgsCreator = withArgsCreator;
_constructorArguments = constructorArgs;
}
@@ -246,7 +248,23 @@
throw new IllegalStateException("No delegate constructor for "+getValueTypeDesc());
}
try {
- return _delegateCreator.call1(delegate);
+ // First simple case: just delegate, no injectables
+ if (_delegateArguments == null) {
+ return _delegateCreator.call1(delegate);
+ }
+ // And then the case with at least one injectable...
+ final int len = _delegateArguments.length;
+ Object[] args = new Object[len];
+ for (int i = 0; i < len; ++i) {
+ CreatorProperty prop = _delegateArguments[i];
+ if (prop == null) { // delegate
+ args[i] = delegate;
+ } else { // nope, injectable:
+ args[i] = ctxt.findInjectableValue(prop.getInjectableValueId(), prop, null);
+ }
+ }
+ // and then try calling with full set of arguments
+ return _delegateCreator.call(args);
} catch (ExceptionInInitializerError e) {
throw wrapException(e);
} catch (Exception e) {
diff --git a/src/test/java/com/fasterxml/jackson/failing/TestDelegatingCreators.java b/src/test/java/com/fasterxml/jackson/failing/TestDelegatingCreators.java
index 307e3b8..1e096f2 100644
--- a/src/test/java/com/fasterxml/jackson/failing/TestDelegatingCreators.java
+++ b/src/test/java/com/fasterxml/jackson/failing/TestDelegatingCreators.java
@@ -23,6 +23,24 @@
age = a;
}
}
+
+ static class FactoryBean711
+ {
+ protected String name1;
+ protected String name2;
+ protected int age;
+
+ private FactoryBean711(int a, String n1, String n2) {
+ age = a;
+ name1 = n1;
+ name2 = n2;
+ }
+
+ @JsonCreator
+ public static FactoryBean711 create(@JacksonInject String n1, int a, @JacksonInject String n2) {
+ return new FactoryBean711(a, n1, n2);
+ }
+ }
/*
/**********************************************************
@@ -46,4 +64,21 @@
assertEquals(38, bean.age);
assertEquals("Pooka", bean.name);
}
+
+ public void testWithFactoryAndDelegate() throws Exception
+ {
+ ObjectMapper mapper = new ObjectMapper();
+ mapper.setInjectableValues(new InjectableValues.Std()
+ .addValue(String.class, "Fygar")
+ );
+ FactoryBean711 bean = null;
+ try {
+ bean = mapper.readValue("38", FactoryBean711.class);
+ } catch (JsonMappingException e) {
+ fail("Did not expect problems, got: "+e.getMessage());
+ }
+ assertEquals(38, bean.age);
+ assertEquals("Fygar", bean.name1);
+ assertEquals("Fygar", bean.name2);
+ }
}
