From 45d287b36813a6d989f5c4a784ac3c166179e82d Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Mon, 28 Nov 2016 23:13:44 -0800 Subject: [PATCH] Fixed #291 --- .../deser/impl/ExternalTypeHandler.java | 108 ++++++++++++++--- .../ext/MultipleExternalIds291Test.java | 111 ++++++++++++++++++ .../failing/TestMultipleExternalIds291.java | 71 ----------- 3 files changed, 201 insertions(+), 89 deletions(-) create mode 100644 src/test/java/com/fasterxml/jackson/databind/jsontype/ext/MultipleExternalIds291Test.java delete mode 100644 src/test/java/com/fasterxml/jackson/failing/TestMultipleExternalIds291.java 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 6155a73dab..1be53a2fe2 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 @@ -21,14 +21,21 @@ public class ExternalTypeHandler private final JavaType _beanType; private final ExtTypedProperty[] _properties; - private final HashMap _nameToPropertyIndex; + + /** + * Mapping from external property ids to one or more indexes; + * in most cases single index as Integer, but + * occasionally same name maps to multiple ones: if so, + * List<Integer>. + */ + private final Map _nameToPropertyIndex; private final String[] _typeIds; private final TokenBuffer[] _tokens; protected ExternalTypeHandler(JavaType beanType, ExtTypedProperty[] properties, - HashMap nameToPropertyIndex, + Map nameToPropertyIndex, String[] typeIds, TokenBuffer[] tokens) { _beanType = beanType; @@ -66,23 +73,43 @@ public ExternalTypeHandler start() { /** * Method called to see if given property/value pair is an external type * id; and if so handle it. This is only to be called in case - * containing POJO has similarly named property as the external type id; + * containing POJO has similarly named property as the external type id AND + * value is of scalar type: * otherwise {@link #handlePropertyValue} should be called instead. */ + @SuppressWarnings("unchecked") public boolean handleTypePropertyValue(JsonParser p, DeserializationContext ctxt, String propName, Object bean) throws IOException { - Integer I = _nameToPropertyIndex.get(propName); - if (I == null) { + Object ob = _nameToPropertyIndex.get(propName); + if (ob == null) { return false; } - int index = I.intValue(); + final String typeId = p.getText(); + // 28-Nov-2016, tatu: For [databind#291], need separate handling + if (ob instanceof List) { + boolean result = false; + for (Integer index : (List) ob) { + if (_handleTypePropertyValue(p, ctxt, propName, bean, + typeId, index.intValue())) { + result = true; + } + } + return result; + } + return _handleTypePropertyValue(p, ctxt, propName, bean, + typeId, ((Integer) ob).intValue()); + } + + private final boolean _handleTypePropertyValue(JsonParser p, DeserializationContext ctxt, + String propName, Object bean, String typeId, int index) + throws IOException + { ExtTypedProperty prop = _properties[index]; - if (!prop.hasTypePropertyName(propName)) { + if (!prop.hasTypePropertyName(propName)) { // when could/should this ever happen? return false; } - String typeId = p.getText(); // note: can NOT skip child values (should always be String anyway) boolean canDeserialize = (bean != null) && (_tokens[index] != null); // Minor optimization: deserialize properties as soon as we have all we need: @@ -104,14 +131,44 @@ public boolean handleTypePropertyValue(JsonParser p, DeserializationContext ctxt * * @return True, if the given property was properly handled */ + @SuppressWarnings("unchecked") public boolean handlePropertyValue(JsonParser p, DeserializationContext ctxt, String propName, Object bean) throws IOException { - Integer I = _nameToPropertyIndex.get(propName); - if (I == null) { + Object ob = _nameToPropertyIndex.get(propName); + if (ob == null) { return false; } - int index = I.intValue(); + // 28-Nov-2016, tatu: For [databind#291], need separate handling + if (ob instanceof List) { + Iterator it = ((List) ob).iterator(); + Integer index = it.next(); + + ExtTypedProperty prop = _properties[index]; + // For now, let's assume it's same type (either type id OR value) + // for all mappings, so we'll only check first one + if (prop.hasTypePropertyName(propName)) { + String typeId = p.getText(); + p.skipChildren(); + _typeIds[index] = typeId; + while (it.hasNext()) { + _typeIds[it.next()] = typeId; + } + } else { + @SuppressWarnings("resource") + TokenBuffer tokens = new TokenBuffer(p, ctxt); + tokens.copyCurrentStructure(p); + _tokens[index] = tokens; + while (it.hasNext()) { + _tokens[it.next()] = tokens; + } + } + return true; + } + + // Otherwise only maps to a single value, in which case we can + // handle things in bit more optimal way... + int index = ((Integer) ob).intValue(); ExtTypedProperty prop = _properties[index]; boolean canDeserialize; if (prop.hasTypePropertyName(propName)) { @@ -125,9 +182,8 @@ public boolean handlePropertyValue(JsonParser p, DeserializationContext ctxt, _tokens[index] = tokens; canDeserialize = (bean != null) && (_typeIds[index] != null); } - /* Minor optimization: let's deserialize properties as soon as - * we have all pertinent information: - */ + // Minor optimization: let's deserialize properties as soon as + // we have all pertinent information: if (canDeserialize) { String typeId = _typeIds[index]; // clear stored data, to avoid deserializing+setting twice: @@ -312,8 +368,8 @@ public static class Builder { private final JavaType _beanType; - private final ArrayList _properties = new ArrayList(); - private final HashMap _nameToPropertyIndex = new HashMap(); + private final List _properties = new ArrayList<>(); + private final Map _nameToPropertyIndex = new HashMap<>(); protected Builder(JavaType t) { _beanType = t; @@ -323,10 +379,26 @@ public void addExternal(SettableBeanProperty property, TypeDeserializer typeDese { Integer index = _properties.size(); _properties.add(new ExtTypedProperty(property, typeDeser)); - _nameToPropertyIndex.put(property.getName(), index); - _nameToPropertyIndex.put(typeDeser.getPropertyName(), index); + _addPropertyIndex(property.getName(), index); + _addPropertyIndex(typeDeser.getPropertyName(), index); } + private void _addPropertyIndex(String name, Integer index) { + Object ob = _nameToPropertyIndex.get(name); + if (ob == null) { + _nameToPropertyIndex.put(name, index); + } else if (ob instanceof List) { + @SuppressWarnings("unchecked") + List list = (List) ob; + list.add(index); + } else { + List list = new LinkedList<>(); + list.add(ob); + list.add(index); + _nameToPropertyIndex.put(name, list); + } + } + /** * Method called after all external properties have been assigned, to further * link property with polymorphic value with possible property for type id diff --git a/src/test/java/com/fasterxml/jackson/databind/jsontype/ext/MultipleExternalIds291Test.java b/src/test/java/com/fasterxml/jackson/databind/jsontype/ext/MultipleExternalIds291Test.java new file mode 100644 index 0000000000..b015dd8e4e --- /dev/null +++ b/src/test/java/com/fasterxml/jackson/databind/jsontype/ext/MultipleExternalIds291Test.java @@ -0,0 +1,111 @@ +package com.fasterxml.jackson.databind.jsontype.ext; + +import com.fasterxml.jackson.annotation.*; +import com.fasterxml.jackson.annotation.JsonTypeInfo.As; +import com.fasterxml.jackson.annotation.JsonTypeInfo.Id; +import com.fasterxml.jackson.databind.*; + +public class MultipleExternalIds291Test extends BaseMapTest +{ + // For [Issue#291] + interface F1 {} + + static class A implements F1 { + public String a; + } + + static class B implements F1 { + public String b; + } + + static interface F2 {} + + static class C implements F2 { + public String c; + } + + static class D implements F2{ + public String d; + } + + static class Container { + @JsonTypeInfo(use = Id.NAME, property = "type", include = As.EXTERNAL_PROPERTY) + @JsonSubTypes({ + @JsonSubTypes.Type(value = A.class, name = "1"), + @JsonSubTypes.Type(value = B.class, name = "2")}) + public F1 field1; + + @JsonTypeInfo(use = Id.NAME, property = "type", include = As.EXTERNAL_PROPERTY) + @JsonSubTypes({ + @JsonSubTypes.Type(value = C.class, name = "1"), + @JsonSubTypes.Type(value = D.class, name = "2")}) + public F2 field2; + } + + static class ContainerWithExtra extends Container { + public String type; + } + + /* + /********************************************************** + /* Test methods + /********************************************************** + */ + + final ObjectMapper MAPPER = objectMapper(); + + // [databind#291] + public void testMultipleValuesSingleExtId() throws Exception + { + // first with ext-id before values + _testMultipleValuesSingleExtId( +"{'type' : '1',\n" ++"'field1' : { 'a' : 'AAA' },\n" ++"'field2' : { 'c' : 'CCC' }\n" ++"}" +); + + // then after + _testMultipleValuesSingleExtId( +"{\n" ++"'field1' : { 'a' : 'AAA' },\n" ++"'field2' : { 'c' : 'CCC' },\n" ++"'type' : '1'\n" ++"}" +); + // and then in-between + _testMultipleValuesSingleExtId( +"{\n" ++"'field1' : { 'a' : 'AAA' },\n" ++"'type' : '1',\n" ++"'field2' : { 'c' : 'CCC' }\n" ++"}" +); + } + + public void _testMultipleValuesSingleExtId(String json) throws Exception + { + json = aposToQuotes(json); + + // First, with base class, no type id field separately + { + Container c = MAPPER.readValue(json, Container.class); + assertNotNull(c); + assertTrue(c.field1 instanceof A); + assertEquals("AAA", ((A) c.field1).a); + assertTrue(c.field2 instanceof C); + assertEquals("CCC", ((C) c.field2).c); + } + + // then with sub-class that does have similarly named property + { + ContainerWithExtra c = MAPPER.readValue(json, ContainerWithExtra.class); + assertNotNull(c); + assertEquals("1", c.type); + assertTrue(c.field1 instanceof A); + assertEquals("AAA", ((A) c.field1).a); + assertTrue(c.field2 instanceof C); + assertEquals("CCC", ((C) c.field2).c); + } + } +} diff --git a/src/test/java/com/fasterxml/jackson/failing/TestMultipleExternalIds291.java b/src/test/java/com/fasterxml/jackson/failing/TestMultipleExternalIds291.java deleted file mode 100644 index 7b942d8b64..0000000000 --- a/src/test/java/com/fasterxml/jackson/failing/TestMultipleExternalIds291.java +++ /dev/null @@ -1,71 +0,0 @@ -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.*; - -public class TestMultipleExternalIds291 extends BaseMapTest -{ - // For [Issue#291] - interface F1 {} - - static class A implements F1 { - public String a; - } - - static class B implements F1 { - public String b; - } - - static interface F2 {} - - static class C implements F2 { - public String c; - } - - static class D implements F2{ - public String d; - } - - static class Container { - public String type; - - @JsonTypeInfo(use = Id.NAME, property = "type", include = As.EXTERNAL_PROPERTY) - @JsonSubTypes({ - @JsonSubTypes.Type(value = A.class, name = "1"), - @JsonSubTypes.Type(value = B.class, name = "2")}) - public F1 field1; - - @JsonTypeInfo(use = Id.NAME, property = "type", include = As.EXTERNAL_PROPERTY) - @JsonSubTypes({ - @JsonSubTypes.Type(value = C.class, name = "1"), - @JsonSubTypes.Type(value = D.class, name = "2")}) - public F2 field2; - } - - /* - /********************************************************** - /* Test methods - /********************************************************** - */ - - // [Issue#291] - public void testMultiple() throws Exception - { - final ObjectMapper mapper = objectMapper(); - final String JSON = -"{\"type\" : \"1\",\n" -+"\"field1\" : {\n" -+" \"a\" : \"AAA\"\n" -+"}, \"field2\" : {\n" -+" \"c\" : \"CCC\"\n" -+"}\n" -+"}"; - - Container c = mapper.readValue(JSON, Container.class); - assertNotNull(c); - assertTrue(c.field1 instanceof A); - assertTrue(c.field2 instanceof C); - } -}