Skip to content

Commit

Permalink
Fix #660
Browse files Browse the repository at this point in the history
  • Loading branch information
@cowtowncoder
cowtowncoder committed Dec 23, 2014
1 parent 1eae322 commit 257ae1c
Show file tree
Hide file tree
Showing 14 changed files with 175 additions and 96 deletions.
1 change: 1 addition & 0 deletions release-notes/VERSION
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ Project: jackson-databind
#653: Jackson doesn't follow JavaBean naming convention (added `MapperFeature.USE_STD_BEAN_NAMING`)
#654: Add support for (re)configuring `JsonGenerator.setRootValueSeparator()` via `ObjectWriter`
#655: Add `ObjectWriter.writeValues()` for writing value sequences
#660: `@JsonCreator`-annotated factory method is ignored if constructor exists
- Allow use of `Shape.ARRAY` for Enums, as an alias to 'use index'
- Start using `JsonGenerator.writeStartArray(int)` to help data formats
that benefit from knowing number of elements in arrays (and would otherwise
Expand Down
38 changes: 19 additions & 19 deletions src/main/java/com/fasterxml/jackson/databind/deser/BasicDeserializerFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ protected void _addDeserializerConstructors(DeserializationContext ctxt, BeanDes
AnnotatedParameter arg = ctor.getParameter(0);
properties[0] = constructCreatorProperty(ctxt, beanDesc, name, 0, arg,
intr.findInjectableValueId(arg));
creators.addPropertyCreator(ctor, properties);
creators.addPropertyCreator(ctor, isCreator, properties);
} else {
/*boolean added = */ _handleSingleArgumentConstructor(ctxt, beanDesc, vchecker, intr, creators,
ctor, isCreator,
Expand Down Expand Up @@ -494,10 +494,10 @@ protected void _addDeserializerConstructors(DeserializationContext ctxt, BeanDes
if (isCreator || explicitNameCount > 0 || injectCount > 0) {
// simple case; everything covered:
if ((namedCount + injectCount) == argCount) {
creators.addPropertyCreator(ctor, properties);
creators.addPropertyCreator(ctor, isCreator, properties);
} else if ((explicitNameCount == 0) && ((injectCount + 1) == argCount)) {
// [712] secondary: all but one injectable, one un-annotated (un-named)
creators.addDelegatingCreator(ctor, properties);
creators.addDelegatingCreator(ctor, isCreator, properties);
} else { // otherwise, epic fail
throw new IllegalArgumentException("Argument #"+nonAnnotatedParam.getIndex()
+" of constructor "+ctor+" has no property name annotation; must have name when multiple-parameter constructor annotated as Creator");
Expand Down Expand Up @@ -544,39 +544,39 @@ protected boolean _handleSingleArgumentConstructor(DeserializationContext ctxt,
{
// otherwise either 'simple' number, String, or general delegate:
Class<?> type = ctor.getRawParameterType(0);
if (type == String.class) {
if (type == String.class || type == CharSequence.class) {
if (isCreator || isVisible) {
creators.addStringCreator(ctor);
creators.addStringCreator(ctor, isCreator);
}
return true;
}
if (type == int.class || type == Integer.class) {
if (isCreator || isVisible) {
creators.addIntCreator(ctor);
creators.addIntCreator(ctor, isCreator);
}
return true;
}
if (type == long.class || type == Long.class) {
if (isCreator || isVisible) {
creators.addLongCreator(ctor);
creators.addLongCreator(ctor, isCreator);
}
return true;
}
if (type == double.class || type == Double.class) {
if (isCreator || isVisible) {
creators.addDoubleCreator(ctor);
creators.addDoubleCreator(ctor, isCreator);
}
return true;
}
if (type == boolean.class || type == Boolean.class) {
if (isCreator || isVisible) {
creators.addBooleanCreator(ctor);
creators.addBooleanCreator(ctor, isCreator);
}
return true;
}
// Delegating Creator ok iff it has @JsonCreator (etc)
if (isCreator) {
creators.addDelegatingCreator(ctor, null);
creators.addDelegatingCreator(ctor, isCreator, null);
return true;
}
return false;
Expand Down Expand Up @@ -686,10 +686,10 @@ protected void _addDeserializerFactoryMethods(DeserializationContext ctxt, BeanD
if (isCreator || explicitNameCount > 0 || injectCount > 0) {
// simple case; everything covered:
if ((namedCount + injectCount) == argCount) {
creators.addPropertyCreator(factory, properties);
creators.addPropertyCreator(factory, isCreator, properties);
} else if ((explicitNameCount == 0) && ((injectCount + 1) == argCount)) {
// [712] secondary: all but one injectable, one un-annotated (un-named)
creators.addDelegatingCreator(factory, properties);
creators.addDelegatingCreator(factory, isCreator, 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-parameter constructor annotated as Creator");
Expand All @@ -706,38 +706,38 @@ protected boolean _handleSingleArgumentFactory(DeserializationConfig config,
{
Class<?> type = factory.getRawParameterType(0);

if (type == String.class) {
if (type == String.class || type == CharSequence.class) {
if (isCreator || vchecker.isCreatorVisible(factory)) {
creators.addStringCreator(factory);
creators.addStringCreator(factory, isCreator);
}
return true;
}
if (type == int.class || type == Integer.class) {
if (isCreator || vchecker.isCreatorVisible(factory)) {
creators.addIntCreator(factory);
creators.addIntCreator(factory, isCreator);
}
return true;
}
if (type == long.class || type == Long.class) {
if (isCreator || vchecker.isCreatorVisible(factory)) {
creators.addLongCreator(factory);
creators.addLongCreator(factory, isCreator);
}
return true;
}
if (type == double.class || type == Double.class) {
if (isCreator || vchecker.isCreatorVisible(factory)) {
creators.addDoubleCreator(factory);
creators.addDoubleCreator(factory, isCreator);
}
return true;
}
if (type == boolean.class || type == Boolean.class) {
if (isCreator || vchecker.isCreatorVisible(factory)) {
creators.addBooleanCreator(factory);
creators.addBooleanCreator(factory, isCreator);
}
return true;
}
if (isCreator) {
creators.addDelegatingCreator(factory, null);
creators.addDelegatingCreator(factory, isCreator, null);
return true;
}
return false;
Expand Down
169 changes: 118 additions & 51 deletions src/main/java/com/fasterxml/jackson/databind/deser/impl/CreatorCollector.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,28 +21,48 @@
*/
public class CreatorCollector
{
// Since 2.5
protected final static int C_DEFAULT = 0;
protected final static int C_STRING = 1;
protected final static int C_INT = 2;
protected final static int C_LONG = 3;
protected final static int C_DOUBLE = 4;
protected final static int C_BOOLEAN = 5;
protected final static int C_DELEGATE = 6;
protected final static int C_PROPS = 7;

protected final static String[] TYPE_DESCS = new String[] {
"default",
"String", "int", "long", "double", "boolean",
"delegate", "property-based"
};

/// Type of bean being created
final protected BeanDescription _beanDesc;

final protected boolean _canFixAccess;

/**
* Reference to the default creator (constructor or factory method).
*<p>
* Note: name is a misnomer, after resolving of [JACKSON-850], since this
* can also point to factory method.
* Set of creators we have collected so far
*
* @since 2.5
*/
protected AnnotatedWithParams _defaultConstructor;

protected AnnotatedWithParams _stringCreator, _intCreator, _longCreator;
protected AnnotatedWithParams _doubleCreator, _booleanCreator;
protected final AnnotatedWithParams[] _creators = new AnnotatedWithParams[8];

protected AnnotatedWithParams _delegateCreator;
/**
* Bitmask of creators that were explicitly marked as creators; false for auto-detected
* (ones included base on naming and/or visibility, not annotation)
*
* @since 2.5
*/
protected int _explicitCreators = 0;

protected boolean _hasNonDefaultCreator = false;

// when there are injectable values along with delegate:
protected CreatorProperty[] _delegateArgs;

protected AnnotatedWithParams _propertyBasedCreator;
protected CreatorProperty[] _propertyBasedArgs = null;
protected CreatorProperty[] _propertyBasedArgs;

protected AnnotatedParameter _incompleteParameter;

Expand All @@ -61,9 +81,9 @@ public CreatorCollector(BeanDescription beanDesc, boolean canFixAccess)
public ValueInstantiator constructValueInstantiator(DeserializationConfig config)
{
JavaType delegateType;
boolean maybeVanilla = _delegateCreator == null;
if (maybeVanilla) {
boolean maybeVanilla = !_hasNonDefaultCreator;

if (maybeVanilla || (_creators[C_DELEGATE] == null)) {
delegateType = null;
} else {
// need to find type...
Expand All @@ -77,20 +97,14 @@ public ValueInstantiator constructValueInstantiator(DeserializationConfig config
}
}
TypeBindings bindings = _beanDesc.bindingsForBeanType();
delegateType = bindings.resolveType(_delegateCreator.getGenericParameterType(ix));
delegateType = bindings.resolveType(_creators[C_DELEGATE].getGenericParameterType(ix));
}

final JavaType type = _beanDesc.getType();

// Any non-standard creator will prevent; with one exception: int-valued constructor
// that standard containers have can be ignored
maybeVanilla &= (_propertyBasedCreator == null)
&& (_delegateCreator == null)
&& (_stringCreator == null)
&& (_longCreator == null)
&& (_doubleCreator == null)
&& (_booleanCreator == null)
;
maybeVanilla &= !_hasNonDefaultCreator;

if (maybeVanilla) {
/* 10-May-2014, tatu: If we have nothing special, and we are dealing with one
Expand All @@ -109,14 +123,14 @@ public ValueInstantiator constructValueInstantiator(DeserializationConfig config
}

StdValueInstantiator inst = new StdValueInstantiator(config, type);
inst.configureFromObjectSettings(_defaultConstructor,
_delegateCreator, delegateType, _delegateArgs,
_propertyBasedCreator, _propertyBasedArgs);
inst.configureFromStringCreator(_stringCreator);
inst.configureFromIntCreator(_intCreator);
inst.configureFromLongCreator(_longCreator);
inst.configureFromDoubleCreator(_doubleCreator);
inst.configureFromBooleanCreator(_booleanCreator);
inst.configureFromObjectSettings(_creators[C_DEFAULT],
_creators[C_DELEGATE], delegateType, _delegateArgs,
_creators[C_PROPS], _propertyBasedArgs);
inst.configureFromStringCreator(_creators[C_STRING]);
inst.configureFromIntCreator(_creators[C_INT]);
inst.configureFromLongCreator(_creators[C_LONG]);
inst.configureFromDoubleCreator(_creators[C_DOUBLE]);
inst.configureFromBooleanCreator(_creators[C_BOOLEAN]);
inst.configureIncompleteParameter(_incompleteParameter);
return inst;
}
Expand All @@ -137,35 +151,36 @@ public ValueInstantiator constructValueInstantiator(DeserializationConfig config
* factory method.
*/
public void setDefaultCreator(AnnotatedWithParams creator) {
_defaultConstructor = _fixAccess(creator);
_creators[C_DEFAULT] = _fixAccess(creator);
}

public void addStringCreator(AnnotatedWithParams creator) {
_stringCreator = verifyNonDup(creator, _stringCreator, "String");
public void addStringCreator(AnnotatedWithParams creator, boolean explicit) {
verifyNonDup(creator, C_STRING, explicit);
}
public void addIntCreator(AnnotatedWithParams creator) {
_intCreator = verifyNonDup(creator, _intCreator, "int");
public void addIntCreator(AnnotatedWithParams creator, boolean explicit) {
verifyNonDup(creator, C_INT, explicit);
}
public void addLongCreator(AnnotatedWithParams creator) {
_longCreator = verifyNonDup(creator, _longCreator, "long");
public void addLongCreator(AnnotatedWithParams creator, boolean explicit) {
verifyNonDup(creator, C_LONG, explicit);
}
public void addDoubleCreator(AnnotatedWithParams creator) {
_doubleCreator = verifyNonDup(creator, _doubleCreator, "double");
public void addDoubleCreator(AnnotatedWithParams creator, boolean explicit) {
verifyNonDup(creator, C_DOUBLE, explicit);
}
public void addBooleanCreator(AnnotatedWithParams creator) {
_booleanCreator = verifyNonDup(creator, _booleanCreator, "boolean");
public void addBooleanCreator(AnnotatedWithParams creator, boolean explicit) {
verifyNonDup(creator, C_BOOLEAN, explicit);
}

public void addDelegatingCreator(AnnotatedWithParams creator,
public void addDelegatingCreator(AnnotatedWithParams creator, boolean explicit,
CreatorProperty[] injectables)
{
_delegateCreator = verifyNonDup(creator, _delegateCreator, "delegate");
verifyNonDup(creator, C_DELEGATE, explicit);
_delegateArgs = injectables;
}

public void addPropertyCreator(AnnotatedWithParams creator, CreatorProperty[] properties)
public void addPropertyCreator(AnnotatedWithParams creator, boolean explicit,
CreatorProperty[] properties)
{
_propertyBasedCreator = verifyNonDup(creator, _propertyBasedCreator, "property-based");
verifyNonDup(creator, C_PROPS, explicit);
// [JACKSON-470] Better ensure we have no duplicate names either...
if (properties.length > 1) {
HashMap<String,Integer> names = new HashMap<String,Integer>();
Expand All @@ -192,6 +207,45 @@ public void addIncompeteParameter(AnnotatedParameter parameter) {
}
}

// Bunch of methods deprecated in 2.5, to be removed from 2.6 or later

@Deprecated // since 2.5
public void addStringCreator(AnnotatedWithParams creator) {
addStringCreator(creator, false);
}
@Deprecated // since 2.5
public void addIntCreator(AnnotatedWithParams creator) {
addBooleanCreator(creator, false);
}
@Deprecated // since 2.5
public void addLongCreator(AnnotatedWithParams creator) {
addBooleanCreator(creator, false);
}
@Deprecated // since 2.5
public void addDoubleCreator(AnnotatedWithParams creator) {
addBooleanCreator(creator, false);
}
@Deprecated // since 2.5
public void addBooleanCreator(AnnotatedWithParams creator) {
addBooleanCreator(creator, false);
}

@Deprecated // since 2.5
public void addDelegatingCreator(AnnotatedWithParams creator, CreatorProperty[] injectables) {
addDelegatingCreator(creator, false, injectables);
}

@Deprecated // since 2.5
public void addPropertyCreator(AnnotatedWithParams creator, CreatorProperty[] properties) {
addPropertyCreator(creator, false, properties);
}

@Deprecated // since 2.5, remove from 2.6
protected AnnotatedWithParams verifyNonDup(AnnotatedWithParams newOne, int typeIndex) {
verifyNonDup(newOne, typeIndex, false);
return _creators[typeIndex];
}

/*
/**********************************************************
/* Accessors
Expand All @@ -202,7 +256,7 @@ public void addIncompeteParameter(AnnotatedParameter parameter) {
* @since 2.1
*/
public boolean hasDefaultCreator() {
return _defaultConstructor != null;
return _creators[C_DEFAULT] != null;
}

/*
Expand All @@ -219,16 +273,29 @@ private <T extends AnnotatedMember> T _fixAccess(T member)
return member;
}

protected AnnotatedWithParams verifyNonDup(AnnotatedWithParams newOne, AnnotatedWithParams oldOne,
String type)
protected void verifyNonDup(AnnotatedWithParams newOne, int typeIndex, boolean explicit)
{
final int mask = (1 << typeIndex);
_hasNonDefaultCreator = true;
AnnotatedWithParams oldOne = _creators[typeIndex];
// already had an explicitly marked one?
if (oldOne != null) {
// important: ok to override factory with constructor; but not within same type, so:
if ((_explicitCreators & mask) != 0) { // already had explicitly annotated, leave as-is
// but skip, if new one not annotated
if (!explicit) {
return;
}
}
// one more thing: ok to override in sub-class
if (oldOne.getClass() == newOne.getClass()) {
throw new IllegalArgumentException("Conflicting "+type+" creators: already had "+oldOne+", encountered "+newOne);
throw new IllegalArgumentException("Conflicting "+TYPE_DESCS[typeIndex]
+" creators: already had explicitly marked "+oldOne+", encountered "+newOne);
}
}
return _fixAccess(newOne);
if (explicit) {
_explicitCreators |= mask;
}
_creators[typeIndex] = _fixAccess(newOne);
}

/*
Expand Down
Loading

0 comments on commit 257ae1c

Please sign in to comment.