From f2bb0dfb8d11979d63cbecd18775a3cc25133fbb Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Thu, 14 Apr 2016 10:46:32 -0700 Subject: [PATCH] Fix #1194 --- release-notes/VERSION | 2 + .../deser/impl/ExternalTypeHandler.java | 17 ++-- .../jsontype/impl/TypeNameIdResolver.java | 28 +++--- .../jsontype/ExternalTypeId198Test.java | 70 +++++++++++++++ .../databind/jsontype/TestExternalId.java | 87 +++++++------------ .../jackson/failing/ExternalId96Test.java | 62 +++++++++++++ 6 files changed, 192 insertions(+), 74 deletions(-) create mode 100644 src/test/java/com/fasterxml/jackson/databind/jsontype/ExternalTypeId198Test.java create mode 100644 src/test/java/com/fasterxml/jackson/failing/ExternalId96Test.java diff --git a/release-notes/VERSION b/release-notes/VERSION index 3cd2cde214..487bfdf28d 100644 --- a/release-notes/VERSION +++ b/release-notes/VERSION @@ -5,11 +5,13 @@ Project: jackson-databind ------------------------------------------------------------------------ 2.7.4 (not yet released) + #1178: `@JsonSerialize(contentAs=superType)` behavior disallowed in 2.7 #1189: Converter called twice results in ClassCastException (reported by carrino@github) #1191: Non-matching quotes used in error message for date parsing #1194: Incorrect signature for generic type via `JavaType.getGenericSignature +#1198: Problem with `@JsonTypeInfo.As.EXTERNAL_PROPERTY`, `defaultImpl`, missing type id, NPE - Improve handling of custom content (de)serializers for `AtomicReference` 2.7.3 (16-Mar-2016) diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/impl/ExternalTypeHandler.java b/src/main/java/com/fasterxml/jackson/databind/deser/impl/ExternalTypeHandler.java index 50d0548687..26af9bcfa8 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/impl/ExternalTypeHandler.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/impl/ExternalTypeHandler.java @@ -257,7 +257,7 @@ protected final void _deserializeAndSet(JsonParser p, DeserializationContext ctx TokenBuffer merged = new TokenBuffer(p, ctxt); merged.writeStartArray(); merged.writeString(typeId); - + merged.copyCurrentStructure(p2); merged.writeEndArray(); // needs to point to START_OBJECT (or whatever first token is) @@ -265,7 +265,7 @@ protected final void _deserializeAndSet(JsonParser p, DeserializationContext ctx mp.nextToken(); _properties[index].getProperty().deserializeAndSet(mp, ctxt, bean); } - + /* /********************************************************** /* Helper classes @@ -284,7 +284,7 @@ public void addExternal(SettableBeanProperty property, TypeDeserializer typeDese _nameToPropertyIndex.put(property.getName(), index); _nameToPropertyIndex.put(typeDeser.getPropertyName(), index); } - + public ExternalTypeHandler build() { return new ExternalTypeHandler(_properties.toArray(new ExtTypedProperty[_properties.size()]), _nameToPropertyIndex, null, null); @@ -296,7 +296,7 @@ private final static class ExtTypedProperty private final SettableBeanProperty _property; private final TypeDeserializer _typeDeserializer; private final String _typePropertyName; - + public ExtTypedProperty(SettableBeanProperty property, TypeDeserializer typeDeser) { _property = property; @@ -312,6 +312,11 @@ public boolean hasDefaultType() { return _typeDeserializer.getDefaultImpl() != null; } + /** + * Specialized called when we need to expose type id of `defaultImpl` when + * serializing: we may need to expose it for assignment to a property, or + * it may be requested as visible for some other reason. + */ public String getDefaultTypeId() { Class defaultType = _typeDeserializer.getDefaultImpl(); if (defaultType == null) { @@ -319,9 +324,9 @@ public String getDefaultTypeId() { } return _typeDeserializer.getTypeIdResolver().idFromValueAndType(null, defaultType); } - + public String getTypePropertyName() { return _typePropertyName; } - + public SettableBeanProperty getProperty() { return _property; } diff --git a/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/TypeNameIdResolver.java b/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/TypeNameIdResolver.java index 734cae341f..9025daff9f 100644 --- a/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/TypeNameIdResolver.java +++ b/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/TypeNameIdResolver.java @@ -1,5 +1,7 @@ package com.fasterxml.jackson.databind.jsontype.impl; +import java.util.*; + import com.fasterxml.jackson.annotation.JsonTypeInfo; import com.fasterxml.jackson.databind.BeanDescription; import com.fasterxml.jackson.databind.DatabindContext; @@ -7,10 +9,6 @@ import com.fasterxml.jackson.databind.cfg.MapperConfig; import com.fasterxml.jackson.databind.jsontype.NamedType; -import java.util.Collection; -import java.util.HashMap; -import java.util.TreeSet; - public class TypeNameIdResolver extends TypeIdResolverBase { protected final MapperConfig _config; @@ -18,15 +16,15 @@ public class TypeNameIdResolver extends TypeIdResolverBase /** * Mappings from class name to type id, used for serialization */ - protected final HashMap _typeToId; + protected final Map _typeToId; /** * Mappings from type id to JavaType, used for deserialization */ - protected final HashMap _idToType; + protected final Map _idToType; protected TypeNameIdResolver(MapperConfig config, JavaType baseType, - HashMap typeToId, HashMap idToType) + Map typeToId, Map idToType) { super(baseType, config.getTypeFactory()); _config = config; @@ -39,14 +37,17 @@ public static TypeNameIdResolver construct(MapperConfig config, JavaType base { // sanity check if (forSer == forDeser) throw new IllegalArgumentException(); - HashMap typeToId = null; - HashMap idToType = null; + Map typeToId = null; + Map idToType = null; if (forSer) { typeToId = new HashMap(); } if (forDeser) { idToType = new HashMap(); + // 14-Apr-2016, tatu: Apparently needed for special case of `defaultImpl`; + // see [databind#1198] for details. + typeToId = new TreeMap(); } if (subtypes != null) { for (NamedType t : subtypes) { @@ -59,10 +60,8 @@ public static TypeNameIdResolver construct(MapperConfig config, JavaType base typeToId.put(cls.getName(), id); } if (forDeser) { - /* 24-Feb-2011, tatu: [JACKSON-498] One more problem; sometimes - * we have same name for multiple types; if so, use most specific - * one. - */ + // One more problem; sometimes we have same name for multiple types; + // if so, use most specific JavaType prev = idToType.get(id); if (prev != null) { // Can only override if more specific if (cls.isAssignableFrom(prev.getRawClass())) { // nope, more generic (or same) @@ -87,12 +86,13 @@ public String idFromValue(Object value) protected String idFromClass(Class clazz) { - if(clazz==null){ + if (clazz == null) { return null; } Class cls = _typeFactory.constructType(clazz).getRawClass(); final String key = cls.getName(); String name; + synchronized (_typeToId) { name = _typeToId.get(key); if (name == null) { diff --git a/src/test/java/com/fasterxml/jackson/databind/jsontype/ExternalTypeId198Test.java b/src/test/java/com/fasterxml/jackson/databind/jsontype/ExternalTypeId198Test.java new file mode 100644 index 0000000000..effef58200 --- /dev/null +++ b/src/test/java/com/fasterxml/jackson/databind/jsontype/ExternalTypeId198Test.java @@ -0,0 +1,70 @@ +package com.fasterxml.jackson.databind.jsontype; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonSubTypes; +import com.fasterxml.jackson.annotation.JsonTypeInfo; + +import com.fasterxml.jackson.databind.*; + +public class ExternalTypeId198Test extends BaseMapTest +{ + public enum Attacks { KICK, PUNCH } + + static class Character { + public String name; + public Attacks preferredAttack; + + @JsonTypeInfo(use=JsonTypeInfo.Id.NAME, defaultImpl=Kick.class, + include=JsonTypeInfo.As.EXTERNAL_PROPERTY, property="preferredAttack") + @JsonSubTypes({ + @JsonSubTypes.Type(value=Kick.class, name="KICK"), + @JsonSubTypes.Type(value=Punch.class, name="PUNCH") + }) + public Attack attack; + } + + public static abstract class Attack { + public String side; + + @JsonCreator + public Attack(String side) { + this.side = side; + } + } + + public static class Kick extends Attack { + @JsonCreator + public Kick(String side) { + super(side); + } + } + + public static class Punch extends Attack { + @JsonCreator + public Punch(String side) { + super(side); + } + } + + final ObjectMapper MAPPER = new ObjectMapper(); + + public void testFails() throws Exception { + String json = "{ \"name\": \"foo\", \"attack\":\"right\" } }"; + + Character character = MAPPER.readValue(json, Character.class); + + assertNotNull(character); + assertNotNull(character.attack); + assertEquals("foo", character.name); + } + + public void testWorks() throws Exception { + String json = "{ \"name\": \"foo\", \"preferredAttack\": \"KICK\", \"attack\":\"right\" } }"; + + Character character = MAPPER.readValue(json, Character.class); + + assertNotNull(character); + assertNotNull(character.attack); + assertEquals("foo", character.name); + } +} diff --git a/src/test/java/com/fasterxml/jackson/databind/jsontype/TestExternalId.java b/src/test/java/com/fasterxml/jackson/databind/jsontype/TestExternalId.java index 3186c520f0..7bdcd09b98 100644 --- a/src/test/java/com/fasterxml/jackson/databind/jsontype/TestExternalId.java +++ b/src/test/java/com/fasterxml/jackson/databind/jsontype/TestExternalId.java @@ -74,7 +74,7 @@ public ExternalBeanWithCreator(@JsonProperty("foo") int f) value = new ValueBean(f); } } - + @JsonTypeName("vbean") static class ValueBean { public int value; @@ -251,6 +251,36 @@ static class Payload928 { public String something; } + enum Type965 { BIG_DECIMAL } + + static class Wrapper965 { + protected Type965 typeEnum; + + protected Object value; + + @JsonGetter("type") + String getTypeString() { + return typeEnum.name(); + } + + @JsonSetter("type") + void setTypeString(String type) { + this.typeEnum = Type965.valueOf(type); + } + + @JsonGetter(value = "objectValue") + Object getValue() { + return value; + } + + @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.EXTERNAL_PROPERTY, property = "type") + @JsonSubTypes({ @JsonSubTypes.Type(name = "BIG_DECIMAL", value = BigDecimal.class) }) + @JsonSetter(value = "objectValue") + private void setValue(Object value) { + this.value = value; + } + } + /* /********************************************************** /* Unit tests, serialization @@ -395,29 +425,7 @@ public void testIssue831() throws Exception assertEquals("dog", result.petType); } - // For [Issue#96]: should allow use of default impl, if property missing - /* 18-Jan-2013, tatu: Unfortunately this collides with [Issue#118], and I don't - * know what the best resolution is. For now at least - */ - /* - public void testWithDefaultAndMissing() throws Exception - { - ExternalBeanWithDefault input = new ExternalBeanWithDefault(13); - // baseline: include type, verify things work: - String fullJson = MAPPER.writeValueAsString(input); - ExternalBeanWithDefault output = MAPPER.readValue(fullJson, ExternalBeanWithDefault.class); - assertNotNull(output); - assertNotNull(output.bean); - // and then try without type info... - ExternalBeanWithDefault defaulted = MAPPER.readValue("{\"bean\":{\"value\":13}}", - ExternalBeanWithDefault.class); - assertNotNull(defaulted); - assertNotNull(defaulted.bean); - assertSame(ValueBean.class, defaulted.bean.getClass()); - } - */ - - // For [Issue#118] + // For [databind#118] // Note: String works fine, since no type id will used; other scalar types have issues public void testWithScalar118() throws Exception { @@ -431,7 +439,7 @@ public void testWithScalar118() throws Exception assertTrue(result.value instanceof java.util.Date); } - // For [Issue#118] using "natural" type(s) + // For [databind#118] using "natural" type(s) public void testWithNaturalScalar118() throws Exception { ExternalTypeWithNonPOJO input = new ExternalTypeWithNonPOJO(Integer.valueOf(13)); @@ -511,35 +519,6 @@ public void testInverseExternalId928() throws Exception assertEquals(Payload928.class, envelope2._payload.getClass()); } - enum Type965 { BIG_DECIMAL } - - static class Wrapper965 { - protected Type965 typeEnum; - - protected Object value; - - @JsonGetter("type") - String getTypeString() { - return typeEnum.name(); - } - - @JsonSetter("type") - void setTypeString(String type) { - this.typeEnum = Type965.valueOf(type); - } - - @JsonGetter(value = "objectValue") - Object getValue() { - return value; - } - - @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.EXTERNAL_PROPERTY, property = "type") - @JsonSubTypes({ @JsonSubTypes.Type(name = "BIG_DECIMAL", value = BigDecimal.class) }) - @JsonSetter(value = "objectValue") - private void setValue(Object value) { - this.value = value; - } - } // for [databind#965] public void testBigDecimal965() throws Exception { diff --git a/src/test/java/com/fasterxml/jackson/failing/ExternalId96Test.java b/src/test/java/com/fasterxml/jackson/failing/ExternalId96Test.java new file mode 100644 index 0000000000..0d19357bdd --- /dev/null +++ b/src/test/java/com/fasterxml/jackson/failing/ExternalId96Test.java @@ -0,0 +1,62 @@ +package com.fasterxml.jackson.failing; + +import com.fasterxml.jackson.annotation.*; +import com.fasterxml.jackson.annotation.JsonTypeInfo.As; +import com.fasterxml.jackson.annotation.JsonTypeInfo.Id; + +import com.fasterxml.jackson.databind.BaseMapTest; +import com.fasterxml.jackson.databind.ObjectMapper; + +// Tests for External type id, one that exists at same level as typed Object, +// that is, property is not within typed object but a member of its parent. +public class ExternalId96Test extends BaseMapTest +{ + // for [databind#96] + static class ExternalBeanWithDefault + { + @JsonTypeInfo(use=Id.CLASS, include=As.EXTERNAL_PROPERTY, property="extType", + defaultImpl=ValueBean.class) + public Object bean; + + public ExternalBeanWithDefault() { } + public ExternalBeanWithDefault(int v) { + bean = new ValueBean(v); + } + } + + @JsonTypeName("vbean") + static class ValueBean { + public int value; + + public ValueBean() { } + public ValueBean(int v) { value = v; } + } + + /* + /********************************************************** + /* Unit tests, deserialization + /********************************************************** + */ + + private final ObjectMapper MAPPER = new ObjectMapper(); + + // For [databind#96]: should allow use of default impl, if property missing + /* 18-Jan-2013, tatu: Unfortunately this collides with [databind#118], and I don't + * know what the best resolution is. For now at least + */ + public void testWithDefaultAndMissing() throws Exception + { + ExternalBeanWithDefault input = new ExternalBeanWithDefault(13); + // baseline: include type, verify things work: + String fullJson = MAPPER.writeValueAsString(input); + ExternalBeanWithDefault output = MAPPER.readValue(fullJson, ExternalBeanWithDefault.class); + assertNotNull(output); + assertNotNull(output.bean); + // and then try without type info... + ExternalBeanWithDefault defaulted = MAPPER.readValue("{\"bean\":{\"value\":13}}", + ExternalBeanWithDefault.class); + assertNotNull(defaulted); + assertNotNull(defaulted.bean); + assertSame(ValueBean.class, defaulted.bean.getClass()); + } +}