From d55cab3629bef89c3e0f90f057c1542abc2180ab Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Tue, 9 Dec 2014 22:14:41 -0800 Subject: [PATCH] First fix for #622 that seems to work (after minor fixes to test also; id class MUST implement #equals() and #hashCode()!) Will see if polymorphic case would support, and if not, what is needed to support it. --- .../databind/deser/BeanDeserializer.java | 20 ++- .../databind/deser/BeanDeserializerBase.java | 68 ++++---- .../databind/deser/impl/ObjectIdReader.java | 32 +++- .../deser/impl/ObjectIdValueProperty.java | 5 - .../failing/JSOGDeserialize622Test.java | 148 ++++++++++-------- 5 files changed, 162 insertions(+), 111 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializer.java b/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializer.java index 7a507e4b40..21afb6e456 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializer.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializer.java @@ -251,12 +251,24 @@ private final Object vanillaDeserialize(JsonParser p, } /** - * General version used when handling needs more advanced - * features. + * General version used when handling needs more advanced features. */ @Override public Object deserializeFromObject(JsonParser p, DeserializationContext ctxt) throws IOException { + /* 09-Dec-2014, tatu: As per [#622], we need to allow Object Id references + * to come in as JSON Objects as well; but for now assume they will + * be simple, single-prooerty references, which means that we can + * recognize them without having to buffer anything. + * Once again, if we must, we can do more complex handling with buffering, + * but let's only do that if and when that becomes necessary. + */ + if (_objectIdReader != null && _objectIdReader.maySerializeAsObject()) { + if (p.hasTokenId(JsonTokenId.ID_FIELD_NAME) + && _objectIdReader.isValidReferencePropertyName(p.getCurrentName(), p)) { + return deserializeFromObjectId(p, ctxt); + } + } if (_nonStandardCreation) { if (_unwrappedPropertyHandler != null) { return deserializeWithUnwrapped(p, ctxt); @@ -276,14 +288,14 @@ public Object deserializeFromObject(JsonParser p, DeserializationContext ctxt) t if (_needViewProcesing) { Class view = ctxt.getActiveView(); if (view != null) { - return deserializeWithView(jp, ctxt, bean, view); + return deserializeWithView(p, ctxt, bean, view); } } */ return bean; } final Object bean = _valueInstantiator.createUsingDefault(ctxt); - // [databind#631]: Assign current value, to be accessible by custom serializers + // [databind#631]: Assign current value, to be accessible by custom deserializers p.setCurrentValue(bean); if (p.canReadObjectId()) { Object id = p.getObjectId(); diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerBase.java b/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerBase.java index 8964f09e31..b39c0bd9ac 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerBase.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerBase.java @@ -926,34 +926,34 @@ public void replaceProperty(SettableBeanProperty original, * General version used when handling needs more advanced * features. */ - public abstract Object deserializeFromObject(JsonParser jp, DeserializationContext ctxt) + public abstract Object deserializeFromObject(JsonParser p, DeserializationContext ctxt) throws IOException; @Override - public Object deserializeWithType(JsonParser jp, DeserializationContext ctxt, + public Object deserializeWithType(JsonParser p, DeserializationContext ctxt, TypeDeserializer typeDeserializer) throws IOException { // 16-Feb-2012, tatu: ObjectId may be used as well... need to check that first if (_objectIdReader != null) { // 05-Aug-2013, tatu: May use native Object Id - if (jp.canReadObjectId()) { - Object id = jp.getObjectId(); + if (p.canReadObjectId()) { + Object id = p.getObjectId(); if (id != null) { - Object ob = typeDeserializer.deserializeTypedFromObject(jp, ctxt); - return _handleTypedObjectId(jp, ctxt, ob, id); + Object ob = typeDeserializer.deserializeTypedFromObject(p, ctxt); + return _handleTypedObjectId(p, ctxt, ob, id); } } // or, Object Ids Jackson explicitly sets - JsonToken t = jp.getCurrentToken(); + JsonToken t = p.getCurrentToken(); // for now (2.2.x) we only allow scalar types (Strings, integral numbers): // NOTE: may need to allow handling of structured values in future for JSOG - if (t != null && t.isScalarValue()) { - return deserializeFromObjectId(jp, ctxt); + if (t != null && (t.isScalarValue() || _objectIdReader.maySerializeAsObject())) { + return deserializeFromObjectId(p, ctxt); } } // In future could check current token... for now this should be enough: - return typeDeserializer.deserializeTypedFromObject(jp, ctxt); + return typeDeserializer.deserializeTypedFromObject(p, ctxt); } /** @@ -964,7 +964,7 @@ public Object deserializeWithType(JsonParser jp, DeserializationContext ctxt, */ protected Object _handleTypedObjectId(JsonParser jp, DeserializationContext ctxt, Object pojo, Object rawId) - throws IOException, JsonProcessingException + throws IOException { /* 07-Aug-2013, tatu: One more challenge: type of id may not be type * of property we are expecting later on; specifically, numeric ids @@ -1034,7 +1034,7 @@ protected Object _convertObjectId(JsonParser jp, DeserializationContext ctxt, protected Object deserializeWithObjectId(JsonParser jp, DeserializationContext ctxt) throws IOException { return deserializeFromObject(jp, ctxt); } - + /** * Method called in cases where it looks like we got an Object Id * to parse and use as a reference. @@ -1054,7 +1054,7 @@ protected Object deserializeFromObjectId(JsonParser jp, DeserializationContext c protected Object deserializeFromObjectUsingNonDefault(JsonParser jp, DeserializationContext ctxt) throws IOException - { + { if (_delegateDeserializer != null) { return _valueInstantiator.createUsingDelegate(ctxt, _delegateDeserializer.deserialize(jp, ctxt)); @@ -1115,14 +1115,14 @@ public Object deserializeFromNumber(JsonParser jp, DeserializationContext ctxt) } return bean; } - throw ctxt.instantiationException(getBeanClass(), "no suitable creator method found to deserialize from JSON integer number"); + throw ctxt.instantiationException(handledType(), "no suitable creator method found to deserialize from JSON integer number"); } - public Object deserializeFromString(JsonParser jp, DeserializationContext ctxt) throws IOException + public Object deserializeFromString(JsonParser p, DeserializationContext ctxt) throws IOException { // First things first: id Object Id is used, most likely that's it if (_objectIdReader != null) { - return deserializeFromObjectId(jp, ctxt); + return deserializeFromObjectId(p, ctxt); } /* Bit complicated if we have delegating creator; may need to use it, @@ -1130,14 +1130,14 @@ public Object deserializeFromString(JsonParser jp, DeserializationContext ctxt) */ if (_delegateDeserializer != null) { if (!_valueInstantiator.canCreateFromString()) { - Object bean = _valueInstantiator.createUsingDelegate(ctxt, _delegateDeserializer.deserialize(jp, ctxt)); + Object bean = _valueInstantiator.createUsingDelegate(ctxt, _delegateDeserializer.deserialize(p, ctxt)); if (_injectables != null) { injectValues(ctxt, bean); } return bean; } } - return _valueInstantiator.createFromString(ctxt, jp.getText()); + return _valueInstantiator.createFromString(ctxt, p.getText()); } /** @@ -1145,52 +1145,52 @@ public Object deserializeFromString(JsonParser jp, DeserializationContext ctxt) * number. */ @SuppressWarnings("incomplete-switch") - public Object deserializeFromDouble(JsonParser jp, DeserializationContext ctxt) throws IOException + public Object deserializeFromDouble(JsonParser p, DeserializationContext ctxt) throws IOException { - switch (jp.getNumberType()) { + switch (p.getNumberType()) { case FLOAT: // no separate methods for taking float... case DOUBLE: if (_delegateDeserializer != null) { if (!_valueInstantiator.canCreateFromDouble()) { - Object bean = _valueInstantiator.createUsingDelegate(ctxt, _delegateDeserializer.deserialize(jp, ctxt)); + Object bean = _valueInstantiator.createUsingDelegate(ctxt, _delegateDeserializer.deserialize(p, ctxt)); if (_injectables != null) { injectValues(ctxt, bean); } return bean; } } - return _valueInstantiator.createFromDouble(ctxt, jp.getDoubleValue()); + return _valueInstantiator.createFromDouble(ctxt, p.getDoubleValue()); } // actually, could also be BigDecimal, so: if (_delegateDeserializer != null) { - return _valueInstantiator.createUsingDelegate(ctxt, _delegateDeserializer.deserialize(jp, ctxt)); + return _valueInstantiator.createUsingDelegate(ctxt, _delegateDeserializer.deserialize(p, ctxt)); } - throw ctxt.instantiationException(getBeanClass(), "no suitable creator method found to deserialize from JSON floating-point number"); + throw ctxt.instantiationException(handledType(), "no suitable creator method found to deserialize from JSON floating-point number"); } /** * Method called to deserialize POJO value from a JSON boolean value (true, false) */ - public Object deserializeFromBoolean(JsonParser jp, DeserializationContext ctxt) throws IOException + public Object deserializeFromBoolean(JsonParser p, DeserializationContext ctxt) throws IOException { if (_delegateDeserializer != null) { if (!_valueInstantiator.canCreateFromBoolean()) { - Object bean = _valueInstantiator.createUsingDelegate(ctxt, _delegateDeserializer.deserialize(jp, ctxt)); + Object bean = _valueInstantiator.createUsingDelegate(ctxt, _delegateDeserializer.deserialize(p, ctxt)); if (_injectables != null) { injectValues(ctxt, bean); } return bean; } } - boolean value = (jp.getCurrentToken() == JsonToken.VALUE_TRUE); + boolean value = (p.getCurrentToken() == JsonToken.VALUE_TRUE); return _valueInstantiator.createFromBoolean(ctxt, value); } - public Object deserializeFromArray(JsonParser jp, DeserializationContext ctxt) throws IOException + public Object deserializeFromArray(JsonParser p, DeserializationContext ctxt) throws IOException { if (_delegateDeserializer != null) { try { - Object bean = _valueInstantiator.createUsingDelegate(ctxt, _delegateDeserializer.deserialize(jp, ctxt)); + Object bean = _valueInstantiator.createUsingDelegate(ctxt, _delegateDeserializer.deserialize(p, ctxt)); if (_injectables != null) { injectValues(ctxt, bean); } @@ -1199,18 +1199,18 @@ public Object deserializeFromArray(JsonParser jp, DeserializationContext ctxt) t wrapInstantiationProblem(e, ctxt); } } else if (ctxt.isEnabled(DeserializationFeature.UNWRAP_SINGLE_VALUE_ARRAYS)) { - JsonToken t = jp.nextToken(); + JsonToken t = p.nextToken(); if (t == JsonToken.END_ARRAY && ctxt.isEnabled(DeserializationFeature.ACCEPT_EMPTY_ARRAY_AS_NULL_OBJECT)) { return null; } - final Object value = deserialize(jp, ctxt); - if (jp.nextToken() != JsonToken.END_ARRAY) { - throw ctxt.wrongTokenException(jp, JsonToken.END_ARRAY, + final Object value = deserialize(p, ctxt); + if (p.nextToken() != JsonToken.END_ARRAY) { + throw ctxt.wrongTokenException(p, JsonToken.END_ARRAY, "Attempted to unwrap single value array for single '" + _valueClass.getName() + "' value but there was more than a single value in the array"); } return value; } else if (ctxt.isEnabled(DeserializationFeature.ACCEPT_EMPTY_ARRAY_AS_NULL_OBJECT)) { - JsonToken t = jp.nextToken(); + JsonToken t = p.nextToken(); if (t == JsonToken.END_ARRAY) { return null; } diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/impl/ObjectIdReader.java b/src/main/java/com/fasterxml/jackson/databind/deser/impl/ObjectIdReader.java index bad83ba31c..6e90e35d2b 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/impl/ObjectIdReader.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/impl/ObjectIdReader.java @@ -5,9 +5,7 @@ import com.fasterxml.jackson.annotation.ObjectIdGenerator; import com.fasterxml.jackson.annotation.ObjectIdResolver; import com.fasterxml.jackson.annotation.SimpleObjectIdResolver; - import com.fasterxml.jackson.core.JsonParser; - import com.fasterxml.jackson.databind.*; import com.fasterxml.jackson.databind.deser.SettableBeanProperty; @@ -97,6 +95,36 @@ public JsonDeserializer getDeserializer() { public JavaType getIdType() { return _idType; } + + /** + * Convenience method, equivalent to calling: + * + * readerInstance.generator.maySerializeAsObject(); + * + * and used to determine whether Object Ids handled by the underlying + * generator may be in form of (JSON) Objects. + * Used for optimizing handling in cases where method returns false. + * + * @since 2.5 + */ + public boolean maySerializeAsObject() { + return generator.maySerializeAsObject(); + } + + /** + * Convenience method, equivalent to calling: + * + * readerInstance.generator.isValidReferencePropertyName(name, parser); + * + * and used to determine whether Object Ids handled by the underlying + * generator may be in form of (JSON) Objects. + * Used for optimizing handling in cases where method returns false. + * + * @since 2.5 + */ + public boolean isValidReferencePropertyName(String name, JsonParser parser) { + return generator.isValidReferencePropertyName(name, parser); + } /** * Method called to read value that is expected to be an Object Reference diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/impl/ObjectIdValueProperty.java b/src/main/java/com/fasterxml/jackson/databind/deser/impl/ObjectIdValueProperty.java index ac9136e4d3..cf9fa17d76 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/impl/ObjectIdValueProperty.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/impl/ObjectIdValueProperty.java @@ -20,11 +20,6 @@ public final class ObjectIdValueProperty protected final ObjectIdReader _objectIdReader; - @Deprecated // since 2.2 - public ObjectIdValueProperty(ObjectIdReader objectIdReader) { - this(objectIdReader, PropertyMetadata.STD_REQUIRED); - } - public ObjectIdValueProperty(ObjectIdReader objectIdReader, PropertyMetadata metadata) { diff --git a/src/test/java/com/fasterxml/jackson/failing/JSOGDeserialize622Test.java b/src/test/java/com/fasterxml/jackson/failing/JSOGDeserialize622Test.java index bcd3a5b81d..7247b235c6 100644 --- a/src/test/java/com/fasterxml/jackson/failing/JSOGDeserialize622Test.java +++ b/src/test/java/com/fasterxml/jackson/failing/JSOGDeserialize622Test.java @@ -7,10 +7,8 @@ import com.fasterxml.jackson.annotation.ObjectIdGenerator; import com.fasterxml.jackson.core.JsonParser; -import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; - import com.fasterxml.jackson.databind.*; /** @@ -19,21 +17,21 @@ */ public class JSOGDeserialize622Test extends BaseMapTest { - /** the key of the property that holds the ref */ - public static final String REF_KEY = "@ref"; + /** the key of the property that holds the ref */ + public static final String REF_KEY = "@ref"; - /** - * JSON input - */ - private static final String EXP_EXAMPLE_JSOG = aposToQuotes( - "{'@id':'1','foo':66,'next':{'"+REF_KEY+"':'1'}}"); + /** + * JSON input + */ + private static final String EXP_EXAMPLE_JSOG = aposToQuotes( + "{'@id':'1','foo':66,'next':{'"+REF_KEY+"':'1'}}"); - private final ObjectMapper mapper = new ObjectMapper(); + private final ObjectMapper mapper = new ObjectMapper(); - /** - * Customer IdGenerator - */ - static class JSOGGenerator extends ObjectIdGenerator { + /** + * Customer IdGenerator + */ + static class JSOGGenerator extends ObjectIdGenerator { private static final long serialVersionUID = 1L; protected transient int _nextValue; @@ -71,68 +69,86 @@ public com.fasterxml.jackson.annotation.ObjectIdGenerator.IdKey key(Object key) return new IdKey(getClass(), _scope, key); } + // important: otherwise won't get proper handling + @Override + public boolean maySerializeAsObject() { return true; } + + @Override + public boolean isValidReferencePropertyName(String name, Object parser) { + return REF_KEY.equals(name); + } + @Override public JSOGRef generateId(Object forPojo) { int id = _nextValue; ++_nextValue; return new JSOGRef(id); } - } + } - /** - * The reference deserializer - */ - static class JSOGRefDeserializer extends JsonDeserializer - { - @Override - public JSOGRef deserialize(JsonParser jp, DeserializationContext ctx) throws IOException, JsonProcessingException { - JsonNode node = jp.readValueAsTree(); - return node.isTextual() - ? new JSOGRef(node.asInt()) : new JSOGRef(node.get(REF_KEY).asInt()); + /** + * The reference deserializer + */ + static class JSOGRefDeserializer extends JsonDeserializer + { + @Override + public JSOGRef deserialize(JsonParser jp, DeserializationContext ctx) throws IOException { + JsonNode node = jp.readValueAsTree(); + return node.isTextual() + ? new JSOGRef(node.asInt()) : new JSOGRef(node.get(REF_KEY).asInt()); + } } - } - /** - * The reference object - */ - @JsonDeserialize(using=JSOGRefDeserializer.class) - static class JSOGRef - { - @JsonProperty(REF_KEY) - public int ref; + /** + * The reference object + */ + @JsonDeserialize(using=JSOGRefDeserializer.class) + static class JSOGRef + { + @JsonProperty(REF_KEY) + public int ref; + + public JSOGRef() { } + + public JSOGRef(int val) { + ref = val; + } + + @Override + public String toString() { return "[JSOGRef#"+ref+"]"; } + + @Override + public int hashCode() { + return ref; + } + + @Override + public boolean equals(Object other) { + return (other instanceof JSOGRef) + && ((JSOGRef) other).ref == this.ref; + } + } - public JSOGRef() { } + /** + * Example class using JSOGGenerator + */ + @JsonIdentityInfo(generator=JSOGGenerator.class, property="@id") + public static class IdentifiableExampleJSOG { + public int foo; + public IdentifiableExampleJSOG next; + } - public JSOGRef(int val) { - ref = val; + /* + /********************************************************************** + /* Test methods + /********************************************************************** + */ + + // for [databind#622] + public void testStructJSOGRef() throws Exception { + IdentifiableExampleJSOG result = mapper.readValue(EXP_EXAMPLE_JSOG, + IdentifiableExampleJSOG.class); + assertEquals(66, result.foo); + assertSame(result, result.next); } - } - - - /** - * Example class using JSOGGenerator - */ - @JsonIdentityInfo(generator=JSOGGenerator.class) - public static class IdentifiableExampleJSOG { - public int foo; - public IdentifiableExampleJSOG next; - } - - /* - /********************************************************************** - /* Test methods - /********************************************************************** - */ - - // for [databind#622] - public void testStructJSOGRef() throws Exception { - // Because the value ({@ref:1}) is not scalar, parser thinks it is not an id - // and tries to deserialize as normal a new IdentifiableExampleJSOG - // then complains about unrecognized field "@ref" - IdentifiableExampleJSOG result = mapper.readValue(EXP_EXAMPLE_JSOG, - IdentifiableExampleJSOG.class); - - assertEquals(66, result.foo); - assertSame(result, result.next); - } }