From 89662b14ef9092fdecf812c8fc7d18e7de1dd0a5 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Mon, 21 Nov 2016 18:50:02 -0800 Subject: [PATCH] Fixed #1376 --- release-notes/VERSION | 2 + .../databind/AnnotationIntrospector.java | 12 +++- .../jackson/databind/BeanDescription.java | 67 ++++++++++++------- .../deser/BeanDeserializerFactory.java | 15 +---- .../AnnotationIntrospectorPair.java | 20 ++++-- .../introspect/BasicBeanDescription.java | 63 +++++++++-------- .../JacksonAnnotationIntrospector.java | 22 +++--- .../introspect/POJOPropertiesCollector.java | 39 ++++++----- ...nyGetter.java => TestAnyGetterAccess.java} | 2 +- ...tAnyProperties.java => AnySetterTest.java} | 26 ++++++- .../TestPOJOPropertiesCollector.java | 11 +-- 11 files changed, 171 insertions(+), 108 deletions(-) rename src/test/java/com/fasterxml/jackson/databind/access/{TestSerAnyGetter.java => TestAnyGetterAccess.java} (98%) rename src/test/java/com/fasterxml/jackson/databind/deser/{TestAnyProperties.java => AnySetterTest.java} (94%) diff --git a/release-notes/VERSION b/release-notes/VERSION index c1fd9e783b..8a3594132c 100644 --- a/release-notes/VERSION +++ b/release-notes/VERSION @@ -27,6 +27,8 @@ Project: jackson-databind by passing `MappingConfig` #1371: Add `MapperFeature.INFER_CREATOR_FROM_CONSTRUCTOR_PROPERTIES` to allow disabling use of `@CreatorProperties` as explicit `@JsonCreator` equivalent +#1376: Add ability to disable JsonAnySetter/JsonAnyGetter via mixin + (suggested by brentryan@github) #1399: Add support for `@JsonSetter(merge=OptBoolean.TRUE`) to allow "deep update" #1406: `ObjectMapper.readTree()` methods do not return `null` on end-of-input (reported by Fabrizio C) diff --git a/src/main/java/com/fasterxml/jackson/databind/AnnotationIntrospector.java b/src/main/java/com/fasterxml/jackson/databind/AnnotationIntrospector.java index 4f3c2f2195..6c341c8c1a 100644 --- a/src/main/java/com/fasterxml/jackson/databind/AnnotationIntrospector.java +++ b/src/main/java/com/fasterxml/jackson/databind/AnnotationIntrospector.java @@ -1189,8 +1189,10 @@ public PropertyName findNameForDeserialization(Annotated a) { * * @return True if such annotation is found (and is not disabled), * false otherwise + * + * @since 2.9 */ - public boolean hasAnySetterAnnotation(AnnotatedMethod am) { + public Boolean hasAnySetter(Annotated a) { return false; } @@ -1263,6 +1265,14 @@ public JsonCreator.Mode findCreatorBinding(Annotated a) { return null; } + /** + * @deprecated Since 2.9 use {@link #hasAnySetter} instead. + */ + @Deprecated // since 2.9 + public boolean hasAnySetterAnnotation(AnnotatedMethod am) { + return false; + } + /* /********************************************************** /* Overridable methods: may be used as low-level extension diff --git a/src/main/java/com/fasterxml/jackson/databind/BeanDescription.java b/src/main/java/com/fasterxml/jackson/databind/BeanDescription.java index f0945e0707..add17f3b1b 100644 --- a/src/main/java/com/fasterxml/jackson/databind/BeanDescription.java +++ b/src/main/java/com/fasterxml/jackson/databind/BeanDescription.java @@ -23,7 +23,7 @@ public abstract class BeanDescription { /** * Bean type information, including raw class and possible - * * generics information + * generics information */ protected final JavaType _type; @@ -155,42 +155,63 @@ protected BeanDescription(JavaType type) { /* Basic API for finding property accessors /********************************************************** */ - + + /** + * Method for locating accessor (readable field, or "getter" method) + * that has + * {@link com.fasterxml.jackson.annotation.JsonValue} annotation, + * if any. If multiple ones are found, + * an error is reported by throwing {@link IllegalArgumentException} + * + * @since 2.9 + */ + public abstract AnnotatedMember findJsonValueAccessor(); + public abstract AnnotatedMember findAnyGetter(); /** - * Method used to locate the method of introspected class that - * implements {@link com.fasterxml.jackson.annotation.JsonAnySetter}. If no such method exists - * null is returned. If more than one are found, an exception - * is thrown. + * Method used to locate a mutator (settable field, or 2-argument set method) + * of introspected class that + * implements {@link com.fasterxml.jackson.annotation.JsonAnySetter}. + * If no such mutator exists null is returned. If more than one are found, + * an exception is thrown. * Additional checks are also made to see that method signature * is acceptable: needs to take 2 arguments, first one String or * Object; second any can be any type. + * + * @since 2.9 */ - public abstract AnnotatedMethod findAnySetter(); + public abstract AnnotatedMember findAnySetterAccessor(); + + public abstract AnnotatedMethod findMethod(String name, Class[] paramTypes); + @Deprecated // since 2.9 + public abstract AnnotatedMethod findJsonValueMethod(); + /** - * Method used to locate the field of the class that implements - * {@link com.fasterxml.jackson.annotation.JsonAnySetter} If no such method - * exists null is returned. If more than one are found, an exception is thrown. - * - * @since 2.8 + * @deprecated Since 2.9: use {@link #findAnySetterAccessor} instead */ - public abstract AnnotatedMember findAnySetterField(); + @Deprecated + public AnnotatedMethod findAnySetter() { + AnnotatedMember m = findAnySetterAccessor(); + if (m instanceof AnnotatedMethod) { + return (AnnotatedMethod) m; + } + return null; + } /** - * Method for locating the getter method that is annotated with - * {@link com.fasterxml.jackson.annotation.JsonValue} annotation, - * if any. If multiple ones are found, - * an error is reported by throwing {@link IllegalArgumentException} + * @deprecated Since 2.9: use {@link #findAnySetterAccessor} instead */ - public abstract AnnotatedMember findJsonValueAccessor(); - - @Deprecated // since 2.9 - public abstract AnnotatedMethod findJsonValueMethod(); + @Deprecated + public AnnotatedMember findAnySetterField() { + AnnotatedMember m = findAnySetterAccessor(); + if (m instanceof AnnotatedField) { + return m; + } + return null; + } - public abstract AnnotatedMethod findMethod(String name, Class[] paramTypes); - /* /********************************************************** /* Basic API, class configuration 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 8037fce13c..b2d8c70b71 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerFactory.java @@ -455,19 +455,10 @@ protected void addBeanProps(DeserializationContext ctxt, } // Also, do we have a fallback "any" setter? - AnnotatedMethod anySetterMethod = beanDesc.findAnySetter(); - AnnotatedMember anySetterField = null; - if (anySetterMethod != null) { - builder.setAnySetter(constructAnySetter(ctxt, beanDesc, anySetterMethod)); + AnnotatedMember anySetter = beanDesc.findAnySetterAccessor(); + if (anySetter != null) { + builder.setAnySetter(constructAnySetter(ctxt, beanDesc, anySetter)); } else { - anySetterField = beanDesc.findAnySetterField(); - if (anySetterField != null) { - builder.setAnySetter(constructAnySetter(ctxt, beanDesc, anySetterField)); - } - } - // NOTE: we do NOT add @JsonIgnore'd properties into blocked ones if there's any-setter - // Implicit ones via @JsonIgnore and equivalent? - if (anySetterMethod == null && anySetterField == null) { Collection ignored2 = beanDesc.getIgnoredPropertyNames(); if (ignored2 != null) { for (String propName : ignored2) { diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotationIntrospectorPair.java b/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotationIntrospectorPair.java index 9a9d62fdec..77cca6b426 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotationIntrospectorPair.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/AnnotationIntrospectorPair.java @@ -720,6 +720,15 @@ public PropertyName findNameForDeserialization(Annotated a) return n; } + @Override + public Boolean hasAnySetter(Annotated a) { + Boolean b = _primary.hasAnySetter(a); + if (b == null) { + b = _secondary.hasAnySetter(a); + } + return b; + } + @Override public JsonSetter.Value findSetterInfo(Annotated a) { JsonSetter.Value v2 = _secondary.findSetterInfo(a); @@ -728,11 +737,6 @@ public JsonSetter.Value findSetterInfo(Annotated a) { ? v1 : v2.withOverrides(v1); } - @Override - public boolean hasAnySetterAnnotation(AnnotatedMethod am) { - return _primary.hasAnySetterAnnotation(am) || _secondary.hasAnySetterAnnotation(am); - } - @Override @Deprecated // since 2.9 public boolean hasCreatorAnnotation(Annotated a) { @@ -755,6 +759,12 @@ public JsonCreator.Mode findCreatorAnnotation(MapperConfig config, Annotated return (mode == null) ? _secondary.findCreatorAnnotation(config, a) : mode; } + @Override + @Deprecated // since 2.9 + public boolean hasAnySetterAnnotation(AnnotatedMethod am) { + return _primary.hasAnySetterAnnotation(am) || _secondary.hasAnySetterAnnotation(am); + } + protected boolean _isExplicitClassOrOb(Object maybeCls, Class implicit) { if (maybeCls instanceof Class) { Class cls = (Class) maybeCls; diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/BasicBeanDescription.java b/src/main/java/com/fasterxml/jackson/databind/introspect/BasicBeanDescription.java index 968e56a6ef..ffeffdba17 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/BasicBeanDescription.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/BasicBeanDescription.java @@ -293,25 +293,39 @@ public AnnotatedConstructor findDefaultConstructor() { } @Override - public AnnotatedMethod findAnySetter() throws IllegalArgumentException + public AnnotatedMember findAnySetterAccessor() throws IllegalArgumentException { - AnnotatedMethod anySetter = (_propCollector == null) ? null - : _propCollector.getAnySetterMethod(); - if (anySetter != null) { - /* Also, let's be somewhat strict on how field name is to be - * passed; String, Object make sense, others not - * so much. - */ - /* !!! 18-May-2009, tatu: how about enums? Can add support if - * requested; easy enough for devs to add support within - * method. - */ - Class type = anySetter.getRawParameterType(0); - if (type != String.class && type != Object.class) { - throw new IllegalArgumentException("Invalid 'any-setter' annotation on method "+anySetter.getName()+"(): first argument not of type String or Object, but "+type.getName()); + if (_propCollector != null) { + AnnotatedMethod anyMethod = _propCollector.getAnySetterMethod(); + if (anyMethod != null) { + // Also, let's be somewhat strict on how field name is to be + // passed; String, Object make sense, others not so much. + + /* !!! 18-May-2009, tatu: how about enums? Can add support if + * requested; easy enough for devs to add support within method. + */ + Class type = anyMethod.getRawParameterType(0); + if ((type != String.class) && (type != Object.class)) { + throw new IllegalArgumentException(String.format( +"Invalid 'any-setter' annotation on method '%s()': first argument not of type String or Object, but %s", +anyMethod.getName(), type.getName())); + } + return anyMethod; + } + AnnotatedMember anyField = _propCollector.getAnySetterField(); + if (anyField != null) { + // For now let's require a Map; in future can add support for other + // types like perhaps Iterable? + Class type = anyField.getRawType(); + if (!Map.class.isAssignableFrom(type)) { + throw new IllegalArgumentException(String.format( +"Invalid 'any-setter' annotation on field '%s': type is not instance of java.util.Map", +anyField.getName())); + } + return anyField; } } - return anySetter; + return null; } @Override @@ -465,23 +479,6 @@ public AnnotatedMember findAnyGetter() throws IllegalArgumentException return anyGetter; } - @Override - public AnnotatedMember findAnySetterField() throws IllegalArgumentException { - AnnotatedMember anySetter = (_propCollector == null) ? null : _propCollector.getAnySetterField(); - if (anySetter != null) { - /* - * For now let's require a Map; in future can add support for other - * types like perhaps Iterable? - */ - Class type = anySetter.getRawType(); - if (!Map.class.isAssignableFrom(type)) { - throw new IllegalArgumentException("Invalid 'any-setter' annotation on field " + anySetter.getName() - + "(): type is not instance of java.util.Map"); - } - } - return anySetter; - } - @Override public Map findBackReferenceProperties() { diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/JacksonAnnotationIntrospector.java b/src/main/java/com/fasterxml/jackson/databind/introspect/JacksonAnnotationIntrospector.java index a1fe95fe0c..cae04c7ea9 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/JacksonAnnotationIntrospector.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/JacksonAnnotationIntrospector.java @@ -992,8 +992,7 @@ public Boolean hasAnyGetter(Annotated a) { @Override @Deprecated // since 2.9 - public boolean hasAnyGetterAnnotation(AnnotatedMethod am) - { + public boolean hasAnyGetterAnnotation(AnnotatedMethod am) { // No dedicated disabling; regular @JsonIgnore used if needs to be ignored (handled separately) return _hasAnnotation(am, JsonAnyGetter.class); } @@ -1203,13 +1202,12 @@ public PropertyName findNameForDeserialization(Annotated a) } @Override - public boolean hasAnySetterAnnotation(AnnotatedMethod am) - { - /* No dedicated disabling; regular @JsonIgnore used - * if needs to be ignored (and if so, is handled prior - * to this method getting called) - */ - return _hasAnnotation(am, JsonAnySetter.class); + public Boolean hasAnySetter(Annotated a) { + JsonAnySetter ann = _findAnnotation(a, JsonAnySetter.class); + if (ann == null) { + return null; + } + return ann.enabled(); } @Override @@ -1217,6 +1215,12 @@ public JsonSetter.Value findSetterInfo(Annotated a) { return JsonSetter.Value.from(_findAnnotation(a, JsonSetter.class)); } + @Override + @Deprecated // since 2.9 + public boolean hasAnySetterAnnotation(AnnotatedMethod am) { + return _hasAnnotation(am, JsonAnySetter.class); + } + @Override @Deprecated // since 2.9 public boolean hasCreatorAnnotation(Annotated a) diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java index 3f06e72a22..3f3a80fbfa 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java @@ -3,7 +3,6 @@ import java.lang.reflect.Modifier; import java.util.*; -import com.fasterxml.jackson.annotation.JsonAnySetter; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.databind.*; import com.fasterxml.jackson.databind.cfg.HandlerInstantiator; @@ -189,8 +188,9 @@ public AnnotatedMember getJsonValueAccessor() // If @JsonValue defined, must have a single one if (_jsonValueAccessors != null) { if (_jsonValueAccessors.size() > 1) { - reportProblem("Multiple 'as-value' properties defined ("+_jsonValueAccessors.get(0)+" vs " - +_jsonValueAccessors.get(1)+")"); + reportProblem("Multiple 'as-value' properties defined (%s vs %s)", + _jsonValueAccessors.get(0), + _jsonValueAccessors.get(1)); } // otherwise we won't greatly care return _jsonValueAccessors.get(0); @@ -205,14 +205,14 @@ public AnnotatedMember getAnyGetter() } if (_anyGetters != null) { if (_anyGetters.size() > 1) { - reportProblem("Multiple 'any-getters' defined ("+_anyGetters.get(0)+" vs " - +_anyGetters.get(1)+")"); + reportProblem("Multiple 'any-getters' defined (%s vs %s)", + _anyGetters.get(0), _anyGetters.get(1)); } return _anyGetters.getFirst(); } return null; } - + public AnnotatedMember getAnySetterField() { if (!_collected) { @@ -220,8 +220,8 @@ public AnnotatedMember getAnySetterField() } if (_anySetterField != null) { if (_anySetterField.size() > 1) { - reportProblem("Multiple 'any-Setters' defined ("+_anySetters.get(0)+" vs " - +_anySetterField.get(1)+")"); + reportProblem("Multiple 'any-setter' fields defined (%s vs %s)", + _anySetterField.get(0), _anySetterField.get(1)); } return _anySetterField.getFirst(); } @@ -235,8 +235,8 @@ public AnnotatedMethod getAnySetterMethod() } if (_anySetters != null) { if (_anySetters.size() > 1) { - reportProblem("Multiple 'any-setters' defined ("+_anySetters.get(0)+" vs " - +_anySetters.get(1)+")"); + reportProblem("Multiple 'any-setter' methods defined (%s vs %s)", + _anySetters.get(0), _anySetters.get(1)); } return _anySetters.getFirst(); } @@ -395,8 +395,7 @@ protected void _addFields(Map props) continue; } // @JsonAnySetter? - // !!! 20-Nov-2016, tatu: This is wrong; needs to go via AnnotationIntrospector! - if (f.hasAnnotation(JsonAnySetter.class)) { + if (Boolean.TRUE.equals(ai.hasAnySetter(f))) { if (_anySetterField == null) { _anySetterField = new LinkedList(); } @@ -407,7 +406,6 @@ protected void _addFields(Map props) if (implName == null) { implName = f.getName(); } - PropertyName pn; if (ai == null) { @@ -550,11 +548,13 @@ protected void _addMethods(Map props) } else if (argCount == 1) { // setters _addSetterMethod(props, m, ai); } else if (argCount == 2) { // any getter? - if (ai != null && ai.hasAnySetterAnnotation(m)) { - if (_anySetters == null) { - _anySetters = new LinkedList(); + if (ai != null) { + if (Boolean.TRUE.equals(ai.hasAnySetter(m))) { + if (_anySetters == null) { + _anySetters = new LinkedList(); + } + _anySetters.add(m); } - _anySetters.add(m); } } } @@ -1032,7 +1032,10 @@ protected void _sortProperties(Map props) /********************************************************** */ - protected void reportProblem(String msg) { + protected void reportProblem(String msg, Object... args) { + if (args.length > 0) { + msg = String.format(msg, args); + } throw new IllegalArgumentException("Problem with definition of "+_classDef+": "+msg); } diff --git a/src/test/java/com/fasterxml/jackson/databind/access/TestSerAnyGetter.java b/src/test/java/com/fasterxml/jackson/databind/access/TestAnyGetterAccess.java similarity index 98% rename from src/test/java/com/fasterxml/jackson/databind/access/TestSerAnyGetter.java rename to src/test/java/com/fasterxml/jackson/databind/access/TestAnyGetterAccess.java index 4ca471b97f..311a86b873 100644 --- a/src/test/java/com/fasterxml/jackson/databind/access/TestSerAnyGetter.java +++ b/src/test/java/com/fasterxml/jackson/databind/access/TestAnyGetterAccess.java @@ -9,7 +9,7 @@ * Separate tests located in different package than code being * exercised; needed to trigger some access-related failures. */ -public class TestSerAnyGetter +public class TestAnyGetterAccess extends BaseMapTest { /* diff --git a/src/test/java/com/fasterxml/jackson/databind/deser/TestAnyProperties.java b/src/test/java/com/fasterxml/jackson/databind/deser/AnySetterTest.java similarity index 94% rename from src/test/java/com/fasterxml/jackson/databind/deser/TestAnyProperties.java rename to src/test/java/com/fasterxml/jackson/databind/deser/AnySetterTest.java index 6aa0aecded..598e9b2c45 100644 --- a/src/test/java/com/fasterxml/jackson/databind/deser/TestAnyProperties.java +++ b/src/test/java/com/fasterxml/jackson/databind/deser/AnySetterTest.java @@ -10,7 +10,7 @@ * Unit tests for verifying that {@link JsonAnySetter} annotation * works as expected. */ -public class TestAnyProperties +public class AnySetterTest extends BaseMapTest { static class MapImitator @@ -28,6 +28,16 @@ void addEntry(String key, Object value) } } + // for [databind#1376] + static class MapImitatorDisabled extends MapImitator + { + @Override + @JsonAnySetter(enabled=false) + void addEntry(String key, Object value) { + throw new RuntimeException("Should not get called"); + } + } + /** * Let's also verify that it is possible to define different * value: not often useful, but possible. @@ -228,6 +238,18 @@ public void testSimpleMapImitation() throws Exception assertEquals(Integer.valueOf(3), l.get(2)); } + public void testAnySetterDisable() throws Exception + { + try { + MAPPER.readValue(aposToQuotes("{'value':3}"), + MapImitatorDisabled.class); + fail("Should not pass"); + } catch (JsonMappingException e) { + verifyException(e, "Unrecognized field \"value\""); + } + + } + public void testSimpleTyped() throws Exception { MapImitatorWithValue mapHolder = MAPPER.readValue @@ -245,7 +267,7 @@ public void testBrokenWithDoubleAnnotations() throws Exception Broken b = MAPPER.readValue("{ \"a\" : 3 }", Broken.class); fail("Should have gotten an exception"); } catch (JsonMappingException e) { - verifyException(e, "Multiple 'any-setters'"); + verifyException(e, "Multiple 'any-setter' methods"); } } diff --git a/src/test/java/com/fasterxml/jackson/databind/introspect/TestPOJOPropertiesCollector.java b/src/test/java/com/fasterxml/jackson/databind/introspect/TestPOJOPropertiesCollector.java index 432d9ab90b..8c9994bbf2 100644 --- a/src/test/java/com/fasterxml/jackson/databind/introspect/TestPOJOPropertiesCollector.java +++ b/src/test/java/com/fasterxml/jackson/databind/introspect/TestPOJOPropertiesCollector.java @@ -438,13 +438,16 @@ public void testUseAnnotationsFalse() throws Exception public void testJackson744() throws Exception { - BeanDescription beanDesc = MAPPER.getDeserializationConfig().introspect(MAPPER.constructType(Issue744Bean.class)); + BeanDescription beanDesc = MAPPER.getDeserializationConfig().introspect + (MAPPER.constructType(Issue744Bean.class)); assertNotNull(beanDesc); - AnnotatedMethod setter = beanDesc.findAnySetter(); + AnnotatedMember setter = beanDesc.findAnySetterAccessor(); assertNotNull(setter); + assertEquals("addAdditionalProperty", setter.getName()); + assertTrue(setter instanceof AnnotatedMethod); } - // [#269]: Support new @JsonPropertyDescription + // [databind#269]: Support new @JsonPropertyDescription public void testPropertyDesc() throws Exception { // start via deser @@ -455,7 +458,7 @@ public void testPropertyDesc() throws Exception _verifyProperty(beanDesc, true, false, "13"); } - // [#438]: Support @JsonProperty.index + // [databind#438]: Support @JsonProperty.index public void testPropertyIndex() throws Exception { BeanDescription beanDesc = MAPPER.getDeserializationConfig().introspect(MAPPER.constructType(PropDescBean.class));