From 2a07652d1d2c449799de476803df05a10e848fc1 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Mon, 23 Nov 2020 17:16:17 -0800 Subject: [PATCH] Fix #232 --- release-notes/VERSION-2.x | 2 ++ .../jackson/dataformat/yaml/YAMLParser.java | 36 +++++++++++++++---- .../snakeyaml/error/MarkedYAMLException.java | 3 +- .../PolymorphicWithObjectId25Test.java | 2 +- .../yaml/misc/ObjectAndTypeId231Test.java | 12 +++++-- .../ObjectAndTypeId232Test.java | 17 ++++++--- .../yaml/type/PolymorphicIdTest.java | 1 - 7 files changed, 57 insertions(+), 16 deletions(-) rename yaml/src/test/java/com/fasterxml/jackson/dataformat/yaml/{failing => misc}/ObjectAndTypeId232Test.java (76%) diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index 22b5e425..ab34379b 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -24,6 +24,8 @@ Modules: #231: (yaml) Typed object with anchor throws Already had POJO for id (note: actual fix in `jackson-annotations`) (reported by almson@github) +#232: (yaml) Typed object throws "Missing type id" when annotated with @JsonIdentityInfo + (reported by almson@github) #233: (yaml) Support decoding Binary, Octal and Hex numbers as integers - Add configurability of "YAML version generator is to follow" via "YAMLFactory.builder()" - SnakeYAML 1.26 -> 1.27 diff --git a/yaml/src/main/java/com/fasterxml/jackson/dataformat/yaml/YAMLParser.java b/yaml/src/main/java/com/fasterxml/jackson/dataformat/yaml/YAMLParser.java index e2e72a9e..2fdaed16 100644 --- a/yaml/src/main/java/com/fasterxml/jackson/dataformat/yaml/YAMLParser.java +++ b/yaml/src/main/java/com/fasterxml/jackson/dataformat/yaml/YAMLParser.java @@ -110,6 +110,15 @@ private Feature(boolean defaultState) { */ protected Event _lastEvent; + /** + * To keep track of tags ("type ids"), need to either get tags for all + * events, or, keep tag of relevant event that might have it: this is + * different from {@code _lastEvent} in some cases. + * + * @since 2.12 + */ + protected Event _lastTagEvent; + /** * We need to keep track of text values. */ @@ -139,7 +148,7 @@ private Feature(boolean defaultState) { * structured types, value whose first token current token is. */ protected String _currentAnchor; - + /* /********************************************************************** /* Life-cycle @@ -387,6 +396,7 @@ public JsonToken nextToken() throws IOException // is null ok? Assume it is, for now, consider to be same as end-of-doc if (evt == null) { _currentAnchor = null; + _lastTagEvent = null; return (_currToken = null); } _lastEvent = evt; @@ -397,6 +407,7 @@ public JsonToken nextToken() throws IOException if (_currToken != JsonToken.FIELD_NAME) { if (!evt.is(Event.ID.Scalar)) { _currentAnchor = null; + _lastTagEvent = null; // end is fine if (evt.is(Event.ID.MappingEnd)) { if (!_parsingContext.inObject()) { // sanity check is optional, but let's do it for now @@ -407,6 +418,7 @@ public JsonToken nextToken() throws IOException } _reportError("Expected a field name (Scalar value in YAML), got this instead: "+evt); } + // 20-Feb-2019, tatu: [dataformats-text#123] Looks like YAML exposes Anchor for Object at point // where we return START_OBJECT (which makes sense), but, alas, Jackson expects that at point // after first FIELD_NAME. So we will need to defer clearing of the anchor slightly, @@ -415,9 +427,15 @@ public JsonToken nextToken() throws IOException // test case given. final ScalarEvent scalar = (ScalarEvent) evt; final String newAnchor = scalar.getAnchor(); - if ((newAnchor != null) || (_currToken != JsonToken.START_OBJECT)) { + final boolean firstEntry = (_currToken == JsonToken.START_OBJECT); + if ((newAnchor != null) || !firstEntry) { _currentAnchor = scalar.getAnchor(); } + // 23-Nov-2020, tatu: [dataformats-text#232] shows case where ref to type id + // needs to be similarly deferred... + if (!firstEntry) { + _lastTagEvent = evt; + } final String name = scalar.getValue(); _currentFieldName = name; _parsingContext.setCurrentName(name); @@ -429,6 +447,7 @@ public JsonToken nextToken() throws IOException // Ugh. Why not expose id, to be able to Switch? _currentAnchor = null; + _lastTagEvent = evt; // scalar values are probably the commonest: if (evt.is(Event.ID.Scalar)) { @@ -1000,11 +1019,16 @@ public String getObjectId() throws IOException public String getTypeId() throws IOException { String tag; - if (_lastEvent instanceof CollectionStartEvent) { - tag = ((CollectionStartEvent) _lastEvent).getTag(); - } else if (_lastEvent instanceof ScalarEvent) { - tag = ((ScalarEvent) _lastEvent).getTag(); + + if (_lastTagEvent instanceof CollectionStartEvent) { + tag = ((CollectionStartEvent) _lastTagEvent).getTag(); +//System.err.println("getTypeId() at "+currentToken()+", last was collection ("+_lastTagEvent.getClass().getSimpleName()+") -> "+tag); + } else if (_lastTagEvent instanceof ScalarEvent) { + tag = ((ScalarEvent) _lastTagEvent).getTag(); +//System.err.println("getTypeId() at "+currentToken()+", last was scalar -> "+tag+", scalar == "+_lastEvent); + } else { +//System.err.println("getTypeId(), something else, curr token: "+currentToken()); return null; } if (tag != null) { diff --git a/yaml/src/main/java/com/fasterxml/jackson/dataformat/yaml/snakeyaml/error/MarkedYAMLException.java b/yaml/src/main/java/com/fasterxml/jackson/dataformat/yaml/snakeyaml/error/MarkedYAMLException.java index 8a50767c..0cfca44b 100644 --- a/yaml/src/main/java/com/fasterxml/jackson/dataformat/yaml/snakeyaml/error/MarkedYAMLException.java +++ b/yaml/src/main/java/com/fasterxml/jackson/dataformat/yaml/snakeyaml/error/MarkedYAMLException.java @@ -5,7 +5,8 @@ /** * Replacement for formerly shaded exception type from SnakeYAML; included * in 2.8 solely for backwards compatibility: new code that relies on Jackson 2.8 - * and alter should NOT use this type but only base type {@link YAMLException}. + * and after should NOT use this type but only base type + * {@link com.fasterxml.jackson.dataformat.yaml.JacksonYAMLParseException}. * * @deprecated Since 2.8 */ diff --git a/yaml/src/test/java/com/fasterxml/jackson/dataformat/yaml/failing/PolymorphicWithObjectId25Test.java b/yaml/src/test/java/com/fasterxml/jackson/dataformat/yaml/failing/PolymorphicWithObjectId25Test.java index 007de1c7..80de3e67 100644 --- a/yaml/src/test/java/com/fasterxml/jackson/dataformat/yaml/failing/PolymorphicWithObjectId25Test.java +++ b/yaml/src/test/java/com/fasterxml/jackson/dataformat/yaml/failing/PolymorphicWithObjectId25Test.java @@ -46,7 +46,7 @@ public void testPolymorphicAndObjectId25() throws Exception +"type: \"node\"\n" +"next:\n" +" &id2 name: \"second\"\n" - +" next: *id1" + +" next: *id1\n" ; NodeWithStringId node = MAPPER.readValue(yml, NodeWithStringId.class); diff --git a/yaml/src/test/java/com/fasterxml/jackson/dataformat/yaml/misc/ObjectAndTypeId231Test.java b/yaml/src/test/java/com/fasterxml/jackson/dataformat/yaml/misc/ObjectAndTypeId231Test.java index 186341dc..22344043 100644 --- a/yaml/src/test/java/com/fasterxml/jackson/dataformat/yaml/misc/ObjectAndTypeId231Test.java +++ b/yaml/src/test/java/com/fasterxml/jackson/dataformat/yaml/misc/ObjectAndTypeId231Test.java @@ -33,15 +33,21 @@ public void testTypeAndObjectId231() throws Exception { String yaml = "list:\n" + " - !Derived &id1\n" + - " a: foo"; + " a: foo\n"+ + " - !Derived &id2\n" + + " a: bar\n"+ + ""; Container container = MAPPER.readValue(yaml, Container.class); assertNotNull(container); assertNotNull(container.list); - assertEquals(1, container.list.size()); + assertEquals(2, container.list.size()); Base item = container.list.get(0); assertEquals(Derived.class, item.getClass()); - assertEquals("foo", ((Derived) item).a); + + item = container.list.get(1); + assertEquals(Derived.class, item.getClass()); + assertEquals("bar", ((Derived) item).a); } } diff --git a/yaml/src/test/java/com/fasterxml/jackson/dataformat/yaml/failing/ObjectAndTypeId232Test.java b/yaml/src/test/java/com/fasterxml/jackson/dataformat/yaml/misc/ObjectAndTypeId232Test.java similarity index 76% rename from yaml/src/test/java/com/fasterxml/jackson/dataformat/yaml/failing/ObjectAndTypeId232Test.java rename to yaml/src/test/java/com/fasterxml/jackson/dataformat/yaml/misc/ObjectAndTypeId232Test.java index 98944f26..592be9b3 100644 --- a/yaml/src/test/java/com/fasterxml/jackson/dataformat/yaml/failing/ObjectAndTypeId232Test.java +++ b/yaml/src/test/java/com/fasterxml/jackson/dataformat/yaml/misc/ObjectAndTypeId232Test.java @@ -1,4 +1,4 @@ -package com.fasterxml.jackson.dataformat.yaml.failing; +package com.fasterxml.jackson.dataformat.yaml.misc; import java.util.List; @@ -19,7 +19,7 @@ static class Container232 { @JsonProperty List list; } - + @JsonTypeInfo(use = Id.NAME) @JsonSubTypes({@JsonSubTypes.Type(name="Derived", value=Derived232.class)}) @JsonIdentityInfo(generator = ObjectIdGenerators.StringIdGenerator.class) @@ -43,12 +43,21 @@ public void testTypedYAML232() throws Exception { String yaml = "list:\n" + " - !Derived\n" + - " a: foo"; + " a: foo\n"+ + " - !Derived\n" + + " a: bar\n"+ + ""; Container232 container = MAPPER.readValue(yaml, Container232.class); assertNotNull(container); assertNotNull(container.list); - assertEquals(1, container.list.size()); + assertEquals(2, container.list.size()); + assertNotNull(container.list.get(0)); assertEquals(Derived232.class, container.list.get(0).getClass()); + assertEquals("foo", ((Derived232) container.list.get(0)).a); + + assertNotNull(container.list.get(1)); + assertEquals(Derived232.class, container.list.get(1).getClass()); + assertEquals("bar", ((Derived232) container.list.get(1)).a); } } diff --git a/yaml/src/test/java/com/fasterxml/jackson/dataformat/yaml/type/PolymorphicIdTest.java b/yaml/src/test/java/com/fasterxml/jackson/dataformat/yaml/type/PolymorphicIdTest.java index e7618a7b..5fddea30 100644 --- a/yaml/src/test/java/com/fasterxml/jackson/dataformat/yaml/type/PolymorphicIdTest.java +++ b/yaml/src/test/java/com/fasterxml/jackson/dataformat/yaml/type/PolymorphicIdTest.java @@ -5,7 +5,6 @@ import com.fasterxml.jackson.annotation.*; import com.fasterxml.jackson.databind.*; import com.fasterxml.jackson.dataformat.yaml.ModuleTestBase; -import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; public class PolymorphicIdTest extends ModuleTestBase {