Skip to content

Commit

Permalink
Fix #1498
Browse files Browse the repository at this point in the history
  • Loading branch information
cowtowncoder committed Sep 18, 2020
1 parent 0ac0b46 commit cc6d372
Show file tree
Hide file tree
Showing 6 changed files with 91 additions and 20 deletions.
2 changes: 2 additions & 0 deletions release-notes/CREDITS-2.x
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,8 @@ Fernando Otero (zeitos@github)
Lovro Pandžić (lpandzic@github)
* Reported #421: @JsonCreator not used in case of multiple creators with parameter names
(2.5.0)
* Requested #1498: Allow handling of single-arg constructor as property based by default
(2.12.0)

Adam Stroud (adstro@github)
* Contributed #576: Add fluent API for adding mixins
Expand Down
2 changes: 2 additions & 0 deletions release-notes/VERSION-2.x
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ Project: jackson-databind
(reported by Mike G; fix contributed by Ville K)
#1296: Add `@JsonIncludeProperties(propertyNames)` (reverse of `@JsonIgnoreProperties`)
(contributed Baptiste P)
#1498: Allow handling of single-arg constructor as property based by default
(requested by Lovro P)
#1852: Allow case insensitive deserialization of String value into
`boolean`/`Boolean` (esp for Excel)
(requested by Patrick J)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.fasterxml.jackson.databind.cfg;

import com.fasterxml.jackson.databind.util.ClassUtil;

/**
* Configurable handler used to select aspects of selecting
* constructor to use as "Creator" for POJOs.
Expand Down Expand Up @@ -143,17 +145,17 @@ protected ConstructorDetector(SingleArgConstructor singleArgMode) {
this(singleArgMode, false, false);
}

protected ConstructorDetector withSingleArgMode(SingleArgConstructor singleArgMode) {
public ConstructorDetector withSingleArgMode(SingleArgConstructor singleArgMode) {
return new ConstructorDetector(singleArgMode,
_requireCtorAnnotation, _allowJDKTypeCtors);
}

protected ConstructorDetector withRequireAnnotation(boolean state) {
public ConstructorDetector withRequireAnnotation(boolean state) {
return new ConstructorDetector(_singleArgMode,
state, _allowJDKTypeCtors);
}

protected ConstructorDetector withAllowJDKTypes(boolean state) {
public ConstructorDetector withAllowJDKTypes(boolean state) {
return new ConstructorDetector(_singleArgMode,
_requireCtorAnnotation, state);
}
Expand Down Expand Up @@ -183,4 +185,13 @@ public boolean singleArgCreatorDefaultsToDelegating() {
public boolean singleArgCreatorDefaultsToProperties() {
return _singleArgMode == SingleArgConstructor.PROPERTIES;
}

public boolean allowImplicitCreators(Class<?> rawType) {
// May not allow implicit creator introspection at all:
if (_requireCtorAnnotation) {
return false;
}
// But if it is allowed, may further limit use for JDK types
return _allowJDKTypeCtors || !ClassUtil.isJDKClass(rawType);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -252,12 +252,16 @@ protected ValueInstantiator _constructDefaultValueInstantiator(DeserializationCo
throws JsonMappingException
{
final CreatorCollectionState ccState;
final boolean findImplicit;

{
final DeserializationConfig config = ctxt.getConfig();
// need to construct suitable visibility checker:
final VisibilityChecker<?> vchecker = config.getDefaultVisibilityChecker(beanDesc.getBeanClass(),
beanDesc.getClassInfo());
// 18-Sep-2020, tatu: Although by default implicit introspection is allowed, 2.12
// has settings to prevent that either generally, or at least for JDK types
findImplicit = config.getConstructorDetector().allowImplicitCreators(beanDesc.getBeanClass());

// 24-Sep-2014, tatu: Tricky part first; need to merge resolved property information
// (which has creator parameters sprinkled around) with actual creator
Expand All @@ -274,7 +278,7 @@ protected ValueInstantiator _constructDefaultValueInstantiator(DeserializationCo
}

// Start with explicitly annotated factory methods
_addExplicitFactoryCreators(ctxt, ccState);
_addExplicitFactoryCreators(ctxt, ccState, findImplicit);

// constructors only usable on concrete types:
if (beanDesc.getType().isConcrete()) {
Expand All @@ -296,16 +300,16 @@ protected ValueInstantiator _constructDefaultValueInstantiator(DeserializationCo
// TODO: look for `@JsonCreator` annotated ones, throw explicit exception?
;
} else {
_addExplicitConstructorCreators(ctxt, ccState);
if (!ccState.hasExplicitFactories() && !ccState.hasExplicitConstructors()
&& ccState.hasImplicitConstructorCandidates()) {
_addExplicitConstructorCreators(ctxt, ccState, findImplicit);
if (ccState.hasImplicitConstructorCandidates()
&& !ccState.hasExplicitFactories() && !ccState.hasExplicitConstructors()) {
_addImplicitConstructorCreators(ctxt, ccState, ccState.implicitConstructorCandidates());
}
}
}
// and finally, implicitly found factory methods if nothing explicit found
if (!ccState.hasExplicitFactories() && !ccState.hasExplicitConstructors()
&& ccState.hasImplicitFactoryCandidates()) {
if (ccState.hasImplicitFactoryCandidates()
&& !ccState.hasExplicitFactories() && !ccState.hasExplicitConstructors()) {
_addImplicitFactoryCreators(ctxt, ccState, ccState.implicitFactoryCandidates());
}
return ccState.creators.constructValueInstantiator(ctxt);
Expand Down Expand Up @@ -418,7 +422,7 @@ protected void _addRecordConstructor(DeserializationContext ctxt, CreatorCollect
*/

protected void _addExplicitConstructorCreators(DeserializationContext ctxt,
CreatorCollectionState ccState)
CreatorCollectionState ccState, boolean findImplicit)
throws JsonMappingException
{
final BeanDescription beanDesc = ccState.beanDesc;
Expand All @@ -444,7 +448,7 @@ protected void _addExplicitConstructorCreators(DeserializationContext ctxt,
}
if (creatorMode == null) {
// let's check Visibility here, to avoid further processing for non-visible?
if (vchecker.isCreatorVisible(ctor)) {
if (findImplicit && vchecker.isCreatorVisible(ctor)) {
ccState.addImplicitConstructorCandidate(CreatorCandidate.construct(intr,
ctor, creatorParams.get(ctor)));
}
Expand Down Expand Up @@ -623,7 +627,7 @@ protected void _addImplicitConstructorCreators(DeserializationContext ctxt,
*/

protected void _addExplicitFactoryCreators(DeserializationContext ctxt,
CreatorCollectionState ccState)
CreatorCollectionState ccState, boolean findImplicit)
throws JsonMappingException
{
final BeanDescription beanDesc = ccState.beanDesc;
Expand All @@ -638,7 +642,7 @@ protected void _addExplicitFactoryCreators(DeserializationContext ctxt,
final int argCount = factory.getParameterCount();
if (creatorMode == null) {
// Only potentially accept 1-argument factory methods
if ((argCount == 1) && vchecker.isCreatorVisible(factory)) {
if (findImplicit && (argCount == 1) && vchecker.isCreatorVisible(factory)) {
ccState.addImplicitFactoryCandidate(CreatorCandidate.construct(intr, factory, null));
}
continue;
Expand Down
13 changes: 10 additions & 3 deletions src/main/java/com/fasterxml/jackson/databind/util/ClassUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -1131,14 +1131,21 @@ public static boolean isJacksonStdImpl(Class<?> implClass) {
}

/**
* Some aspects of handling need to be changed for JDK types (and possibly
* some extensions under {@code javax.}?): for example, forcing of access
* Accessor for checking whether given {@code Class} is under Java package
* of {@code java.*} or {@code javax.*} (including all sub-packages).
*<p>
* Added since some aspects of handling need to be changed for JDK types (and
* possibly some extensions under {@code javax.}?): for example, forcing of access
* will not work well for future JDKs (12 and later).
*<p>
* Note: in Jackson 2.11 only returned true for {@code java.*} (and not {@code javax.*});
* was changed in 2.12.
*
* @since 2.11
*/
public static boolean isJDKClass(Class<?> rawType) {
return rawType.getName().startsWith("java.");
final String clsName = rawType.getName();
return clsName.startsWith("java.") || clsName.startsWith("javax.");
}

/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public SingleArg2CtorsNotAnnotated(@ImplicitName("value") long value) {
v = (int) (value * 2);
}
}

static class SingleArg1498 {
final int _bar;

Expand All @@ -77,10 +77,22 @@ static class SingleArg1498 {
}
}

static class TwoArgsNotAnnotated {
protected int _a, _b;

public TwoArgsNotAnnotated(@ImplicitName("a") int a, @ImplicitName("b") int b) {
_a = a;
_b = b;
}
}

private final ObjectMapper MAPPER_PROPS = mapperFor(ConstructorDetector.USE_PROPERTIES_BASED);
private final ObjectMapper MAPPER_DELEGATING = mapperFor(ConstructorDetector.USE_DELEGATING);
private final ObjectMapper MAPPER_EXPLICIT = mapperFor(ConstructorDetector.EXPLICIT_ONLY);

private final ObjectMapper MAPPER_MUST_ANNOTATE = mapperFor(ConstructorDetector.DEFAULT
.withRequireAnnotation(true));

/*
/**********************************************************************
/* Test methods, selecting from 1-arg constructors, properties-based
Expand All @@ -96,7 +108,6 @@ public void test1ArgDefaultsToPropertiesNonAnnotated() throws Exception

public void test1ArgDefaultsToPropertiesNoMode() throws Exception
{
// and similarly for mode-less
SingleArgNoMode value = MAPPER_PROPS.readValue(a2q("{'value' : 137 }"),
SingleArgNoMode.class);
assertEquals(137, value.v);
Expand All @@ -105,12 +116,20 @@ public void test1ArgDefaultsToPropertiesNoMode() throws Exception
// And specific test for original [databind#1498]
public void test1ArgDefaultsToPropertiesIssue1498() throws Exception
{
// and similarly for mode-less
SingleArg1498 value = MAPPER_PROPS.readValue(a2q("{'bar' : 404 }"),
SingleArg1498.class);
assertEquals(404, value._bar);
}

// This was working already but verify
public void testMultiArgAsProperties() throws Exception
{
TwoArgsNotAnnotated value = MAPPER_PROPS.readValue(a2q("{'a' : 3, 'b':4 }"),
TwoArgsNotAnnotated.class);
assertEquals(3, value._a);
assertEquals(4, value._b);
}

// 18-Sep-2020, tatu: For now there is a problematic case of multiple eligible
// choices; not cleanly solvable for 2.12
public void test1ArgDefaultsToPropsMultipleCtors() throws Exception
Expand Down Expand Up @@ -166,7 +185,7 @@ public void test1ArgDefaultsToHeuristics() throws Exception

/*
/**********************************************************************
/* Test methods, selecting from 1-arg constructors, explicit fail
/* Test methods, selecting from 1-arg constructors, explicit fails
/**********************************************************************
*/

Expand Down Expand Up @@ -197,6 +216,32 @@ public void test1ArgFailsNoMode() throws Exception
}
}

public void test1ArgRequiresAnnotation() throws Exception
{
// First: if there is a 0-arg ctor, fine, must use that
SingleArgNotAnnotated value = MAPPER_MUST_ANNOTATE.readValue("{ }",
SingleArgNotAnnotated.class);
assertEquals(new SingleArgNotAnnotated().v, value.v);

// But if no such ctor, will fail
try {
MAPPER_MUST_ANNOTATE.readValue(" { } ", SingleArg1498.class);
fail("Should not pass");
} catch (InvalidDefinitionException e) {
verifyException(e, "no Creators, like default constructor");
}
}

public void testMultiArgRequiresAnnotation() throws Exception
{
try {
MAPPER_MUST_ANNOTATE.readValue(" { } ", TwoArgsNotAnnotated.class);
fail("Should not pass");
} catch (InvalidDefinitionException e) {
verifyException(e, "no Creators, like default constructor");
}
}

/*
/**********************************************************************
/* Helper methods
Expand Down

0 comments on commit cc6d372

Please sign in to comment.