TINKERPOP-1738 Bump to jackson 2.9.4 This fixes a problem where Jackson was missing an error in GraphSON type information depending on the order of the JSON elements. It required some internal changes to the GraphSONTypeSerializer given API shifts in jackson - should not really affect TinkerPop users. Modified the original test that demonstrated this problem to be a bit more robust.
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/f35f04a8 Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/f35f04a8 Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/f35f04a8 Branch: refs/heads/TINKERPOP-1447 Commit: f35f04a8da88280e8b121356ac5839fc0b8ac859 Parents: a61dd58 Author: Stephen Mallette <sp...@genoprime.com> Authored: Tue Feb 13 10:16:46 2018 -0500 Committer: Stephen Mallette <sp...@genoprime.com> Committed: Fri Mar 2 09:11:34 2018 -0500 ---------------------------------------------------------------------- CHANGELOG.asciidoc | 1 + .../io/graphson/GraphSONTypeSerializer.java | 94 ++++++-------------- ...aphSONMapperV2d0PartialEmbeddedTypeTest.java | 32 +++++++ gremlin-shaded/pom.xml | 2 +- 4 files changed, 61 insertions(+), 68 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/f35f04a8/CHANGELOG.asciidoc ---------------------------------------------------------------------- diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index a24b1fb..f99d1a2 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -27,6 +27,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima * Modified `GremlinDslProcessor` so that it generated the `getAnonymousTraversalClass()` method to return the DSL version of `__`. * Added the "Kitchen Sink" test data set. * Fixed deserialization of `P.not()` for GraphSON. +* Bumped to Jackson 2.9.4. * Added `idleConnectionTimeout` and `keepAliveInterval` to Gremlin Server that enables a "ping" and auto-close for seemingly dead clients. * Fixed a bug in `NumberHelper` that led to wrong min/max results if numbers exceeded the Integer limits. * Delayed setting of the request identifier until `RequestMessage` construction by the builder. http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/f35f04a8/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONTypeSerializer.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONTypeSerializer.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONTypeSerializer.java index 78c670a..8b1ba25 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONTypeSerializer.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONTypeSerializer.java @@ -39,6 +39,8 @@ import org.apache.tinkerpop.gremlin.util.function.HashMapSupplier; import org.apache.tinkerpop.gremlin.util.function.Lambda; import org.apache.tinkerpop.shaded.jackson.annotation.JsonTypeInfo; import org.apache.tinkerpop.shaded.jackson.core.JsonGenerator; +import org.apache.tinkerpop.shaded.jackson.core.JsonToken; +import org.apache.tinkerpop.shaded.jackson.core.type.WritableTypeId; import org.apache.tinkerpop.shaded.jackson.databind.BeanProperty; import org.apache.tinkerpop.shaded.jackson.databind.jsontype.TypeIdResolver; import org.apache.tinkerpop.shaded.jackson.databind.jsontype.TypeSerializer; @@ -79,7 +81,7 @@ public class GraphSONTypeSerializer extends TypeSerializer { @Override public JsonTypeInfo.As getTypeInclusion() { - return null; + return JsonTypeInfo.As.WRAPPER_OBJECT; } @Override @@ -93,79 +95,37 @@ public class GraphSONTypeSerializer extends TypeSerializer { } @Override - public void writeTypePrefixForScalar(final Object o, final JsonGenerator jsonGenerator) throws IOException { - if (canWriteTypeId()) { - writeTypePrefix(jsonGenerator, getTypeIdResolver().idFromValueAndType(o, getClassFromObject(o))); + public WritableTypeId writeTypePrefix(final JsonGenerator jsonGenerator, final WritableTypeId writableTypeId) throws IOException { + if (writableTypeId.valueShape == JsonToken.VALUE_STRING) { + if (canWriteTypeId()) { + writeTypePrefix(jsonGenerator, getTypeIdResolver().idFromValueAndType(writableTypeId.forValue, getClassFromObject(writableTypeId.forValue))); + } + } else if (writableTypeId.valueShape == JsonToken.START_OBJECT) { + jsonGenerator.writeStartObject(); + } else if (writableTypeId.valueShape == JsonToken.START_ARRAY) { + jsonGenerator.writeStartArray(); + } else { + throw new IllegalStateException("Could not write prefix"); } - } - - @Override - public void writeTypePrefixForObject(final Object o, final JsonGenerator jsonGenerator) throws IOException { - jsonGenerator.writeStartObject(); - // TODO: FULL_TYPES should be implemented here as : if (fullTypesModeEnabled()) writeTypePrefix(Map); - } - - @Override - public void writeTypePrefixForArray(final Object o, final JsonGenerator jsonGenerator) throws IOException { - jsonGenerator.writeStartArray(); - // TODO: FULL_TYPES should be implemented here as : if (fullTypesModeEnabled()) writeTypePrefix(List); - } - - @Override - public void writeTypeSuffixForScalar(final Object o, final JsonGenerator jsonGenerator) throws IOException { - if (canWriteTypeId()) { - writeTypeSuffix(jsonGenerator); - } - } - - @Override - public void writeTypeSuffixForObject(final Object o, final JsonGenerator jsonGenerator) throws IOException { - jsonGenerator.writeEndObject(); - // TODO: FULL_TYPES should be implemented here as : if (fullTypesModeEnabled()) writeTypeSuffix(Map); - } - - @Override - public void writeTypeSuffixForArray(final Object o, final JsonGenerator jsonGenerator) throws IOException { - jsonGenerator.writeEndArray(); - // TODO: FULL_TYPES should be implemented here as : if (fullTypesModeEnabled()) writeTypeSuffix(List); - } - @Override - public void writeCustomTypePrefixForScalar(final Object o, final JsonGenerator jsonGenerator, final String s) throws IOException { - if (canWriteTypeId()) { - writeTypePrefix(jsonGenerator, s); - } + return writableTypeId; } @Override - public void writeCustomTypePrefixForObject(final Object o, final JsonGenerator jsonGenerator, final String s) throws IOException { - jsonGenerator.writeStartObject(); - // TODO: FULL_TYPES should be implemented here as : if (fullTypesModeEnabled()) writeTypePrefix(s); - } - - @Override - public void writeCustomTypePrefixForArray(final Object o, final JsonGenerator jsonGenerator, final String s) throws IOException { - jsonGenerator.writeStartArray(); - // TODO: FULL_TYPES should be implemented here as : if (fullTypesModeEnabled()) writeTypePrefix(s); - } - - @Override - public void writeCustomTypeSuffixForScalar(final Object o, final JsonGenerator jsonGenerator, final String s) throws IOException { - if (canWriteTypeId()) { - writeTypeSuffix(jsonGenerator); + public WritableTypeId writeTypeSuffix(final JsonGenerator jsonGenerator, final WritableTypeId writableTypeId) throws IOException { + if (writableTypeId.valueShape == JsonToken.VALUE_STRING) { + if (canWriteTypeId()) { + writeTypeSuffix(jsonGenerator); + } + } else if (writableTypeId.valueShape == JsonToken.START_OBJECT) { + jsonGenerator.writeEndObject(); + } else if (writableTypeId.valueShape == JsonToken.START_ARRAY) { + jsonGenerator.writeEndArray(); + } else { + throw new IllegalStateException("Could not write suffix"); } - } - - @Override - public void writeCustomTypeSuffixForObject(final Object o, final JsonGenerator jsonGenerator, final String s) throws IOException { - jsonGenerator.writeEndObject(); - // TODO: FULL_TYPES should be implemented here as : if (fullTypesModeEnabled()) writeTypeSuffix(s); - } - @Override - public void writeCustomTypeSuffixForArray(final Object o, final JsonGenerator jsonGenerator,final String s) throws IOException { - jsonGenerator.writeEndArray(); - // TODO: FULL_TYPES should be implemented here as : if (fullTypesModeEnabled()) writeTypeSuffix(s); + return writableTypeId; } private boolean canWriteTypeId() { http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/f35f04a8/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapperV2d0PartialEmbeddedTypeTest.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapperV2d0PartialEmbeddedTypeTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapperV2d0PartialEmbeddedTypeTest.java index 5b009e1..c388e61 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapperV2d0PartialEmbeddedTypeTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONMapperV2d0PartialEmbeddedTypeTest.java @@ -19,8 +19,10 @@ package org.apache.tinkerpop.gremlin.structure.io.graphson; import org.apache.tinkerpop.gremlin.process.remote.traversal.DefaultRemoteTraverser; +import org.apache.tinkerpop.gremlin.process.traversal.Bytecode; import org.apache.tinkerpop.gremlin.process.traversal.P; import org.apache.tinkerpop.gremlin.process.traversal.Traverser; +import org.apache.tinkerpop.shaded.jackson.databind.JsonMappingException; import org.apache.tinkerpop.shaded.jackson.databind.ObjectMapper; import org.junit.Test; @@ -40,6 +42,7 @@ import java.util.Map; import java.util.UUID; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.core.IsInstanceOf.instanceOf; import static org.hamcrest.core.StringContains.containsString; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; @@ -56,6 +59,35 @@ public class GraphSONMapperV2d0PartialEmbeddedTypeTest extends AbstractGraphSONT .typeInfo(TypeInfo.PARTIAL_TYPES) .create() .createMapper(); + @Test + public void elementOrderShouldNotMatter() throws Exception { + final String bytecodeJSONFail1 = "{\"@type\":\"g:Bytecode\",\"@value\":{\"step\":[[\"addV\",\"poc_int\"],[\"property\",\"bigint1value\",{\"@type\":\"g:Int32\",\"@value\":-4294967295}]]}}"; + final String bytecodeJSONFail2 = "{\"@value\":{\"step\":[[\"addV\",\"poc_int\"],[\"property\",\"bigint1value\",{\"@value\":-4294967295,\"@type\":\"g:Int32\"}]]},\"@type\":\"g:Bytecode\"}"; + + // first validate the failures of TINKERPOP-1738 - prior to the jackson fix on 2.9.4 one of these would have + // passed based on the ordering of the properties + try { + mapper.readValue(bytecodeJSONFail1, Bytecode.class); + fail("Should have thrown an error because 'bigint1value' is not an int32"); + } catch (Exception ex) { + assertThat(ex, instanceOf(JsonMappingException.class)); + } + + try { + mapper.readValue(bytecodeJSONFail2, Bytecode.class); + fail("Should have thrown an error because 'bigint1value' is not an int32"); + } catch (Exception ex) { + assertThat(ex, instanceOf(JsonMappingException.class)); + } + + // now do a legit parsing based on order + final String bytecodeJSON1 = "{\"@type\":\"g:Bytecode\",\"@value\":{\"step\":[[\"addV\",\"poc_int\"],[\"property\",\"bigint1value\",{\"@type\":\"g:Int64\",\"@value\":-4294967295}]]}}"; + final String bytecodeJSON2 = "{\"@value\":{\"step\":[[\"addV\",\"poc_int\"],[\"property\",\"bigint1value\",{\"@value\":-4294967295,\"@type\":\"g:Int64\"}]]},\"@type\":\"g:Bytecode\"}"; + + final Bytecode bytecode1 = mapper.readValue(bytecodeJSON1, Bytecode.class); + final Bytecode bytecode2 = mapper.readValue(bytecodeJSON2, Bytecode.class); + assertEquals(bytecode1, bytecode2); + } @Test public void shouldSerializeDeserializeNestedCollectionsAndMapAndTypedValuesCorrectly() throws Exception { http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/f35f04a8/gremlin-shaded/pom.xml ---------------------------------------------------------------------- diff --git a/gremlin-shaded/pom.xml b/gremlin-shaded/pom.xml index 2219a4e..aeda3d4 100644 --- a/gremlin-shaded/pom.xml +++ b/gremlin-shaded/pom.xml @@ -50,7 +50,7 @@ limitations under the License. <groupId>com.fasterxml.jackson.core</groupId> <artifactId>jackson-databind</artifactId> <!-- Update the shade plugin config whenever you change this version --> - <version>2.8.10</version> + <version>2.9.4</version> <optional>true</optional> </dependency> </dependencies>
