This is an automated email from the ASF dual-hosted git repository. spmallette pushed a commit to branch TINKERPOP-3182 in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit 7850cf375ebd6f2567ef6012808c07f706d86cff Author: Stephen Mallette <[email protected]> AuthorDate: Fri Aug 22 15:38:24 2025 -0400 TINKERPOP-3182 Fixed inconsistency in Path GraphSON serialization Path objects were being forcibly detached by reference in GraphSON v1/v2 which put it at odds with v1 untyped. Inclusion or removal of properties should be controlled via materializeProperties only. --- CHANGELOG.asciidoc | 2 + .../structure/io/binary/types/PathSerializer.java | 4 +- .../io/graphson/GraphSONSerializersV2.java | 11 ++- .../io/graphson/GraphSONSerializersV3.java | 10 ++- .../DriverRemoteConnection/GraphTraversalTests.cs | 47 +++++++++++ gremlin-go/driver/connection_test.go | 54 ++++++++++++ .../gremlin-javascript/lib/structure/graph.js | 17 +++- .../test/integration/client-tests.js | 70 ++++++++++++++-- .../test/integration/traversal-test.js | 73 +++++++++++++++++ .../test/unit/graphbinary/PathSerializer-test.js | 92 +++++++++++++++++++++ .../gremlin-javascript/test/unit/graphson-test.js | 38 +++++++-- .../tests/driver/test_driver_remote_connection.py | 15 ++++ .../apache/tinkerpop/gremlin/server/Context.java | 8 +- .../GremlinServerSerializationIntegrateTest.java | 95 +++++++++++++++++++++- .../gremlin/structure/SerializationTest.java | 29 +++---- .../org/apache/tinkerpop/gremlin/util/Tokens.java | 5 +- 16 files changed, 516 insertions(+), 54 deletions(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index a138ee22b5..95ad39aef7 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -25,6 +25,8 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima * Improved Java driver host availability on connection pool initialization. * Added getter for `parameterItems` and `valueTraversal` on `DifferenceStep`. +* Added properties to `Element` objects found in a `Path` for GraphSON v2 and v3 and GraphBinary. +* Fixed edge properties for GraphBinary which were not deserializing properly. [[release-3-7-4]] === TinkerPop 3.7.4 (Release Date: August 1, 2025) diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/binary/types/PathSerializer.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/binary/types/PathSerializer.java index b59436c8e0..830ffdc4e0 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/binary/types/PathSerializer.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/binary/types/PathSerializer.java @@ -52,7 +52,9 @@ public class PathSerializer extends SimpleTypeSerializer<Path> { path.extend(objects.get(ix), labels.get(ix)); } - return ReferenceFactory.detach(path); + // before 3.7.5, we forced detachment to reference here. better to just allow materializeProperties to control + // detachment or not. + return path; } @Override diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV2.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV2.java index e633efb49d..a36d3d9a0b 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV2.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV2.java @@ -258,10 +258,15 @@ class GraphSONSerializersV2 { throws IOException, JsonGenerationException { jsonGenerator.writeStartObject(); + // prior to 3.7.5, the code was such that: // 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()); + // + // however, there was the idea that v2 untyped, with just a couple documented exceptions, should essentially + // match v1 which does include the properties (Path objects are not documented as exceptions). as of 3.7.5, + // we remove detachment to references and allow users to control the inclusion or exclusion of properties + // with the materializeProperties option. + jsonGenerator.writeObjectField(GraphSONTokens.LABELS, path.labels()); + jsonGenerator.writeObjectField(GraphSONTokens.OBJECTS, path.objects()); jsonGenerator.writeEndObject(); } diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV3.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV3.java index 2d9024aa60..c8a7cf2dc5 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV3.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV3.java @@ -281,10 +281,14 @@ class GraphSONSerializersV3 { throws IOException, JsonGenerationException { jsonGenerator.writeStartObject(); + // prior to 3.7.5, the code was such that: // 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()); + // + // however, there was the idea that v3 untyped should essentially match v1 which does include the + // properties. as of 3.7.5, we remove detachment to references and allow users to control the inclusion + // or exclusion of properties with the materializeProperties option. + jsonGenerator.writeObjectField(GraphSONTokens.LABELS, path.labels()); + jsonGenerator.writeObjectField(GraphSONTokens.OBJECTS, path.objects()); jsonGenerator.writeEndObject(); } diff --git a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Process/Traversal/DriverRemoteConnection/GraphTraversalTests.cs b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Process/Traversal/DriverRemoteConnection/GraphTraversalTests.cs index 717bf211ec..a355ce6e3b 100644 --- a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Process/Traversal/DriverRemoteConnection/GraphTraversalTests.cs +++ b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Process/Traversal/DriverRemoteConnection/GraphTraversalTests.cs @@ -333,5 +333,52 @@ namespace Gremlin.Net.IntegrationTest.Process.Traversal.DriverRemoteConnection Assert.True(vp.Properties == null || vp.Properties.Length == 0); } } + + [Fact] + public void shouldUseMaterializedPropertiesTokenInPath() + { + var connection = _connectionFactory.CreateRemoteConnection(); + var g = AnonymousTraversalSource.Traversal().WithRemote(connection); + var p = g.With("materializeProperties", "tokens").V().Has("name", "marko").OutE().InV().HasLabel("software").Path().Next(); + + Assert.NotNull(p); + Assert.Equal(3, p.Count); + + var a = p[0] as Vertex; + var b = p[1] as Edge; + var c = p[2] as Vertex; + + Assert.NotNull(a); + Assert.NotNull(b); + Assert.NotNull(c); + + // GraphSON will deserialize into null and GraphBinary to [] + Assert.True(a.Properties == null || a.Properties.Length == 0); + Assert.True(b.Properties == null || b.Properties.Length == 0); + Assert.True(c.Properties == null || c.Properties.Length == 0); + } + + [Fact] + public void shouldMaterializePropertiesAllInPath() + { + var connection = _connectionFactory.CreateRemoteConnection(); + var g = AnonymousTraversalSource.Traversal().WithRemote(connection); + var p = g.With("materializeProperties", "all").V().Has("name", "marko").OutE().InV().HasLabel("software").Path().Next(); + + Assert.NotNull(p); + Assert.Equal(3, p.Count); + + var a = p[0] as Vertex; + var b = p[1] as Edge; + var c = p[2] as Vertex; + + Assert.NotNull(a); + Assert.NotNull(b); + Assert.NotNull(c); + + Assert.True(a.Properties != null && a.Properties.Length > 0); + Assert.True(b.Properties != null && b.Properties.Length > 0); + Assert.True(c.Properties != null && c.Properties.Length > 0); + } } } \ No newline at end of file diff --git a/gremlin-go/driver/connection_test.go b/gremlin-go/driver/connection_test.go index 677f9bccab..863c264c09 100644 --- a/gremlin-go/driver/connection_test.go +++ b/gremlin-go/driver/connection_test.go @@ -1266,5 +1266,59 @@ func TestConnection(t *testing.T) { assert.True(t, ok) assert.Equal(t, 0, len(properties)) } + + // Path elements should also have no materialized properties when tokens is set + r, err := g.With("materializeProperties", MaterializeProperties.Tokens).V().Has("person", "name", "marko").OutE().InV().HasLabel("software").Path().Next() + assert.Nil(t, err) + p, err := r.GetPath() + assert.Nil(t, err) + assert.NotNil(t, p) + assert.Equal(t, 3, len(p.Objects)) + + // first element should be a Vertex + if a, ok := p.Objects[0].(*Vertex); assert.True(t, ok) { + props, ok := a.Properties.([]interface{}) + assert.True(t, ok) + assert.Equal(t, 0, len(props)) + } + // second element should be an Edge + if b, ok := p.Objects[1].(*Edge); assert.True(t, ok) { + props, ok := b.Properties.([]interface{}) + assert.True(t, ok) + assert.Equal(t, 0, len(props)) + } + // third element should be a Vertex + if c, ok := p.Objects[2].(*Vertex); assert.True(t, ok) { + props, ok := c.Properties.([]interface{}) + assert.True(t, ok) + assert.Equal(t, 0, len(props)) + } + + // Path elements should have materialized properties when all is set + r, err = g.With("materializeProperties", MaterializeProperties.All).V().Has("person", "name", "marko").OutE().InV().HasLabel("software").Path().Next() + assert.Nil(t, err) + p, err = r.GetPath() + assert.Nil(t, err) + assert.NotNil(t, p) + assert.Equal(t, 3, len(p.Objects)) + + // first element should be a Vertex with properties present + if a, ok := p.Objects[0].(*Vertex); assert.True(t, ok) { + props, ok := a.Properties.([]interface{}) + assert.True(t, ok) + assert.Greater(t, len(props), 0) + } + // second element should be an Edge with properties present + if b, ok := p.Objects[1].(*Edge); assert.True(t, ok) { + props, ok := b.Properties.([]interface{}) + assert.True(t, ok) + assert.Greater(t, len(props), 0) + } + // third element should be a Vertex with properties present + if c, ok := p.Objects[2].(*Vertex); assert.True(t, ok) { + props, ok := c.Properties.([]interface{}) + assert.True(t, ok) + assert.Greater(t, len(props), 0) + } }) } diff --git a/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/structure/graph.js b/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/structure/graph.js index 4171c3556e..9e0b19f30f 100644 --- a/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/structure/graph.js +++ b/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/structure/graph.js @@ -79,10 +79,19 @@ class Edge extends Element { this.inV = inV; this.properties = {}; if (properties) { - const keys = Object.keys(properties); - for (let i = 0; i < keys.length; i++) { - const k = keys[i]; - this.properties[k] = properties[k].value; + if (Array.isArray(properties)) { + // Handle array of Property objects + for (let i = 0; i < properties.length; i++) { + const prop = properties[i]; + this.properties[prop.key] = prop.value; + } + } else { + // Handle object format as before + const keys = Object.keys(properties); + for (let i = 0; i < keys.length; i++) { + const k = keys[i]; + this.properties[k] = properties[k].value; + } } } } diff --git a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/integration/client-tests.js b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/integration/client-tests.js index a52b92ff6a..a61701d255 100644 --- a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/integration/client-tests.js +++ b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/integration/client-tests.js @@ -25,14 +25,16 @@ const graphModule = require('../../lib/structure/graph'); const helper = require('../helper'); const t = require('../../lib/process/traversal'); -let client; +let client, clientCrew; describe('Client', function () { before(function () { client = helper.getClient('gmodern'); + clientCrew = helper.getClient('gcrew') return client.open(); }); after(function () { + clientCrew.close(); return client.close(); }); describe('#submit()', function () { @@ -84,12 +86,16 @@ describe('Client', function () { assert.ok(vertex instanceof graphModule.Vertex); let age, name if (vertex.properties instanceof Array) { - age = vertex.properties[1] - name = vertex.properties[0] + const ageProps = vertex.properties.filter(p => p.key === 'age'); + const nameProps = vertex.properties.filter(p => p.key === 'name'); + age = ageProps[0]; + name = nameProps[0]; } else { age = vertex.properties.age[0] name = vertex.properties.name[0] } + assert.ok(age); + assert.ok(name); assert.strictEqual(age.value, 29); assert.strictEqual(name.value, 'marko'); }); @@ -113,19 +119,63 @@ describe('Client', function () { assert.strictEqual(result.length, 1); const vertex = result.first(); assert.ok(vertex instanceof graphModule.Vertex); + // if/then until TINKERPOP-3186 let age, name if (vertex.properties instanceof Array) { - age = vertex.properties[1] - name = vertex.properties[0] + const ageProps = vertex.properties.filter(p => p.key === 'age'); + const nameProps = vertex.properties.filter(p => p.key === 'name'); + age = ageProps[0]; + name = nameProps[0]; } else { age = vertex.properties.age[0] name = vertex.properties.name[0] } + assert.ok(age); + assert.ok(name); assert.strictEqual(age.value, 29); assert.strictEqual(name.value, 'marko'); }); }); + it('should handle Edge properties for gremlin request', function () { + return client.submit('g.E().has("weight", 0.5).limit(1)') + .then(function (result) { + assert.ok(result); + assert.strictEqual(result.length, 1); + const edge = result.first(); + assert.ok(edge instanceof graphModule.Edge); + assert.strictEqual(edge.label, 'knows'); + assert.strictEqual(edge.properties.weight, 0.5); + assert.ok(edge.inV); + assert.ok(edge.outV); + }); + }); + + it('should handle VertexProperty metadata for gremlin request', function () { + return clientCrew.submit('g.V(7).properties("location").limit(1)') + .then(function (result) { + assert.ok(result); + assert.strictEqual(result.length, 1); + const prop = result.first(); + assert.ok(prop instanceof graphModule.VertexProperty); + assert.strictEqual(prop.key, 'location'); + assert.strictEqual(prop.value, 'centreville'); + + // Check meta-properties - TINKERPOP-3186 + if (prop.properties instanceof Object && !(prop.properties instanceof Array)) { + assert.strictEqual(prop.properties.startTime, 1990); + assert.strictEqual(prop.properties.endTime, 2000); + } else { + const startTime = prop.properties.find(p => p.key === 'startTime'); + const endTime = prop.properties.find(p => p.key === 'endTime'); + assert.ok(startTime); + assert.ok(endTime); + assert.strictEqual(startTime.value, 1990); + assert.strictEqual(endTime.value, 2000); + } + }); + }); + it('should skip Vertex properties for gremlin request with tokens', function () { return client.submit('g.with("materializeProperties", "tokens").V(1)') .then(function (result) { @@ -306,10 +356,12 @@ function assertVertexProperties(vertex) { const vertexProperty = locations[0]; assert.strictEqual(vertexProperty.value, 'centreville'); if (vertexProperty.properties instanceof Array) { - assert.strictEqual(vertexProperty.properties[0].key, 'startTime'); - assert.strictEqual(vertexProperty.properties[0].value, 1990); - assert.strictEqual(vertexProperty.properties[1].key, 'endTime'); - assert.strictEqual(vertexProperty.properties[1].value, 2000); + const start = vertexProperty.properties.find(p => p.key === 'startTime'); + const end = vertexProperty.properties.find(p => p.key === 'endTime'); + assert.ok(start); + assert.ok(end); + assert.strictEqual(start.value, 1990); + assert.strictEqual(end.value, 2000); } else { assert.strictEqual(vertexProperty.properties.startTime, 1990); assert.strictEqual(vertexProperty.properties.endTime, 2000); diff --git a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/integration/traversal-test.js b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/integration/traversal-test.js index 97b522e33d..cfffd57cc0 100644 --- a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/integration/traversal-test.js +++ b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/integration/traversal-test.js @@ -158,6 +158,79 @@ describe('Traversal', function () { list.forEach(vp => assert.ok(vp.properties === undefined || vp.properties.length === 0)); }); }); + it('should skip path element properties when tokens is set', function () { + var g = traversal().withRemote(connection); + return g.with_("materializeProperties", "tokens").V().has('name','marko').outE().inV().hasLabel('software').path().next().then(function (item) { + const p = item.value; + assert.ok(p); + assert.strictEqual(p.objects.length, 3); + const a = p.objects[0]; + const b = p.objects[1]; + const c = p.objects[2]; + assert.ok(a instanceof Vertex); + assert.ok(b instanceof Edge); + assert.ok(c instanceof Vertex); + assert.ok(a.properties === undefined || a.properties.length === 0); + assert.strictEqual(Object.keys(b.properties).length, 0); + assert.ok(c.properties === undefined || c.properties.length === 0); + }); + }); + it('should materialize path element properties when all is set', function () { + var g = traversal().withRemote(connection); + return g.with_("materializeProperties", "all").V().has('name','marko').outE().inV().hasLabel('software').path().next().then(function (item) { + const p = item.value; + assert.ok(p); + assert.strictEqual(p.objects.length, 3); + const a = p.objects[0]; + const b = p.objects[1]; + const c = p.objects[2]; + assert.ok(a instanceof Vertex); + assert.ok(b instanceof Edge); + assert.ok(c instanceof Vertex); + assert.ok(a.properties); + // these assertions are if/then because of how mixed up javascript is right now on serialization + // standards as discussed on TINKERPOP-3186 - can fix that on a breaking change...until then we + // just retain existing weirdness. + let aNameProps, aAgeProps; + if (a.properties instanceof Array) { + aNameProps = a.properties.filter(p => p.key === 'name'); + aAgeProps = a.properties.filter(p => p.key === 'age'); + } else { + aNameProps = a.properties['name']; + aAgeProps = a.properties['age']; + } + assert.ok(aNameProps); + assert.strictEqual(aNameProps.length, 1); + assert.strictEqual(aNameProps[0].value, 'marko'); + assert.ok(aAgeProps); + assert.strictEqual(aAgeProps.length, 1); + assert.strictEqual(aAgeProps[0].value, 29); + assert.ok(b.properties); + let bWeight; + if (b.properties instanceof Array) { + bWeight = b.properties.filter(p => p.key === 'weight'); + } else { + bWeight = b.properties['weight']; + } + assert.ok(bWeight !== undefined); + assert.strictEqual(bWeight, 0.4); + assert.ok(c.properties); + let cNameProps, cLangProps; + if (c.properties instanceof Array) { + cNameProps = c.properties.filter(p => p.key === 'name'); + cLangProps = c.properties.filter(p => p.key === 'lang'); + } else { + cNameProps = c.properties['name']; + cLangProps = c.properties['lang']; + } + assert.ok(cNameProps); + assert.strictEqual(cNameProps.length, 1); + assert.strictEqual(cNameProps[0].value, 'lop'); + assert.ok(cLangProps); + assert.strictEqual(cLangProps.length, 1); + assert.strictEqual(cLangProps[0].value, 'java'); + }); + }); }); describe('lambdas', function() { it('should handle 1-arg lambdas', function() { diff --git a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/graphbinary/PathSerializer-test.js b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/graphbinary/PathSerializer-test.js index a65a842c5f..95198c4862 100644 --- a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/graphbinary/PathSerializer-test.js +++ b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/graphbinary/PathSerializer-test.js @@ -149,4 +149,96 @@ describe('GraphBinary.PathSerializer', () => { )) ); + // Additional complex path tests with Vertex and Edge objects + describe('complex V-E-V path round-trip', () => { + function buildVertex(id, label, propsObj) { + if (!propsObj) return new g.Vertex(id, label, undefined); + // build array of VertexProperty entries as expected by Vertex + const vps = Object.keys(propsObj).map((k, idx) => new g.VertexProperty(idx + 1, k, propsObj[k], undefined)); + return new g.Vertex(id, label, vps); + } + + function buildEdge(id, outV, label, inV, propsObj) { + // Edge expects properties as a map of key -> g.Property (the constructor extracts .value) + let props = undefined; + if (propsObj) { + props = {}; + for (const k of Object.keys(propsObj)) props[k] = new g.Property(k, propsObj[k]); + } + return new g.Edge(id, outV, label, inV, props); + } + + it('should serialize/deserialize Path with V-E-V that includes properties', () => { + const vOut = buildVertex(1, 'person', { name: 'marko', age: 29 }); + const vIn = buildVertex(3, 'software', { name: 'lop', lang: 'java' }); + const e = buildEdge(9, vOut, 'created', vIn, { weight: 0.4 }); + + const p = new g.Path([[ 'a' ], [ 'b' ], [ 'c' ]], [ vOut, e, vIn ]); + + // round-trip with non-fully-qualified format + const buf = pathSerializer.serialize(p, false); + const { v: deser, len } = pathSerializer.deserialize(buf, false); + + // sanity on buffer consumed + assert.strictEqual(len, buf.length); + + // assert labels + assert.strictEqual(deser.labels.length, 3); + assert.strictEqual(deser.labels[0].length, 1); + assert.strictEqual(deser.labels[1].length, 1); + assert.strictEqual(deser.labels[2].length, 1); + + // assert objects and their types + assert.strictEqual(deser.objects.length, 3); + const a = deser.objects[0]; + const b = deser.objects[1]; + const c = deser.objects[2]; + assert.ok(a instanceof g.Vertex); + assert.ok(b instanceof g.Edge); + assert.ok(c instanceof g.Vertex); + + // Vertex properties should be present (VertexSerializer preserves them) + assert.ok(Array.isArray(a.properties)); + assert.ok(a.properties.length > 0); + assert.ok(Array.isArray(c.properties)); + assert.ok(c.properties.length > 0); + + // Edge properties are not serialized in GraphBinary (EdgeSerializer writes null), expect empty object + assert.ok(b.properties); + assert.strictEqual(Object.keys(b.properties).length, 0); + }); + + it('should serialize/deserialize Path with V-E-V without properties', () => { + const vOut = buildVertex(1, 'person', undefined); + const vIn = buildVertex(3, 'software', undefined); + const e = buildEdge(9, vOut, 'created', vIn, undefined); + + const p = new g.Path([[ 'a' ], [ 'b' ], [ 'c' ]], [ vOut, e, vIn ]); + + // round-trip with fully-qualified format + const fqBuf = pathSerializer.serialize(p, true); + const { v: deser, len } = pathSerializer.deserialize(fqBuf, true); + + assert.strictEqual(len, fqBuf.length); + + // types + const a = deser.objects[0]; + const b = deser.objects[1]; + const c = deser.objects[2]; + assert.ok(a instanceof g.Vertex); + assert.ok(b instanceof g.Edge); + assert.ok(c instanceof g.Vertex); + + // Vertex properties default to [] when null/undefined + assert.ok(Array.isArray(a.properties)); + assert.strictEqual(a.properties.length, 0); + assert.ok(Array.isArray(c.properties)); + assert.strictEqual(c.properties.length, 0); + + // Edge properties default to {} when null/undefined + assert.ok(b.properties); + assert.strictEqual(Object.keys(b.properties).length, 0); + }); + }); + }); diff --git a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/graphson-test.js b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/graphson-test.js index cc7febc4d6..8e2c2d660d 100644 --- a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/graphson-test.js +++ b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/graphson-test.js @@ -128,12 +128,28 @@ describe('GraphSONReader', function () { assert.ok(result.objects); assert.ok(result.labels); assert.strictEqual(result.objects[2], 'lop'); - assert.ok(result.objects[0] instanceof graph.Vertex); - assert.ok(result.objects[1] instanceof graph.Vertex); - assert.strictEqual(result.objects[0].label, 'person'); - assert.strictEqual(result.objects[1].label, 'software'); + const a = result.objects[0]; + const bc = result.objects[1]; + assert.ok(a instanceof graph.Vertex); + assert.ok(bc instanceof graph.Vertex); + assert.strictEqual(a.label, 'person'); + assert.strictEqual(bc.label, 'software'); + assert.ok(a.properties); + assert.ok(a.properties['name']); + assert.strictEqual(a.properties['name'].length, 1); + assert.strictEqual(a.properties['name'][0].value, 'marko'); + assert.ok(a.properties['age']); + assert.strictEqual(a.properties['age'].length, 1); + assert.strictEqual(a.properties['age'][0].value, 29); + assert.ok(bc.properties); + assert.ok(bc.properties['name']); + assert.strictEqual(bc.properties['name'].length, 1); + assert.strictEqual(bc.properties['name'][0].value, 'lop'); + assert.ok(bc.properties['lang']); + assert.strictEqual(bc.properties['lang'].length, 1); + assert.strictEqual(bc.properties['lang'][0].value, 'java'); }); - it('should parse paths from GraphSON3', function () { + it('should parse paths from GraphSON3 without properties', function () { const obj = { "@type" : "g:Path", "@value" : { @@ -180,10 +196,14 @@ describe('GraphSONReader', function () { assert.ok(result.objects); assert.ok(result.labels); assert.strictEqual(result.objects[2], 'lop'); - assert.ok(result.objects[0] instanceof graph.Vertex); - assert.ok(result.objects[1] instanceof graph.Vertex); - assert.strictEqual(result.objects[0].label, 'person'); - assert.strictEqual(result.objects[1].label, 'software'); + const a = result.objects[0]; + const bc = result.objects[1]; + assert.ok(a instanceof graph.Vertex); + assert.ok(bc instanceof graph.Vertex); + assert.strictEqual(a.label, 'person'); + assert.strictEqual(bc.label, 'software'); + assert.ok(a.properties === undefined); + assert.ok(bc.properties === undefined); }); }); describe('GraphSONWriter', function () { diff --git a/gremlin-python/src/main/python/tests/driver/test_driver_remote_connection.py b/gremlin-python/src/main/python/tests/driver/test_driver_remote_connection.py index 4d995d406d..732aafcb2b 100644 --- a/gremlin-python/src/main/python/tests/driver/test_driver_remote_connection.py +++ b/gremlin-python/src/main/python/tests/driver/test_driver_remote_connection.py @@ -127,6 +127,21 @@ class TestDriverRemoteConnection(object): results = g.with_("materializeProperties", "tokens").V().properties().to_list() for vp in results: assert vp.properties is None or len(vp.properties) == 0 + # # + # test materializeProperties in Path - GraphSON will deserialize into None and GraphBinary to [] + p = g.with_("materializeProperties", "tokens").V().has('name', 'marko').outE().inV().has_label('software').path().next() + assert 3 == len(p.objects) + assert p.objects[0].properties is None or len(p.objects[0].properties) == 0 + assert p.objects[1].properties is None or len(p.objects[1].properties) == 0 + assert p.objects[2].properties is None or len(p.objects[2].properties) == 0 + # # + # test materializeProperties in Path - 'all' should materialize properties on each element + p = g.with_("materializeProperties", "all").V().has('name', 'marko').outE().inV().has_label('software').path().next() + assert 3 == len(p.objects) + assert p.objects[0].properties is not None and len(p.objects[0].properties) > 0 + # edges have dict-like properties; ensure not empty + assert p.objects[1].properties is not None and len(p.objects[1].properties) > 0 + assert p.objects[2].properties is not None and len(p.objects[2].properties) > 0 def test_lambda_traversals(self, remote_connection): statics.load_statics(globals()) diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/Context.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/Context.java index 8636954ad5..b0eb6aedbe 100644 --- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/Context.java +++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/Context.java @@ -18,6 +18,7 @@ */ package org.apache.tinkerpop.gremlin.server; +import org.apache.tinkerpop.gremlin.process.traversal.Path; import org.apache.tinkerpop.gremlin.process.traversal.traverser.util.AbstractTraverser; import org.apache.tinkerpop.gremlin.structure.Element; import org.apache.tinkerpop.gremlin.structure.util.reference.ReferenceFactory; @@ -327,8 +328,13 @@ public class Context { final Object firstElement = aggregate.get(0); if (firstElement instanceof Element) { - for (int i = 0; i < aggregate.size(); i++) + for (int i = 0; i < aggregate.size(); i++) { aggregate.set(i, ReferenceFactory.detach((Element) aggregate.get(i))); + } + } else if (firstElement instanceof Path) { + for (int i = 0; i < aggregate.size(); i++) { + aggregate.set(i, ReferenceFactory.detach((Path) aggregate.get(i))); + } } else if (firstElement instanceof AbstractTraverser) { for (final Object item : aggregate) ((AbstractTraverser) item).detach(); diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerSerializationIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerSerializationIntegrateTest.java index ebe6dfdf6c..cca1b80394 100644 --- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerSerializationIntegrateTest.java +++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerSerializationIntegrateTest.java @@ -22,11 +22,13 @@ import org.apache.tinkerpop.gremlin.driver.Client; import org.apache.tinkerpop.gremlin.driver.Cluster; import org.apache.tinkerpop.gremlin.driver.remote.DriverRemoteConnection; import org.apache.tinkerpop.gremlin.process.traversal.AnonymousTraversalSource; +import org.apache.tinkerpop.gremlin.process.traversal.Path; import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; import org.apache.tinkerpop.gremlin.structure.Edge; import org.apache.tinkerpop.gremlin.structure.Property; import org.apache.tinkerpop.gremlin.structure.Vertex; import org.apache.tinkerpop.gremlin.structure.VertexProperty; +import org.apache.tinkerpop.gremlin.util.Tokens; import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils; import org.apache.tinkerpop.gremlin.util.ser.AbstractMessageSerializer; import org.apache.tinkerpop.gremlin.util.ser.GraphBinaryMessageSerializerV1; @@ -42,7 +44,11 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.core.Is.is; +import static org.hamcrest.CoreMatchers.instanceOf; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; @RunWith(Parameterized.class) public class GremlinServerSerializationIntegrateTest extends AbstractGremlinServerIntegrationTest { @@ -56,7 +62,7 @@ public class GremlinServerSerializationIntegrateTest extends AbstractGremlinServ this.serializer = serializer; } - @Parameterized.Parameters + @Parameterized.Parameters(name = "{0}") public static Collection serializers() { return Arrays.asList(new Object[][]{ {new GraphBinaryMessageSerializerV1()}, @@ -105,7 +111,7 @@ public class GremlinServerSerializationIntegrateTest extends AbstractGremlinServ @Test public void shouldSkipVertexPropertiesForBytecode() { - final Vertex vertex = g.with("materializeProperties", "tokens").V(1).next(); + final Vertex vertex = g.with(Tokens.ARGS_MATERIALIZE_PROPERTIES, Tokens.MATERIALIZE_PROPERTIES_TOKENS).V(1).next(); assertVertexWithoutProperties(vertex); } @@ -133,7 +139,7 @@ public class GremlinServerSerializationIntegrateTest extends AbstractGremlinServ @Test public void shouldSkipEdgePropertiesForBytecode() { - final Edge edge = g.with("materializeProperties", "tokens").E(7).next(); + final Edge edge = g.with(Tokens.ARGS_MATERIALIZE_PROPERTIES, Tokens.MATERIALIZE_PROPERTIES_TOKENS).E(7).next(); assertEdgeWithoutProperties(edge); } @@ -161,11 +167,36 @@ public class GremlinServerSerializationIntegrateTest extends AbstractGremlinServ @Test public void shouldSkipVertexPropertyPropertiesForBytecode() { - final Vertex vertex = g.with("materializeProperties", "tokens").V(7).next(); + final Vertex vertex = g.with(Tokens.ARGS_MATERIALIZE_PROPERTIES, Tokens.MATERIALIZE_PROPERTIES_TOKENS).V(7).next(); assertVertexWithoutVertexProperties(vertex); } + @Test + public void shouldDeserializePathPropertiesForScripts() { + final Path p = client.submit("gmodern.V().has('name','marko').outE().inV().hasLabel('software').path()").one().getPath(); + assertPathElementsWithProperties(p); + } + + @Test + public void shouldDeserializePathPropertiesForScriptsWithTokens() { + final Path p = client.submit("gmodern.with('materializeProperties','tokens').V().has('name','marko').outE().inV().hasLabel('software').path()").one().getPath(); + assertPathElementsWithoutProperties(p); + } + + @Test + public void shouldDeserializePathPropertiesForScriptsForBytecode() { + final Path p = g.V().has("name", "marko").outE().inV().hasLabel("software").path().next(); + assertPathElementsWithProperties(p); + } + + @Test + public void shouldDeserializePathPropertiesForScriptsWithTokensForBytecode() { + final Path p = g.with(Tokens.ARGS_MATERIALIZE_PROPERTIES, Tokens.MATERIALIZE_PROPERTIES_TOKENS). + V().has("name", "marko").outE().inV().hasLabel("software").path().next(); + assertPathElementsWithoutProperties(p); + } + // asserted vertex 7 from crew graph private void assertVertexWithVertexProperties(final Vertex vertex) { assertEquals(7, vertex.id()); @@ -224,4 +255,60 @@ public class GremlinServerSerializationIntegrateTest extends AbstractGremlinServ assertEquals(1, IteratorUtils.count(edge.properties())); assertEquals(0.5, edge.property("weight").value()); } + + private void assertPathElementsWithProperties(final Path p) { + // expect a V-E-V path + assertEquals(3, p.size()); + + final Object a = p.get(0); + final Object b = p.get(1); + final Object c = p.get(2); + + // basic type assertions (prefer Hamcrest) + assertThat(a, instanceOf(Vertex.class)); + assertThat(b, instanceOf(Edge.class)); + assertThat(c, instanceOf(Vertex.class)); + + // properties should be present on each element in the path with expected key/values + final Vertex vOut = (Vertex) a; + final Edge e = (Edge) b; + final Vertex vIn = (Vertex) c; + + // vertex 'a' should be marko with age 29 + assertThat(IteratorUtils.count(vOut.properties()) > 0, is(true)); + assertEquals("marko", vOut.value("name")); + assertEquals(Integer.valueOf(29), vOut.value("age")); + + // edge 'b' should have weight 0.4 + assertThat(IteratorUtils.count(e.properties()) > 0, is(true)); + assertEquals(0.4, (Double) e.value("weight"), 0.0000001d); + + // vertex 'c' should be lop with lang java + assertThat(IteratorUtils.count(vIn.properties()) > 0, is(true)); + assertEquals("lop", vIn.value("name")); + assertEquals("java", vIn.value("lang")); + } + + private void assertPathElementsWithoutProperties(final Path p) { + // expect a V-E-V path + assertEquals(3, p.size()); + + final Object a = p.get(0); + final Object b = p.get(1); + final Object c = p.get(2); + + // basic type assertions (prefer Hamcrest) + assertThat(a, instanceOf(Vertex.class)); + assertThat(b, instanceOf(Edge.class)); + assertThat(c, instanceOf(Vertex.class)); + + // properties should NOT be present on each element in the path when materializeProperties is 'tokens' + final Vertex vOut = (Vertex) a; + final Edge e = (Edge) b; + final Vertex vIn = (Vertex) c; + + assertEquals(0, IteratorUtils.count(vOut.properties())); + assertEquals(0, IteratorUtils.count(e.properties())); + assertEquals(0, IteratorUtils.count(vIn.properties())); + } } diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/SerializationTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/SerializationTest.java index 9ddcf2321c..38c72092f6 100644 --- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/SerializationTest.java +++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/SerializationTest.java @@ -53,6 +53,7 @@ import java.util.concurrent.TimeUnit; import org.apache.tinkerpop.gremlin.process.traversal.step.util.Tree; import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -721,25 +722,21 @@ public class SerializationTest { final Vertex detachedVOut = detached.get("a"); assertEquals(vOut.label(), detachedVOut.label()); assertEquals(vOut.id(), detachedVOut.id()); - - // this is a SimpleTraverser so no properties are present in detachment - assertFalse(detachedVOut.properties().hasNext()); + assertEquals("marko", detachedVOut.value("name").toString()); + assertEquals(Integer.valueOf(29), detachedVOut.value("age")); final Edge e = p.get("b"); final Edge detachedE = detached.get("b"); assertEquals(e.label(), detachedE.label()); assertEquals(e.id(), detachedE.id()); - - // this is a SimpleTraverser so no properties are present in detachment - assertFalse(detachedE.properties().hasNext()); + assertEquals((double) e.value("weight"), detachedE.value("weight"), 0.0001d); final Vertex vIn = p.get("c"); final Vertex detachedVIn = detached.get("c"); assertEquals(vIn.label(), detachedVIn.label()); assertEquals(vIn.id(), detachedVIn.id()); - - // this is a SimpleTraverser so no properties are present in detachment - assertFalse(detachedVIn.properties().hasNext()); + assertEquals("lop", detachedVIn.value("name").toString()); + assertEquals("java", detachedVIn.value("lang").toString()); } @Test @@ -923,25 +920,21 @@ public class SerializationTest { final Vertex detachedVOut = detached.get("a"); assertEquals(vOut.label(), detachedVOut.label()); assertEquals(vOut.id(), detachedVOut.id()); - - // this is a SimpleTraverser so no properties are present in detachment - assertFalse(detachedVOut.properties().hasNext()); + assertEquals("marko", detachedVOut.value("name").toString()); + assertEquals(Integer.valueOf(29), detachedVOut.value("age")); final Edge e = p.get("b"); final Edge detachedE = detached.get("b"); assertEquals(e.label(), detachedE.label()); assertEquals(e.id(), detachedE.id()); - - // this is a SimpleTraverser so no properties are present in detachment - assertFalse(detachedE.properties().hasNext()); + assertEquals((double) e.value("weight"), detachedE.value("weight"), 0.0001d); final Vertex vIn = p.get("c"); final Vertex detachedVIn = detached.get("c"); assertEquals(vIn.label(), detachedVIn.label()); assertEquals(vIn.id(), detachedVIn.id()); - - // this is a SimpleTraverser so no properties are present in detachment - assertFalse(detachedVIn.properties().hasNext()); + assertEquals("lop", detachedVIn.value("name").toString()); + assertEquals("java", detachedVIn.value("lang").toString()); } @Test diff --git a/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/Tokens.java b/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/Tokens.java index 63918524ad..eaa3308bbc 100644 --- a/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/Tokens.java +++ b/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/Tokens.java @@ -79,6 +79,7 @@ public final class Tokens { public static final String ARGS_HOST = "host"; public static final String ARGS_SESSION = "session"; public static final String ARGS_MANAGE_TRANSACTION = "manageTransaction"; + /** * The name of the argument that allows to control the serialization of properties on the server. */ @@ -88,8 +89,8 @@ public final class Tokens { * The name of the value denoting that all properties of Element should be returned. * Should be used with {@code ARGS_MATERIALIZE_PROPERTIES} */ - public static final String MATERIALIZE_PROPERTIES_ALL = "all"; + /** * The name of the value denoting that only `ID` and `Label` of Element should be returned. * Should be used with {@code ARGS_MATERIALIZE_PROPERTIES} @@ -130,7 +131,7 @@ public final class Tokens { public static final String STATUS_ATTRIBUTE_STACK_TRACE = "stackTrace"; /** - * A {@link ResultSet#statusAttributes()} key for user-facing warnings. + * A {@code ResultSet#statusAttributes()} key for user-facing warnings. * <p> * Implementations that set this key should consider using one of * these two recommended value types:
