You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by "GumpacG (via GitHub)" <gi...@apache.org> on 2026/05/15 20:08:32 UTC
[PR] Subgraph support for GLVs [tinkerpop]
GumpacG opened a new pull request, #3428:
URL: https://github.com/apache/tinkerpop/pull/3428
### Description
Brings gremlin-javascript, gremlin-dotnet, and gremlin-go to feature parity with gremlin-python for the GraphBinary 4.0 Graph type (0x10), so that g.X().subgraph('sg').cap('sg').next() from any of the four GLVs returns a usable Graph data container instead of throwing an unknown-type error.
Added unit tests and feature tests.
Assisted-by: Devin: Claude Opus 4.7
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Subgraph support for GLVs [tinkerpop]
Posted by "kenhuuu (via GitHub)" <gi...@apache.org>.
kenhuuu commented on PR #3428:
URL: https://github.com/apache/tinkerpop/pull/3428#issuecomment-4537203155
What does `subgraph("sg").cap("sg").asString` return? I noticed that you made a change to what the string output of the Graph is and I'm wondering if it impacts this step. Are there tests for this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Subgraph support for GLVs [tinkerpop]
Posted by "spmallette (via GitHub)" <gi...@apache.org>.
spmallette commented on code in PR #3428:
URL: https://github.com/apache/tinkerpop/pull/3428#discussion_r3274260542
##########
docs/src/upgrade/release-4.x.x.asciidoc:
##########
@@ -433,6 +426,60 @@ beyond this limit will be rejected with an error.
See: link:https://issues.apache.org/jira/browse/TINKERPOP-3247[TINKERPOP-3247]
+==== Subgraph Support in GLVs
Review Comment:
I think we need to mention python here too? it's essentially all the GLVs being announced for the first time. when i did python i don't think i added any docs because i knew all these other ones had to land too.
i also think this doesn't read like Upgrade Documentation. It should be doing more to explain the relevance of this feature and less about technical details and minutiae.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Subgraph support for GLVs [tinkerpop]
Posted by "GumpacG (via GitHub)" <gi...@apache.org>.
GumpacG commented on code in PR #3428:
URL: https://github.com/apache/tinkerpop/pull/3428#discussion_r3285004364
##########
docs/src/upgrade/release-4.x.x.asciidoc:
##########
@@ -433,6 +426,60 @@ beyond this limit will be rejected with an error.
See: link:https://issues.apache.org/jira/browse/TINKERPOP-3247[TINKERPOP-3247]
+==== Subgraph Support in GLVs
Review Comment:
I've changed it to mention that all GLVs support subgraphs and took out the technical details. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Subgraph support for GLVs [tinkerpop]
Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on code in PR #3428:
URL: https://github.com/apache/tinkerpop/pull/3428#discussion_r3277647629
##########
gremlin-go/driver/graphBinaryDeserializer.go:
##########
@@ -416,6 +418,194 @@ func (d *GraphBinaryDeserializer) readEdge() (*Edge, error) {
return e, nil
}
+// readGraph reads a GraphBinary 4.0 Graph value (after the 0x10 type code + 0x00
+// value flag have already been consumed). Mirrors org.apache.tinkerpop.gremlin.
+// structure.io.binary.types.GraphSerializer (Java).
Review Comment:
Similar to above, this comment probably doesn't need to exist. I think the name is self-descriptive enough.
##########
gremlin-go/driver/graph.go:
##########
@@ -26,7 +26,25 @@ import (
)
// Graph is used to store the graph.
+// In-memory collections of vertices and edges populated by GraphBinary 4.0
+// Graph (0x10) deserialization so that subgraph() results can be returned as
+// a usable data container. Maps are keyed by element id.
Review Comment:
Nit: Might be a bit too much context here, we probably don't want to remember to update for GraphBinary 5 and such.
```suggestion
// In-memory collections of vertices and edges, typically produced by a subgraph() traversal. Maps are keyed by element id.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Subgraph support for GLVs [tinkerpop]
Posted by "GumpacG (via GitHub)" <gi...@apache.org>.
GumpacG commented on PR #3428:
URL: https://github.com/apache/tinkerpop/pull/3428#issuecomment-4558554387
> What does `subgraph("sg").cap("sg").asString()` return? I noticed that you made a change to what the string output of the Graph is and I'm wondering if it impacts this step. Are there tests for this?
`asString()` is untouched and should remain the same. I added a feature test for this. I just corrected `toString()` for GLVs as it used to return `"graph[]"`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Subgraph support for GLVs [tinkerpop]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #3428:
URL: https://github.com/apache/tinkerpop/pull/3428#issuecomment-4463918059
## [Codecov](https://app.codecov.io/gh/apache/tinkerpop/pull/3428?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 77.38%. Comparing base ([`cfd6889`](https://app.codecov.io/gh/apache/tinkerpop/commit/cfd68897ea111e3ae7100dbc2f0f7a364fd0ffa5?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`6986189`](https://app.codecov.io/gh/apache/tinkerpop/commit/6986189a802eda655b392d25ce76f21d2d587100?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1052 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #3428 +/- ##
============================================
- Coverage 77.87% 77.38% -0.50%
============================================
Files 1015 27 -988
Lines 59308 5964 -53344
Branches 6835 0 -6835
============================================
- Hits 46184 4615 -41569
+ Misses 10817 1073 -9744
+ Partials 2307 276 -2031
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/tinkerpop/pull/3428?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Subgraph support for GLVs [tinkerpop]
Posted by "kenhuuu (via GitHub)" <gi...@apache.org>.
kenhuuu commented on PR #3428:
URL: https://github.com/apache/tinkerpop/pull/3428#issuecomment-4559919348
> > What does `subgraph("sg").cap("sg").asString()` return? I noticed that you made a change to what the string output of the Graph is and I'm wondering if it impacts this step. Are there tests for this?
>
> `asString()` is untouched and should remain the same. I added a feature test for this. I just corrected `toString()` for GLVs as it used to return `"graph[]"`.
Yea, there's still something a bit odd about this to me but I guess its comes down again to the fact that Java has more capabilities than the other GLVs. The deserializer in Java actually turns the Graph into a TinkerGraph so the toString() and asString() match, but you don't have this in the other GLVs. In any case, it's fairly small and I don't think anyone will probably notice.
VOTE +1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Subgraph support for GLVs [tinkerpop]
Posted by "kenhuuu (via GitHub)" <gi...@apache.org>.
kenhuuu commented on PR #3428:
URL: https://github.com/apache/tinkerpop/pull/3428#issuecomment-4606464579
Merging as lazy consensus has been reached.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Subgraph support for GLVs [tinkerpop]
Posted by "xiazcy (via GitHub)" <gi...@apache.org>.
xiazcy commented on code in PR #3428:
URL: https://github.com/apache/tinkerpop/pull/3428#discussion_r3270197477
##########
gremlin-go/driver/graphBinaryGraph_test.go:
##########
@@ -0,0 +1,239 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+*/
+
+package gremlingo
+
+import (
+ "bytes"
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+ "golang.org/x/text/language"
+)
+
+// TestGraphSerializerRoundTrip verifies GraphBinary 4.0 Graph (0x10)
+// serialization/deserialization round-trip preserves vertices, edges,
+// vertex-properties and properties.
+func TestGraphSerializerRoundTrip(t *testing.T) {
Review Comment:
Let's not create a new test file for just one graph type. I'd add `TestGraphSerializerRoundTrip` into the file `graphBinarySerializer_test.go`, which already contains round trip serialization tests, and `TestGraphString` into `graph_test.go`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Subgraph support for GLVs [tinkerpop]
Posted by "kenhuuu (via GitHub)" <gi...@apache.org>.
kenhuuu merged PR #3428:
URL: https://github.com/apache/tinkerpop/pull/3428
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Subgraph support for GLVs [tinkerpop]
Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on PR #3428:
URL: https://github.com/apache/tinkerpop/pull/3428#issuecomment-4548118569
VOTE +1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Subgraph support for GLVs [tinkerpop]
Posted by "xiazcy (via GitHub)" <gi...@apache.org>.
xiazcy commented on code in PR #3428:
URL: https://github.com/apache/tinkerpop/pull/3428#discussion_r3270199344
##########
gremlin-go/driver/graphBinaryGraph_test.go:
##########
@@ -0,0 +1,239 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+*/
+
+package gremlingo
+
+import (
+ "bytes"
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+ "golang.org/x/text/language"
+)
+
+// TestGraphSerializerRoundTrip verifies GraphBinary 4.0 Graph (0x10)
+// serialization/deserialization round-trip preserves vertices, edges,
+// vertex-properties and properties.
+func TestGraphSerializerRoundTrip(t *testing.T) {
+ t.Run("preserves vertices, edges, vertex-properties and properties", func(t *testing.T) {
+ graph := NewGraph()
+
+ v1 := &Vertex{Element: Element{Id: int32(1), Label: "person"}}
+ v2 := &Vertex{Element: Element{Id: int32(2), Label: "person"}}
+
+ // VertexProperty on v1 with a meta-property
+ vp1 := &VertexProperty{
+ Element: Element{Id: int32(4), Label: "name"},
+ Key: "name",
+ Value: "marko",
+ Vertex: *v1,
+ }
+ vp1.Properties = []interface{}{&Property{Key: "acl", Value: "public"}}
+ v1.Properties = []interface{}{vp1}
+ v2.Properties = []interface{}{}
+
+ graph.Vertices[v1.Id] = v1
+ graph.Vertices[v2.Id] = v2
+
+ // Edge v1 -knows-> v2 with weight property
+ e1 := &Edge{
+ Element: Element{Id: int32(3), Label: "knows"},
+ InV: *v2,
+ OutV: *v1,
+ }
+ e1.Properties = []interface{}{&Property{Key: "weight", Value: 0.5}}
+ graph.Edges[e1.Id] = e1
+
+ var buffer bytes.Buffer
+ serializer := graphBinaryTypeSerializer{newLogHandler(&defaultLogger{}, Error, language.English)}
+
+ // Round-trip via fully-qualified write/read.
+ assert.Nil(t, serializer.write(graph, &buffer))
+
+ d := NewGraphBinaryDeserializer(bytes.NewReader(buffer.Bytes()))
+ out, err := d.ReadFullyQualified()
+ assert.Nil(t, err)
+
+ rg, ok := out.(*Graph)
+ assert.True(t, ok, "expected *Graph result, got %T", out)
+ assert.Equal(t, 2, len(rg.Vertices))
+ assert.Equal(t, 1, len(rg.Edges))
+
+ rv1 := rg.Vertices[int32(1)]
+ assert.NotNil(t, rv1)
+ assert.Equal(t, "person", rv1.Label)
+ rv1Props, _ := rv1.Properties.([]interface{})
+ assert.Equal(t, 1, len(rv1Props))
+ rvp1, _ := rv1Props[0].(*VertexProperty)
+ assert.NotNil(t, rvp1)
+ assert.Equal(t, "name", rvp1.Label)
+ assert.Equal(t, "marko", rvp1.Value)
+
+ rvp1Meta, _ := rvp1.Properties.([]interface{})
+ assert.Equal(t, 1, len(rvp1Meta))
+ meta, _ := rvp1Meta[0].(*Property)
+ assert.NotNil(t, meta)
+ assert.Equal(t, "acl", meta.Key)
+ assert.Equal(t, "public", meta.Value)
+
+ rv2 := rg.Vertices[int32(2)]
+ assert.NotNil(t, rv2)
+ assert.Equal(t, "person", rv2.Label)
+
+ re1 := rg.Edges[int32(3)]
+ assert.NotNil(t, re1)
+ assert.Equal(t, "knows", re1.Label)
+ assert.Equal(t, int32(1), re1.OutV.Id)
+ assert.Equal(t, int32(2), re1.InV.Id)
+
+ re1Props, _ := re1.Properties.([]interface{})
+ assert.Equal(t, 1, len(re1Props))
+ w, _ := re1Props[0].(*Property)
+ assert.NotNil(t, w)
+ assert.Equal(t, "weight", w.Key)
+ assert.Equal(t, 0.5, w.Value)
+ })
+
+ t.Run("handles empty graph", func(t *testing.T) {
+ graph := NewGraph()
+
+ var buffer bytes.Buffer
+ serializer := graphBinaryTypeSerializer{newLogHandler(&defaultLogger{}, Error, language.English)}
+ assert.Nil(t, serializer.write(graph, &buffer))
+
+ d := NewGraphBinaryDeserializer(bytes.NewReader(buffer.Bytes()))
+ out, err := d.ReadFullyQualified()
+ assert.Nil(t, err)
+
+ rg, ok := out.(*Graph)
+ assert.True(t, ok, "expected *Graph result, got %T", out)
+ assert.Equal(t, 0, len(rg.Vertices))
+ assert.Equal(t, 0, len(rg.Edges))
+ })
+
+ t.Run("handles vertices without properties and edges without properties", func(t *testing.T) {
+ graph := NewGraph()
+
+ v1 := &Vertex{Element: Element{Id: int32(10), Label: "person"}}
+ v2 := &Vertex{Element: Element{Id: int32(20), Label: "software"}}
+ v1.Properties = []interface{}{}
+ v2.Properties = []interface{}{}
+ graph.Vertices[v1.Id] = v1
+ graph.Vertices[v2.Id] = v2
+
+ e := &Edge{
+ Element: Element{Id: int32(30), Label: "created"},
+ InV: *v2,
+ OutV: *v1,
+ }
+ e.Properties = []interface{}{}
+ graph.Edges[e.Id] = e
+
+ var buffer bytes.Buffer
+ serializer := graphBinaryTypeSerializer{newLogHandler(&defaultLogger{}, Error, language.English)}
+ assert.Nil(t, serializer.write(graph, &buffer))
+
+ d := NewGraphBinaryDeserializer(bytes.NewReader(buffer.Bytes()))
+ out, err := d.ReadFullyQualified()
+ assert.Nil(t, err)
+
+ rg, _ := out.(*Graph)
+ assert.Equal(t, 2, len(rg.Vertices))
+ assert.Equal(t, 1, len(rg.Edges))
+
+ re := rg.Edges[int32(30)]
+ assert.Equal(t, "created", re.Label)
+ assert.Equal(t, int32(10), re.OutV.Id)
+ assert.Equal(t, int32(20), re.InV.Id)
+ })
+
+ t.Run("handles string ids", func(t *testing.T) {
+ graph := NewGraph()
+
+ v1 := &Vertex{Element: Element{Id: "a", Label: "person"}}
+ v2 := &Vertex{Element: Element{Id: "b", Label: "person"}}
+ v1.Properties = []interface{}{}
+ v2.Properties = []interface{}{}
+ graph.Vertices[v1.Id] = v1
+ graph.Vertices[v2.Id] = v2
+
+ e := &Edge{
+ Element: Element{Id: "e1", Label: "knows"},
+ InV: *v2,
+ OutV: *v1,
+ }
+ e.Properties = []interface{}{}
+ graph.Edges[e.Id] = e
+
+ var buffer bytes.Buffer
+ serializer := graphBinaryTypeSerializer{newLogHandler(&defaultLogger{}, Error, language.English)}
+ assert.Nil(t, serializer.write(graph, &buffer))
+
+ d := NewGraphBinaryDeserializer(bytes.NewReader(buffer.Bytes()))
+ out, err := d.ReadFullyQualified()
+ assert.Nil(t, err)
+
+ rg, _ := out.(*Graph)
+ assert.Equal(t, 2, len(rg.Vertices))
+ assert.Equal(t, 1, len(rg.Edges))
+
+ re := rg.Edges["e1"]
+ assert.NotNil(t, re)
+ assert.Equal(t, "a", re.OutV.Id)
+ assert.Equal(t, "b", re.InV.Id)
+ })
+}
+
+// TestGraphString verifies the *Graph String() representation
+// renders as graph[vertices:N edges:M].
+func TestGraphString(t *testing.T) {
+ t.Run("empty graph", func(t *testing.T) {
+ g := NewGraph()
+ assert.Equal(t, "graph[vertices:0 edges:0]", g.String())
+ })
+
+ t.Run("graph with vertices and edges", func(t *testing.T) {
+ g := NewGraph()
+ v1 := &Vertex{Element: Element{Id: int32(1), Label: "person"}}
+ v2 := &Vertex{Element: Element{Id: int32(2), Label: "person"}}
+ g.Vertices[v1.Id] = v1
+ g.Vertices[v2.Id] = v2
+ g.Edges[int32(3)] = &Edge{
+ Element: Element{Id: int32(3), Label: "knows"},
+ InV: *v2, OutV: *v1,
+ }
+ assert.Equal(t, "graph[vertices:2 edges:1]", g.String())
+ })
+}
+
+// TestGraphSerializerRegistry verifies the graph dataType is wired up correctly.
+func TestGraphSerializerRegistry(t *testing.T) {
Review Comment:
Is this set of tests necessary? Wouldn't roundtrip tests already catches these?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Subgraph support for GLVs [tinkerpop]
Posted by "GumpacG (via GitHub)" <gi...@apache.org>.
GumpacG commented on code in PR #3428:
URL: https://github.com/apache/tinkerpop/pull/3428#discussion_r3284998044
##########
gremlin-go/driver/graphBinaryGraph_test.go:
##########
@@ -0,0 +1,239 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+*/
+
+package gremlingo
+
+import (
+ "bytes"
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+ "golang.org/x/text/language"
+)
+
+// TestGraphSerializerRoundTrip verifies GraphBinary 4.0 Graph (0x10)
+// serialization/deserialization round-trip preserves vertices, edges,
+// vertex-properties and properties.
+func TestGraphSerializerRoundTrip(t *testing.T) {
+ t.Run("preserves vertices, edges, vertex-properties and properties", func(t *testing.T) {
+ graph := NewGraph()
+
+ v1 := &Vertex{Element: Element{Id: int32(1), Label: "person"}}
+ v2 := &Vertex{Element: Element{Id: int32(2), Label: "person"}}
+
+ // VertexProperty on v1 with a meta-property
+ vp1 := &VertexProperty{
+ Element: Element{Id: int32(4), Label: "name"},
+ Key: "name",
+ Value: "marko",
+ Vertex: *v1,
+ }
+ vp1.Properties = []interface{}{&Property{Key: "acl", Value: "public"}}
+ v1.Properties = []interface{}{vp1}
+ v2.Properties = []interface{}{}
+
+ graph.Vertices[v1.Id] = v1
+ graph.Vertices[v2.Id] = v2
+
+ // Edge v1 -knows-> v2 with weight property
+ e1 := &Edge{
+ Element: Element{Id: int32(3), Label: "knows"},
+ InV: *v2,
+ OutV: *v1,
+ }
+ e1.Properties = []interface{}{&Property{Key: "weight", Value: 0.5}}
+ graph.Edges[e1.Id] = e1
+
+ var buffer bytes.Buffer
+ serializer := graphBinaryTypeSerializer{newLogHandler(&defaultLogger{}, Error, language.English)}
+
+ // Round-trip via fully-qualified write/read.
+ assert.Nil(t, serializer.write(graph, &buffer))
+
+ d := NewGraphBinaryDeserializer(bytes.NewReader(buffer.Bytes()))
+ out, err := d.ReadFullyQualified()
+ assert.Nil(t, err)
+
+ rg, ok := out.(*Graph)
+ assert.True(t, ok, "expected *Graph result, got %T", out)
+ assert.Equal(t, 2, len(rg.Vertices))
+ assert.Equal(t, 1, len(rg.Edges))
+
+ rv1 := rg.Vertices[int32(1)]
+ assert.NotNil(t, rv1)
+ assert.Equal(t, "person", rv1.Label)
+ rv1Props, _ := rv1.Properties.([]interface{})
+ assert.Equal(t, 1, len(rv1Props))
+ rvp1, _ := rv1Props[0].(*VertexProperty)
+ assert.NotNil(t, rvp1)
+ assert.Equal(t, "name", rvp1.Label)
+ assert.Equal(t, "marko", rvp1.Value)
+
+ rvp1Meta, _ := rvp1.Properties.([]interface{})
+ assert.Equal(t, 1, len(rvp1Meta))
+ meta, _ := rvp1Meta[0].(*Property)
+ assert.NotNil(t, meta)
+ assert.Equal(t, "acl", meta.Key)
+ assert.Equal(t, "public", meta.Value)
+
+ rv2 := rg.Vertices[int32(2)]
+ assert.NotNil(t, rv2)
+ assert.Equal(t, "person", rv2.Label)
+
+ re1 := rg.Edges[int32(3)]
+ assert.NotNil(t, re1)
+ assert.Equal(t, "knows", re1.Label)
+ assert.Equal(t, int32(1), re1.OutV.Id)
+ assert.Equal(t, int32(2), re1.InV.Id)
+
+ re1Props, _ := re1.Properties.([]interface{})
+ assert.Equal(t, 1, len(re1Props))
+ w, _ := re1Props[0].(*Property)
+ assert.NotNil(t, w)
+ assert.Equal(t, "weight", w.Key)
+ assert.Equal(t, 0.5, w.Value)
+ })
+
+ t.Run("handles empty graph", func(t *testing.T) {
+ graph := NewGraph()
+
+ var buffer bytes.Buffer
+ serializer := graphBinaryTypeSerializer{newLogHandler(&defaultLogger{}, Error, language.English)}
+ assert.Nil(t, serializer.write(graph, &buffer))
+
+ d := NewGraphBinaryDeserializer(bytes.NewReader(buffer.Bytes()))
+ out, err := d.ReadFullyQualified()
+ assert.Nil(t, err)
+
+ rg, ok := out.(*Graph)
+ assert.True(t, ok, "expected *Graph result, got %T", out)
+ assert.Equal(t, 0, len(rg.Vertices))
+ assert.Equal(t, 0, len(rg.Edges))
+ })
+
+ t.Run("handles vertices without properties and edges without properties", func(t *testing.T) {
+ graph := NewGraph()
+
+ v1 := &Vertex{Element: Element{Id: int32(10), Label: "person"}}
+ v2 := &Vertex{Element: Element{Id: int32(20), Label: "software"}}
+ v1.Properties = []interface{}{}
+ v2.Properties = []interface{}{}
+ graph.Vertices[v1.Id] = v1
+ graph.Vertices[v2.Id] = v2
+
+ e := &Edge{
+ Element: Element{Id: int32(30), Label: "created"},
+ InV: *v2,
+ OutV: *v1,
+ }
+ e.Properties = []interface{}{}
+ graph.Edges[e.Id] = e
+
+ var buffer bytes.Buffer
+ serializer := graphBinaryTypeSerializer{newLogHandler(&defaultLogger{}, Error, language.English)}
+ assert.Nil(t, serializer.write(graph, &buffer))
+
+ d := NewGraphBinaryDeserializer(bytes.NewReader(buffer.Bytes()))
+ out, err := d.ReadFullyQualified()
+ assert.Nil(t, err)
+
+ rg, _ := out.(*Graph)
+ assert.Equal(t, 2, len(rg.Vertices))
+ assert.Equal(t, 1, len(rg.Edges))
+
+ re := rg.Edges[int32(30)]
+ assert.Equal(t, "created", re.Label)
+ assert.Equal(t, int32(10), re.OutV.Id)
+ assert.Equal(t, int32(20), re.InV.Id)
+ })
+
+ t.Run("handles string ids", func(t *testing.T) {
+ graph := NewGraph()
+
+ v1 := &Vertex{Element: Element{Id: "a", Label: "person"}}
+ v2 := &Vertex{Element: Element{Id: "b", Label: "person"}}
+ v1.Properties = []interface{}{}
+ v2.Properties = []interface{}{}
+ graph.Vertices[v1.Id] = v1
+ graph.Vertices[v2.Id] = v2
+
+ e := &Edge{
+ Element: Element{Id: "e1", Label: "knows"},
+ InV: *v2,
+ OutV: *v1,
+ }
+ e.Properties = []interface{}{}
+ graph.Edges[e.Id] = e
+
+ var buffer bytes.Buffer
+ serializer := graphBinaryTypeSerializer{newLogHandler(&defaultLogger{}, Error, language.English)}
+ assert.Nil(t, serializer.write(graph, &buffer))
+
+ d := NewGraphBinaryDeserializer(bytes.NewReader(buffer.Bytes()))
+ out, err := d.ReadFullyQualified()
+ assert.Nil(t, err)
+
+ rg, _ := out.(*Graph)
+ assert.Equal(t, 2, len(rg.Vertices))
+ assert.Equal(t, 1, len(rg.Edges))
+
+ re := rg.Edges["e1"]
+ assert.NotNil(t, re)
+ assert.Equal(t, "a", re.OutV.Id)
+ assert.Equal(t, "b", re.InV.Id)
+ })
+}
+
+// TestGraphString verifies the *Graph String() representation
+// renders as graph[vertices:N edges:M].
+func TestGraphString(t *testing.T) {
+ t.Run("empty graph", func(t *testing.T) {
+ g := NewGraph()
+ assert.Equal(t, "graph[vertices:0 edges:0]", g.String())
+ })
+
+ t.Run("graph with vertices and edges", func(t *testing.T) {
+ g := NewGraph()
+ v1 := &Vertex{Element: Element{Id: int32(1), Label: "person"}}
+ v2 := &Vertex{Element: Element{Id: int32(2), Label: "person"}}
+ g.Vertices[v1.Id] = v1
+ g.Vertices[v2.Id] = v2
+ g.Edges[int32(3)] = &Edge{
+ Element: Element{Id: int32(3), Label: "knows"},
+ InV: *v2, OutV: *v1,
+ }
+ assert.Equal(t, "graph[vertices:2 edges:1]", g.String())
+ })
+}
+
+// TestGraphSerializerRegistry verifies the graph dataType is wired up correctly.
+func TestGraphSerializerRegistry(t *testing.T) {
Review Comment:
Not necessarily, I added these tests to graphBinarySerializer_test.go as that's where similar tests are as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Subgraph support for GLVs [tinkerpop]
Posted by "GumpacG (via GitHub)" <gi...@apache.org>.
GumpacG commented on code in PR #3428:
URL: https://github.com/apache/tinkerpop/pull/3428#discussion_r3285005449
##########
gremlin-go/driver/graphBinaryDeserializer.go:
##########
@@ -416,6 +418,194 @@ func (d *GraphBinaryDeserializer) readEdge() (*Edge, error) {
return e, nil
}
+// readGraph reads a GraphBinary 4.0 Graph value (after the 0x10 type code + 0x00
+// value flag have already been consumed). Mirrors org.apache.tinkerpop.gremlin.
+// structure.io.binary.types.GraphSerializer (Java).
Review Comment:
Removed. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org