diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 822a3f442ce..edc3e109967 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -36,6 +36,7 @@ This release also includes changes from <>. * Fix cases where Map keys of incomparable types could panic in `gremlin-go`. * Fixed an issue where missing necessary parameters for logging, resulting in '%!x(MISSING)' output in `gremlin-go`. * Added getter method to `ConcatStep`, `ConjoinStep`, `SplitGlobalStep` and `SplitLocalStep` for their private fields. +* When using `ReferenceElementStrategy` with `GraphBinaryV1`, properties on elements will be returned as `null` instead of empty list, to stay compatible with older drivers. * Gremlin Server docker containers shutdown gracefully when receiving a SIGTERM. * Added DefaultIdManager.STRING for proper string id creation/handling. * Allowed specification of an `Operator` as a reducer in `withSideEffect` when parsing with the grammar. diff --git a/docs/src/reference/gremlin-applications.asciidoc b/docs/src/reference/gremlin-applications.asciidoc index a2412642fbe..196e59fd4d7 100644 --- a/docs/src/reference/gremlin-applications.asciidoc +++ b/docs/src/reference/gremlin-applications.asciidoc @@ -2336,6 +2336,45 @@ List results = g.V().hasLabel("person").elementMap('name').toList(); Both of the above requests return a list of `Map` instances that contain the `id`, `label` and the "name" property. +*Compatibility* + +*It is not recommended to use 3.6.x or below driver versions with 3.7.x or above Gremlin Server*, as some older drivers do not construct +graph elements with properties and thus are not designed to handle the returned properties by default; however, compatibility +can be achieved by configuring `ReferenceElementStrategy` in the server such that properties are not returned. +Per-request configuration option `materializeProperties` is not supported older driver versions. + +Also note that older drivers of different language variants will handle incoming properties differently with different +serializers used. Drivers using `GraphSON` serializers will remain compatible, but may encounter deserialization errors +with `GraphBinary`. Below is a table documenting GLV behaviors using `GraphBinary` when properties are returned by the +default 3.7.x server, as well as if `ReferenceElementStrategy` is configured (i.e. mimic the behavior +of a 3.6.x server). This can be observed with the results of `g.V().next()`. Note that only `gremlin-driver` +and `gremlin-javacript` have the `properties` attribute in the Element objects, all other GLVs only have `id` and `label`. + +[cols="1,1,1"] +|=== +|3.6.x drivers with `GraphBinary` |Behavior with default 3.7.x Server | Behavior with `ReferenceElementStrategy` + +|`gremlin-driver` +|Properties returned as empty iterator +|Properties returned as empty iterator + +|`gremlin-dotnet` +|Skips properties in Elements +|Skips properties in Elements + +|`gremlin-javascript` +|Deserialization error +|Properties returned as empty list + +|`gremlin-python` +|Deserialization error +|Skips properties in Elements + +|`gremlin-go` +|Deserialization error +|Skips properties in Elements +|=== + TIP: Consider utilizing `ReferenceElementStrategy` whenever creating a `GraphTraversalSource` in Java to ensure the most portable Gremlin. diff --git a/docs/src/upgrade/release-3.7.x.asciidoc b/docs/src/upgrade/release-3.7.x.asciidoc index 4dda8ba74d4..8fd5605f63b 100644 --- a/docs/src/upgrade/release-3.7.x.asciidoc +++ b/docs/src/upgrade/release-3.7.x.asciidoc @@ -21,7 +21,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima *Gremfir Master of the Pan Flute* -=== TinkerPop 3.7.3 +== TinkerPop 3.7.3 *Release Date: NOT OFFICIALLY RELEASED YET* @@ -30,6 +30,25 @@ complete list of all the modifications that are part of this release. === Upgrading for Users +==== GraphBinary Deserialization of Reference Element Properties with ReferenceElementStrategy +When properties on element were introduced and returned as default in 3.7.0, setting ReferenceElementStrategy on the +server meant as a way to send reference element without properties, for lightweight wire transfer and compatibility +reasons. However, an issue was discovered where using GraphBinary, the 3.7.x server were not serializing properties as +`null` as per IO spec, but as empty lists instead. This caused deserialization failures in Python, JavaScript and Go +driver versions 3.6.x or below. + +A fix was introduced to correct such error, where Gremlin Server versions 3.7.3 and above will return element properties +as `null` when ReferenceElementStrategy is applied, or when `token` is used with `materializedProperties` option in 3.7.x +drivers. However, this also led to a change in 3.7.x driver behavior, where all non-Java drivers returns `null` instead +of empty list. As such, an additional change was introduced in these GLVs, where `null` properties from reference elements +will now deserialized into an empty list, to maintain such behavior with older 3.7.x drivers. This should minimize impact for users. + +One caveat is that when using 3.7.0 to 3.7.2 drivers to connect to 3.7.3 and above server, these drivers will not contain +the deserialization change and return `null` as properties. In these cases, it is recommended to upgrade to 3.7.3 drivers. + +See: link:https://tinkerpop.apache.org/docs/x.y.z/reference/#_properties_of_elements[Properties of Elements], +link:https://issues.apache.org/jira/browse/TINKERPOP-3105[TINKERPOP-3105] + == TinkerPop 3.7.2 *Release Date: April 8, 2024* @@ -705,7 +724,7 @@ For 3.7: ===== Enabling the previous behavior Note that drivers from earlier versions like 3.5 and 3.6 will not be able to retrieve properties on elements. Older -drivers connecting to 3.7.x servers should disable this functionality server-side in one of two ways: +drivers connecting to 3.7.x servers should disable this functionality server-side: *Configure Gremlin Server to not return properties* - update Gremlin Server initialization script with `ReferenceElementStrategy`. This configuration is essentially the one used in older versions of the server by default. @@ -715,7 +734,7 @@ drivers connecting to 3.7.x servers should disable this functionality server-sid globals << [g : traversal().withEmbedded(graph).withStrategies(ReferenceElementStrategy)] ---- -*Disable property inclusion per request* - the `materializeProperties` has a `tokens` option for this purpose. +For 3.7 drivers, properties on elements can also be disabled per request using the `tokens` option with `materializeProperties`. [source,csharp] ---- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/binary/types/EdgeSerializer.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/binary/types/EdgeSerializer.java index fdfb94f05ad..09b7c10f3d9 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/binary/types/EdgeSerializer.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/binary/types/EdgeSerializer.java @@ -27,6 +27,7 @@ import org.apache.tinkerpop.gremlin.structure.io.binary.GraphBinaryWriter; import org.apache.tinkerpop.gremlin.structure.util.detached.DetachedEdge; import org.apache.tinkerpop.gremlin.structure.util.detached.DetachedVertex; +import org.apache.tinkerpop.gremlin.structure.util.reference.ReferenceEdge; import java.io.IOException; import java.util.List; @@ -81,7 +82,7 @@ protected void writeValue(final Edge value, final Buffer buffer, final GraphBina // we don't serialize the parent Vertex for edges. context.write(null, buffer); - if (value.properties() == null) { + if (value instanceof ReferenceEdge) { context.write(null, buffer); } else { diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/binary/types/VertexPropertySerializer.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/binary/types/VertexPropertySerializer.java index 9850c13e253..c904de630fd 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/binary/types/VertexPropertySerializer.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/binary/types/VertexPropertySerializer.java @@ -26,6 +26,7 @@ import org.apache.tinkerpop.gremlin.structure.VertexProperty; import org.apache.tinkerpop.gremlin.structure.io.Buffer; import org.apache.tinkerpop.gremlin.structure.util.detached.DetachedVertexProperty; +import org.apache.tinkerpop.gremlin.structure.util.reference.ReferenceVertexProperty; import java.io.IOException; import java.util.Collections; @@ -67,9 +68,14 @@ protected void writeValue(final VertexProperty value, final Buffer buffer, final // we don't serialize the parent vertex, let's hold a place for it context.write(null, buffer); - final List asList = value.graph().features().vertex().supportsMetaProperties() ? - IteratorUtils.toList(value.properties()) : - Collections.emptyList(); - context.write(asList, buffer); + if (value instanceof ReferenceVertexProperty) { + context.write(null, buffer); + } + else { + final List asList = value.graph().features().vertex().supportsMetaProperties() ? + IteratorUtils.toList(value.properties()) : + Collections.emptyList(); + context.write(asList, buffer); + } } } diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/binary/types/VertexSerializer.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/binary/types/VertexSerializer.java index 5dbdfd1b0c3..ff5b99ea1ce 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/binary/types/VertexSerializer.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/binary/types/VertexSerializer.java @@ -27,6 +27,7 @@ import org.apache.tinkerpop.gremlin.structure.io.binary.GraphBinaryWriter; import org.apache.tinkerpop.gremlin.structure.util.detached.DetachedVertex; import org.apache.tinkerpop.gremlin.structure.util.detached.DetachedVertexProperty; +import org.apache.tinkerpop.gremlin.structure.util.reference.ReferenceVertex; import java.io.IOException; import java.util.List; @@ -60,7 +61,7 @@ protected Vertex readValue(final Buffer buffer, final GraphBinaryReader context) protected void writeValue(final Vertex value, final Buffer buffer, final GraphBinaryWriter context) throws IOException { context.write(value.id(), buffer); context.writeValue(value.label(), buffer, false); - if (value.properties() == null) { + if (value instanceof ReferenceVertex) { context.write(null, buffer); } else { diff --git a/gremlin-dotnet/src/Gremlin.Net/Structure/IO/GraphBinary/Types/EdgeSerializer.cs b/gremlin-dotnet/src/Gremlin.Net/Structure/IO/GraphBinary/Types/EdgeSerializer.cs index 13de71b3477..bfb9acfef11 100644 --- a/gremlin-dotnet/src/Gremlin.Net/Structure/IO/GraphBinary/Types/EdgeSerializer.cs +++ b/gremlin-dotnet/src/Gremlin.Net/Structure/IO/GraphBinary/Types/EdgeSerializer.cs @@ -21,6 +21,7 @@ #endregion +using System; using System.Collections.Generic; using System.IO; using System.Threading; @@ -76,7 +77,7 @@ protected override async Task ReadValueAsync(Stream stream, GraphBinaryRea await reader.ReadAsync(stream, cancellationToken).ConfigureAwait(false); var properties = await reader.ReadAsync(stream, cancellationToken).ConfigureAwait(false); - var propertiesAsArray = (properties as List)?.ToArray(); + var propertiesAsArray = null == properties ? Array.Empty() : (properties as List)?.ToArray(); return new Edge(id, outV, label, inV, propertiesAsArray); } diff --git a/gremlin-dotnet/src/Gremlin.Net/Structure/IO/GraphBinary/Types/VertexPropertySerializer.cs b/gremlin-dotnet/src/Gremlin.Net/Structure/IO/GraphBinary/Types/VertexPropertySerializer.cs index f347e0a3850..ea99aec9dc6 100644 --- a/gremlin-dotnet/src/Gremlin.Net/Structure/IO/GraphBinary/Types/VertexPropertySerializer.cs +++ b/gremlin-dotnet/src/Gremlin.Net/Structure/IO/GraphBinary/Types/VertexPropertySerializer.cs @@ -21,6 +21,7 @@ #endregion +using System; using System.Collections.Generic; using System.IO; using System.Threading; @@ -68,7 +69,7 @@ protected override async Task ReadValueAsync(Stream stream, Grap await reader.ReadAsync(stream, cancellationToken).ConfigureAwait(false); var properties = await reader.ReadAsync(stream, cancellationToken).ConfigureAwait(false); - var propertiesAsArray = (properties as List)?.ToArray(); + var propertiesAsArray = null == properties ? Array.Empty() : (properties as List)?.ToArray(); return new VertexProperty(id, label, value, null, propertiesAsArray); } diff --git a/gremlin-dotnet/src/Gremlin.Net/Structure/IO/GraphBinary/Types/VertexSerializer.cs b/gremlin-dotnet/src/Gremlin.Net/Structure/IO/GraphBinary/Types/VertexSerializer.cs index 0dd492cac49..51d27021d9f 100644 --- a/gremlin-dotnet/src/Gremlin.Net/Structure/IO/GraphBinary/Types/VertexSerializer.cs +++ b/gremlin-dotnet/src/Gremlin.Net/Structure/IO/GraphBinary/Types/VertexSerializer.cs @@ -21,6 +21,7 @@ #endregion +using System; using System.Collections.Generic; using System.IO; using System.Threading; @@ -57,7 +58,7 @@ protected override async Task ReadValueAsync(Stream stream, GraphBinaryR var label = (string)await reader.ReadNonNullableValueAsync(stream, cancellationToken).ConfigureAwait(false); var properties = await reader.ReadAsync(stream, cancellationToken).ConfigureAwait(false); - var propertiesAsArray = (properties as List)?.ToArray(); + var propertiesAsArray = null == properties ? Array.Empty() : (properties as List)?.ToArray(); return new Vertex(id, label, propertiesAsArray); } 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 2f48dcae7b4..717bf211ec7 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 @@ -291,5 +291,47 @@ public async Task ShouldThrowExceptionOnRollbackWhenGraphNotSupportTx() var exception = await Assert.ThrowsAsync(async () => await tx.RollbackAsync()); Assert.Equal("ServerError: Graph does not support transactions", exception.Message); } + + [Fact] + public void shouldUseMaterializedPropertiesTokenInV() + { + var connection = _connectionFactory.CreateRemoteConnection(); + var g = AnonymousTraversalSource.Traversal().WithRemote(connection); + var vertices = g.With("materializeProperties", "tokens").V().ToList(); + foreach (var v in vertices) + { + Assert.NotNull(v); + // GraphSON will deserialize into null and GraphBinary to [] + Assert.True(v.Properties == null || v.Properties.Length == 0); + } + } + + [Fact] + public void shouldUseMaterializedPropertiesTokenInE() + { + var connection = _connectionFactory.CreateRemoteConnection(); + var g = AnonymousTraversalSource.Traversal().WithRemote(connection); + var edges = g.With("materializeProperties", "tokens").E().ToList(); + foreach (var e in edges) + { + Assert.NotNull(e); + // GraphSON will deserialize into null and GraphBinary to [] + Assert.True(e.Properties == null || e.Properties.Length == 0); + } + } + + [Fact] + public void shouldUseMaterializedPropertiesTokenInVP() + { + var connection = _connectionFactory.CreateRemoteConnection(); + var g = AnonymousTraversalSource.Traversal().WithRemote(connection); + var vps = g.With("materializeProperties", "tokens").V().Properties().ToList(); + foreach (var vp in vps) + { + Assert.NotNull(vp); + // GraphSON will deserialize into null and GraphBinary to [] + Assert.True(vp.Properties == null || vp.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 92dce790b9a..a4d0dd66f0c 100644 --- a/gremlin-go/driver/connection_test.go +++ b/gremlin-go/driver/connection_test.go @@ -1230,4 +1230,41 @@ func TestConnection(t *testing.T) { AssertMarkoVertexWithProperties(t, r) }) + + t.Run("Test DriverRemoteConnection Traversal With materializeProperties in Modern Graph", func(t *testing.T) { + skipTestsIfNotEnabled(t, integrationTestSuiteName, testNoAuthEnable) + + g := getModernGraph(t, testNoAuthUrl, &AuthInfo{}, &tls.Config{}) + defer g.remoteConnection.Close() + + vertices, err := g.With("materializeProperties", MaterializeProperties.Tokens).V().ToList() + assert.Nil(t, err) + for _, res := range vertices { + v, _ := res.GetVertex() + assert.Nil(t, err) + properties, ok := v.Properties.([]interface{}) + assert.True(t, ok) + assert.Equal(t, 0, len(properties)) + } + + edges, err := g.With("materializeProperties", MaterializeProperties.Tokens).E().ToList() + assert.Nil(t, err) + for _, res := range edges { + e, _ := res.GetEdge() + assert.Nil(t, err) + properties, ok := e.Properties.([]interface{}) + assert.True(t, ok) + assert.Equal(t, 0, len(properties)) + } + + vps, err := g.With("materializeProperties", MaterializeProperties.Tokens).V().Properties().ToList() + assert.Nil(t, err) + for _, res := range vps { + vp, _ := res.GetVertexProperty() + assert.Nil(t, err) + properties, ok := vp.Properties.([]interface{}) + assert.True(t, ok) + assert.Equal(t, 0, len(properties)) + } + }) } diff --git a/gremlin-go/driver/graphBinary.go b/gremlin-go/driver/graphBinary.go index 839a8919f84..39c6d2f0293 100644 --- a/gremlin-go/driver/graphBinary.go +++ b/gremlin-go/driver/graphBinary.go @@ -1069,10 +1069,15 @@ func vertexReaderReadingProperties(data *[]byte, i *int, readProperties bool) (i } v.Label = label.(string) if readProperties { - v.Properties, err = readFullyQualifiedNullable(data, i, true) + props, err := readFullyQualifiedNullable(data, i, true) if err != nil { return nil, err } + // null properties are returned as empty slices + v.Properties = make([]interface{}, 0) + if props != nil { + v.Properties = props + } } return v, nil } @@ -1101,10 +1106,15 @@ func edgeReader(data *[]byte, i *int) (interface{}, error) { } e.OutV = *v.(*Vertex) *i += 2 - e.Properties, err = readFullyQualifiedNullable(data, i, true) + props, err := readFullyQualifiedNullable(data, i, true) if err != nil { return nil, err } + // null properties are returned as empty slices + e.Properties = make([]interface{}, 0) + if props != nil { + e.Properties = props + } return e, nil } @@ -1149,7 +1159,11 @@ func vertexPropertyReader(data *[]byte, i *int) (interface{}, error) { return nil, err } - vp.Properties = props + // null properties are returned as empty slices + vp.Properties = make([]interface{}, 0) + if props != nil { + vp.Properties = props + } return vp, nil } diff --git a/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/structure/io/binary/internals/VertexPropertySerializer.js b/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/structure/io/binary/internals/VertexPropertySerializer.js index b1cfc6d0012..597c0ab4be5 100644 --- a/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/structure/io/binary/internals/VertexPropertySerializer.js +++ b/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/structure/io/binary/internals/VertexPropertySerializer.js @@ -164,7 +164,10 @@ module.exports = class VertexPropertySerializer { // TODO: should we verify that properties is null? cursor = cursor.slice(properties_len); - const v = new g.VertexProperty(id, label, value, properties); + // null properties are deserialized into empty lists + const vp_props = properties ? properties : []; + + const v = new g.VertexProperty(id, label, value, vp_props); return { v, len }; } catch (err) { throw this.ioc.utils.des_error({ serializer: this, args: arguments, cursor, err }); diff --git a/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/structure/io/binary/internals/VertexSerializer.js b/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/structure/io/binary/internals/VertexSerializer.js index 3d050c578d6..65ec4e465d3 100644 --- a/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/structure/io/binary/internals/VertexSerializer.js +++ b/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/structure/io/binary/internals/VertexSerializer.js @@ -127,7 +127,10 @@ module.exports = class VertexSerializer { } cursor = cursor.slice(properties_len); - const v = new g.Vertex(id, label, properties); + // null properties are deserialized into empty lists + const vertex_props = properties ? properties : []; + + const v = new g.Vertex(id, label, vertex_props); return { v, len }; } catch (err) { throw this.ioc.utils.des_error({ serializer: this, args: arguments, cursor, err }); 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 5d1546959bb..97b522e33d6 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 @@ -26,7 +26,7 @@ const Mocha = require('mocha'); const assert = require('assert'); const { AssertionError } = require('assert'); const DriverRemoteConnection = require('../../lib/driver/driver-remote-connection'); -const { Vertex } = require('../../lib/structure/graph'); +const { Vertex, Edge, VertexProperty} = require('../../lib/structure/graph'); const { traversal } = require('../../lib/process/anonymous-traversal'); const { GraphTraversalSource, GraphTraversal, statics } = require('../../lib/process/graph-traversal'); const { SubgraphStrategy, ReadOnlyStrategy, SeedStrategy, @@ -129,6 +129,36 @@ describe('Traversal', function () { }); }); }); + describe('#materializeProperties()', function () { + it('should skip vertex properties when tokens is set', function () { + var g = traversal().withRemote(connection); + return g.with_("materializeProperties", "tokens").V().toList().then(function (list) { + assert.ok(list); + assert.strictEqual(list.length, 6); + list.forEach(v => assert.ok(v instanceof Vertex)); + list.forEach(v => assert.ok(v.properties === undefined || v.properties.length === 0)); + }); + }); + it('should skip edge properties when tokens is set', function () { + var g = traversal().withRemote(connection); + return g.with_("materializeProperties", "tokens").E().toList().then(function (list) { + assert.ok(list); + assert.strictEqual(list.length, 6); + list.forEach(e => assert.ok(e instanceof Edge)); + // due to the way edge is constructed, edge properties will be {} regardless if it's null or [] + list.forEach(e => assert.strictEqual(Object.keys(e.properties).length, 0)); + }); + }); + it('should skip vertex property properties when tokens is set', function () { + var g = traversal().withRemote(connection); + return g.with_("materializeProperties", "tokens").V().properties().toList().then(function (list) { + assert.ok(list); + assert.strictEqual(list.length, 12); + list.forEach(vp => assert.ok(vp instanceof VertexProperty)); + list.forEach(vp => assert.ok(vp.properties === undefined || vp.properties.length === 0)); + }); + }); + }); describe('lambdas', function() { it('should handle 1-arg lambdas', function() { const g = traversal().withRemote(connection); diff --git a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/graphbinary/AnySerializer-test.js b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/graphbinary/AnySerializer-test.js index d4b5debe30c..b827ebb9d52 100644 --- a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/graphbinary/AnySerializer-test.js +++ b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/graphbinary/AnySerializer-test.js @@ -193,12 +193,12 @@ describe('GraphBinary.AnySerializer', () => { }, // VertexSerializer - { v: new Vertex('A', 'Person', null), + { v: new Vertex('A', 'Person', []), b: [ DataType.VERTEX,0x00, DataType.STRING,0x00, 0x00,0x00,0x00,0x01, 0x41, // {id} 0x00,0x00,0x00,0x06, ...from('Person'), // {label} - 0xFE,0x01, // {properties} + 0x09,0x00,0x00,0x00,0x00,0x00, // {properties} ] }, @@ -457,13 +457,13 @@ describe('GraphBinary.AnySerializer', () => { // VERTEX { v:null, b:[0x11,0x01] }, - { v:new Vertex('00010203-0405-0607-0809-0a0b0c0d0e0f', 'A', null), + { v:new Vertex('00010203-0405-0607-0809-0a0b0c0d0e0f', 'A', []), b:[0x11,0x00, 0x0C,0x00,0x00,0x01,0x02,0x03,0x04,0x05,0x06,0x07,0x08,0x09,0x0A,0x0B,0x0C,0x0D,0x0E,0x0F, 0x00,0x00,0x00,0x01,0x41, 0xFE,0x01] }, // VERTEXPROPERTY { v:null, b:[0x12,0x01] }, - { v:new VertexProperty('00010203-0405-0607-0809-0a0b0c0d0e0f', 'Label', 42, null), + { v:new VertexProperty('00010203-0405-0607-0809-0a0b0c0d0e0f', 'Label', 42, []), b:[ 0x12,0x00, 0x0C,0x00, 0x00,0x01,0x02,0x03,0x04,0x05,0x06,0x07,0x08,0x09,0x0A,0x0B,0x0C,0x0D,0x0E,0x0F, diff --git a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/graphbinary/VertexPropertySerializer-test.js b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/graphbinary/VertexPropertySerializer-test.js index 5b7f4a26d86..16444aef8f2 100644 --- a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/graphbinary/VertexPropertySerializer-test.js +++ b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/graphbinary/VertexPropertySerializer-test.js @@ -37,17 +37,17 @@ describe('GraphBinary.VertexPropertySerializer', () => { const cases = [ { v:undefined, fq:1, b:[0x12,0x01], av:null }, - { v:undefined, fq:0, b:[0xFE,0x01, 0x00,0x00,0x00,0x00, 0xFE,0x01, 0xFE,0x01, 0xFE,0x01], av:new g.VertexProperty(null,'',null,null) }, + { v:undefined, fq:0, b:[0xFE,0x01, 0x00,0x00,0x00,0x00, 0xFE,0x01, 0xFE,0x01, 0xFE,0x01], av:new g.VertexProperty(null,'',null,[]) }, { v:null, fq:1, b:[0x12,0x01] }, - { v:null, fq:0, b:[0xFE,0x01, 0x00,0x00,0x00,0x00, 0xFE,0x01, 0xFE,0x01, 0xFE,0x01], av:new g.VertexProperty(null,'',null,null) }, + { v:null, fq:0, b:[0xFE,0x01, 0x00,0x00,0x00,0x00, 0xFE,0x01, 0xFE,0x01, 0xFE,0x01], av:new g.VertexProperty(null,'',null,[]) }, - { v:new g.VertexProperty('Id', 'Label', 'Value', null), + { v:new g.VertexProperty('Id', 'Label', 'Value', []), b:[ 0x03,0x00, 0x00,0x00,0x00,0x02, ...from('Id'), 0x00,0x00,0x00,0x05, ...from('Label'), 0x03,0x00, 0x00,0x00,0x00,0x05, ...from('Value'), 0xFE,0x01, - 0xFE,0x01, + 0x09,0x00,0x00,0x00,0x00,0x00, ] }, diff --git a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/graphbinary/VertexSerializer-test.js b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/graphbinary/VertexSerializer-test.js index 9622593e7a3..774037bfc6e 100644 --- a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/graphbinary/VertexSerializer-test.js +++ b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/graphbinary/VertexSerializer-test.js @@ -37,9 +37,9 @@ describe('GraphBinary.VertexSerializer', () => { const cases = [ { v:undefined, fq:1, b:[0x11,0x01], av:null }, - { v:undefined, fq:0, b:[0x03,0x00,0x00,0x00,0x00,0x00, 0x00,0x00,0x00,0x00, 0xFE,0x01], av:new g.Vertex('','',null) }, + { v:undefined, fq:0, b:[0x03,0x00,0x00,0x00,0x00,0x00, 0x00,0x00,0x00,0x00, 0xFE,0x01], av:new g.Vertex('','',[]) }, { v:null, fq:1, b:[0x11,0x01] }, - { v:null, fq:0, b:[0x03,0x00,0x00,0x00,0x00,0x00, 0x00,0x00,0x00,0x00, 0xFE,0x01], av:new g.Vertex('','',null) }, + { v:null, fq:0, b:[0x03,0x00,0x00,0x00,0x00,0x00, 0x00,0x00,0x00,0x00, 0xFE,0x01], av:new g.Vertex('','',[]) }, { v:new g.Vertex(42, 'A', -1), b:[ @@ -51,7 +51,7 @@ describe('GraphBinary.VertexSerializer', () => { // real case with id of UUID type, but JS does not have UUID type, it's presented as a string instead { des:1, - v:new g.Vertex('28f38e3d-7739-4c99-8284-eb43db2a80f1', 'Person', null), + v:new g.Vertex('28f38e3d-7739-4c99-8284-eb43db2a80f1', 'Person', []), b:[ 0x0C,0x00, 0x28,0xF3,0x8E,0x3D, 0x77,0x39, 0x4C,0x99, 0x82,0x84, 0xEB,0x43,0xDB,0x2A,0x80,0xF1, // id 0x00,0x00,0x00,0x06, ...from('Person'), // label diff --git a/gremlin-python/src/main/python/gremlin_python/structure/io/graphbinaryV1.py b/gremlin-python/src/main/python/gremlin_python/structure/io/graphbinaryV1.py index cb9284f5d28..20228bcd1c8 100644 --- a/gremlin-python/src/main/python/gremlin_python/structure/io/graphbinaryV1.py +++ b/gremlin-python/src/main/python/gremlin_python/structure/io/graphbinaryV1.py @@ -603,7 +603,9 @@ def _read_edge(cls, b, r): inv = Vertex(r.read_object(b), r.to_object(b, DataType.string, False)) outv = Vertex(r.read_object(b), r.to_object(b, DataType.string, False)) b.read(2) - properties = r.read_object(b) + props = r.read_object(b) + # null properties are returned as empty lists + properties = [] if props is None else props edge = Edge(edgeid, outv, edgelbl, inv, properties) return edge @@ -682,7 +684,12 @@ def objectify(cls, buff, reader, nullable=True): @classmethod def _read_vertex(cls, b, r): - vertex = Vertex(r.read_object(b), r.to_object(b, DataType.string, False), r.read_object(b)) + vertex_id = r.read_object(b) + vertex_label = r.to_object(b, DataType.string, False) + props = r.read_object(b) + # null properties are returned as empty lists + properties = [] if props is None else props + vertex = Vertex(vertex_id, vertex_label, properties) return vertex @@ -709,7 +716,9 @@ def objectify(cls, buff, reader, nullable=True): def _read_vertexproperty(cls, b, r): vp = VertexProperty(r.read_object(b), r.to_object(b, DataType.string, False), r.read_object(b), None) b.read(2) - vp.properties = r.read_object(b) + properties = r.read_object(b) + # null properties are returned as empty lists + vp.properties = [] if properties is None else properties return vp diff --git a/gremlin-python/src/main/python/tests/driver/test_client.py b/gremlin-python/src/main/python/tests/driver/test_client.py index 3d84994ca2a..fdd2100ef63 100644 --- a/gremlin-python/src/main/python/tests/driver/test_client.py +++ b/gremlin-python/src/main/python/tests/driver/test_client.py @@ -176,6 +176,19 @@ def test_client_gremlin(client): vertex = result[0] assert 1 == vertex.id assert 0 == len(vertex.properties) + ## + result_set = client.submit('g.with("materializeProperties", "tokens").E(7)') + result = result_set.all().result() + assert 1 == len(result) + edge = result[0] + assert 7 == edge.id + assert 0 == len(edge.properties) + ## + result_set = client.submit('g.with("materializeProperties", "tokens").V(1).properties()') + result = result_set.all().result() + assert 2 == len(result) + for vp in result: + assert 0 == len(vp.properties) def test_client_bytecode(client): 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 961b1b70897..f6feff1ffca 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 @@ -109,6 +109,21 @@ def test_traversals(self, remote_connection): if not isinstance(remote_connection._client._message_serializer, GraphSONSerializersV2d0): results = g.V().has('person', 'name', 'marko').both('knows').groupCount().by(__.values('name').fold()).next() assert {tuple(['vadas']): 1, tuple(['josh']): 1} == results + # # + # test materializeProperties in V - GraphSON will deserialize into None and GraphBinary to [] + results = g.with_("materializeProperties", "tokens").V().to_list() + for v in results: + assert v.properties is None or len(v.properties) == 0 + # # + # test materializeProperties in E - GraphSON will deserialize into None and GraphBinary to [] + results = g.with_("materializeProperties", "tokens").E().to_list() + for e in results: + assert e.properties is None or len(e.properties) == 0 + # # + # test materializeProperties in VP - GraphSON will deserialize into None and GraphBinary to [] + results = g.with_("materializeProperties", "tokens").V().properties().to_list() + for vp in results: + assert vp.properties is None or len(vp.properties) == 0 def test_lambda_traversals(self, remote_connection): statics.load_statics(globals()) diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java index 01121310e98..0458c86cc81 100644 --- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java +++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java @@ -32,6 +32,7 @@ import org.apache.tinkerpop.gremlin.server.channel.WebSocketChannelizer; import org.apache.tinkerpop.gremlin.server.channel.WebSocketTestChannelizer; import org.apache.tinkerpop.gremlin.server.channel.WsAndHttpTestChannelizer; +import org.apache.tinkerpop.gremlin.structure.VertexProperty; import org.apache.tinkerpop.gremlin.util.ExceptionHelper; import org.apache.tinkerpop.gremlin.TestHelper; import org.apache.tinkerpop.gremlin.driver.Client; @@ -63,6 +64,7 @@ import org.apache.tinkerpop.gremlin.structure.Graph; import org.apache.tinkerpop.gremlin.structure.T; import org.apache.tinkerpop.gremlin.structure.Vertex; +import org.apache.tinkerpop.gremlin.structure.Edge; import org.apache.tinkerpop.gremlin.util.function.Lambda; import org.hamcrest.CoreMatchers; import org.hamcrest.Matchers; @@ -107,6 +109,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.fail; import static org.junit.Assume.assumeThat; @@ -1373,4 +1376,26 @@ public void shouldGenerateFailureErrorResponseStatusCode() throws Exception { assertEquals(ResponseStatusCode.SERVER_ERROR_FAIL_STEP, ((ResponseException) t).getResponseStatusCode()); } } + + @Test + public void shouldReturnEmptyPropertiesWithMaterializeProperties() { + final Cluster cluster = TestClientFactory.build().create(); + final GraphTraversalSource g = traversal().withRemote(DriverRemoteConnection.using(cluster)); + + final Vertex v1 = g.addV("person").property("name", "marko").next(); + final Vertex r1 = g.V().next(); + assertEquals(v1.properties().next(), r1.properties().next()); + final Vertex r1_tokens = g.with("materializeProperties", "tokens").V().next(); + assertFalse(r1_tokens.properties().hasNext()); + + final VertexProperty vp1 = (VertexProperty) g.with("materializeProperties", "tokens").V().properties().next(); + assertFalse(vp1.properties().hasNext()); + + final Vertex v2 = g.addV("person").property("name", "stephen").next(); + g.V(v1).addE("knows").to(v2).property("weight", 0.75).iterate(); + final Edge r2 = g.E().next(); + assertEquals(r2.properties().next(), r2.properties().next()); + final Edge r2_tokens = g.with("materializeProperties", "tokens").E().next(); + assertFalse(r2_tokens.properties().hasNext()); + } }