Skip to content

Commit

Permalink
More work on #1498, now cover handling of annotated-but-mode-less case
Browse files Browse the repository at this point in the history
  • Loading branch information
cowtowncoder committed Sep 14, 2020
1 parent f939280 commit a1c4cbc
Show file tree
Hide file tree
Showing 3 changed files with 267 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public enum SingleArgConstructor {
* {@link com.fasterxml.jackson.databind.exc.InvalidDefinitionException}
* in ambiguous case.
*/
FAIL;
REQUIRE_MODE;
}

// @FunctionalInterface
Expand Down Expand Up @@ -115,10 +115,10 @@ public AnnotatedConstructor select(DatabindContext ctxt,

/**
* Instance similar to {@link #DEFAULT} except that for single-argument case
* uses setting of {@link SingleArgConstructor#FAIL}.
* uses setting of {@link SingleArgConstructor#REQUIRE_MODE}.
*/
public final static ConstructorDetector EXPLICIT_ONLY
= new ConstructorDetector(SingleArgConstructor.FAIL);
= new ConstructorDetector(SingleArgConstructor.REQUIRE_MODE);

/*
/**********************************************************************
Expand Down Expand Up @@ -209,4 +209,12 @@ public boolean requireCtorAnnotation() {
public boolean allowJDKTypeConstructors() {
return _allowJDKTypeCtors;
}

public boolean singleArgCreatorDefaultsToDelegating() {
return _singleArgMode == SingleArgConstructor.DELEGATING;
}

public boolean singleArgCreatorDefaultsToProperties() {
return _singleArgMode == SingleArgConstructor.PROPERTIES;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import com.fasterxml.jackson.databind.*;
import com.fasterxml.jackson.databind.cfg.ConfigOverride;
import com.fasterxml.jackson.databind.cfg.ConstructorDetector;
import com.fasterxml.jackson.databind.cfg.DeserializerFactoryConfig;
import com.fasterxml.jackson.databind.cfg.HandlerInstantiator;
import com.fasterxml.jackson.databind.deser.impl.CreatorCandidate;
Expand Down Expand Up @@ -401,6 +402,7 @@ protected void _addDeserializerConstructors(DeserializationContext ctxt,
}
continue;
}

switch (creatorMode) {
case DELEGATING:
_addExplicitDelegatingCreator(ctxt, beanDesc, creators,
Expand All @@ -412,7 +414,8 @@ protected void _addDeserializerConstructors(DeserializationContext ctxt,
break;
default:
_addExplicitAnyCreator(ctxt, beanDesc, creators,
CreatorCandidate.construct(intr, ctor, creatorParams.get(ctor)));
CreatorCandidate.construct(intr, ctor, creatorParams.get(ctor)),
ctxt.getConfig().getConstructorDetector());
break;
}
++explCount;
Expand Down Expand Up @@ -666,59 +669,107 @@ protected void _addExplicitPropertyCreator(DeserializationContext ctxt,
creators.addPropertyCreator(candidate.creator(), true, properties);
}

@Deprecated // since 2.12, remove from 2.13 or later
protected void _addExplicitAnyCreator(DeserializationContext ctxt,
BeanDescription beanDesc, CreatorCollector creators,
CreatorCandidate candidate)
throws JsonMappingException
{
_addExplicitAnyCreator(ctxt, beanDesc, creators, candidate,
ctxt.getConfig().getConstructorDetector());
}

/**
* Helper method called when there is explicit "is-creator" marker, but no mode declaration.
* Helper method called when there is explicit "is-creator" marker,
* but no mode declaration.
*
* @since 2.9.2
* @since 2.12
*/
protected void _addExplicitAnyCreator(DeserializationContext ctxt,
BeanDescription beanDesc, CreatorCollector creators,
CreatorCandidate candidate)
CreatorCandidate candidate, ConstructorDetector ctorDetector)
throws JsonMappingException
{
// Looks like there's bit of magic regarding 1-parameter creators; others simpler:
if (1 != candidate.paramCount()) {
// Ok: for delegates, we want one and exactly one parameter without
// injection AND without name
int oneNotInjected = candidate.findOnlyParamWithoutInjection();
if (oneNotInjected >= 0) {
// getting close; but most not have name
if (candidate.paramName(oneNotInjected) == null) {
_addExplicitDelegatingCreator(ctxt, beanDesc, creators, candidate);
return;

// 13-Sep-2020, tatu: Can only be delegating if not forced to be properties-based
if (!ctorDetector.singleArgCreatorDefaultsToProperties()) {
int oneNotInjected = candidate.findOnlyParamWithoutInjection();
if (oneNotInjected >= 0) {
// getting close; but most not have name (or be explicitly specified
// as default-to-delegate)
if (ctorDetector.singleArgCreatorDefaultsToDelegating()
|| candidate.paramName(oneNotInjected) == null) {
_addExplicitDelegatingCreator(ctxt, beanDesc, creators, candidate);
return;
}
}
}
_addExplicitPropertyCreator(ctxt, beanDesc, creators, candidate);
return;
}
AnnotatedParameter param = candidate.parameter(0);
JacksonInject.Value injectId = candidate.injection(0);
PropertyName paramName = candidate.explicitParamName(0);
BeanPropertyDefinition paramDef = candidate.propertyDef(0);

// If there's injection or explicit name, should be properties-based
boolean useProps = (paramName != null) || (injectId != null);
if (!useProps && (paramDef != null)) {
// One more thing: if implicit name matches property with a getter
// or field, we'll consider it property-based as well

// 25-May-2018, tatu: as per [databind#2051], looks like we have to get
// not implicit name, but name with possible strategy-based-rename
// paramName = candidate.findImplicitParamName(0);
// And here be the "simple" single-argument construct
final AnnotatedParameter param = candidate.parameter(0);
final JacksonInject.Value injectId = candidate.injection(0);
PropertyName paramName = null;

boolean useProps;
switch (ctorDetector.singleArgMode()) {
case DELEGATING:
useProps = false;
break;
case PROPERTIES:
useProps = true;
// 13-Sep-2020, tatu: since we are configured to prefer Properties-style,
// any name (explicit OR implicit does):
paramName = candidate.paramName(0);
useProps = (paramName != null) && paramDef.couldSerialize();
break;

case REQUIRE_MODE:
ctxt.reportBadTypeDefinition(beanDesc,
"Single-argument constructor (%s) is annotated but no 'mode' defined; `CreatorDetector`"
+ "configured with `SingleArgConstructor.REQUIRE_MODE`",
candidate.creator());
return;
case HEURISTIC:
default:
{ // Note: behavior pre-Jackson-2.12
final BeanPropertyDefinition paramDef = candidate.propertyDef(0);
// with heuristic, need to start with just explicit name
paramName = candidate.explicitParamName(0);

// If there's injection or explicit name, should be properties-based
useProps = (paramName != null) || (injectId != null);
if (!useProps && (paramDef != null)) {
// One more thing: if implicit name matches property with a getter
// or field, we'll consider it property-based as well

// 25-May-2018, tatu: as per [databind#2051], looks like we have to get
// not implicit name, but name with possible strategy-based-rename
// paramName = candidate.findImplicitParamName(0);
paramName = candidate.paramName(0);
useProps = (paramName != null) && paramDef.couldSerialize();
}
}
}

if (useProps) {
SettableBeanProperty[] properties = new SettableBeanProperty[] {
constructCreatorProperty(ctxt, beanDesc, paramName, 0, param, injectId)
};
creators.addPropertyCreator(candidate.creator(), true, properties);
return;
}

_handleSingleArgumentCreator(creators, candidate.creator(), true, true);

// one more thing: sever link to creator property, to avoid possible later
// problems with "unresolved" constructor property
final BeanPropertyDefinition paramDef = candidate.propertyDef(0);
if (paramDef != null) {
((POJOPropertyBuilder) paramDef).removeConstructors();
}
Expand Down Expand Up @@ -842,7 +893,10 @@ private void _checkImplicitlyNamedConstructors(DeserializationContext ctxt,
case DEFAULT:
default:
_addExplicitAnyCreator(ctxt, beanDesc, creators,
CreatorCandidate.construct(intr, factory, creatorParams.get(factory)));
CreatorCandidate.construct(intr, factory, creatorParams.get(factory)),
// 13-Sep-2020, tatu: Factory methods do not follow config settings
// (as of Jackson 2.12)
ConstructorDetector.DEFAULT);
break;
}
++explCount;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
package com.fasterxml.jackson.databind.deser.creators;

import java.lang.annotation.*;

import com.fasterxml.jackson.annotation.JsonCreator;

import com.fasterxml.jackson.databind.*;
import com.fasterxml.jackson.databind.cfg.*;
import com.fasterxml.jackson.databind.exc.InvalidDefinitionException;
import com.fasterxml.jackson.databind.introspect.AnnotatedMember;
import com.fasterxml.jackson.databind.introspect.JacksonAnnotationIntrospector;
import com.fasterxml.jackson.databind.json.JsonMapper;

// Tests for [databind#1498] (Jackson 2.12)
public class ConstructorDetector1498Test extends BaseMapTest
{
// Helper annotation to work around lack of implicit name access with Jackson 2.x
@Target(ElementType.PARAMETER)
@Retention(RetentionPolicy.RUNTIME)
@interface ImplicitName {
String value();
}

// And annotation introspector to make use of it
@SuppressWarnings("serial")
static class CtorNameIntrospector extends JacksonAnnotationIntrospector
{
@Override
public String findImplicitPropertyName(//MapperConfig<?> config,
AnnotatedMember member) {
final ImplicitName ann = member.getAnnotation(ImplicitName.class);
return (ann == null) ? null : ann.value();
}
}

static class SingleArgNotAnnotated {
protected int v;

// SingleArgNotAnnotated() { throw new Error("Should not be used"); }

public SingleArgNotAnnotated(int value) {
v = value;
}
}

static class SingleArgNoMode {
protected int v;

// SingleArgNoMode() { throw new Error("Should not be used"); }

@JsonCreator
public SingleArgNoMode(@ImplicitName("value") int value) {
v = value;
}
}

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);

/*
/**********************************************************************
/* Test methods, selecting from 1-arg constructors, properties-based
/**********************************************************************
*/

/*
public void test1ArgDefaultsToPropertiesNonAnnotated() throws Exception
{
SingleArgNotAnnotated value = MAPPER_PROPS.readValue(a2q("{'value' : 137 }"),
SingleArgNotAnnotated.class);
assertEquals(137, value.v);
}
*/

public void test1ArgDefaultsToPropertiesNoMode() throws Exception
{
// and similarly for mode-less
SingleArgNoMode value = MAPPER_PROPS.readValue(a2q("{'value' : 137 }"),
SingleArgNoMode.class);
assertEquals(137, value.v);
}

/*
/**********************************************************************
/* Test methods, selecting from 1-arg constructors, delegating
/**********************************************************************
*/

/*
public void test1ArgDefaultsToDelegatingNoAnnotation() throws Exception
{
// No annotation, fail
try {
MAPPER_DELEGATING.readValue(" 2812 ", SingleArgNotAnnotated.class);
fail("Should not pass");
} catch (Exception e) {
verifyException(e, "foobar");
}
}
*/

public void test1ArgDefaultsToDelegatingNoMode() throws Exception
{
// One with `@JsonCreator` no mode annotation (ok since indicated)
SingleArgNoMode value = MAPPER_DELEGATING.readValue(" 2812 ", SingleArgNoMode.class);
assertEquals(2812, value.v);
}

/*
/**********************************************************************
/* Test methods, selecting from 1-arg constructors, other
/**********************************************************************
*/

public void test1ArgDefaultsToHeuristics() throws Exception
{
final ObjectMapper mapper = mapperFor(ConstructorDetector.DEFAULT);
final String DOC = " 13117 ";

// First: unannotated is ok, defaults to delegating
SingleArgNotAnnotated v1 = mapper.readValue(DOC, SingleArgNotAnnotated.class);
assertEquals(13117, v1.v);

// and ditto for mode-less
SingleArgNoMode v2 = mapper.readValue(DOC, SingleArgNoMode.class);
assertEquals(13117, v2.v);
}

/*
public void test1ArgFailsNoAnnotation() throws Exception
{
// First: fail if nothing annotated (for 1-arg case)
try {
MAPPER_EXPLICIT.readValue(" 2812 ", SingleArgNotAnnotated.class);
fail("Should not pass");
} catch (Exception e) {
verifyException(e, "foobar");
}
}
*/

public void test1ArgFailsNoMode() throws Exception
{
// Second: also fail also if no "mode" indicated
try {
MAPPER_EXPLICIT.readValue(" 2812 ", SingleArgNoMode.class);
fail("Should not pass");
} catch (InvalidDefinitionException e) {
verifyException(e, "no 'mode' defined");
verifyException(e, "SingleArgConstructor.REQUIRE_MODE");
}
}

/*
/**********************************************************************
/* Test methods
/**********************************************************************
*/

/*
/**********************************************************************
/* Helper methods
/**********************************************************************
*/

private JsonMapper.Builder mapperBuilder() {
return JsonMapper.builder()
.annotationIntrospector(new CtorNameIntrospector());
}

private ObjectMapper mapperFor(ConstructorDetector cd) {
return mapperBuilder()
.constructorDetector(cd)
.build();
}
}

0 comments on commit a1c4cbc

Please sign in to comment.