From cc6d3729b017c7d19b47a7dca5fedf932c546608 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Fri, 18 Sep 2020 14:55:54 -0700 Subject: [PATCH] Fix #1498 --- release-notes/CREDITS-2.x | 2 + release-notes/VERSION-2.x | 2 + .../databind/cfg/ConstructorDetector.java | 17 ++++-- .../deser/BasicDeserializerFactory.java | 24 +++++---- .../jackson/databind/util/ClassUtil.java | 13 +++-- .../creators/ConstructorDetector1498Test.java | 53 +++++++++++++++++-- 6 files changed, 91 insertions(+), 20 deletions(-) diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x index b38d9477e2..d91bbe72d9 100644 --- a/release-notes/CREDITS-2.x +++ b/release-notes/CREDITS-2.x @@ -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 diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index f251a66fa2..27086c0381 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -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) diff --git a/src/main/java/com/fasterxml/jackson/databind/cfg/ConstructorDetector.java b/src/main/java/com/fasterxml/jackson/databind/cfg/ConstructorDetector.java index f0f0883e7b..d9786a15dd 100644 --- a/src/main/java/com/fasterxml/jackson/databind/cfg/ConstructorDetector.java +++ b/src/main/java/com/fasterxml/jackson/databind/cfg/ConstructorDetector.java @@ -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. @@ -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); } @@ -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); + } } diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/BasicDeserializerFactory.java b/src/main/java/com/fasterxml/jackson/databind/deser/BasicDeserializerFactory.java index 96f1532f35..5e347188aa 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/BasicDeserializerFactory.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/BasicDeserializerFactory.java @@ -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 @@ -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()) { @@ -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); @@ -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; @@ -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))); } @@ -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; @@ -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; diff --git a/src/main/java/com/fasterxml/jackson/databind/util/ClassUtil.java b/src/main/java/com/fasterxml/jackson/databind/util/ClassUtil.java index 073bb4e271..f2686bbefa 100644 --- a/src/main/java/com/fasterxml/jackson/databind/util/ClassUtil.java +++ b/src/main/java/com/fasterxml/jackson/databind/util/ClassUtil.java @@ -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). + *

+ * 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). + *

+ * 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."); } /* diff --git a/src/test/java/com/fasterxml/jackson/databind/deser/creators/ConstructorDetector1498Test.java b/src/test/java/com/fasterxml/jackson/databind/deser/creators/ConstructorDetector1498Test.java index cb0c4c31df..847e8baa8a 100644 --- a/src/test/java/com/fasterxml/jackson/databind/deser/creators/ConstructorDetector1498Test.java +++ b/src/test/java/com/fasterxml/jackson/databind/deser/creators/ConstructorDetector1498Test.java @@ -67,7 +67,7 @@ public SingleArg2CtorsNotAnnotated(@ImplicitName("value") long value) { v = (int) (value * 2); } } - + static class SingleArg1498 { final int _bar; @@ -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 @@ -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); @@ -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 @@ -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 /********************************************************************** */ @@ -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