From 567d6c09a868175626a0b404dc1a8f128f7746f2 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Tue, 14 Jun 2016 22:14:57 -0700 Subject: [PATCH] Fix #1261 --- release-notes/CREDITS | 5 + release-notes/VERSION | 2 + .../databind/deser/BeanDeserializer.java | 61 ++++- .../databind/deser/std/MapDeserializer.java | 3 +- .../objectid/ObjectWithCreator1261Test.java | 107 +++++++++ .../failing/ObjectWithCreator1261Test.java | 213 ------------------ 6 files changed, 173 insertions(+), 218 deletions(-) create mode 100644 src/test/java/com/fasterxml/jackson/databind/objectid/ObjectWithCreator1261Test.java delete mode 100644 src/test/java/com/fasterxml/jackson/failing/ObjectWithCreator1261Test.java diff --git a/release-notes/CREDITS b/release-notes/CREDITS index a646aa9bb1..3e8ace67cc 100644 --- a/release-notes/CREDITS +++ b/release-notes/CREDITS @@ -494,3 +494,8 @@ Maarten Billemont (lhunath@github) Vladimir Kulev (lightoze@github) * Reported #1028: Ignore USE_BIG_DECIMAL_FOR_FLOATS for NaN/Infinity (2.8.0) + +Ari Fogel (arifogel@github) + * Reported 1261, contributed fix for: `@JsonIdentityInfo` deserialization fails with + combination of forward references, `@JsonCreator` + (2.8.0) diff --git a/release-notes/VERSION b/release-notes/VERSION index 5c2123e0dd..ff609bdf26 100644 --- a/release-notes/VERSION +++ b/release-notes/VERSION @@ -45,6 +45,8 @@ Project: jackson-databind #1233: Add support for `JsonFormat.Feature.WRITE_SORTED_MAP_ENTRIES` #1235: `java.nio.file.Path` support incomplete (reported by, fix contributed by Benson M) +#1261: JsonIdentityInfo broken deserialization involving forward references and/or cycles + (reported by, fix contributed by Ari F) 2.7.5 (11-Jun-2016) 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 cda7dbe638..8360b5118c 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializer.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializer.java @@ -6,6 +6,7 @@ import com.fasterxml.jackson.core.*; import com.fasterxml.jackson.databind.*; import com.fasterxml.jackson.databind.deser.impl.*; +import com.fasterxml.jackson.databind.deser.impl.ReadableObjectId.Referring; import com.fasterxml.jackson.databind.util.NameTransformer; import com.fasterxml.jackson.databind.util.TokenBuffer; @@ -384,6 +385,7 @@ protected Object _deserializeUsingPropertyBased(final JsonParser p, final Deseri TokenBuffer unknown = null; JsonToken t = p.getCurrentToken(); + List referrings = null; for (; t == JsonToken.FIELD_NAME; t = p.nextToken()) { String propName = p.getCurrentName(); p.nextToken(); // to point to value @@ -426,11 +428,21 @@ protected Object _deserializeUsingPropertyBased(final JsonParser p, final Deseri // regular property? needs buffering SettableBeanProperty prop = _beanProperties.find(propName); if (prop != null) { - buffer.bufferProperty(prop, _deserializeWithErrorWrapping(p, ctxt, prop)); + try { + buffer.bufferProperty(prop, _deserializeWithErrorWrapping(p, ctxt, prop)); + } catch (UnresolvedForwardReference reference) { + // 14-Jun-2016, tatu: As per [databind#1261], looks like we need additional + // handling of forward references here. Not exactly sure why existing + // facilities did not cover, but this does appear to solve the problem + BeanReferring referring = handleUnresolvedReference(p, prop, buffer, reference); + if (referrings == null) { + referrings = new ArrayList(); + } + referrings.add(referring); + } continue; } - // As per [JACKSON-313], things marked as ignorable should not be - // passed to any setter + // Things marked as ignorable should not be passed to any setter if (_ignorableProps != null && _ignorableProps.contains(propName)) { handleIgnoredProperty(p, ctxt, handledType(), propName); continue; @@ -460,6 +472,11 @@ protected Object _deserializeUsingPropertyBased(final JsonParser p, final Deseri wrapInstantiationProblem(e, ctxt); bean = null; // never gets here } + if (referrings != null) { + for (BeanReferring referring : referrings) { + referring.setBean(bean); + } + } if (unknown != null) { // polymorphic? if (bean.getClass() != _beanType.getRawClass()) { @@ -471,6 +488,20 @@ protected Object _deserializeUsingPropertyBased(final JsonParser p, final Deseri return bean; } + /** + * @since 2.8 + */ + private BeanReferring handleUnresolvedReference(JsonParser p, + SettableBeanProperty prop, PropertyValueBuffer buffer, + UnresolvedForwardReference reference) + throws JsonMappingException + { + BeanReferring referring = new BeanReferring(reference, prop.getType().getRawClass(), + buffer, prop); + reference.getRoid().appendReferring(referring); + return referring; + } + protected final Object _deserializeWithErrorWrapping(JsonParser p, DeserializationContext ctxt, SettableBeanProperty prop) throws IOException @@ -920,4 +951,28 @@ protected Exception _creatorReturnedNullException() { } return _nullFromCreator; } + + /** + * @since 2.8 + */ + static class BeanReferring extends Referring { + private final SettableBeanProperty _prop; + private Object _bean; + + public void setBean(Object bean) { + _bean = bean; + } + + BeanReferring(UnresolvedForwardReference ref, + Class valueType, PropertyValueBuffer buffer, SettableBeanProperty prop) + { + super(ref, valueType); + _prop = prop; + } + + @Override + public void handleResolvedForwardReference(Object id, Object value) throws IOException { + _prop.set(_bean, value); + } + } } diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/std/MapDeserializer.java b/src/main/java/com/fasterxml/jackson/databind/deser/std/MapDeserializer.java index d671503281..3dc7c4b061 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/std/MapDeserializer.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/std/MapDeserializer.java @@ -247,7 +247,6 @@ public JsonDeserializer createContextual(DeserializationContext ctxt, vd = findConvertingContentDeserializer(ctxt, property, vd); } final JavaType vt = _mapType.getContentType(); -System.err.println("Map deser for "+_mapType+":\n vt == "+vt); if (vd == null) { vd = ctxt.findContextualValueDeserializer(vt, property); } else { // if directly assigned, probably not yet contextual, so: @@ -668,7 +667,7 @@ public void resolveForwardReference(Object id, Object value) throws IOException * The resolved object associated with {@link #key} comes before the values in * {@link #next}. */ - final static class MapReferring extends Referring { + static class MapReferring extends Referring { private final MapReferringAccumulator _parent; public final Map next = new LinkedHashMap(); diff --git a/src/test/java/com/fasterxml/jackson/databind/objectid/ObjectWithCreator1261Test.java b/src/test/java/com/fasterxml/jackson/databind/objectid/ObjectWithCreator1261Test.java new file mode 100644 index 0000000000..e673d73c83 --- /dev/null +++ b/src/test/java/com/fasterxml/jackson/databind/objectid/ObjectWithCreator1261Test.java @@ -0,0 +1,107 @@ +package com.fasterxml.jackson.databind.objectid; + +import java.util.*; + +import com.fasterxml.jackson.annotation.*; +import com.fasterxml.jackson.databind.*; + +public class ObjectWithCreator1261Test + extends BaseMapTest +{ + static class Answer + { + public SortedMap parents; + + public Answer() { + parents = new TreeMap(); + } + } + + @JsonIdentityInfo(generator = ObjectIdGenerators.IntSequenceGenerator.class, property = "@id") + static class Parent + { + public Map children; + + @JsonIdentityReference(alwaysAsId = true) + public Child favoriteChild; + + public String name; + + protected Parent() { } + + public Parent(String name, boolean ignored) { + children = new TreeMap(); + this.name = name; + } + } + + @JsonInclude(JsonInclude.Include.NON_NULL) + @JsonIdentityInfo(generator = ObjectIdGenerators.IntSequenceGenerator.class, property = "@id") + static class Child + { + public String name; + + @JsonIdentityReference(alwaysAsId = true) + public Parent parent; + + @JsonIdentityReference(alwaysAsId = true) + public List parentAsList; + + public String someNullProperty; + + protected Child() { } + + @JsonCreator + public Child(@JsonProperty("name") String name, + @JsonProperty("someNullProperty") String someNullProperty) { + this.name = name; + this.someNullProperty = someNullProperty; + } + + public Child(String n) { + name = n; + } + } + + public void testObjectIds1261() throws Exception + { + ObjectMapper mapper = new ObjectMapper(); + mapper.enable(SerializationFeature.INDENT_OUTPUT); + mapper.enable(MapperFeature.SORT_PROPERTIES_ALPHABETICALLY); + + Answer initialAnswer = createInitialAnswer(); + String initialAnswerString = mapper.writeValueAsString(initialAnswer); +// System.out.println("Initial answer:\n"+initialAnswerString); + JsonNode tree = mapper.readTree(initialAnswerString); + Answer deserializedAnswer = mapper.readValue(initialAnswerString, + Answer.class); + String reserializedAnswerString = mapper + .writeValueAsString(deserializedAnswer); + JsonNode newTree = mapper.readTree(reserializedAnswerString); + if (!tree.equals(newTree)) { + fail("Original and recovered Json are different. Recovered = \n" + + reserializedAnswerString + "\n"); + } + } + + private Answer createInitialAnswer() { + Answer answer = new Answer(); + String child1Name = "child1"; + String child2Name = "child2"; + String parent1Name = "parent1"; + String parent2Name = "parent2"; + Parent parent1 = new Parent(parent1Name, false); + answer.parents.put(parent1Name, parent1); + Child child1 = new Child(child1Name); + child1.parent = parent1; + child1.parentAsList = Collections.singletonList(parent1); + Child child2 = new Child(child2Name); + Parent parent2 = new Parent(parent2Name, false); + child2.parent = parent2; + child2.parentAsList = Collections.singletonList(parent2); + parent1.children.put(child1Name, child1); + parent1.children.put(child2Name, child2); + answer.parents.put(parent2Name, parent2); + return answer; + } +} diff --git a/src/test/java/com/fasterxml/jackson/failing/ObjectWithCreator1261Test.java b/src/test/java/com/fasterxml/jackson/failing/ObjectWithCreator1261Test.java deleted file mode 100644 index c0f23ddcc1..0000000000 --- a/src/test/java/com/fasterxml/jackson/failing/ObjectWithCreator1261Test.java +++ /dev/null @@ -1,213 +0,0 @@ -package com.fasterxml.jackson.failing; - -import java.util.*; - -import com.fasterxml.jackson.annotation.*; -import com.fasterxml.jackson.databind.*; - -public class ObjectWithCreator1261Test - extends BaseMapTest -{ - static class Answer - { - public SortedMap parents; - - public Answer() { - parents = new TreeMap(); - } - } - - @JsonIdentityInfo(generator = ObjectIdGenerators.IntSequenceGenerator.class, property = "@id") - static class Parent - { - public Map children; - - @JsonIdentityReference(alwaysAsId = true) - public Child favoriteChild; - - public String name; - - protected Parent() { } - - public Parent(String name, boolean ignored) { - children = new TreeMap(); - this.name = name; - } - } - - @JsonInclude(JsonInclude.Include.NON_NULL) - @JsonIdentityInfo(generator = ObjectIdGenerators.IntSequenceGenerator.class, property = "@id") - static class Child - { - public String name; - - @JsonIdentityReference(alwaysAsId = true) - public Parent parent; - - @JsonIdentityReference(alwaysAsId = true) - public List parentAsList; - - public String someNullProperty; - - protected Child() { } - - @JsonCreator - public Child(@JsonProperty("name") String name, - @JsonProperty("someNullProperty") String someNullProperty) { - this.name = name; - this.someNullProperty = someNullProperty; - } - - public Child(String n) { - name = n; - } - } - - public void testObjectIds1261() throws Exception - { - ObjectMapper mapper = new ObjectMapper(); - mapper.enable(SerializationFeature.INDENT_OUTPUT); - mapper.enable(MapperFeature.SORT_PROPERTIES_ALPHABETICALLY); - - Answer initialAnswer = createInitialAnswer(); - String initialAnswerString = mapper.writeValueAsString(initialAnswer); -// System.out.println("Initial answer:\n"+initialAnswerString); - JsonNode tree = mapper.readTree(initialAnswerString); - Answer deserializedAnswer = mapper.readValue(initialAnswerString, - Answer.class); - String reserializedAnswerString = mapper - .writeValueAsString(deserializedAnswer); - JsonNode newTree = mapper.readTree(reserializedAnswerString); - if (!tree.equals(newTree)) { - fail("Original and recovered Json are different. Recovered = \n" - + reserializedAnswerString + "\n"); - } - } - - private Answer createInitialAnswer() { - Answer answer = new Answer(); - String child1Name = "child1"; - String child2Name = "child2"; - String parent1Name = "parent1"; - String parent2Name = "parent2"; - Parent parent1 = new Parent(parent1Name, false); - answer.parents.put(parent1Name, parent1); - Child child1 = new Child(child1Name); - child1.parent = parent1; - child1.parentAsList = Collections.singletonList(parent1); - Child child2 = new Child(child2Name); - Parent parent2 = new Parent(parent2Name, false); - child2.parent = parent2; - child2.parentAsList = Collections.singletonList(parent2); - parent1.children.put(child1Name, child1); - parent1.children.put(child2Name, child2); - answer.parents.put(parent2Name, parent2); - return answer; - } - // 14-Jun-2016, tatu: Original test, has a cycle which may render it impossible - // to support (although better error message would be nice). - // Left here in case we'll fix the other problem above. - - /* - static class Answer { - private List _parents; - - public Answer() { - _parents = new ArrayList(); - } - - @JsonCreator - public Answer(@JsonProperty("parents") List parents) { - _parents = parents; - } - - @JsonProperty("parents") - public List getParents() { - return _parents; - } - - } - - @JsonIdentityInfo(generator = ObjectIdGenerators.IntSequenceGenerator.class, property = "@id") - static class Parent - { - public List children; - - @JsonIdentityReference(alwaysAsId = true) - public Child aFavoriteChild; - - public Parent() { - children = new ArrayList(); - } - - @JsonCreator - public Parent(@JsonProperty("children") List children - ,@JsonProperty("aFavoriteChild") Child favoriteChild - ) - { - this.children = children; - this.aFavoriteChild = favoriteChild; - } - - public List getChildren() { return children; } - - @JsonIgnore - public void setFavoriteChild(Child c) { aFavoriteChild = c; } - } - - @JsonIdentityInfo(generator = ObjectIdGenerators.IntSequenceGenerator.class, property = "@id") - static class Child - { - @JsonIdentityReference(alwaysAsId = true) - public Parent parent; - - public List parentAsList; - - public Child() { } - public Child(Parent p) { - parent = p; - parentAsList = Collections.singletonList(parent); - } - } - - public void testWithCreators() throws Exception - { - ObjectMapper mapper = new ObjectMapper(); - mapper.enable(SerializationFeature.INDENT_OUTPUT); - - Answer initialAnswer = createInitialAnswer(); - String initialAnswerString = mapper.writeValueAsString(initialAnswer); - Answer deserializedAnswer = mapper.readValue(initialAnswerString, - Answer.class); - String reserializedAnswerString = mapper - .writeValueAsString(deserializedAnswer); - JsonNode tree = mapper.readTree(initialAnswerString); - JsonNode newTree = mapper.readTree(reserializedAnswerString); - if (!tree.equals(newTree)) { - System.err - .print("\nOriginal and recovered Json are different. Recovered = \n" - + reserializedAnswerString + "\n"); - } - } - - private static Answer createInitialAnswer() { - Answer answer = new Answer(); - Parent parent1 = new Parent(); - answer.getParents().add(parent1); - Child child1 = new Child(parent1); - Child child2 = new Child(parent1); - Child child3 = new Child(parent1); - Child child4 = new Child(parent1); - parent1.getChildren().add(child1); - parent1.getChildren().add(child2); - parent1.getChildren().add(child3); - parent1.getChildren().add(child4); - Parent parent2 = new Parent(); - answer.getParents().add(parent2); - Child child5 = new Child(parent2); - parent2.getChildren().add(child5); - parent1.setFavoriteChild(child5); - return answer; - } -*/ -}