TINKERPOP-1676 Removed properties from graphson serialization of Path Path should not have properties on elements (if they are present). That is inconsistent with gryo and was unintentially allowed.
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/e5d2d2bc Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/e5d2d2bc Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/e5d2d2bc Branch: refs/heads/TINKERPOP-1681 Commit: e5d2d2bc000654fd026521219cf30bac265139a3 Parents: 02b0073 Author: Stephen Mallette <sp...@genoprime.com> Authored: Wed May 24 13:38:16 2017 -0400 Committer: Stephen Mallette <sp...@genoprime.com> Committed: Thu May 25 14:52:30 2017 -0400 ---------------------------------------------------------------------- CHANGELOG.asciidoc | 1 + docs/src/dev/io/graphson.asciidoc | 126 +------------------ .../upgrade/release-3.2.x-incubating.asciidoc | 12 ++ .../io/graphson/GraphSONSerializersV2d0.java | 33 +++-- 4 files changed, 42 insertions(+), 130 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/e5d2d2bc/CHANGELOG.asciidoc ---------------------------------------------------------------------- diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index f0d7b17..750b4dd 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -26,6 +26,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima TinkerPop 3.2.5 (Release Date: NOT OFFICIALLY RELEASED YET) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +* Fixed inconsistency in GraphSON serialization of `Path` where properties of graph elements were being included when serialized. * Improved performance and memory usage of GraphSON when serializing `TinkerGraph` and graph elements. * Removed use of `stream()` in `DetachedEdge` and `DetachedVertex`. * Deprecated a constructor in `DetachedEdge` that made use of `Pair` in favor of a new one that just uses the objects that were in the `Pair`. http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/e5d2d2bc/docs/src/dev/io/graphson.asciidoc ---------------------------------------------------------------------- diff --git a/docs/src/dev/io/graphson.asciidoc b/docs/src/dev/io/graphson.asciidoc index fe56c27..e4c671d 100644 --- a/docs/src/dev/io/graphson.asciidoc +++ b/docs/src/dev/io/graphson.asciidoc @@ -149,12 +149,12 @@ file.withWriter { writer -> writer.write(toJson(Operator.sum, "Operator")) writer.write(toJson(Order.incr, "Order")) writer.write(toJson(Pop.all, "Pop")) - writer.write(toJson(Pick.any, "Pick")) + writer.write(toJson(org.apache.tinkerpop.gremlin.process.traversal.step.TraversalOptionParent.Pick.any, "Pick")) writer.write(toJson(org.apache.tinkerpop.gremlin.util.function.Lambda.function("{ it.get() }"), "Lambda")) tm = g.V().hasLabel('person').out().out().tree().profile().next() metrics = new org.apache.tinkerpop.gremlin.process.traversal.util.MutableMetrics(tm.getMetrics(0)); metrics.addNested(new org.apache.tinkerpop.gremlin.process.traversal.util.MutableMetrics(tm.getMetrics(1))); - writer.write(toJson(m, "Metrics")) + writer.write(toJson(metrics, "Metrics")) writer.write(toJson(P.gt(0), "P")) writer.write(toJson(P.gt(0).and(P.lt(10)), "P and")) writer.write(toJson(P.gt(0).or(P.within(-1, -10, -100)), "P or")) @@ -1662,97 +1662,7 @@ Path "@type" : "g:Int32", "@value" : 1 }, - "label" : "person", - "properties" : { - "name" : [ { - "@type" : "g:VertexProperty", - "@value" : { - "id" : { - "@type" : "g:Int64", - "@value" : 0 - }, - "value" : "marko", - "label" : "name" - } - } ], - "location" : [ { - "@type" : "g:VertexProperty", - "@value" : { - "id" : { - "@type" : "g:Int64", - "@value" : 6 - }, - "value" : "san diego", - "label" : "location", - "properties" : { - "startTime" : { - "@type" : "g:Int32", - "@value" : 1997 - }, - "endTime" : { - "@type" : "g:Int32", - "@value" : 2001 - } - } - } - }, { - "@type" : "g:VertexProperty", - "@value" : { - "id" : { - "@type" : "g:Int64", - "@value" : 7 - }, - "value" : "santa cruz", - "label" : "location", - "properties" : { - "startTime" : { - "@type" : "g:Int32", - "@value" : 2001 - }, - "endTime" : { - "@type" : "g:Int32", - "@value" : 2004 - } - } - } - }, { - "@type" : "g:VertexProperty", - "@value" : { - "id" : { - "@type" : "g:Int64", - "@value" : 8 - }, - "value" : "brussels", - "label" : "location", - "properties" : { - "startTime" : { - "@type" : "g:Int32", - "@value" : 2004 - }, - "endTime" : { - "@type" : "g:Int32", - "@value" : 2005 - } - } - } - }, { - "@type" : "g:VertexProperty", - "@value" : { - "id" : { - "@type" : "g:Int64", - "@value" : 9 - }, - "value" : "santa fe", - "label" : "location", - "properties" : { - "startTime" : { - "@type" : "g:Int32", - "@value" : 2005 - } - } - } - } ] - } + "label" : "person" } }, { "@type" : "g:Vertex", @@ -1761,20 +1671,7 @@ Path "@type" : "g:Int32", "@value" : 10 }, - "label" : "software", - "properties" : { - "name" : [ { - "@type" : "g:VertexProperty", - "@value" : { - "id" : { - "@type" : "g:Int64", - "@value" : 4 - }, - "value" : "gremlin", - "label" : "name" - } - } ] - } + "label" : "software" } }, { "@type" : "g:Vertex", @@ -1783,20 +1680,7 @@ Path "@type" : "g:Int32", "@value" : 11 }, - "label" : "software", - "properties" : { - "name" : [ { - "@type" : "g:VertexProperty", - "@value" : { - "id" : { - "@type" : "g:Int64", - "@value" : 5 - }, - "value" : "tinkergraph", - "label" : "name" - } - } ] - } + "label" : "software" } } ] } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/e5d2d2bc/docs/src/upgrade/release-3.2.x-incubating.asciidoc ---------------------------------------------------------------------- diff --git a/docs/src/upgrade/release-3.2.x-incubating.asciidoc b/docs/src/upgrade/release-3.2.x-incubating.asciidoc index a2045e4..36e0aae 100644 --- a/docs/src/upgrade/release-3.2.x-incubating.asciidoc +++ b/docs/src/upgrade/release-3.2.x-incubating.asciidoc @@ -32,6 +32,18 @@ Please see the link:https://github.com/apache/tinkerpop/blob/3.2.5/CHANGELOG.asc Upgrading for Users ~~~~~~~~~~~~~~~~~~~ +GraphSON Path Serialization +^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Serialization of `Path` with GraphSON was inconsistent with Gryo in that all the properties on any elements of +the `Path` were being included. With Gryo that, correctly, was not happening as that could be extraordinarily +expensive. GraphSON serialization has now been modified to properly not include properties. That change can cause +breaks in application code if that application code tries to access properties on elements in a `Path` as they +will no longer be there. Applications that require the properties will need to alter their Gremlin to better +restrict the data they want to retrieve. + +See: link:https://issues.apache.org/jira/browse/TINKERPOP-1676[TINKERPOP-1676] + Authentication Configuration ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/e5d2d2bc/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV2d0.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV2d0.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV2d0.java index bdf3fe5..ffde32c 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV2d0.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV2d0.java @@ -35,6 +35,7 @@ import org.apache.tinkerpop.gremlin.structure.Vertex; import org.apache.tinkerpop.gremlin.structure.VertexProperty; import org.apache.tinkerpop.gremlin.structure.util.Comparators; import org.apache.tinkerpop.gremlin.structure.util.detached.DetachedEdge; +import org.apache.tinkerpop.gremlin.structure.util.detached.DetachedFactory; import org.apache.tinkerpop.gremlin.structure.util.detached.DetachedProperty; import org.apache.tinkerpop.gremlin.structure.util.detached.DetachedUtil; import org.apache.tinkerpop.gremlin.structure.util.detached.DetachedVertex; @@ -47,9 +48,11 @@ import org.apache.tinkerpop.shaded.jackson.core.JsonProcessingException; import org.apache.tinkerpop.shaded.jackson.core.JsonToken; import org.apache.tinkerpop.shaded.jackson.databind.DeserializationContext; import org.apache.tinkerpop.shaded.jackson.databind.JavaType; +import org.apache.tinkerpop.shaded.jackson.databind.JsonNode; import org.apache.tinkerpop.shaded.jackson.databind.SerializerProvider; import org.apache.tinkerpop.shaded.jackson.databind.deser.std.StdDeserializer; import org.apache.tinkerpop.shaded.jackson.databind.jsontype.TypeSerializer; +import org.apache.tinkerpop.shaded.jackson.databind.node.ArrayNode; import org.apache.tinkerpop.shaded.jackson.databind.ser.std.StdKeySerializer; import org.apache.tinkerpop.shaded.jackson.databind.ser.std.StdScalarSerializer; import org.apache.tinkerpop.shaded.jackson.databind.ser.std.StdSerializer; @@ -241,8 +244,6 @@ class GraphSONSerializersV2d0 { jsonGenerator.writeEndObject(); } - - } final static class PathJacksonSerializer extends StdScalarSerializer<Path> { @@ -256,8 +257,10 @@ class GraphSONSerializersV2d0 { throws IOException, JsonGenerationException { jsonGenerator.writeStartObject(); - jsonGenerator.writeObjectField(GraphSONTokens.LABELS, path.labels()); - jsonGenerator.writeObjectField(GraphSONTokens.OBJECTS, path.objects()); + // paths shouldn't serialize with properties if the path contains graph elements + final Path p = DetachedFactory.detach(path, false); + jsonGenerator.writeObjectField(GraphSONTokens.LABELS, p.labels()); + jsonGenerator.writeObjectField(GraphSONTokens.OBJECTS, p.objects()); jsonGenerator.writeEndObject(); } @@ -534,24 +537,36 @@ class GraphSONSerializersV2d0 { } } - static class PathJacksonDeserializer extends AbstractObjectDeserializer<Path> { + static class PathJacksonDeserializer extends StdDeserializer<Path> { public PathJacksonDeserializer() { super(Path.class); } @Override - public Path createObject(final Map<String, Object> pathData) { + public Path deserialize(final JsonParser jsonParser, final DeserializationContext deserializationContext) throws IOException, JsonProcessingException { + final JsonNode n = jsonParser.readValueAsTree(); final Path p = MutablePath.make(); + final JavaType setType = deserializationContext.getConfig().getTypeFactory().constructCollectionType(HashSet.class, String.class); - final List labels = (List) pathData.get(GraphSONTokens.LABELS); - final List objects = (List) pathData.get(GraphSONTokens.OBJECTS); + final ArrayNode labels = (ArrayNode) n.get(GraphSONTokens.LABELS); + final ArrayNode objects = (ArrayNode) n.get(GraphSONTokens.OBJECTS); for (int i = 0; i < objects.size(); i++) { - p.extend(objects.get(i), new HashSet((List) labels.get(i))); + final JsonParser po = objects.get(i).traverse(); + po.nextToken(); + final JsonParser pl = labels.get(i).traverse(); + pl.nextToken(); + p.extend(deserializationContext.readValue(po, Object.class), deserializationContext.readValue(pl, setType)); } + return p; } + + @Override + public boolean isCachable() { + return true; + } } static class VertexPropertyJacksonDeserializer extends StdDeserializer<VertexProperty> {