From ffcf128a353dfd8759c1b5bdee85ca471b332ee7 Mon Sep 17 00:00:00 2001 From: Ken Hu <106191785+kenhuuu@users.noreply.github.com> Date: Tue, 26 May 2026 17:35:09 -0700 Subject: [PATCH 1/2] Update gremlin-go submit to block until response headers arrive Split executeAndStream() into sendRequest() (synchronous HTTP call) and streamResponse() (async body streaming). submit() now blocks until the server acknowledges the request (response headers received), then streams the body in the background. Non-GraphBinary HTTP errors (400/500 with text/JSON bodies) are now returned directly from submit() instead of being embedded in the ResultSet. Tests updated accordingly. Assisted-by: Kiro:claude-opus-4-6 --- CHANGELOG.asciidoc | 1 + gremlin-go/driver/connection.go | 113 ++++++++++++++------------ gremlin-go/driver/connection_test.go | 55 ++++++------- gremlin-go/driver/interceptor_test.go | 32 +++----- 4 files changed, 97 insertions(+), 104 deletions(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index e312807286e..e7f8ff90c90 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -28,6 +28,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima * Added typed numeric wrappers and `preciseNumbers` connection option to `gremlin-javascript` for explicit control over numeric type serialization and deserialization. * Added `NextN(n)` to `Traversal` in `gremlin-go` for batched result iteration, providing API parity with `next(n)` in the Java, Python, and .NET GLVs, and updated the Go translators in `gremlin-core` and `gremlin-javascript` to emit `NextN(n)` for the batched form. * Added Gremlator, a single page web application, that translates Gremlin into various programming languages like Javascript and Python. +* Refactored Go driver connection to block until response headers arrive, enabling synchronous error returns and proper transaction ordering. * Removed `uuid` dependency from `gremlin-javascript` in favor of the built-in `globalThis.crypto.randomUUID()`. * Added streaming HTTP response support to `gremlin-driver` for incremental result deserialization over GraphBinary. * Connected HTTP streaming response deserialization to the traversal API in `gremlin-javascript`, enabling `next()` to return the first result without waiting for the full response. diff --git a/gremlin-go/driver/connection.go b/gremlin-go/driver/connection.go index 882161e36c7..8354ec0574a 100644 --- a/gremlin-go/driver/connection.go +++ b/gremlin-go/driver/connection.go @@ -119,28 +119,71 @@ func (c *connection) AddInterceptor(interceptor RequestInterceptor) { c.interceptors = append(c.interceptors, interceptor) } -// submit sends request and streams results directly to ResultSet +// submit sends request and streams results directly to ResultSet. +// Blocks until response headers arrive, ensuring the server has acknowledged +// receipt of the request before returning. func (c *connection) submit(req *RequestMessage) (ResultSet, error) { rs := newChannelResultSet() + // Send the HTTP request synchronously — blocks until response headers arrive + resp, err := c.sendRequest(req) + if err != nil { + rs.Close() + return nil, err + } + + // If the HTTP status indicates an error and the response is not GraphBinary, + // read the body as a text/JSON error message instead of attempting binary + // deserialization which would produce cryptic errors. + contentType := resp.Header.Get(HeaderContentType) + if resp.StatusCode >= 400 && !strings.Contains(contentType, graphBinaryMimeType) { + defer func() { + io.Copy(io.Discard, resp.Body) + resp.Body.Close() + }() + bodyBytes, readErr := io.ReadAll(resp.Body) + if readErr != nil { + rs.Close() + return nil, fmt.Errorf("Gremlin Server returned HTTP %d and failed to read body: %w", + resp.StatusCode, readErr) + } + errorBody := string(bodyBytes) + errorMsg := tryExtractJSONError(errorBody) + if errorMsg == "" { + errorMsg = fmt.Sprintf("Gremlin Server returned HTTP %d: %s", resp.StatusCode, errorBody) + } + c.logHandler.logf(Error, failedToReceiveResponse, errorMsg) + rs.Close() + return nil, fmt.Errorf("%s", errorMsg) + } + + // Stream the response body into the ResultSet asynchronously c.wg.Add(1) go func() { defer c.wg.Done() - c.executeAndStream(req, rs) + // Drain any unread bytes so the connection can be reused gracefully. + // Without this, Go's HTTP client sends a TCP RST instead of FIN, + // causing "Connection reset by peer" errors on the server. + defer func() { + io.Copy(io.Discard, resp.Body) + if err := resp.Body.Close(); err != nil { + c.logHandler.logf(Debug, failedToCloseResponseBody, err.Error()) + } + }() + defer rs.Close() + c.streamResponse(resp, rs) }() return rs, nil } -func (c *connection) executeAndStream(req *RequestMessage, rs ResultSet) { - defer rs.Close() - +// sendRequest builds and sends the HTTP request, blocking until response headers arrive. +func (c *connection) sendRequest(req *RequestMessage) (*http.Response, error) { // Create HttpRequest for interceptors httpReq, err := NewHttpRequest(http.MethodPost, c.url) if err != nil { c.logHandler.logf(Error, failedToSendRequest, err.Error()) - rs.setError(err) - return + return nil, err } // Set default headers before interceptors @@ -154,8 +197,7 @@ func (c *connection) executeAndStream(req *RequestMessage, rs ResultSet) { for _, interceptor := range c.interceptors { if err := interceptor(httpReq); err != nil { c.logHandler.logf(Error, failedToSendRequest, err.Error()) - rs.setError(err) - return + return nil, err } } @@ -165,15 +207,13 @@ func (c *connection) executeAndStream(req *RequestMessage, rs ResultSet) { data, err := c.serializer.SerializeMessage(r) if err != nil { c.logHandler.logf(Error, failedToSendRequest, err.Error()) - rs.setError(err) - return + return nil, err } httpReq.Body = data } else { errMsg := "request body was not serialized; either provide a serializer or add an interceptor that serializes the request" c.logHandler.logf(Error, failedToSendRequest, errMsg) - rs.setError(fmt.Errorf("%s", errMsg)) - return + return nil, fmt.Errorf("%s", errMsg) } } @@ -184,16 +224,14 @@ func (c *connection) executeAndStream(req *RequestMessage, rs ResultSet) { httpGoReq, err = http.NewRequest(httpReq.Method, httpReq.URL.String(), bytes.NewReader(body)) if err != nil { c.logHandler.logf(Error, failedToSendRequest, err.Error()) - rs.setError(err) - return + return nil, err } httpGoReq.Header = httpReq.Headers case io.Reader: httpGoReq, err = http.NewRequest(httpReq.Method, httpReq.URL.String(), body) if err != nil { c.logHandler.logf(Error, failedToSendRequest, err.Error()) - rs.setError(err) - return + return nil, err } httpGoReq.Header = httpReq.Headers case *http.Request: @@ -201,48 +239,21 @@ func (c *connection) executeAndStream(req *RequestMessage, rs ResultSet) { default: errMsg := fmt.Sprintf("unsupported body type after interceptors: %T", body) c.logHandler.logf(Error, failedToSendRequest, errMsg) - rs.setError(fmt.Errorf("%s", errMsg)) - return + return nil, fmt.Errorf("%s", errMsg) } + // This blocks until response headers arrive resp, err := c.httpClient.Do(httpGoReq) if err != nil { c.logHandler.logf(Error, failedToSendRequest, err.Error()) - rs.setError(err) - return + return nil, err } - defer func() { - // Drain any unread bytes so the connection can be reused gracefully. - // Without this, Go's HTTP client sends a TCP RST instead of FIN, - // causing "Connection reset by peer" errors on the server. - io.Copy(io.Discard, resp.Body) - if err := resp.Body.Close(); err != nil { - c.logHandler.logf(Debug, failedToCloseResponseBody, err.Error()) - } - }() - // If the HTTP status indicates an error and the response is not GraphBinary, - // read the body as a text/JSON error message instead of attempting binary - // deserialization which would produce cryptic errors. - contentType := resp.Header.Get(HeaderContentType) - if resp.StatusCode >= 400 && !strings.Contains(contentType, graphBinaryMimeType) { - bodyBytes, readErr := io.ReadAll(resp.Body) - if readErr != nil { - c.logHandler.logf(Error, failedToReceiveResponse, readErr.Error()) - rs.setError(fmt.Errorf("Gremlin Server returned HTTP %d and failed to read body: %w", - resp.StatusCode, readErr)) - return - } - errorBody := string(bodyBytes) - errorMsg := tryExtractJSONError(errorBody) - if errorMsg == "" { - errorMsg = fmt.Sprintf("Gremlin Server returned HTTP %d: %s", resp.StatusCode, errorBody) - } - c.logHandler.logf(Error, failedToReceiveResponse, errorMsg) - rs.setError(fmt.Errorf("%s", errorMsg)) - return - } + return resp, nil +} +// streamResponse reads the response body and pushes results into the ResultSet. +func (c *connection) streamResponse(resp *http.Response, rs ResultSet) { reader, zlibReader, err := c.getReader(resp) if err != nil { c.logHandler.logf(Error, failedToReceiveResponse, err.Error()) diff --git a/gremlin-go/driver/connection_test.go b/gremlin-go/driver/connection_test.go index aebdeb992f9..1b4620a3760 100644 --- a/gremlin-go/driver/connection_test.go +++ b/gremlin-go/driver/connection_test.go @@ -990,12 +990,8 @@ func TestConnectionWithMockServer(t *testing.T) { connectionTimeout: 100 * time.Millisecond, }) - rs, err := conn.submit(&RequestMessage{Gremlin: "g.V()", Fields: map[string]interface{}{}}) - assert.NoError(t, err) // submit returns nil, error goes to ResultSet - - // All() blocks until stream closes, then we can check error - _, _ = rs.All() - assert.Error(t, rs.GetError()) + _, err := conn.submit(&RequestMessage{Gremlin: "g.V()", Fields: map[string]interface{}{}}) + assert.Error(t, err) // connection errors are now returned directly }) t.Run("receives headers from request", func(t *testing.T) { @@ -1035,14 +1031,10 @@ func TestConnectionWithMockServer(t *testing.T) { defer server.Close() conn := newConnection(newTestLogHandler(), server.URL, &connectionSettings{}) - rs, err := conn.submit(&RequestMessage{Gremlin: "g.V()", Fields: map[string]interface{}{}}) - require.NoError(t, err) - - _, _ = rs.All() - rsErr := rs.GetError() - require.Error(t, rsErr) - assert.Contains(t, rsErr.Error(), "HTTP 500") - assert.Contains(t, rsErr.Error(), "Internal Server Error") + _, err := conn.submit(&RequestMessage{Gremlin: "g.V()", Fields: map[string]interface{}{}}) + require.Error(t, err) + assert.Contains(t, err.Error(), "HTTP 500") + assert.Contains(t, err.Error(), "Internal Server Error") }) t.Run("extracts message from JSON error response", func(t *testing.T) { @@ -1054,13 +1046,9 @@ func TestConnectionWithMockServer(t *testing.T) { defer server.Close() conn := newConnection(newTestLogHandler(), server.URL, &connectionSettings{}) - rs, err := conn.submit(&RequestMessage{Gremlin: "g.V()", Fields: map[string]interface{}{}}) - require.NoError(t, err) - - _, _ = rs.All() - rsErr := rs.GetError() - require.Error(t, rsErr) - assert.Equal(t, "Authentication required", rsErr.Error()) + _, err := conn.submit(&RequestMessage{Gremlin: "g.V()", Fields: map[string]interface{}{}}) + require.Error(t, err) + assert.Equal(t, "Authentication required", err.Error()) }) t.Run("falls back to raw body for non-JSON error response", func(t *testing.T) { @@ -1072,14 +1060,10 @@ func TestConnectionWithMockServer(t *testing.T) { defer server.Close() conn := newConnection(newTestLogHandler(), server.URL, &connectionSettings{}) - rs, err := conn.submit(&RequestMessage{Gremlin: "g.V()", Fields: map[string]interface{}{}}) - require.NoError(t, err) - - _, _ = rs.All() - rsErr := rs.GetError() - require.Error(t, rsErr) - assert.Contains(t, rsErr.Error(), "HTTP 502") - assert.Contains(t, rsErr.Error(), "Bad Gateway") + _, err := conn.submit(&RequestMessage{Gremlin: "g.V()", Fields: map[string]interface{}{}}) + require.Error(t, err) + assert.Contains(t, err.Error(), "HTTP 502") + assert.Contains(t, err.Error(), "Bad Gateway") }) t.Run("falls through to GraphBinary deserialization for GraphBinary error responses", func(t *testing.T) { @@ -1127,9 +1111,18 @@ func TestConnectionWithMockServer(t *testing.T) { }) t.Run("close waits for in-flight requests", func(t *testing.T) { + // Server responds with headers immediately but streams body slowly. + // This tests that close() waits for the body-streaming goroutine. server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - time.Sleep(200 * time.Millisecond) + w.Header().Set("Content-Type", graphBinaryMimeType) w.WriteHeader(http.StatusOK) + if f, ok := w.(http.Flusher); ok { + f.Flush() + } + // Delay before writing body so the streaming goroutine is still active + time.Sleep(200 * time.Millisecond) + // Write a minimal valid GraphBinary response (version + no-bulking flag) + w.Write([]byte{0x84, 0x00}) })) defer server.Close() @@ -1142,7 +1135,7 @@ func TestConnectionWithMockServer(t *testing.T) { conn.close() elapsed := time.Since(start) - // close() should have waited for the in-flight goroutine + // close() should have waited for the body-streaming goroutine to finish assert.GreaterOrEqual(t, elapsed.Milliseconds(), int64(150), "close() should wait for in-flight requests to complete") diff --git a/gremlin-go/driver/interceptor_test.go b/gremlin-go/driver/interceptor_test.go index 40a947ac9d0..adb51786fc9 100644 --- a/gremlin-go/driver/interceptor_test.go +++ b/gremlin-go/driver/interceptor_test.go @@ -246,13 +246,9 @@ func TestInterceptor_NilSerializerNoSerialization(t *testing.T) { conn := newConnection(newTestLogHandler(), server.URL, &connectionSettings{}) conn.serializer = nil // explicitly nil serializer - rs, err := conn.submit(&RequestMessage{Gremlin: "g.V()", Fields: map[string]interface{}{}}) - require.NoError(t, err) - - _, _ = rs.All() // drain — this triggers the async executeAndStream - rsErr := rs.GetError() - require.Error(t, rsErr, "should get an error when serializer is nil and no interceptor serializes") - assert.Contains(t, rsErr.Error(), "request body was not serialized", + _, err := conn.submit(&RequestMessage{Gremlin: "g.V()", Fields: map[string]interface{}{}}) + require.Error(t, err, "should get an error when serializer is nil and no interceptor serializes") + assert.Contains(t, err.Error(), "request body was not serialized", "error message should indicate the body was not serialized") } @@ -327,14 +323,10 @@ func TestInterceptor_ErrorPropagation(t *testing.T) { return fmt.Errorf("interceptor failed") }) - rs, err := conn.submit(&RequestMessage{Gremlin: "g.V()", Fields: map[string]interface{}{}}) - require.NoError(t, err) - - _, _ = rs.All() // drain — triggers async executeAndStream - rsErr := rs.GetError() - require.Error(t, rsErr, "interceptor error should propagate to ResultSet") - assert.Contains(t, rsErr.Error(), "interceptor failed", - "ResultSet error should contain the interceptor's error message") + _, err := conn.submit(&RequestMessage{Gremlin: "g.V()", Fields: map[string]interface{}{}}) + require.Error(t, err, "interceptor error should propagate") + assert.Contains(t, err.Error(), "interceptor failed", + "error should contain the interceptor's error message") } // TestInterceptor_UnsupportedBodyType verifies that setting Body to an unsupported type @@ -353,13 +345,9 @@ func TestInterceptor_UnsupportedBodyType(t *testing.T) { return nil }) - rs, err := conn.submit(&RequestMessage{Gremlin: "g.V()", Fields: map[string]interface{}{}}) - require.NoError(t, err) - - _, _ = rs.All() // drain - rsErr := rs.GetError() - require.Error(t, rsErr, "unsupported body type should produce an error") - assert.Contains(t, rsErr.Error(), "unsupported body type", + _, err := conn.submit(&RequestMessage{Gremlin: "g.V()", Fields: map[string]interface{}{}}) + require.Error(t, err, "unsupported body type should produce an error") + assert.Contains(t, err.Error(), "unsupported body type", "error message should indicate unsupported body type") } From 7c2cd79a9f4f184ea15ed8b0a5a62d7acef82348 Mon Sep 17 00:00:00 2001 From: Ken Hu <106191785+kenhuuu@users.noreply.github.com> Date: Wed, 27 May 2026 13:15:04 -0700 Subject: [PATCH 2/2] Add explicit transaction support to non-Java GLVs Implement Transaction classes in gremlin-python, gremlin-go, gremlin-javascript, and gremlin-dotnet that mirror the Java HttpRemoteTransaction. Transaction wraps a Client and provides begin/commit/rollback/close lifecycle plus a submit() method for plain gremlin-lang strings. Key design decisions: - Default close behavior changed from commit to rollback (all GLVs) - Transactions are single-use (terminal state after commit/rollback) - gtx.tx() returns the same Transaction (enables gtx.tx().commit()) Assisted-by: Kiro:claude-opus-4-6 --- CHANGELOG.asciidoc | 2 + docs/src/reference/gremlin-variants.asciidoc | 156 +++++- docs/src/reference/intro.asciidoc | 3 +- docs/src/reference/the-traversal.asciidoc | 18 +- docs/src/upgrade/release-4.x.x.asciidoc | 28 + .../src/Gremlin.Net/Driver/Connection.cs | 9 + .../src/Gremlin.Net/Driver/GremlinClient.cs | 12 + .../Driver/Remote/DriverRemoteConnection.cs | 10 +- .../Remote/TransactionRemoteConnection.cs | 110 ++++ .../Gremlin.Net/Driver/RemoteTransaction.cs | 270 +++++++++ .../src/Gremlin.Net/Driver/Tokens.cs | 5 + .../Process/Remote/IRemoteConnection.cs | 6 +- .../Process/Remote/RemoteTransaction.cs | 87 --- .../Process/Traversal/GraphTraversalSource.cs | 15 +- .../Docs/Reference/GremlinVariantsTests.cs | 51 +- .../Driver/TransactionTests.cs | 468 ++++++++++++++++ .../GraphTraversalTransactionTests.cs | 96 ---- .../Process/Remote/RemoteTransactionTests.cs | 44 -- .../driver/remote/HttpRemoteTransaction.java | 5 +- gremlin-go/driver/client.go | 26 +- gremlin-go/driver/connection.go | 17 + gremlin-go/driver/connection_test.go | 6 +- gremlin-go/driver/error_codes.go | 4 +- gremlin-go/driver/graphTraversal.go | 75 +-- gremlin-go/driver/graphTraversalSource.go | 27 +- gremlin-go/driver/request.go | 4 + gremlin-go/driver/requestOptions.go | 8 + .../driver/resources/error-messages/en.json | 2 + gremlin-go/driver/transaction.go | 195 +++++++ .../driver/transactionRemoteConnection.go | 39 ++ gremlin-go/driver/transaction_test.go | 529 ++++++++++++++++++ gremlin-go/driver/traversal.go | 2 +- gremlin-go/driver/traversal_test.go | 347 ------------ .../gremlin-javascript/lib/driver/client.ts | 20 +- .../lib/driver/connection.ts | 10 + .../lib/driver/driver-remote-connection.ts | 29 +- .../lib/driver/remote-connection.ts | 14 +- .../lib/process/graph-traversal.ts | 21 +- .../lib/process/transaction.ts | 146 ++++- .../test/integration/transaction-tests.js | 374 +++++++++++++ .../test/integration/traversal-test.js | 110 +--- .../test/unit/traversal-test.js | 46 -- .../python/gremlin_python/driver/client.py | 11 +- .../gremlin_python/driver/connection.py | 5 + .../driver/driver_remote_connection.py | 9 - .../python/gremlin_python/driver/request.py | 3 +- .../gremlin_python/driver/transaction.py | 170 ++++++ .../gremlin_python/process/graph_traversal.py | 111 +--- .../gremlin_python/process/traversal.py | 16 - .../integration/driver/test_transaction.py | 345 ++++++++++++ ...GremlinDriverTransactionIntegrateTest.java | 14 +- 51 files changed, 3059 insertions(+), 1071 deletions(-) create mode 100644 gremlin-dotnet/src/Gremlin.Net/Driver/Remote/TransactionRemoteConnection.cs create mode 100644 gremlin-dotnet/src/Gremlin.Net/Driver/RemoteTransaction.cs delete mode 100644 gremlin-dotnet/src/Gremlin.Net/Process/Remote/RemoteTransaction.cs create mode 100644 gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Driver/TransactionTests.cs delete mode 100644 gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Process/Traversal/DriverRemoteConnection/GraphTraversalTransactionTests.cs delete mode 100644 gremlin-dotnet/test/Gremlin.Net.UnitTest/Process/Remote/RemoteTransactionTests.cs create mode 100644 gremlin-go/driver/transaction.go create mode 100644 gremlin-go/driver/transactionRemoteConnection.go create mode 100644 gremlin-go/driver/transaction_test.go create mode 100644 gremlin-js/gremlin-javascript/test/integration/transaction-tests.js create mode 100644 gremlin-python/src/main/python/gremlin_python/driver/transaction.py create mode 100644 gremlin-python/src/main/python/tests/integration/driver/test_transaction.py diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index e7f8ff90c90..c1d610e5513 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -28,6 +28,8 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima * Added typed numeric wrappers and `preciseNumbers` connection option to `gremlin-javascript` for explicit control over numeric type serialization and deserialization. * Added `NextN(n)` to `Traversal` in `gremlin-go` for batched result iteration, providing API parity with `next(n)` in the Java, Python, and .NET GLVs, and updated the Go translators in `gremlin-core` and `gremlin-javascript` to emit `NextN(n)` for the batched form. * Added Gremlator, a single page web application, that translates Gremlin into various programming languages like Javascript and Python. +* Added explicit transaction support to all non-Java GLVs (gremlin-python, gremlin-go, gremlin-javascript, gremlin-dotnet). +* Changed default transaction close behavior from commit to rollback across all GLVs to align with embedded graph defaults. * Refactored Go driver connection to block until response headers arrive, enabling synchronous error returns and proper transaction ordering. * Removed `uuid` dependency from `gremlin-javascript` in favor of the built-in `globalThis.crypto.randomUUID()`. * Added streaming HTTP response support to `gremlin-driver` for incremental result deserialization over GraphBinary. diff --git a/docs/src/reference/gremlin-variants.asciidoc b/docs/src/reference/gremlin-variants.asciidoc index 521a7017bf7..d98e278fb70 100644 --- a/docs/src/reference/gremlin-variants.asciidoc +++ b/docs/src/reference/gremlin-variants.asciidoc @@ -291,8 +291,42 @@ re-construction machine-side. [[gremlin-go-transactions]] === Transactions -IMPORTANT: Transaction support over HTTP is not yet implemented in Gremlin-Go. This will be addressed by the official -4.0.0 release. +Gremlin-Go supports remote transactions over HTTP. Transactions can be created from a `GraphTraversalSource` or +directly from a `Client`. + +==== Via GraphTraversalSource + +[source,go] +---- +g := gremlingo.Traversal_().With(remote) + +tx := g.Tx() +gtx, err := tx.Begin() +if err != nil { log.Fatal(err) } + +_, err = gtx.AddV("person").Property("name", "alice").Iterate() +if err != nil { tx.Rollback(); log.Fatal(err) } + +err = tx.Commit() +if err != nil { log.Fatal(err) } +---- + +==== Via Client + +[source,go] +---- +client, _ := gremlingo.NewClient(url, func(settings *gremlingo.ClientSettings) { + settings.TraversalSource = "g" +}) + +tx := client.Transact() +tx.Begin() +tx.Submit("g.addV('person').property('name','alice')") +tx.Commit() +---- + +NOTE: Transactions are not thread-safe. All operations within a transaction must be sequential. The default behavior +of `Close()` is to rollback. Transactions are single-use and cannot be reused after commit or rollback. [[gremlin-go-gvalue]] === GValue Parameterization @@ -958,15 +992,15 @@ sequentially. ==== Close Behavior -The default behavior of `close()` on a transaction is to commit. This can be changed: +The default behavior of `close()` on a transaction is to rollback. This can be changed: [source,java] ---- RemoteTransaction tx = cluster.transact("g"); -tx.onClose(Transaction.CLOSE_BEHAVIOR.ROLLBACK); +tx.onClose(Transaction.CLOSE_BEHAVIOR.COMMIT); tx.begin(); // ... do work ... -tx.close(); // rolls back instead of committing +tx.close(); // commits instead of rolling back ---- [[gremlin-java-serialization]] @@ -1756,8 +1790,35 @@ To get a full understanding of this section, it would be good to start by readin section of this documentation, which discusses transactions in the general context of TinkerPop itself. This section builds on that content by demonstrating the transactional syntax for Javascript. -IMPORTANT: 4.0.0-beta.2 Release - Transactions are not yet implemented for the HTTP driver. This functionality is -planned for a future release. +Gremlin-Javascript supports remote transactions over HTTP via the Traversal API or the Client API. + +==== Via GraphTraversalSource + +[source,javascript] +---- +const g = traversal().with_(new DriverRemoteConnection('http://localhost:8182/gremlin')); +const tx = g.tx(); +const gtx = await tx.begin(); + +await gtx.addV("person").property("name", "jorge").iterate(); +await gtx.addV("person").property("name", "josh").iterate(); + +await tx.commit(); +---- + +==== Via Client + +[source,javascript] +---- +const client = new Client('http://localhost:8182/gremlin', { traversalSource: 'g' }); +const tx = client.transact(); +await tx.begin(); +await tx.submit("g.addV('person').property('name','alice')"); +await tx.commit(); +---- + +IMPORTANT: Each traversal must be `await`ed before submitting the next. Transactions are not thread-safe. The default +behavior of `close()` is to rollback. Transactions are single-use and cannot be reused after commit or rollback. [source,javascript] ---- @@ -2283,8 +2344,40 @@ encoded in the Gremlin.Net GremlinLang and transmitted to the Gremlin traversal [[gremlin-dotnet-transactions]] === Transactions -IMPORTANT: Transaction support over HTTP is not yet implemented in Gremlin.Net. This will be addressed by the official -4.0.0 release. +Gremlin.Net supports remote transactions over HTTP. Transactions can be created from a `GraphTraversalSource` or +directly from a `GremlinClient`. + +==== Via GraphTraversalSource + +[source,csharp] +---- +var g = AnonymousTraversalSource.Traversal().With(new DriverRemoteConnection("localhost", 8182)); + +var tx = g.Tx(); +var gtx = await tx.BeginAsync(); + +await gtx.AddV("person").Property("name", "jorge").Promise(t => t.Iterate()); +await gtx.AddV("person").Property("name", "josh").Promise(t => t.Iterate()); + +await tx.CommitAsync(); +---- + +==== Via GremlinClient + +[source,csharp] +---- +using var client = new GremlinClient(new GremlinServer("localhost", 8182)); +var tx = client.Transact("g"); +await tx.BeginAsync(); + +await tx.SubmitAsync("g.addV('person').property('name','alice')"); + +await tx.CommitAsync(); +---- + +NOTE: Submissions within a transaction are serialized internally via a semaphore to guarantee the server receives +requests in order. Transactions are not thread-safe. The default behavior of `DisposeAsync()` is to rollback. Use +`await using` for automatic cleanup. Transactions are single-use and cannot be reused after commit or rollback. [[gremlin-dotnet-gvalue]] === GValue Parameterization @@ -2785,8 +2878,49 @@ re-construction machine-side. [[gremlin-python-transactions]] === Transactions -IMPORTANT: Transaction support over HTTP is not yet implemented in Gremlin-Python. This will be addressed by the -official 4.0.0 release. +Gremlin-Python supports remote transactions over HTTP. Transactions can be created from a `GraphTraversalSource` or +directly from a `Client`. + +==== Via GraphTraversalSource + +[source,python] +---- +g = traversal().with_remote(DriverRemoteConnection('http://localhost:8182/gremlin', 'g')) + +tx = g.tx() +gtx = tx.begin() + +gtx.addV('person').property('name', 'jorge').iterate() +gtx.addV('person').property('name', 'josh').iterate() + +tx.commit() +---- + +==== Via Client + +[source,python] +---- +client = Client('http://localhost:8182/gremlin', 'g') + +tx = client.transact() +tx.begin() +tx.submit("g.addV('person').property('name','alice')") +tx.commit() +---- + +==== Context Manager + +[source,python] +---- +with client.transact() as tx: + tx.begin() + tx.submit("g.addV('person').property('name','alice')") + tx.commit() + # if commit() not called or exception occurs, rollback happens on exit +---- + +NOTE: Transactions are not thread-safe. All operations must be sequential. The default behavior of `close()` is to +rollback. Transactions are single-use and cannot be reused after commit or rollback. [[gremlin-python-gvalue]] === GValue Parameterization diff --git a/docs/src/reference/intro.asciidoc b/docs/src/reference/intro.asciidoc index b4cf33536cb..0d4179478dd 100644 --- a/docs/src/reference/intro.asciidoc +++ b/docs/src/reference/intro.asciidoc @@ -446,7 +446,8 @@ regard. Prefer explicit property key names in Gremlin unless it is completely im The third and final point involves transactions. Under this model, one traversal is equivalent to a single transaction and there is no way in TinkerPop to string together multiple traversals into the same transaction. -IMPORTANT: 4.0.0-beta.1 Release - Remote transactions are not supported in this beta. +Remote transactions are supported across all GLVs. See the <> section and each GLV's +specific transaction documentation for syntax and usage details. [[connecting-rgp]] === Remote Gremlin Provider diff --git a/docs/src/reference/the-traversal.asciidoc b/docs/src/reference/the-traversal.asciidoc index ab49ae2614c..bc05a52b44d 100644 --- a/docs/src/reference/the-traversal.asciidoc +++ b/docs/src/reference/the-traversal.asciidoc @@ -107,22 +107,8 @@ GraphTraversalSource gtx2 = tx2.begin(); ---- In remote cases, `GraphTraversalSource` instances spawned from `begin()` are safe to use from a single thread. The -default behavior of `close()` on a `Transaction` for remote cases is to `commit()`, so the following re-write of the -earlier example is also valid: - -[source,java] ----- -// note here that we dispense with creating a Transaction object and -// simply spawn the gtx in a more inline fashion -GraphTraversalSource gtx = g.tx().begin(); -try { - gtx.addV('person').iterate(); - gtx.addV('software').iterate(); - gtx.close(); -} catch (Exception ex) { - tx.rollback(); -} ----- +default behavior of `close()` on a `Transaction` for remote cases is to `rollback()`, ensuring partial work is +discarded if the user forgets to commit. Users must call `commit()` explicitly to persist data. IMPORTANT: Transactions with non-JVM languages are always "remote". For specific transaction syntax in a particular language, please see the "Transactions" sub-section of your language of interest in the diff --git a/docs/src/upgrade/release-4.x.x.asciidoc b/docs/src/upgrade/release-4.x.x.asciidoc index aa14264ca0a..1cdb8136fb6 100644 --- a/docs/src/upgrade/release-4.x.x.asciidoc +++ b/docs/src/upgrade/release-4.x.x.asciidoc @@ -41,6 +41,34 @@ anonymized form. The original gremlator.com was a prototype built by TinkerPop c previous implementation required Java and a running Gremlin Server, whereas the new version runs entirely in the browser with no server infrastructure needed. +==== GLV Transaction Support + +All non-Java Gremlin Language Variants (gremlin-python, gremlin-go, gremlin-javascript, gremlin-dotnet) now support +explicit remote transactions over HTTP. The transaction model is the same across all GLVs: begin a transaction, submit +traversals within it, then commit or rollback. Data is isolated until committed. + +Each GLV provides two entry points: + +* **Traversal API**: `g.tx().begin()` returns a transaction-bound `GraphTraversalSource` +* **Driver API**: `client.transact()` (or `client.Transact()` in .NET) returns a `Transaction` with `submit()` methods + +Key behaviors consistent across all GLVs: + +* Transactions are not thread-safe. All operations must be sequential. +* The default close behavior is rollback (partial work discarded if commit is not called explicitly). +* Transactions are single-use. After commit or rollback, a new transaction must be created. +* `gtx.tx()` returns the same transaction object (enabling `gtx.tx().commit()` pattern from the Traversal API). + +See the <> reference documentation for language-specific +syntax and examples. + +==== Transaction Default Close Behavior Changed + +The default behavior of `close()` on a remote transaction has been changed from `commit` to `rollback` across all +GLVs (including the Java driver). This aligns with the embedded graph transaction default and is the safer behavior: +partial work is discarded if the user forgets to call `commit()`. In Java, the behavior can still be overridden via +`tx.onClose(Transaction.CLOSE_BEHAVIOR.COMMIT)`. + ==== Removed `uuid` Dependency in gremlin-javascript The `uuid` npm package has been removed from `gremlin-javascript`. UUID generation now uses the built-in diff --git a/gremlin-dotnet/src/Gremlin.Net/Driver/Connection.cs b/gremlin-dotnet/src/Gremlin.Net/Driver/Connection.cs index e0b0e30b78b..e5c94611d4a 100644 --- a/gremlin-dotnet/src/Gremlin.Net/Driver/Connection.cs +++ b/gremlin-dotnet/src/Gremlin.Net/Driver/Connection.cs @@ -139,6 +139,15 @@ public async Task> SubmitAsync(RequestMessage requestMessage, headers["bulkResults"] = "true"; } + // Promote transactionId to HTTP header before serialization. + // The field remains in the serialized body as well (dual transmission + // per the HTTP transaction protocol specification). + if (requestMessage.Fields.TryGetValue(Tokens.ArgsTransactionId, out var txIdObj) && + txIdObj is string txId && !string.IsNullOrEmpty(txId)) + { + headers["X-Transaction-Id"] = txId; + } + object body; if (_requestSerializer != null) { diff --git a/gremlin-dotnet/src/Gremlin.Net/Driver/GremlinClient.cs b/gremlin-dotnet/src/Gremlin.Net/Driver/GremlinClient.cs index 50dcbcbd274..768c30ab10d 100644 --- a/gremlin-dotnet/src/Gremlin.Net/Driver/GremlinClient.cs +++ b/gremlin-dotnet/src/Gremlin.Net/Driver/GremlinClient.cs @@ -117,6 +117,18 @@ public async Task> SubmitAsync(RequestMessage requestMessage, .ConfigureAwait(false); } + /// + /// Creates a new for executing operations within an explicit + /// server-side transaction. Transactions are short-lived and single-use: after commit + /// or rollback, create a new RemoteTransaction for the next unit of work. + /// + /// The traversal source alias (e.g. "g" or "gtx"). + /// A new . + public RemoteTransaction Transact(string traversalSource) + { + return new RemoteTransaction(this, traversalSource); + } + #region IDisposable Support private bool _disposed; diff --git a/gremlin-dotnet/src/Gremlin.Net/Driver/Remote/DriverRemoteConnection.cs b/gremlin-dotnet/src/Gremlin.Net/Driver/Remote/DriverRemoteConnection.cs index 3340730c1b9..abcaa28d32f 100644 --- a/gremlin-dotnet/src/Gremlin.Net/Driver/Remote/DriverRemoteConnection.cs +++ b/gremlin-dotnet/src/Gremlin.Net/Driver/Remote/DriverRemoteConnection.cs @@ -47,9 +47,6 @@ public class DriverRemoteConnection : IRemoteConnection, IDisposable // The server filters out options that don't apply, and this allows // providers to use custom request fields via the Client directly or DRC. - /// - public bool IsSessionBound => false; - /// /// Initializes a new . /// @@ -141,12 +138,11 @@ public async Task> SubmitAsync(GremlinLan } /// - /// - /// Transaction support over HTTP is not yet implemented. This will be addressed in a future release. - /// public RemoteTransaction Tx(GraphTraversalSource g) { - throw new NotSupportedException("Transaction support over HTTP is not yet implemented."); + // The g parameter satisfies the IRemoteConnection interface but is unused. + // The traversal source is taken from this connection's configuration. + return new RemoteTransaction(_client, _traversalSource); } /// diff --git a/gremlin-dotnet/src/Gremlin.Net/Driver/Remote/TransactionRemoteConnection.cs b/gremlin-dotnet/src/Gremlin.Net/Driver/Remote/TransactionRemoteConnection.cs new file mode 100644 index 00000000000..9fa1e57441d --- /dev/null +++ b/gremlin-dotnet/src/Gremlin.Net/Driver/Remote/TransactionRemoteConnection.cs @@ -0,0 +1,110 @@ +#region License + +/* + * 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. + */ + +#endregion + +using System; +using System.Threading; +using System.Threading.Tasks; +using Gremlin.Net.Driver.Messages; +using Gremlin.Net.Process.Remote; +using Gremlin.Net.Process.Traversal; +using Gremlin.Net.Process.Traversal.Strategy.Decoration; + +namespace Gremlin.Net.Driver.Remote +{ + /// + /// A that attaches a transactionId to all requests, + /// binding them to a server-side transaction. + /// + internal class TransactionRemoteConnection : IRemoteConnection + { + private readonly IGremlinClient _client; + private readonly string _traversalSource; + private readonly string _transactionId; + private readonly RemoteTransaction _transaction; + private bool _closed; + + public TransactionRemoteConnection(IGremlinClient client, string traversalSource, string transactionId, RemoteTransaction transaction) + { + _client = client; + _traversalSource = traversalSource; + _transactionId = transactionId; + _transaction = transaction; + } + + /// + public async Task> SubmitAsync(GremlinLang gremlinLang, + CancellationToken cancellationToken = default) + { + if (_closed) + { + throw new InvalidOperationException("Transaction is not open"); + } + + gremlinLang.AddG(_traversalSource); + + var requestMsg = RequestMessage.Build(gremlinLang.GetGremlin()) + .AddG(_traversalSource); + + var parametersString = gremlinLang.GetParametersAsString(); + if (parametersString != "[:]") + { + requestMsg.AddBindingsString(parametersString); + } + + foreach (var optionsStrategy in gremlinLang.OptionsStrategies) + { + foreach (var pair in optionsStrategy.Configuration) + { + requestMsg.AddField(pair.Key, pair.Value); + } + } + + // Default bulkResults to "true" if not set per-request + if (!requestMsg.HasField(Tokens.ArgsBulkResults)) + { + requestMsg.AddField(Tokens.ArgsBulkResults, "true"); + } + + // Inject transactionId into the request + requestMsg.AddField(Tokens.ArgsTransactionId, _transactionId); + + // Route through Transaction's serialized submission to guarantee ordering + var resultSet = await _transaction.SubmitAsync(requestMsg.Create(), cancellationToken) + .ConfigureAwait(false); + return new DriverRemoteTraversal(resultSet); + } + + /// + public RemoteTransaction Tx(GraphTraversalSource g) + { + // Return the existing transaction. Calling BeginAsync() on it will throw + // "Transaction already started" since it's already open. + return _transaction; + } + + internal void MarkClosed() + { + _closed = true; + } + } +} diff --git a/gremlin-dotnet/src/Gremlin.Net/Driver/RemoteTransaction.cs b/gremlin-dotnet/src/Gremlin.Net/Driver/RemoteTransaction.cs new file mode 100644 index 00000000000..b7e665b968e --- /dev/null +++ b/gremlin-dotnet/src/Gremlin.Net/Driver/RemoteTransaction.cs @@ -0,0 +1,270 @@ +#region License + +/* + * 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. + */ + +#endregion + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Gremlin.Net.Driver.Messages; +using Gremlin.Net.Driver.Remote; +using Gremlin.Net.Process.Traversal; + +namespace Gremlin.Net.Driver +{ + /// + /// Controls an explicit remote transaction. Created via + /// GremlinClient.Transact() or g.Tx(). + /// The transaction is not started until is called. + /// + /// All submissions are serialized internally via a semaphore to guarantee + /// the server receives requests in order, even if the caller does not await + /// each call before issuing the next. + /// + /// This class is NOT thread-safe. Do not share a RemoteTransaction across + /// multiple threads without external synchronization. + /// + public class RemoteTransaction : IGremlinClient, IAsyncDisposable + { + private readonly IGremlinClient _client; + private readonly string _traversalSource; + // Serializes all submissions to guarantee ordering. Matches the role of + // Java's synchronized on HttpRemoteTransaction.submitInternal. + private readonly SemaphoreSlim _submitLock = new(1, 1); + private string? _transactionId; + private bool _isOpen; + private bool _failed; + private TransactionRemoteConnection? _txConnection; + + internal RemoteTransaction(IGremlinClient client, string traversalSource) + { + _client = client ?? throw new ArgumentNullException(nameof(client)); + _traversalSource = traversalSource ?? throw new ArgumentNullException(nameof(traversalSource)); + } + + /// + /// Gets the server-generated transaction ID, or null if the transaction has not yet been started. + /// + public string? TransactionId => _transactionId; + + /// + /// Gets whether the transaction is currently open. + /// + public bool IsOpen => _isOpen; + + /// + /// Starts the transaction and returns a transaction-bound . + /// + /// The token to cancel the operation. + /// A bound to this transaction. + /// Thrown if the transaction is already started. + public async Task BeginAsync(CancellationToken cancellationToken = default) + { + if (_isOpen || _failed) + { + throw new InvalidOperationException("Transaction already started"); + } + + var requestMsg = RequestMessage.Build("g.tx().begin()") + .AddG(_traversalSource) + .Create(); + + await _submitLock.WaitAsync(cancellationToken).ConfigureAwait(false); + try + { + List results; + try + { + var resultSet = await _client.SubmitAsync(requestMsg, cancellationToken) + .ConfigureAwait(false); + results = await resultSet.ToListAsync(cancellationToken).ConfigureAwait(false); + } + catch + { + _failed = true; + throw; + } + + if (results.Count == 0) + { + _failed = true; + throw new InvalidOperationException("Server did not return transaction ID"); + } + + if (results[0] is Dictionary resultMap && + resultMap.TryGetValue("transactionId", out var txIdObj) && + txIdObj is string txId && !string.IsNullOrEmpty(txId)) + { + _transactionId = txId; + } + else + { + _failed = true; + throw new InvalidOperationException("Server did not return transaction ID in expected format"); + } + + _isOpen = true; + } + finally + { + _submitLock.Release(); + } + + _txConnection = new TransactionRemoteConnection(_client, _traversalSource, _transactionId, this); + return new GraphTraversalSource( + new List(), + new GremlinLang(), + _txConnection); + } + + /// + /// Commits the transaction. + /// + /// The token to cancel the operation. + /// Thrown if the transaction is not open. + public async Task CommitAsync(CancellationToken cancellationToken = default) + { + await CloseTransactionAsync("g.tx().commit()", cancellationToken).ConfigureAwait(false); + } + + /// + /// Rolls back the transaction. + /// + /// The token to cancel the operation. + /// Thrown if the transaction is not open. + public async Task RollbackAsync(CancellationToken cancellationToken = default) + { + await CloseTransactionAsync("g.tx().rollback()", cancellationToken).ConfigureAwait(false); + } + + private async Task CloseTransactionAsync(string script, CancellationToken cancellationToken) + { + if (!_isOpen) + { + throw new InvalidOperationException("Transaction is not open"); + } + + var requestMsg = RequestMessage.Build(script) + .AddG(_traversalSource) + .AddField(Tokens.ArgsTransactionId, _transactionId!) + .Create(); + + await _submitLock.WaitAsync(cancellationToken).ConfigureAwait(false); + try + { + var resultSet = await _client.SubmitAsync(requestMsg, cancellationToken).ConfigureAwait(false); + // Drain the result to surface any GraphBinary-level errors from the response body + await resultSet.ToListAsync(cancellationToken).ConfigureAwait(false); + } + finally + { + _submitLock.Release(); + } + _isOpen = false; + _failed = true; // Terminal state: transaction cannot be reused + _txConnection?.MarkClosed(); + } + + /// + /// Submits a within this transaction. + /// Submissions are serialized to guarantee the server receives them in order. + /// + public async Task> SubmitAsync(RequestMessage requestMessage, + CancellationToken cancellationToken = default) + { + if (!_isOpen) + { + throw new InvalidOperationException("Transaction is not open"); + } + + await _submitLock.WaitAsync(cancellationToken).ConfigureAwait(false); + try + { + return await _client.SubmitAsync(requestMessage, cancellationToken).ConfigureAwait(false); + } + finally + { + _submitLock.Release(); + } + } + + /// + /// Submits a plain gremlin-lang string within this transaction. + /// The transactionId is automatically attached. + /// + /// The Gremlin query string. + /// The token to cancel the operation. + /// A containing the results. + /// Thrown if the transaction is not open. + public async Task> SubmitAsync(string gremlin, CancellationToken cancellationToken = default) + { + if (!_isOpen) + { + throw new InvalidOperationException("Transaction is not open"); + } + + var requestMsg = RequestMessage.Build(gremlin) + .AddG(_traversalSource) + .AddField(Tokens.ArgsTransactionId, _transactionId!) + .Create(); + + await _submitLock.WaitAsync(cancellationToken).ConfigureAwait(false); + try + { + return await _client.SubmitAsync(requestMsg, cancellationToken).ConfigureAwait(false); + } + finally + { + _submitLock.Release(); + } + } + + /// + /// Disposes the transaction asynchronously. Default behavior is rollback. + /// + public async ValueTask DisposeAsync() + { + if (_isOpen) + { + try + { + await RollbackAsync().ConfigureAwait(false); + } + catch + { + _isOpen = false; + _failed = true; + _txConnection?.MarkClosed(); + } + } + } + + /// + /// Synchronous dispose (required by IGremlinClient/IDisposable). + /// Does not attempt rollback. Use await using for proper cleanup. + /// + public void Dispose() + { + } + } +} diff --git a/gremlin-dotnet/src/Gremlin.Net/Driver/Tokens.cs b/gremlin-dotnet/src/Gremlin.Net/Driver/Tokens.cs index fd94e24a773..7f39308ec00 100644 --- a/gremlin-dotnet/src/Gremlin.Net/Driver/Tokens.cs +++ b/gremlin-dotnet/src/Gremlin.Net/Driver/Tokens.cs @@ -89,5 +89,10 @@ public class Tokens /// Argument name for bulk results mode. /// public const string ArgsBulkResults = "bulkResults"; + + /// + /// Argument name for the transaction ID that binds a request to a server-side transaction. + /// + public const string ArgsTransactionId = "transactionId"; } } diff --git a/gremlin-dotnet/src/Gremlin.Net/Process/Remote/IRemoteConnection.cs b/gremlin-dotnet/src/Gremlin.Net/Process/Remote/IRemoteConnection.cs index a571c9546b7..bbef10f47c5 100644 --- a/gremlin-dotnet/src/Gremlin.Net/Process/Remote/IRemoteConnection.cs +++ b/gremlin-dotnet/src/Gremlin.Net/Process/Remote/IRemoteConnection.cs @@ -23,6 +23,7 @@ using System.Threading; using System.Threading.Tasks; +using Gremlin.Net.Driver; using Gremlin.Net.Process.Traversal; namespace Gremlin.Net.Process.Remote @@ -51,10 +52,5 @@ public interface IRemoteConnection /// /// The created . RemoteTransaction Tx(GraphTraversalSource graphTraversalSource); - - /// - /// Determines if the connection is bound to a session. - /// - bool IsSessionBound { get; } } } \ No newline at end of file diff --git a/gremlin-dotnet/src/Gremlin.Net/Process/Remote/RemoteTransaction.cs b/gremlin-dotnet/src/Gremlin.Net/Process/Remote/RemoteTransaction.cs deleted file mode 100644 index 99e7b0b37a7..00000000000 --- a/gremlin-dotnet/src/Gremlin.Net/Process/Remote/RemoteTransaction.cs +++ /dev/null @@ -1,87 +0,0 @@ -#region License - -/* - * 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. - */ - -#endregion - -using System; -using System.Threading; -using System.Threading.Tasks; -using Gremlin.Net.Process.Traversal; - -namespace Gremlin.Net.Process.Remote -{ - /// - /// A controller for a remote transaction that is constructed from g.Tx(). Calling Begin() on - /// this object will produce a new GraphTraversalSource that is bound to a remote transaction over which - /// multiple traversals may be executed in that context. Calling CommitAsync() or - /// RollbackAsync() will then close the transaction and thus, the session. This feature only works with - /// transaction enabled graphs. - /// - public class RemoteTransaction - { - private readonly IRemoteConnection _sessionBasedConnection; - private GraphTraversalSource _g; - - /// - /// Initializes a new instance of the class. - /// - /// The session bound connection that will be used to control this transaction. - /// The graph traversal source from which a session bound traversal source will be created. - public RemoteTransaction(IRemoteConnection connection, GraphTraversalSource g) - { - _sessionBasedConnection = connection; - _g = g; - } - - /// - /// Spawns a that is bound to a remote session which enables a transaction. - /// - /// A bound to a remote session. - /// Thrown if this transaction is already bound to a session. - public GraphTraversalSource Begin() - { - if (_g.IsSessionBound) - { - throw new InvalidOperationException("Transaction already started on this object"); - } - _g = new GraphTraversalSource(_g.TraversalStrategies, _g.GremlinLang, _sessionBasedConnection); - return _g; - } - - /// - /// Commits the transaction. - /// - /// The token to cancel the operation. The default value is None. - public async Task CommitAsync(CancellationToken cancellationToken = default) - { - await _sessionBasedConnection.SubmitAsync(GraphOp.Commit, cancellationToken) - .ConfigureAwait(false); - } - - /// - /// Rolls back the transaction. - /// - public async Task RollbackAsync() - { - await _sessionBasedConnection.SubmitAsync(GraphOp.Rollback).ConfigureAwait(false); - } - } -} \ No newline at end of file diff --git a/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GraphTraversalSource.cs b/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GraphTraversalSource.cs index 581882f4902..fe1f6028a60 100644 --- a/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GraphTraversalSource.cs +++ b/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GraphTraversalSource.cs @@ -24,6 +24,7 @@ using System; using System.Collections.Generic; using System.Linq; +using Gremlin.Net.Driver; using Gremlin.Net.Process.Remote; using Gremlin.Net.Process.Traversal.Strategy.Decoration; using Gremlin.Net.Structure; @@ -39,8 +40,6 @@ namespace Gremlin.Net.Process.Traversal public class GraphTraversalSource { private readonly IRemoteConnection? _connection; - - public bool IsSessionBound => _connection is { IsSessionBound: true }; /// /// Gets or sets the traversal strategies associated with this graph traversal source. @@ -267,18 +266,12 @@ public GraphTraversalSource WithRemote(IRemoteConnection remoteConnection) => GremlinLang.Clone(), remoteConnection); /// - /// Spawns a new object that can then start and stop a transaction. + /// Returns the for this traversal source. If the source is + /// already bound to a transaction, the existing transaction is returned. /// - /// The spawned transaction. - /// Thrown if this traversal source is already bound to a session. + /// The transaction. public RemoteTransaction Tx() { - // you can't do g.tx().begin().tx() - no child transactions - if (IsSessionBound) - { - throw new InvalidOperationException( - "This GraphTraversalSource is already bound to a transaction - child transactions are not supported"); - } return _connection!.Tx(this); } diff --git a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Docs/Reference/GremlinVariantsTests.cs b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Docs/Reference/GremlinVariantsTests.cs index 6b15eee4667..be2506151e4 100644 --- a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Docs/Reference/GremlinVariantsTests.cs +++ b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Docs/Reference/GremlinVariantsTests.cs @@ -148,32 +148,33 @@ public void SubmittingScriptsWithAuthenticationTest() } [Fact(Skip = "No Server under localhost")] - public async Task TransactionsTest() + public async Task TransactionsViaTraversalSourceTest() { -// tag::transactions[] -using var gremlinClient = new GremlinClient(new GremlinServer("localhost", 8182)); -var g = Traversal().With(new DriverRemoteConnection(gremlinClient)); -var tx = g.Tx(); // create a transaction - -// spawn a new GraphTraversalSource binding all traversals established from it to tx -var gtx = tx.Begin(); - -// execute traversals using gtx occur within the scope of the transaction held by tx. the -// tx is closed after calls to CommitAsync or RollbackAsync and cannot be re-used. simply spawn a -// new Transaction from g.Tx() to create a new one as needed. the g context remains -// accessible through all this as a sessionless connection. -try -{ - await gtx.AddV("person").Property("name", "jorge").Promise(t => t.Iterate()); - await gtx.AddV("person").Property("name", "josh").Promise(t => t.Iterate()); - - await tx.CommitAsync(); -} -catch (Exception) -{ - await tx.RollbackAsync(); -} -// end::transactions[] +// tag::transactionsViaTraversalSource[] +var g = AnonymousTraversalSource.Traversal().With(new DriverRemoteConnection("localhost", 8182)); + +var tx = g.Tx(); +var gtx = await tx.BeginAsync(); + +await gtx.AddV("person").Property("name", "jorge").Promise(t => t.Iterate()); +await gtx.AddV("person").Property("name", "josh").Promise(t => t.Iterate()); + +await tx.CommitAsync(); +// end::transactionsViaTraversalSource[] + } + + [Fact(Skip = "No Server under localhost")] + public async Task TransactionsViaClientTest() + { +// tag::transactionsViaClient[] +using var client = new GremlinClient(new GremlinServer("localhost", 8182)); +var tx = client.Transact("g"); +await tx.BeginAsync(); + +await tx.SubmitAsync("g.addV('person').property('name','alice')"); + +await tx.CommitAsync(); +// end::transactionsViaClient[] } } } \ No newline at end of file diff --git a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Driver/TransactionTests.cs b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Driver/TransactionTests.cs new file mode 100644 index 00000000000..8bd01179a53 --- /dev/null +++ b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Driver/TransactionTests.cs @@ -0,0 +1,468 @@ +#region License + +/* + * 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. + */ + +#endregion + +using System; +using System.Collections.Generic; +using System.Threading.Tasks; +using Gremlin.Net.Driver; +using Gremlin.Net.Driver.Exceptions; +using Gremlin.Net.Driver.Messages; +using Gremlin.Net.Driver.Remote; +using Gremlin.Net.IntegrationTest.Util; +using Gremlin.Net.Process.Traversal; +using Xunit; + +namespace Gremlin.Net.IntegrationTest.Driver +{ + public class TransactionTests + { + private static readonly string TestHost = ConfigProvider.Configuration["TestServerIpAddress"]!; + private static readonly int TestPort = Convert.ToInt32(ConfigProvider.Configuration["TestServerPort"]); + + private GremlinClient CreateClient() + { + return new GremlinClient(new GremlinServer(TestHost, TestPort)); + } + + private async Task GetCount(GremlinClient client, string label) + { + var msg = RequestMessage.Build($"g.V().hasLabel('{label}').count()") + .AddG("gtx").Create(); + var resultSet = await client.SubmitAsync(msg); + var enumerator = resultSet.GetAsyncEnumerator(); + await enumerator.MoveNextAsync(); + return enumerator.Current; + } + + /// + /// Drops all vertices from the transactional graph to prevent cross-test contamination. + /// + private async Task DropGraph(GremlinClient client) + { + var msg = RequestMessage.Build("g.V().drop()").AddG("gtx").Create(); + await client.SubmitAsync(msg); + } + + [Fact] + public async Task ShouldCommitTransaction() + { + using var client = CreateClient(); + await DropGraph(client); + var tx = client.Transact("gtx"); + await tx.BeginAsync(); + Assert.True(tx.IsOpen); + + await tx.SubmitAsync("g.addV('person').property('name','alice')"); + + // Uncommitted data not visible outside the transaction + Assert.Equal(0L, await GetCount(client, "person")); + + await tx.CommitAsync(); + Assert.False(tx.IsOpen); + + // Committed data visible + Assert.Equal(1L, await GetCount(client, "person")); + } + + [Fact] + public async Task ShouldRollbackTransaction() + { + using var client = CreateClient(); + await DropGraph(client); + var tx = client.Transact("gtx"); + await tx.BeginAsync(); + + await tx.SubmitAsync("g.addV('person').property('name','bob')"); + + await tx.RollbackAsync(); + Assert.False(tx.IsOpen); + + Assert.Equal(0L, await GetCount(client, "person")); + } + + [Fact] + public async Task ShouldSupportIntraTransactionConsistency() + { + using var client = CreateClient(); + await DropGraph(client); + var tx = client.Transact("gtx"); + await tx.BeginAsync(); + + await tx.SubmitAsync("g.addV('test').property('name','A')"); + + // Read-your-own-writes + var resultSet = await tx.SubmitAsync("g.V().hasLabel('test').count()"); + var enumerator = resultSet.GetAsyncEnumerator(); + await enumerator.MoveNextAsync(); + Assert.Equal(1L, enumerator.Current); + + await tx.SubmitAsync("g.addV('test').property('name','B')"); + await tx.CommitAsync(); + + Assert.Equal(2L, await GetCount(client, "test")); + } + + [Fact] + public async Task ShouldThrowOnSubmitAfterCommit() + { + using var client = CreateClient(); + var tx = client.Transact("gtx"); + await tx.BeginAsync(); + await tx.SubmitAsync("g.addV()"); + await tx.CommitAsync(); + + await Assert.ThrowsAsync( + () => tx.SubmitAsync("g.V().count()")); + } + + [Fact] + public async Task ShouldThrowOnSubmitAfterRollback() + { + using var client = CreateClient(); + var tx = client.Transact("gtx"); + await tx.BeginAsync(); + await tx.SubmitAsync("g.addV()"); + await tx.RollbackAsync(); + + await Assert.ThrowsAsync( + () => tx.SubmitAsync("g.V().count()")); + } + + [Fact] + public async Task ShouldThrowOnDoubleBegin() + { + using var client = CreateClient(); + var tx = client.Transact("gtx"); + await tx.BeginAsync(); + + await Assert.ThrowsAsync(() => tx.BeginAsync()); + } + + [Fact] + public async Task ShouldThrowOnCommitWhenNotOpen() + { + using var client = CreateClient(); + var tx = client.Transact("gtx"); + + await Assert.ThrowsAsync(() => tx.CommitAsync()); + } + + [Fact] + public async Task ShouldThrowOnRollbackWhenNotOpen() + { + using var client = CreateClient(); + var tx = client.Transact("gtx"); + + await Assert.ThrowsAsync(() => tx.RollbackAsync()); + } + + [Fact] + public async Task ShouldReturnNullTransactionIdBeforeBegin() + { + using var client = CreateClient(); + var tx = client.Transact("gtx"); + + Assert.Null(tx.TransactionId); + + await tx.BeginAsync(); + Assert.NotNull(tx.TransactionId); + Assert.True(tx.TransactionId!.Length > 0); + } + + [Fact] + public async Task ShouldRollbackOnDisposeByDefault() + { + using var client = CreateClient(); + await DropGraph(client); + + await using (var tx = client.Transact("gtx")) + { + await tx.BeginAsync(); + await tx.SubmitAsync("g.addV('person').property('name','dispose_test')"); + // No commit - dispose should rollback + } + + Assert.Equal(0L, await GetCount(client, "person")); + } + + [Fact] + public async Task ShouldIsolateConcurrentTransactions() + { + using var client = CreateClient(); + await DropGraph(client); + var tx1 = client.Transact("gtx"); + await tx1.BeginAsync(); + var tx2 = client.Transact("gtx"); + await tx2.BeginAsync(); + + await tx1.SubmitAsync("g.addV('tx1')"); + await tx2.SubmitAsync("g.addV('tx2')"); + + // tx1 should not see tx2's data + var rs = await tx1.SubmitAsync("g.V().hasLabel('tx2').count()"); + var e = rs.GetAsyncEnumerator(); + await e.MoveNextAsync(); + Assert.Equal(0L, e.Current); + + await tx1.CommitAsync(); + await tx2.CommitAsync(); + + Assert.Equal(1L, await GetCount(client, "tx1")); + Assert.Equal(1L, await GetCount(client, "tx2")); + } + + [Fact] + public async Task ShouldOpenAndCloseManyTransactionsSequentially() + { + using var client = CreateClient(); + await DropGraph(client); + const int numTransactions = 50; + + for (int i = 0; i < numTransactions; i++) + { + var tx = client.Transact("gtx"); + await tx.BeginAsync(); + await tx.SubmitAsync("g.addV('stress')"); + await tx.CommitAsync(); + } + + Assert.Equal((long)numTransactions, await GetCount(client, "stress")); + } + + [Fact] + public async Task ShouldKeepTransactionOpenAfterTraversalError() + { + using var client = CreateClient(); + await DropGraph(client); + var tx = client.Transact("gtx"); + await tx.BeginAsync(); + + await tx.SubmitAsync("g.addV('good_vertex')"); + + try + { + await tx.SubmitAsync("g.V().fail()"); + } + catch + { + // expected + } + + Assert.True(tx.IsOpen); + await tx.RollbackAsync(); + + Assert.Equal(0L, await GetCount(client, "good_vertex")); + } + + [Fact] + public async Task ShouldWorkWithTraversalApi() + { + using var client = CreateClient(); + await DropGraph(client); + var connection = new DriverRemoteConnection(client, "gtx"); + + var g = AnonymousTraversalSource.Traversal().With(connection); + + var tx = g.Tx(); + var gtx = await tx.BeginAsync(); + await gtx.AddV("val").Promise(t => t.Iterate()); + await tx.CommitAsync(); + + var count = await g.V().HasLabel("val").Count().Promise(t => t.Next()); + Assert.Equal(1L, count); + } + + [Fact] + public async Task ShouldRejectBeginOnNonTransactionalGraph() + { + using var client = CreateClient(); + var tx = client.Transact("gclassic"); + + var ex = await Assert.ThrowsAsync(() => tx.BeginAsync()); + Assert.Contains("Graph does not support transactions", ex.Message); + } + + [Fact] + public async Task ShouldReturnSameTransactionFromGtxTx() + { + using var client = CreateClient(); + await DropGraph(client); + var connection = new DriverRemoteConnection(client, "gtx"); + var g = AnonymousTraversalSource.Traversal().With(connection); + + var tx = g.Tx(); + var gtx = await tx.BeginAsync(); + var sameTx = gtx.Tx(); + + Assert.Same(tx, sameTx); + + await sameTx.RollbackAsync(); + } + + [Fact] + public async Task ShouldThrowOnBeginFromGtxTx() + { + using var client = CreateClient(); + await DropGraph(client); + var connection = new DriverRemoteConnection(client, "gtx"); + var g = AnonymousTraversalSource.Traversal().With(connection); + + var tx = g.Tx(); + var gtx = await tx.BeginAsync(); + var sameTx = gtx.Tx(); + + await Assert.ThrowsAsync(() => sameTx.BeginAsync()); + + await tx.RollbackAsync(); + } + + [Fact] + public async Task ShouldThrowOnDoubleCommit() + { + using var client = CreateClient(); + await DropGraph(client); + var tx = client.Transact("gtx"); + await tx.BeginAsync(); + await tx.CommitAsync(); + + await Assert.ThrowsAsync(() => tx.CommitAsync()); + } + + [Fact] + public async Task ShouldThrowOnDoubleRollback() + { + using var client = CreateClient(); + await DropGraph(client); + var tx = client.Transact("gtx"); + await tx.BeginAsync(); + await tx.RollbackAsync(); + + await Assert.ThrowsAsync(() => tx.RollbackAsync()); + } + + [Fact] + public async Task ShouldNotAllowBeginAfterCommit() + { + using var client = CreateClient(); + await DropGraph(client); + var tx = client.Transact("gtx"); + await tx.BeginAsync(); + await tx.CommitAsync(); + + await Assert.ThrowsAsync(() => tx.BeginAsync()); + } + + [Fact] + public async Task ShouldNotAllowBeginAfterRollback() + { + using var client = CreateClient(); + await DropGraph(client); + var tx = client.Transact("gtx"); + await tx.BeginAsync(); + await tx.RollbackAsync(); + + await Assert.ThrowsAsync(() => tx.BeginAsync()); + } + + [Fact] + public async Task ShouldIsolateTransactionalAndNonTransactionalRequests() + { + using var client = CreateClient(); + await DropGraph(client); + var tx = client.Transact("gtx"); + await tx.BeginAsync(); + + await tx.SubmitAsync("g.addV('tx_data')"); + + // Non-transactional read should not see uncommitted data + Assert.Equal(0L, await GetCount(client, "tx_data")); + + await tx.CommitAsync(); + + Assert.Equal(1L, await GetCount(client, "tx_data")); + } + + [Fact] + public async Task ShouldCommitViaGtxTx() + { + using var client = CreateClient(); + await DropGraph(client); + var connection = new DriverRemoteConnection(client, "gtx"); + var g = AnonymousTraversalSource.Traversal().With(connection); + + var tx = g.Tx(); + var gtx = await tx.BeginAsync(); + await gtx.AddV("gtx_commit_test").Promise(t => t.Iterate()); + await gtx.Tx().CommitAsync(); + + Assert.Equal(1L, await GetCount(client, "gtx_commit_test")); + } + + [Fact] + public async Task ShouldCleanUpOnBeginFailure() + { + using var client = CreateClient(); + var tx = client.Transact("gclassic"); + + try + { + await tx.BeginAsync(); + } + catch + { + // expected + } + + Assert.False(tx.IsOpen); + Assert.Null(tx.TransactionId); + + // Cannot begin again + await Assert.ThrowsAsync(() => tx.BeginAsync()); + } + [Fact] + public async Task ShouldSerializeUnawaitedSubmissions() + { + using var client = CreateClient(); + await DropGraph(client); + var tx = client.Transact("gtx"); + await tx.BeginAsync(); + + // Fire submissions from the same thread without awaiting between them. + // The semaphore serializes them so the server processes them in order. + // Each step depends on the prior one completing first. + var t1 = tx.SubmitAsync("g.addV('chain').property('pos', 0)"); + var t2 = tx.SubmitAsync("g.V().has('chain','pos',0).property('pos', 1)"); + var t3 = tx.SubmitAsync("g.V().has('chain','pos',1).property('pos', 2)"); + + await Task.WhenAll(t1, t2, t3); + + // Verify the chain completed: final vertex should have pos=2 + var rs = await tx.SubmitAsync("g.V().has('chain','pos',2).count()"); + var enumerator = rs.GetAsyncEnumerator(); + await enumerator.MoveNextAsync(); + Assert.Equal(1L, enumerator.Current); + + await tx.CommitAsync(); + } + } +} diff --git a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Process/Traversal/DriverRemoteConnection/GraphTraversalTransactionTests.cs b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Process/Traversal/DriverRemoteConnection/GraphTraversalTransactionTests.cs deleted file mode 100644 index 7d9383331da..00000000000 --- a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Process/Traversal/DriverRemoteConnection/GraphTraversalTransactionTests.cs +++ /dev/null @@ -1,96 +0,0 @@ -#region License - -/* - * 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. - */ - -#endregion - -using System; -using System.Threading.Tasks; -using Gremlin.Net.Process.Remote; -using Gremlin.Net.Process.Traversal; -using Xunit; - -namespace Gremlin.Net.IntegrationTest.Process.Traversal.DriverRemoteConnection -{ - public class GraphTraversalTransactionTests : IDisposable - { - private readonly IRemoteConnection _connection = new RemoteConnectionFactory().CreateRemoteConnection("gtx"); - - // TODO: update after tx is implemented in 4.0 - [Fact(Skip = "transaction-related tests are disabled until Tx implemented")] - public async Task ShouldSupportRemoteTransactionsCommit() - { - var g = AnonymousTraversalSource.Traversal().With(_connection); - var tx = g.Tx(); - var gtx = tx.Begin(); - await gtx.AddV("person").Property("name", "jorge").Promise(t => t.Iterate()).ConfigureAwait(false); - await gtx.AddV("person").Property("name", "josh").Promise(t => t.Iterate()).ConfigureAwait(false); - - // Assert within the transaction - var count = await gtx.V().Count().Promise(t => t.Next()).ConfigureAwait(false); - Assert.Equal(2, count); - - // Vertices should not be visible in a different transaction before commiting - count = await g.V().Count().Promise(t => t.Next()).ConfigureAwait(false); - Assert.Equal(0, count); - - // Now commit changes to test outside of the transaction - await tx.CommitAsync().ConfigureAwait(false); - - count = await g.V().Count().Promise(t => t.Next()).ConfigureAwait(false); - Assert.Equal(2, count); - } - - // TODO: update after tx is implemented in 4.0 - [Fact(Skip = "transaction-related tests are disabled until Tx implemented")] - public async Task ShouldSupportRemoteTransactionsRollback() - { - var g = AnonymousTraversalSource.Traversal().With(_connection); - var tx = g.Tx(); - var gtx = tx.Begin(); - await gtx.AddV("person").Property("name", "jorge").Promise(t => t.Iterate()).ConfigureAwait(false); - await gtx.AddV("person").Property("name", "josh").Promise(t => t.Iterate()).ConfigureAwait(false); - - // Assert within the transaction - var count = await gtx.V().Count().Promise(t => t.Next()).ConfigureAwait(false); - Assert.Equal(2, count); - - // Now rollback changes to test outside of the transaction - await tx.RollbackAsync().ConfigureAwait(false); - - count = await g.V().Count().Promise(t => t.Next()).ConfigureAwait(false); - Assert.Equal(0, count); - - g.V().Count().Next(); - } - - public void Dispose() - { - EmptyGraph(); - } - - private void EmptyGraph() - { - var g = AnonymousTraversalSource.Traversal().With(_connection); - g.V().Drop().Iterate(); - } - } - -} \ No newline at end of file diff --git a/gremlin-dotnet/test/Gremlin.Net.UnitTest/Process/Remote/RemoteTransactionTests.cs b/gremlin-dotnet/test/Gremlin.Net.UnitTest/Process/Remote/RemoteTransactionTests.cs deleted file mode 100644 index 14eb904d603..00000000000 --- a/gremlin-dotnet/test/Gremlin.Net.UnitTest/Process/Remote/RemoteTransactionTests.cs +++ /dev/null @@ -1,44 +0,0 @@ -#region License - -/* - * 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. - */ - -#endregion - -using System; -using Gremlin.Net.Driver; -using Gremlin.Net.Driver.Remote; -using Gremlin.Net.Process.Traversal; -using NSubstitute; -using Xunit; - -namespace Gremlin.Net.UnitTest.Process.Remote -{ - public class RemoteTransactionTests - { - [Fact] - public void ShouldThrowNotSupportedExceptionOnTx() - { - var g = AnonymousTraversalSource.Traversal() - .With(new DriverRemoteConnection(Substitute.For())); - - Assert.Throws(() => g.Tx()); - } - } -} \ No newline at end of file diff --git a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/HttpRemoteTransaction.java b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/HttpRemoteTransaction.java index fb4dde34c84..61e2d6bbcd7 100644 --- a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/HttpRemoteTransaction.java +++ b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/HttpRemoteTransaction.java @@ -65,7 +65,10 @@ public class HttpRemoteTransaction implements RemoteTransaction { private static final Logger logger = LoggerFactory.getLogger(HttpRemoteTransaction.class); private static final long CLOSING_MAX_WAIT_MS = 10000; - protected Consumer closeConsumer = CLOSE_BEHAVIOR.COMMIT; + // Rollback-on-close is the default because non-Java GLVs don't support configurable close behavior and must have a + // single default. Rollback is the safest choice since partial work is discarded rather than accidentally persisted + // if a user forgets to call commit(). + protected Consumer closeConsumer = CLOSE_BEHAVIOR.ROLLBACK; private final Client.PinnedClient pinnedClient; private final Cluster cluster; private final Host pinnedHost; diff --git a/gremlin-go/driver/client.go b/gremlin-go/driver/client.go index 5bee34ed8e3..e0e508c29c9 100644 --- a/gremlin-go/driver/client.go +++ b/gremlin-go/driver/client.go @@ -140,6 +140,14 @@ func (client *Client) errorCallback() { client.logHandler.log(Error, errorCallback) } +// Transact creates a new Transaction for executing operations within an explicit +// server-side transaction. Transactions are short-lived and single-use: after +// commit or rollback, create a new Transaction for the next unit of work. +// The traversal source (g alias) is inherited from this Client. +func (client *Client) Transact() *Transaction { + return &Transaction{client: client} +} + // SubmitWithOptions submits a Gremlin script to the server with specified RequestOptions and returns a ResultSet. func (client *Client) SubmitWithOptions(traversalString string, requestOptions RequestOptions) (ResultSet, error) { client.logHandler.logf(Debug, submitStartedString, traversalString) @@ -161,25 +169,31 @@ func (client *Client) Submit(traversalString string, bindings ...map[string]inte // submitGremlinLang submits GremlinLang to the server to execute and returns a ResultSet. func (client *Client) submitGremlinLang(gremlinLang *GremlinLang) (ResultSet, error) { + return client.submitGremlinLangWithBuilder(gremlinLang, new(RequestOptionsBuilder)) +} + +// submitGremlinLangWithBuilder is the core submission function. It extracts bindings +// and options strategies from the GremlinLang, merges them into the provided builder, +// then builds and sends the request. +func (client *Client) submitGremlinLangWithBuilder(gremlinLang *GremlinLang, builder *RequestOptionsBuilder) (ResultSet, error) { client.logHandler.logf(Debug, submitStartedString, *gremlinLang) - requestOptionsBuilder := new(RequestOptionsBuilder) parametersString := gremlinLang.GetParametersAsString() if parametersString != "[:]" { - requestOptionsBuilder.SetBindingsString(parametersString) + builder.SetBindingsString(parametersString) } if len(gremlinLang.optionsStrategies) > 0 { - requestOptionsBuilder = applyOptionsConfig(requestOptionsBuilder, gremlinLang.optionsStrategies[0].configuration) + builder = applyOptionsConfig(builder, gremlinLang.optionsStrategies[0].configuration) } // default bulkResults to true when using DRC through request options // consistent with Java RequestOptions.getRequestOptions and Python extract_request_options - if requestOptionsBuilder.bulkResults == nil { - requestOptionsBuilder.SetBulkResults(true) + if builder.bulkResults == nil { + builder.SetBulkResults(true) } - request := MakeStringRequest(gremlinLang.GetGremlin(), client.traversalSource, requestOptionsBuilder.Create()) + request := MakeStringRequest(gremlinLang.GetGremlin(), client.traversalSource, builder.Create()) return client.conn.submit(&request) } diff --git a/gremlin-go/driver/connection.go b/gremlin-go/driver/connection.go index 8354ec0574a..ea0183626d7 100644 --- a/gremlin-go/driver/connection.go +++ b/gremlin-go/driver/connection.go @@ -192,6 +192,9 @@ func (c *connection) sendRequest(req *RequestMessage) (*http.Response, error) { // Set Body to the raw *RequestMessage so interceptors can inspect/modify it httpReq.Body = req + // Promote transactionId to HTTP header before interceptors and serialization + c.promoteTransactionIdHeader(httpReq, req) + // Apply interceptors — they see *RequestMessage in Body (pre-serialization). // Interceptors may replace Body with []byte, io.Reader, or *http.Request. for _, interceptor := range c.interceptors { @@ -284,6 +287,20 @@ func (c *connection) setHttpRequestHeaders(req *HttpRequest) { } } +// promoteTransactionIdHeader extracts transactionId from the request message +// fields and sets it as the X-Transaction-Id HTTP header. The field remains in +// the body for dual transmission per the HTTP transaction protocol spec. +func (c *connection) promoteTransactionIdHeader(req *HttpRequest, msg *RequestMessage) { + if msg == nil { + return + } + if txId, ok := msg.Fields["transactionId"]; ok { + if txIdStr, ok := txId.(string); ok && txIdStr != "" { + req.Headers.Set("X-Transaction-Id", txIdStr) + } + } +} + func (c *connection) getReader(resp *http.Response) (io.Reader, io.Closer, error) { if resp.Header.Get("Content-Encoding") == "deflate" { zr, err := zlib.NewReader(resp.Body) diff --git a/gremlin-go/driver/connection_test.go b/gremlin-go/driver/connection_test.go index 1b4620a3760..572cc954600 100644 --- a/gremlin-go/driver/connection_test.go +++ b/gremlin-go/driver/connection_test.go @@ -124,9 +124,9 @@ func initializeGraph(t *testing.T, url string, tls *tls.Config) *GraphTraversalS } func resetGraph(t *testing.T, g *GraphTraversalSource) { - defer func(remoteConnection *DriverRemoteConnection) { - remoteConnection.Close() - }(g.remoteConnection) + defer func() { + g.remoteConnection.Close() + }() // Drop the graph and check that it is empty. dropGraph(t, g) readCount(t, g, "", 0) diff --git a/gremlin-go/driver/error_codes.go b/gremlin-go/driver/error_codes.go index 6c0dea424c0..380e73664e3 100644 --- a/gremlin-go/driver/error_codes.go +++ b/gremlin-go/driver/error_codes.go @@ -85,11 +85,13 @@ const ( // Bytecode.go errors err1001ConvertArgumentChildTraversalNotFromAnonError errorCode = "E1001_BYTECODE_CHILD_T_NOT_ANON_ERROR" - // graphTraversal.go errors + // transaction.go errors err1101TransactionRepeatedOpenError errorCode = "E1101_TRANSACTION_REPEATED_OPEN_ERROR" err1102TransactionRollbackNotOpenedError errorCode = "E1102_TRANSACTION_ROLLBACK_NOT_OPENED_ERROR" err1103TransactionCommitNotOpenedError errorCode = "E1103_TRANSACTION_COMMIT_NOT_OPENED_ERROR" err1104TransactionRepeatedCloseError errorCode = "E1104_TRANSACTION_REPEATED_CLOSE_ERROR" + err1105TransactionBeginFailedError errorCode = "E1105_TRANSACTION_BEGIN_FAILED_ERROR" + err1106TransactionNotOpenError errorCode = "E1106_TRANSACTION_NOT_OPEN_ERROR" ) var localizer *i18n.Localizer diff --git a/gremlin-go/driver/graphTraversal.go b/gremlin-go/driver/graphTraversal.go index 03aa6827062..e4782138978 100644 --- a/gremlin-go/driver/graphTraversal.go +++ b/gremlin-go/driver/graphTraversal.go @@ -21,7 +21,6 @@ package gremlingo import ( "math" - "sync" ) type Lambda struct { @@ -36,7 +35,7 @@ type GraphTraversal struct { // NewGraphTraversal make a new GraphTraversal. // why is this taking a non exported field as an exported function - remove or replace? -func NewGraphTraversal(graph *Graph, gremlinLang *GremlinLang, remote *DriverRemoteConnection) *GraphTraversal { +func NewGraphTraversal(graph *Graph, gremlinLang *GremlinLang, remote remoteConnection) *GraphTraversal { if gremlinLang == nil { gremlinLang = NewGremlinLang(nil) } @@ -930,75 +929,3 @@ func int32Args(args []interface{}) []interface{} { } return args } - -type Transaction struct { - g *GraphTraversalSource - remoteConnection *DriverRemoteConnection - isOpen bool - mutex sync.Mutex -} - -func (t *Transaction) Begin() (*GraphTraversalSource, error) { - t.mutex.Lock() - defer t.mutex.Unlock() - - if err := t.verifyTransactionState(false, newError(err1101TransactionRepeatedOpenError)); err != nil { - return nil, err - } - - gts := &GraphTraversalSource{ - graph: t.g.graph, - gremlinLang: t.g.gremlinLang} - return gts, nil -} - -func (t *Transaction) Rollback() error { - t.mutex.Lock() - defer t.mutex.Unlock() - - if err := t.verifyTransactionState(true, newError(err1102TransactionRollbackNotOpenedError)); err != nil { - return err - } - - t.close() - return nil -} - -func (t *Transaction) Commit() error { - t.mutex.Lock() - defer t.mutex.Unlock() - - if err := t.verifyTransactionState(true, newError(err1103TransactionCommitNotOpenedError)); err != nil { - return err - } - - t.close() - return nil -} - -func (t *Transaction) Close() error { - t.mutex.Lock() - defer t.mutex.Unlock() - - if err := t.verifyTransactionState(true, newError(err1104TransactionRepeatedCloseError)); err != nil { - return err - } - - t.close() - return nil -} - -func (t *Transaction) IsOpen() bool { - return t.isOpen -} - -func (t *Transaction) verifyTransactionState(state bool, err error) error { - if t.IsOpen() != state { - return err - } - return nil -} - -func (t *Transaction) close() { - t.isOpen = false -} diff --git a/gremlin-go/driver/graphTraversalSource.go b/gremlin-go/driver/graphTraversalSource.go index c550bd9556a..c03fefa0698 100644 --- a/gremlin-go/driver/graphTraversalSource.go +++ b/gremlin-go/driver/graphTraversalSource.go @@ -27,17 +27,24 @@ func convertStrategyVarargs(strategies []TraversalStrategy) []interface{} { return converted } +// remoteConnection abstracts the submission of traversals to a remote server. +// Both DriverRemoteConnection and transactionRemoteConnection satisfy this interface. +type remoteConnection interface { + submitGremlinLang(gremlinLang *GremlinLang) (ResultSet, error) + Close() +} + // GraphTraversalSource can be used to start GraphTraversal. type GraphTraversalSource struct { graph *Graph gremlinLang *GremlinLang - remoteConnection *DriverRemoteConnection + remoteConnection remoteConnection graphTraversal *GraphTraversal } // NewGraphTraversalSource creates a new GraphTraversalSource from a Graph, DriverRemoteConnection, and any TraversalStrategy. // Graph and DriverRemoteConnection can be set to nil as valid default values. -func NewGraphTraversalSource(graph *Graph, remoteConnection *DriverRemoteConnection, +func NewGraphTraversalSource(graph *Graph, remoteConnection remoteConnection, traversalStrategies ...TraversalStrategy) *GraphTraversalSource { // TODO: revisit when updating strategies gl := NewGremlinLang(nil) @@ -62,7 +69,7 @@ func (gts *GraphTraversalSource) clone() *GraphTraversalSource { return cloneGraphTraversalSource(gts.graph, NewGremlinLang(gts.gremlinLang), gts.remoteConnection) } -func cloneGraphTraversalSource(graph *Graph, gl *GremlinLang, remoteConnection *DriverRemoteConnection) *GraphTraversalSource { +func cloneGraphTraversalSource(graph *Graph, gl *GremlinLang, remoteConnection remoteConnection) *GraphTraversalSource { return &GraphTraversalSource{graph: graph, gremlinLang: gl, remoteConnection: remoteConnection, @@ -230,5 +237,17 @@ func (gts *GraphTraversalSource) Union(args ...interface{}) *GraphTraversal { } func (gts *GraphTraversalSource) Tx() *Transaction { - return &Transaction{g: gts, remoteConnection: gts.remoteConnection} + if gts.remoteConnection == nil { + panic("remote connection is required for transactions") + } + // If this GTS is already bound to a transaction, return that transaction. + if txRC, ok := gts.remoteConnection.(*transactionRemoteConnection); ok { + return txRC.transaction + } + // Extract the Client from the DriverRemoteConnection. + drc, ok := gts.remoteConnection.(*DriverRemoteConnection) + if !ok { + panic("transactions require a DriverRemoteConnection") + } + return &Transaction{client: drc.client} } diff --git a/gremlin-go/driver/request.go b/gremlin-go/driver/request.go index 915cc79f829..65f8662cf0c 100644 --- a/gremlin-go/driver/request.go +++ b/gremlin-go/driver/request.go @@ -77,6 +77,10 @@ func MakeStringRequest(stringGremlin string, traversalSource string, requestOpti newFields["bulkResults"] = strconv.FormatBool(*requestOptions.bulkResults) } + if requestOptions.transactionId != "" { + newFields["transactionId"] = requestOptions.transactionId + } + return RequestMessage{ Gremlin: stringGremlin, Fields: newFields, diff --git a/gremlin-go/driver/requestOptions.go b/gremlin-go/driver/requestOptions.go index 81b23703d2f..53863276d92 100644 --- a/gremlin-go/driver/requestOptions.go +++ b/gremlin-go/driver/requestOptions.go @@ -26,6 +26,7 @@ type RequestOptions struct { bindingsString string materializeProperties string bulkResults *bool + transactionId string } type RequestOptionsBuilder struct { @@ -36,6 +37,7 @@ type RequestOptionsBuilder struct { bindingsString string materializeProperties string bulkResults *bool + transactionId string } func (builder *RequestOptionsBuilder) SetEvaluationTimeout(evaluationTimeout int) *RequestOptionsBuilder { @@ -79,6 +81,11 @@ func (builder *RequestOptionsBuilder) SetBulkResults(bulkResults bool) *RequestO return builder } +func (builder *RequestOptionsBuilder) SetTransactionId(transactionId string) *RequestOptionsBuilder { + builder.transactionId = transactionId + return builder +} + func (builder *RequestOptionsBuilder) AddBinding(key string, binding interface{}) *RequestOptionsBuilder { if builder.bindingsString != "" { panic("cannot mix AddBinding() with SetBindingsString()") @@ -98,6 +105,7 @@ func (builder *RequestOptionsBuilder) Create() RequestOptions { requestOptions.userAgent = builder.userAgent requestOptions.materializeProperties = builder.materializeProperties requestOptions.bulkResults = builder.bulkResults + requestOptions.transactionId = builder.transactionId // convert map bindings to string at creation time, matching Java's RequestOptions.Builder.create() if builder.bindingsString != "" { diff --git a/gremlin-go/driver/resources/error-messages/en.json b/gremlin-go/driver/resources/error-messages/en.json index ecae8a1510a..fc33202bf5f 100644 --- a/gremlin-go/driver/resources/error-messages/en.json +++ b/gremlin-go/driver/resources/error-messages/en.json @@ -51,6 +51,8 @@ "E1102_TRANSACTION_ROLLBACK_NOT_OPENED_ERROR": "E1102: cannot rollback a transaction that is not started", "E1103_TRANSACTION_COMMIT_NOT_OPENED_ERROR": "E1103: cannot commit a transaction that is not started", "E1104_TRANSACTION_REPEATED_CLOSE_ERROR": "E1104: cannot close a transaction that has previously been closed", + "E1105_TRANSACTION_BEGIN_FAILED_ERROR": "E1105: failed to begin transaction: %v", + "E1106_TRANSACTION_NOT_OPEN_ERROR": "E1106: transaction is not open", "E1201_REQUEST_SIZE_EXCEEDS_WRITE_BUFFER_ERROR": "E1201: Request size cannot exceed the configured WriteBufferSize. Consider increasing WriteBufferSize or splitting into smaller requests" } diff --git a/gremlin-go/driver/transaction.go b/gremlin-go/driver/transaction.go new file mode 100644 index 00000000000..9b50bc13d95 --- /dev/null +++ b/gremlin-go/driver/transaction.go @@ -0,0 +1,195 @@ +/* +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 ( + "fmt" + "sync" +) + +// Transaction controls an explicit remote transaction. A thin wrapper around +// a Client that adds transaction lifecycle (begin/commit/rollback/close) and +// attaches a transactionId to every request. +// +// Created via Client.Transact() or GraphTraversalSource.Tx(). The traversal +// source (g alias) is inherited from the Client and cannot be changed. +// +// Transactions are short-lived and single-use. After commit or rollback, the +// transaction ID is invalid and the object cannot be reused. +// +// This struct is NOT safe for concurrent semantic use. The mutex serializes +// requests if a user accidentally shares the Transaction across goroutines. +type Transaction struct { + client *Client + transactionId string + isOpen bool + failed bool + mutex sync.Mutex +} + +func (t *Transaction) Begin() (*GraphTraversalSource, error) { + t.mutex.Lock() + defer t.mutex.Unlock() + + if t.isOpen || t.failed { + return nil, newError(err1101TransactionRepeatedOpenError) + } + + // Submit g.tx().begin() via the Client to obtain a server-generated transactionId + rs, err := t.client.SubmitWithOptions("g.tx().begin()", + RequestOptions{}) + if err != nil { + t.failed = true + return nil, newError(err1105TransactionBeginFailedError, err) + } + + results, err := rs.All() + if err != nil { + t.failed = true + return nil, newError(err1105TransactionBeginFailedError, err) + } + + txId, err := extractTransactionId(results) + if err != nil { + t.failed = true + return nil, err + } + + t.transactionId = txId + t.isOpen = true + + // Create a transaction-bound remote connection that injects transactionId + txDRC := &transactionRemoteConnection{ + transaction: t, + } + gts := &GraphTraversalSource{ + graph: nil, + gremlinLang: NewGremlinLang(nil), + remoteConnection: txDRC, + } + return gts, nil +} + +func (t *Transaction) Commit() error { + return t.closeTransaction("g.tx().commit()") +} + +func (t *Transaction) Rollback() error { + return t.closeTransaction("g.tx().rollback()") +} + +// Close rolls back the transaction if still open. This is the safe default: +// partial work is discarded rather than accidentally persisted. +func (t *Transaction) Close() error { + t.mutex.Lock() + defer t.mutex.Unlock() + + if !t.isOpen { + return nil + } + + return t.closeTransactionLocked("g.tx().rollback()") +} + +func (t *Transaction) IsOpen() bool { + t.mutex.Lock() + defer t.mutex.Unlock() + return t.isOpen +} + +// TransactionId returns the server-generated transaction ID, or empty string if not yet begun. +func (t *Transaction) TransactionId() string { + t.mutex.Lock() + defer t.mutex.Unlock() + return t.transactionId +} + +func (t *Transaction) closeTransaction(script string) error { + t.mutex.Lock() + defer t.mutex.Unlock() + + if !t.isOpen { + return newError(err1102TransactionRollbackNotOpenedError) + } + + return t.closeTransactionLocked(script) +} + +func (t *Transaction) closeTransactionLocked(script string) error { + opts := new(RequestOptionsBuilder) + opts.SetTransactionId(t.transactionId) + rs, err := t.client.SubmitWithOptions(script, opts.Create()) + if err != nil { + return err + } + // Drain the result set to ensure the response is fully consumed + _, err = rs.All() + if err != nil { + return err + } + // Only mark closed after server confirms success + t.isOpen = false + t.failed = true // Terminal state: transaction cannot be reused + return nil +} + +// Submit sends a plain gremlin-lang string within this transaction. +// This is the driver-level API for users who don't want to use the Traversal API. +// The transactionId is automatically attached. +func (t *Transaction) Submit(gremlin string, options ...RequestOptions) (ResultSet, error) { + t.mutex.Lock() + defer t.mutex.Unlock() + + if !t.isOpen { + return nil, newError(err1106TransactionNotOpenError) + } + + var opts RequestOptions + if len(options) > 0 { + opts = options[0] + } + opts.transactionId = t.transactionId + + return t.client.SubmitWithOptions(gremlin, opts) +} + +func extractTransactionId(results []*Result) (string, error) { + if len(results) == 0 { + return "", fmt.Errorf("server did not return transaction ID") + } + + resultVal := results[0].GetInterface() + resultMap, ok := resultVal.(map[interface{}]interface{}) + if !ok { + return "", fmt.Errorf("server did not return transaction ID in expected format") + } + + txId, ok := resultMap["transactionId"] + if !ok { + return "", fmt.Errorf("server did not return transaction ID in expected format") + } + + s, ok := txId.(string) + if !ok || s == "" { + return "", fmt.Errorf("server did not return transaction ID in expected format") + } + + return s, nil +} diff --git a/gremlin-go/driver/transactionRemoteConnection.go b/gremlin-go/driver/transactionRemoteConnection.go new file mode 100644 index 00000000000..59025df2bf1 --- /dev/null +++ b/gremlin-go/driver/transactionRemoteConnection.go @@ -0,0 +1,39 @@ +/* +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 + +// transactionRemoteConnection is a remote connection that attaches a +// transactionId to all requests, binding them to a server-side transaction. +type transactionRemoteConnection struct { + transaction *Transaction +} + +func (t *transactionRemoteConnection) Close() { + // No-op: the transaction lifecycle is managed by Transaction.Close/Commit/Rollback +} + +func (t *transactionRemoteConnection) submitGremlinLang(gremlinLang *GremlinLang) (ResultSet, error) { + if !t.transaction.isOpen { + return nil, newError(err1106TransactionNotOpenError) + } + builder := new(RequestOptionsBuilder) + builder.SetTransactionId(t.transaction.transactionId) + return t.transaction.client.submitGremlinLangWithBuilder(gremlinLang, builder) +} diff --git a/gremlin-go/driver/transaction_test.go b/gremlin-go/driver/transaction_test.go new file mode 100644 index 00000000000..a746fb3dcff --- /dev/null +++ b/gremlin-go/driver/transaction_test.go @@ -0,0 +1,529 @@ +/* +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 ( + "crypto/tls" + "testing" + + "github.com/stretchr/testify/assert" +) + +func newTxRemoteConnection(t *testing.T) *DriverRemoteConnection { + url := getEnvOrDefaultString("GREMLIN_SERVER_URL", noAuthUrl) + remote, err := NewDriverRemoteConnection(url, + func(settings *DriverRemoteConnectionSettings) { + settings.TlsConfig = &tls.Config{} + settings.TraversalSource = "gtx" + }) + assert.Nil(t, err) + assert.NotNil(t, remote) + return remote +} + +func newTxClient(t *testing.T) *Client { + url := getEnvOrDefaultString("GREMLIN_SERVER_URL", noAuthUrl) + client, err := NewClient(url, func(settings *ClientSettings) { + settings.TlsConfig = &tls.Config{} + settings.TraversalSource = "gtx" + }) + assert.Nil(t, err) + assert.NotNil(t, client) + return client +} + +// dropTxGraph removes all vertices from the transactional graph to prevent cross-test contamination. +func dropTxGraph(t *testing.T, client *Client) { + rs, err := client.Submit("g.V().drop()") + assert.Nil(t, err) + _, err = rs.All() + assert.Nil(t, err) +} + +func getTxCount(t *testing.T, client *Client, label string) int64 { + rs, err := client.Submit("g.V().hasLabel('" + label + "').count()") + assert.Nil(t, err) + results, err := rs.All() + assert.Nil(t, err) + assert.Equal(t, 1, len(results)) + val, err := results[0].GetInt64() + assert.Nil(t, err) + return val +} + +func TestTransactionCommit(t *testing.T) { + client := newTxClient(t) + defer client.Close() + dropTxGraph(t, client) + + tx := client.Transact() + _, err := tx.Begin() + assert.Nil(t, err) + assert.True(t, tx.IsOpen()) + + _, err = tx.Submit("g.addV('person').property('name','alice')") + assert.Nil(t, err) + + // Uncommitted data not visible outside the transaction + assert.Equal(t, int64(0), getTxCount(t, client, "person")) + + err = tx.Commit() + assert.Nil(t, err) + assert.False(t, tx.IsOpen()) + + // Committed data visible + assert.Equal(t, int64(1), getTxCount(t, client, "person")) +} + +func TestTransactionRollback(t *testing.T) { + client := newTxClient(t) + defer client.Close() + dropTxGraph(t, client) + + tx := client.Transact() + _, err := tx.Begin() + assert.Nil(t, err) + + _, err = tx.Submit("g.addV('person').property('name','bob')") + assert.Nil(t, err) + + err = tx.Rollback() + assert.Nil(t, err) + assert.False(t, tx.IsOpen()) + + // Data discarded + assert.Equal(t, int64(0), getTxCount(t, client, "person")) +} + +func TestTransactionIntraConsistency(t *testing.T) { + client := newTxClient(t) + defer client.Close() + dropTxGraph(t, client) + + tx := client.Transact() + _, err := tx.Begin() + assert.Nil(t, err) + + _, err = tx.Submit("g.addV('test').property('name','A')") + assert.Nil(t, err) + + // Read-your-own-writes + rs, err := tx.Submit("g.V().hasLabel('test').count()") + assert.Nil(t, err) + results, err := rs.All() + assert.Nil(t, err) + val, err := results[0].GetInt64() + assert.Nil(t, err) + assert.Equal(t, int64(1), val) + + _, err = tx.Submit("g.addV('test').property('name','B')") + assert.Nil(t, err) + + err = tx.Commit() + assert.Nil(t, err) + + assert.Equal(t, int64(2), getTxCount(t, client, "test")) +} + +func TestTransactionSubmitAfterCommit(t *testing.T) { + client := newTxClient(t) + defer client.Close() + + tx := client.Transact() + _, err := tx.Begin() + assert.Nil(t, err) + + _, err = tx.Submit("g.addV()") + assert.Nil(t, err) + + err = tx.Commit() + assert.Nil(t, err) + + _, err = tx.Submit("g.V().count()") + assert.NotNil(t, err) + assert.Contains(t, err.Error(), "transaction is not open") +} + +func TestTransactionSubmitAfterRollback(t *testing.T) { + client := newTxClient(t) + defer client.Close() + + tx := client.Transact() + _, err := tx.Begin() + assert.Nil(t, err) + + _, err = tx.Submit("g.addV()") + assert.Nil(t, err) + + err = tx.Rollback() + assert.Nil(t, err) + + _, err = tx.Submit("g.V().count()") + assert.NotNil(t, err) + assert.Contains(t, err.Error(), "transaction is not open") +} + +func TestTransactionDoubleBegin(t *testing.T) { + client := newTxClient(t) + defer client.Close() + + tx := client.Transact() + _, err := tx.Begin() + assert.Nil(t, err) + + _, err = tx.Begin() + assert.NotNil(t, err) +} + +func TestTransactionCommitWhenNotOpen(t *testing.T) { + client := newTxClient(t) + defer client.Close() + + tx := client.Transact() + err := tx.Commit() + assert.NotNil(t, err) +} + +func TestTransactionRollbackWhenNotOpen(t *testing.T) { + client := newTxClient(t) + defer client.Close() + + tx := client.Transact() + err := tx.Rollback() + assert.NotNil(t, err) +} + +func TestTransactionIdBeforeAndAfterBegin(t *testing.T) { + client := newTxClient(t) + defer client.Close() + + tx := client.Transact() + assert.Equal(t, "", tx.TransactionId()) + + _, err := tx.Begin() + assert.Nil(t, err) + assert.NotEqual(t, "", tx.TransactionId()) + assert.True(t, len(tx.TransactionId()) > 0) + + tx.Rollback() +} + +func TestTransactionRollbackOnCloseByDefault(t *testing.T) { + client := newTxClient(t) + defer client.Close() + dropTxGraph(t, client) + + tx := client.Transact() + _, err := tx.Begin() + assert.Nil(t, err) + + _, err = tx.Submit("g.addV('person').property('name','close_test')") + assert.Nil(t, err) + + err = tx.Close() + assert.Nil(t, err) + assert.False(t, tx.IsOpen()) + + // Data should NOT persist (rollback is default) + assert.Equal(t, int64(0), getTxCount(t, client, "person")) +} + +func TestTransactionIsolateConcurrent(t *testing.T) { + client := newTxClient(t) + defer client.Close() + dropTxGraph(t, client) + + tx1 := client.Transact() + _, err := tx1.Begin() + assert.Nil(t, err) + + tx2 := client.Transact() + _, err = tx2.Begin() + assert.Nil(t, err) + + _, err = tx1.Submit("g.addV('tx1')") + assert.Nil(t, err) + _, err = tx2.Submit("g.addV('tx2')") + assert.Nil(t, err) + + // tx1 should not see tx2's data + rs, err := tx1.Submit("g.V().hasLabel('tx2').count()") + assert.Nil(t, err) + results, _ := rs.All() + val, _ := results[0].GetInt64() + assert.Equal(t, int64(0), val) + + err = tx1.Commit() + assert.Nil(t, err) + err = tx2.Commit() + assert.Nil(t, err) + + assert.Equal(t, int64(1), getTxCount(t, client, "tx1")) + assert.Equal(t, int64(1), getTxCount(t, client, "tx2")) +} + +func TestTransactionSequentialStress(t *testing.T) { + client := newTxClient(t) + defer client.Close() + dropTxGraph(t, client) + + numTransactions := 50 + for i := 0; i < numTransactions; i++ { + tx := client.Transact() + _, err := tx.Begin() + assert.Nil(t, err) + _, err = tx.Submit("g.addV('stress')") + assert.Nil(t, err) + err = tx.Commit() + assert.Nil(t, err) + } + + assert.Equal(t, int64(numTransactions), getTxCount(t, client, "stress")) +} + +func TestTransactionKeepOpenAfterTraversalError(t *testing.T) { + client := newTxClient(t) + defer client.Close() + dropTxGraph(t, client) + + tx := client.Transact() + _, err := tx.Begin() + assert.Nil(t, err) + + _, err = tx.Submit("g.addV('good_vertex')") + assert.Nil(t, err) + + // Submit a bad traversal - GraphBinary errors surface when draining the ResultSet + // because the server responds with HTTP 200 + error in the body + rs, err := tx.Submit("g.V().fail()") + if err == nil { + _, err = rs.All() + } + assert.NotNil(t, err) + + // Transaction should still be open + assert.True(t, tx.IsOpen()) + err = tx.Rollback() + assert.Nil(t, err) + + assert.Equal(t, int64(0), getTxCount(t, client, "good_vertex")) +} + +func TestTransactionWithTraversalAPI(t *testing.T) { + remote := newTxRemoteConnection(t) + defer remote.Close() + + g := Traversal_().With(remote) + + tx := g.Tx() + gtx, err := tx.Begin() + assert.Nil(t, err) + + promise := gtx.AddV("val").Iterate() + assert.Nil(t, <-promise) + + err = tx.Commit() + assert.Nil(t, err) + + count, err := g.V().HasLabel("val").Count().ToList() + assert.Nil(t, err) + val, err := count[0].GetInt64() + assert.Nil(t, err) + assert.Equal(t, int64(1), val) +} + +func TestTransactionRejectBeginOnNonTransactionalGraph(t *testing.T) { + url := getEnvOrDefaultString("GREMLIN_SERVER_URL", noAuthUrl) + client, err := NewClient(url, func(settings *ClientSettings) { + settings.TlsConfig = &tls.Config{} + settings.TraversalSource = "gclassic" + }) + assert.Nil(t, err) + defer client.Close() + + tx := client.Transact() + _, err = tx.Begin() + assert.NotNil(t, err) + assert.Contains(t, err.Error(), "Graph does not support transactions") +} + +func TestTransactionCleanUpOnBeginFailure(t *testing.T) { + url := getEnvOrDefaultString("GREMLIN_SERVER_URL", noAuthUrl) + client, err := NewClient(url, func(settings *ClientSettings) { + settings.TlsConfig = &tls.Config{} + settings.TraversalSource = "gclassic" + }) + assert.Nil(t, err) + defer client.Close() + + tx := client.Transact() + _, err = tx.Begin() + assert.NotNil(t, err) + + // Transaction should not be open after failed begin + assert.False(t, tx.IsOpen()) + assert.Equal(t, "", tx.TransactionId()) + + // Cannot begin again + _, err = tx.Begin() + assert.NotNil(t, err) +} + +func TestTransactionReturnsSameTxFromGtxTx(t *testing.T) { + client := newTxClient(t) + defer client.Close() + dropTxGraph(t, client) + + remote := newTxRemoteConnection(t) + defer remote.Close() + g := Traversal_().With(remote) + + tx := g.Tx() + gtx, err := tx.Begin() + assert.Nil(t, err) + + sameTx := gtx.Tx() + assert.Equal(t, tx, sameTx) + + err = tx.Rollback() + assert.Nil(t, err) +} + +func TestTransactionBeginFromGtxTxThrows(t *testing.T) { + client := newTxClient(t) + defer client.Close() + + remote := newTxRemoteConnection(t) + defer remote.Close() + g := Traversal_().With(remote) + + tx := g.Tx() + gtx, err := tx.Begin() + assert.Nil(t, err) + + sameTx := gtx.Tx() + _, err = sameTx.Begin() + assert.NotNil(t, err) + + tx.Rollback() +} + +func TestTransactionCommitViaGtxTx(t *testing.T) { + client := newTxClient(t) + defer client.Close() + dropTxGraph(t, client) + + remote := newTxRemoteConnection(t) + defer remote.Close() + g := Traversal_().With(remote) + + tx := g.Tx() + gtx, err := tx.Begin() + assert.Nil(t, err) + + promise := gtx.AddV("gtx_commit_test").Iterate() + assert.Nil(t, <-promise) + + err = gtx.Tx().Commit() + assert.Nil(t, err) + + assert.Equal(t, int64(1), getTxCount(t, client, "gtx_commit_test")) +} + +func TestTransactionDoubleCommit(t *testing.T) { + client := newTxClient(t) + defer client.Close() + dropTxGraph(t, client) + + tx := client.Transact() + _, err := tx.Begin() + assert.Nil(t, err) + err = tx.Commit() + assert.Nil(t, err) + + err = tx.Commit() + assert.NotNil(t, err) +} + +func TestTransactionDoubleRollback(t *testing.T) { + client := newTxClient(t) + defer client.Close() + dropTxGraph(t, client) + + tx := client.Transact() + _, err := tx.Begin() + assert.Nil(t, err) + err = tx.Rollback() + assert.Nil(t, err) + + err = tx.Rollback() + assert.NotNil(t, err) +} + +func TestTransactionIsolateFromNonTx(t *testing.T) { + client := newTxClient(t) + defer client.Close() + dropTxGraph(t, client) + + tx := client.Transact() + _, err := tx.Begin() + assert.Nil(t, err) + + _, err = tx.Submit("g.addV('tx_data')") + assert.Nil(t, err) + + // Non-transactional read should not see uncommitted data + assert.Equal(t, int64(0), getTxCount(t, client, "tx_data")) + + err = tx.Commit() + assert.Nil(t, err) + + assert.Equal(t, int64(1), getTxCount(t, client, "tx_data")) +} + +func TestTransactionBeginAfterCommit(t *testing.T) { + client := newTxClient(t) + defer client.Close() + dropTxGraph(t, client) + + tx := client.Transact() + _, err := tx.Begin() + assert.Nil(t, err) + err = tx.Commit() + assert.Nil(t, err) + + _, err = tx.Begin() + assert.NotNil(t, err) +} + +func TestTransactionBeginAfterRollback(t *testing.T) { + client := newTxClient(t) + defer client.Close() + dropTxGraph(t, client) + + tx := client.Transact() + _, err := tx.Begin() + assert.Nil(t, err) + err = tx.Rollback() + assert.Nil(t, err) + + _, err = tx.Begin() + assert.NotNil(t, err) +} diff --git a/gremlin-go/driver/traversal.go b/gremlin-go/driver/traversal.go index 3ea36875a74..a78f6ee9c2b 100644 --- a/gremlin-go/driver/traversal.go +++ b/gremlin-go/driver/traversal.go @@ -36,7 +36,7 @@ type Traverser struct { type Traversal struct { graph *Graph GremlinLang *GremlinLang - remote *DriverRemoteConnection + remote remoteConnection results ResultSet lastTraverser *Traverser // current traverser being lazily unrolled } diff --git a/gremlin-go/driver/traversal_test.go b/gremlin-go/driver/traversal_test.go index bc3fb339163..83c264ec333 100644 --- a/gremlin-go/driver/traversal_test.go +++ b/gremlin-go/driver/traversal_test.go @@ -55,320 +55,6 @@ func TestTraversal(t *testing.T) { assert.NotNil(t, <-promise) }) - // TODO enable when transaction is implemented - //t.Run("Test Transaction commit", func(t *testing.T) { - // // Start a transaction traversal. - // remote := newConnection(t) - // g := Traversal_().With(remote) - // startCount := getCount(t, g) - // tx := g.Tx() - // - // // Except transaction to not be open until begin is called. - // assert.False(t, tx.IsOpen()) - // gtx, _ := tx.Begin() - // assert.True(t, tx.IsOpen()) - // - // addV(t, gtx, "lyndon") - // addV(t, gtx, "valentyn") - // assert.Equal(t, startCount, getCount(t, g)) - // assert.Equal(t, startCount+2, getCount(t, gtx)) - // - // // Commit the transaction, this should close it. - // // Our vertex count outside the transaction should be 2 + the start count. - // err := tx.Commit() - // assert.Nil(t, err) - // - // assert.False(t, tx.IsOpen()) - // assert.Equal(t, startCount+2, getCount(t, g)) - // - // dropGraphCheckCount(t, g) - // verifyGtxClosed(t, gtx) - //}) - // - //t.Run("Test Transaction rollback", func(t *testing.T) { - // // Start a transaction traversal. - // remote := newConnection(t) - // g := Traversal_().With(remote) - // startCount := getCount(t, g) - // tx := g.Tx() - // - // // Except transaction to not be open until begin is called. - // assert.False(t, tx.IsOpen()) - // gtx, _ := tx.Begin() - // assert.True(t, tx.IsOpen()) - // - // addV(t, gtx, "lyndon") - // addV(t, gtx, "valentyn") - // assert.Equal(t, startCount, getCount(t, g)) - // assert.Equal(t, startCount+2, getCount(t, gtx)) - // - // // Rollback the transaction, this should close it. - // // Our vertex count outside the transaction should be the start count. - // err := tx.Rollback() - // assert.Nil(t, err) - // - // assert.False(t, tx.IsOpen()) - // assert.Equal(t, startCount, getCount(t, g)) - // - // dropGraphCheckCount(t, g) - // verifyGtxClosed(t, gtx) - //}) - // - //t.Run("Test Transaction flows", func(t *testing.T) { - // // Start a transaction traversal. - // remote := newConnection(t) - // g := Traversal_().With(remote) - // tx := g.Tx() - // assert.False(t, tx.IsOpen()) - // - // // Commit should return error when transaction not started - // err := tx.Commit() - // assert.NotNil(t, err) - // - // // Rollback should return error when transaction not started - // err = tx.Rollback() - // assert.NotNil(t, err) - // - // // Create transaction and verify it is open. - // gtx, err := tx.Begin() - // assert.Nil(t, err) - // assert.NotNil(t, gtx) - // assert.True(t, tx.IsOpen()) - // - // // Can't open inner transaction. - // innerTx, err := gtx.Tx().Begin() - // assert.Nil(t, innerTx) - // assert.NotNil(t, err) - // - // // Commit this unused transaction and verify it is no longer open. - // err = tx.Commit() - // assert.Nil(t, err) - // assert.False(t, tx.IsOpen()) - // - // // Create another transaction and verify it is open. - // gtx, err = tx.Begin() - // assert.Nil(t, err) - // assert.NotNil(t, gtx) - // assert.True(t, tx.IsOpen()) - // - // // Rollback this unused transaction and verify it is no longer open. - // err = tx.Rollback() - // assert.Nil(t, err) - // assert.False(t, tx.IsOpen()) - //}) - // - //t.Run("Test multi commit Transaction", func(t *testing.T) { - // // Start a transaction traversal. - // remote := newConnection(t) - // g := Traversal_().With(remote) - // startCount := getCount(t, g) - // - // // Create two transactions. - // tx1 := g.Tx() - // tx2 := g.Tx() - // - // // Generate two GraphTraversalSource's for each transaction with begin. - // gtx1, _ := tx1.Begin() - // gtx2, _ := tx2.Begin() - // verifyTxState(t, true, tx1, tx2) - // - // // Add node to gtx1, which should be visible to gtx1, not gtx2. - // addNodeValidateTransactionState(t, g, gtx1, startCount, startCount, tx1, tx2) - // - // // Add node to gtx2, which should be visible to gtx2, not gtx1 - // addNodeValidateTransactionState(t, g, gtx2, startCount, startCount, tx1, tx2) - // - // // Add node to gtx1, which should be visible to gtx1, not gtx2. Note previous node also added. - // addNodeValidateTransactionState(t, g, gtx1, startCount, startCount+1, tx1, tx2) - // - // tx1.Commit() - // verifyTxState(t, false, tx1) - // verifyTxState(t, true, tx2) - // assert.Equal(t, startCount+2, getCount(t, g)) - // - // tx2.Commit() - // verifyTxState(t, false, tx1, tx2) - // assert.Equal(t, startCount+3, getCount(t, g)) - //}) - // - //t.Run("Test multi rollback Transaction", func(t *testing.T) { - // // Start a transaction traversal. - // remote := newConnection(t) - // g := Traversal_().With(remote) - // startCount := getCount(t, g) - // - // // Create two transactions. - // tx1 := g.Tx() - // tx2 := g.Tx() - // - // // Generate two GraphTraversalSource's for each transaction with begin. - // gtx1, _ := tx1.Begin() - // gtx2, _ := tx2.Begin() - // verifyTxState(t, true, tx1, tx2) - // - // // Add node to gtx1, which should be visible to gtx1, not gtx2. - // addNodeValidateTransactionState(t, g, gtx1, startCount, startCount, tx1, tx2) - // - // // Add node to gtx2, which should be visible to gtx2, not gtx1 - // addNodeValidateTransactionState(t, g, gtx2, startCount, startCount, tx1, tx2) - // - // // Add node to gtx1, which should be visible to gtx1, not gtx2. Note previous node also added. - // addNodeValidateTransactionState(t, g, gtx1, startCount, startCount+1, tx1, tx2) - // - // tx1.Rollback() - // verifyTxState(t, false, tx1) - // verifyTxState(t, true, tx2) - // assert.Equal(t, startCount, getCount(t, g)) - // - // tx2.Rollback() - // verifyTxState(t, false, tx1, tx2) - // assert.Equal(t, startCount, getCount(t, g)) - //}) - // - //t.Run("Test multi commit and rollback Transaction", func(t *testing.T) { - // // Start a transaction traversal. - // remote := newConnection(t) - // g := Traversal_().With(remote) - // startCount := getCount(t, g) - // - // // Create two transactions. - // tx1 := g.Tx() - // tx2 := g.Tx() - // - // // Generate two GraphTraversalSource's for each transaction with begin. - // gtx1, _ := tx1.Begin() - // gtx2, _ := tx2.Begin() - // verifyTxState(t, true, tx1, tx2) - // - // // Add node to gtx1, which should be visible to gtx1, not gtx2. - // addNodeValidateTransactionState(t, g, gtx1, startCount, startCount, tx1, tx2) - // - // // Add node to gtx2, which should be visible to gtx2, not gtx1 - // addNodeValidateTransactionState(t, g, gtx2, startCount, startCount, tx1, tx2) - // - // // Add node to gtx1, which should be visible to gtx1, not gtx2. Note previous node also added. - // addNodeValidateTransactionState(t, g, gtx1, startCount, startCount+1, tx1, tx2) - // - // tx1.Commit() - // verifyTxState(t, false, tx1) - // verifyTxState(t, true, tx2) - // assert.Equal(t, startCount+2, getCount(t, g)) - // - // tx2.Rollback() - // verifyTxState(t, false, tx1, tx2) - // assert.Equal(t, startCount+2, getCount(t, g)) - //}) - // - //t.Run("Test Transaction close", func(t *testing.T) { - // // Start a transaction traversal. - // remote := newConnection(t) - // g := Traversal_().With(remote) - // dropGraphCheckCount(t, g) - // - // // Create two transactions. - // tx1 := g.Tx() - // tx2 := g.Tx() - // - // // Generate two GraphTraversalSource's for each transaction with begin. - // gtx1, _ := tx1.Begin() - // gtx2, _ := tx2.Begin() - // verifyTxState(t, true, tx1, tx2) - // - // // Add stuff to both gtx. - // addNodeValidateTransactionState(t, g, gtx1, 0, 0, tx1, tx2) - // addNodeValidateTransactionState(t, g, gtx2, 0, 0, tx1, tx2) - // addNodeValidateTransactionState(t, g, gtx2, 0, 1, tx1, tx2) - // addNodeValidateTransactionState(t, g, gtx2, 0, 2, tx1, tx2) - // - // // someone gets lazy and doesn't commit/rollback and just calls close() - the graph - // // will decide how to treat the transaction - // tx1.Close() - // tx2.Close() - // - // verifyGtxClosed(t, gtx1) - // verifyGtxClosed(t, gtx2) - // - // remote = newConnection(t) - // g = Traversal_().With(remote) - // assert.Equal(t, int32(0), getCount(t, g)) - //}) - // - //t.Run("Test Transaction close tx from parent", func(t *testing.T) { - // // Start a transaction traversal. - // remote := newConnection(t) - // g := Traversal_().With(remote) - // dropGraphCheckCount(t, g) - // - // // Create two transactions. - // tx1 := g.Tx() - // tx2 := g.Tx() - // - // // Generate two GraphTraversalSource's for each transaction with begin. - // gtx1, _ := tx1.Begin() - // gtx2, _ := tx2.Begin() - // verifyTxState(t, true, tx1, tx2) - // - // // Add stuff to both gtx. - // addNodeValidateTransactionState(t, g, gtx1, 0, 0, tx1, tx2) - // addNodeValidateTransactionState(t, g, gtx2, 0, 0, tx1, tx2) - // addNodeValidateTransactionState(t, g, gtx2, 0, 1, tx1, tx2) - // addNodeValidateTransactionState(t, g, gtx2, 0, 2, tx1, tx2) - // - // // someone gets lazy and doesn't commit/rollback and just calls Close() but on the parent - // // DriverRemoteConnection for all the session that were created via Tx() - the graph - // // will decide how to treat the transaction - // remote.Close() - // - // assert.False(t, tx1.IsOpen()) - // assert.False(t, tx2.IsOpen()) - // verifyGtxClosed(t, gtx1) - // verifyGtxClosed(t, gtx2) - // - // remote = newConnection(t) - // g = Traversal_().With(remote) - // assert.Equal(t, int32(0), getCount(t, g)) - //}) - // - //t.Run("Test commit if no transaction started", func(t *testing.T) { - // // Start a traversal. - // g := newWithOptionsConnection(t) - // - // // Create transactions - // tx := g.Tx() - // - // // try to commit - // err := tx.Commit() - // assert.Equal(t, "E1103: cannot commit a transaction that is not started", err.Error()) - //}) - // - //t.Run("Test rollback if no transaction started", func(t *testing.T) { - // // Start a traversal. - // g := newWithOptionsConnection(t) - // - // // Create transactions - // tx := g.Tx() - // - // // try to rollback - // err := tx.Rollback() - // assert.Equal(t, "E1102: cannot rollback a transaction that is not started", err.Error()) - //}) - // - //t.Run("Test commit if no transaction support for Graph", func(t *testing.T) { - // // Start a traversal. - // g := newWithOptionsConnection(t) - // - // // Create transactions - // tx := g.Tx() - // - // _, err := tx.Begin() - // assert.Nil(t, err) - // - // // try to commit - // err = tx.Commit() - // assert.True(t, strings.HasPrefix(err.Error(), - // "E0502: error in read loop, error message '{code:244 message:Graph does not support transactions")) - //}) - t.Run("Test WithOptions.Tokens WithOptions.None", func(t *testing.T) { skipTestsIfNotEnabled(t, integrationTestSuiteName, getEnvOrDefaultBool("RUN_INTEGRATION_WITH_ALIAS_TESTS", true)) @@ -813,39 +499,6 @@ func newTestRemoteConnection(t *testing.T) *DriverRemoteConnection { return remote } -func addNodeValidateTransactionState(t *testing.T, g, gAddTo *GraphTraversalSource, - gStartCount, gAddToStartCount int32, txVerifyList ...*Transaction) { - // Add a single node to gAddTo, but not g. - // Check that vertex count in g is gStartCount and vertex count in gAddTo is gAddToStartCount + 1. - addV(t, gAddTo, "lyndon") - assert.Equal(t, gAddToStartCount+1, getCount(t, gAddTo)) - assert.Equal(t, gStartCount, getCount(t, g)) - verifyTxState(t, true, txVerifyList...) -} - -func verifyTxState(t *testing.T, expected bool, gtxList ...*Transaction) { - for _, tx := range gtxList { - assert.Equal(t, expected, tx.IsOpen()) - } -} - -func addV(t *testing.T, g *GraphTraversalSource, name string) { - promise := g.AddV("person").Property("name", name).Iterate() - assert.Nil(t, <-promise) -} - -func dropGraphCheckCount(t *testing.T, g *GraphTraversalSource) { - dropGraph(t, g) - assert.Equal(t, int32(0), getCount(t, g)) -} - -func verifyGtxClosed(t *testing.T, gtx *GraphTraversalSource) { - // Attempt to add an additional vertex to the transaction. This should return an error since it - // has been closed. - promise := gtx.AddV("failure").Iterate() - assert.NotNil(t, <-promise) -} - func getCount(t *testing.T, g *GraphTraversalSource) int32 { count, err := g.V().Count().ToList() assert.Nil(t, err) diff --git a/gremlin-js/gremlin-javascript/lib/driver/client.ts b/gremlin-js/gremlin-javascript/lib/driver/client.ts index 47acaa2dc21..c45aaff25ca 100644 --- a/gremlin-js/gremlin-javascript/lib/driver/client.ts +++ b/gremlin-js/gremlin-javascript/lib/driver/client.ts @@ -19,6 +19,7 @@ import Connection, { ConnectionOptions } from './connection.js'; import {RequestMessage} from "./request-message.js"; +import { Transaction } from '../process/transaction.js'; export type RequestOptions = { bindings?: any; @@ -30,6 +31,7 @@ export type RequestOptions = { materializeProperties?: string; bulkResults?: boolean; params?: Record; + transactionId?: string; }; export type ClientOptions = ConnectionOptions & RequestOptions & { processor?: string }; @@ -46,8 +48,8 @@ export default class Client { * @param {ClientOptions} [options] The connection options. */ constructor( - url: string, - private readonly options: ClientOptions = {}, + readonly url: string, + readonly options: ClientOptions = {}, ) { this._connection = new Connection(url, options); } @@ -122,6 +124,9 @@ export default class Client { if (requestOptions?.bulkResults) { requestBuilder.addBulkResults(requestOptions.bulkResults); } + if (requestOptions?.transactionId) { + requestBuilder.addField('transactionId', requestOptions.transactionId); + } return requestBuilder.create(); } @@ -134,6 +139,17 @@ export default class Client { return this._connection.close(); } + /** + * Creates a new Transaction for executing operations within an explicit transaction. + * Transactions are short-lived and single-use. After commit or rollback, + * create a new Transaction for the next unit of work. + * The traversal source (g alias) is inherited from this Client. + * @returns {Transaction} + */ + transact(): Transaction { + return new Transaction(this); + } + /** * Adds an event listener to the connection * @param {String} event The event name that you want to listen to. diff --git a/gremlin-js/gremlin-javascript/lib/driver/connection.ts b/gremlin-js/gremlin-javascript/lib/driver/connection.ts index 12f7da04674..ef57960b735 100644 --- a/gremlin-js/gremlin-javascript/lib/driver/connection.ts +++ b/gremlin-js/gremlin-javascript/lib/driver/connection.ts @@ -239,6 +239,16 @@ export default class Connection extends EventEmitter { body, }; + // Promote transactionId to HTTP header before interceptors run. + // The field remains in the serialized body as well (dual transmission + // per the HTTP transaction protocol specification). + if (body instanceof RequestMessage) { + const fields = body.getFields(); + if (fields.has('transactionId')) { + httpRequest.headers['X-Transaction-Id'] = fields.get('transactionId'); + } + } + for (let i = 0; i < this._interceptors.length; i++) { try { httpRequest = await this._interceptors[i](httpRequest); diff --git a/gremlin-js/gremlin-javascript/lib/driver/driver-remote-connection.ts b/gremlin-js/gremlin-javascript/lib/driver/driver-remote-connection.ts index d293462a3c4..78caf3bec89 100644 --- a/gremlin-js/gremlin-javascript/lib/driver/driver-remote-connection.ts +++ b/gremlin-js/gremlin-javascript/lib/driver/driver-remote-connection.ts @@ -32,16 +32,28 @@ import { ConnectionOptions } from './connection.js'; * Represents the default `RemoteConnection` implementation. */ export default class DriverRemoteConnection extends RemoteConnection { - private readonly _client: Client; + // Exposed as package-internal for Transaction to submit begin/commit/rollback. + // Not part of the public API contract. + readonly _client: Client; + private _transactionId?: string; /** * Creates a new instance of {@link DriverRemoteConnection}. * @param {String} url The resource uri. * @param {ConnectionOptions} [options] The connection options. + * @param {String} [transactionId] Optional transaction ID to attach to all requests. */ - constructor(url: string, options: ConnectionOptions = {}) { + constructor(url: string, options: ConnectionOptions = {}, transactionId?: string) { super(url, options); this._client = new Client(url, options); + this._transactionId = transactionId; + } + + /** + * Returns the transaction ID if this connection is bound to a transaction, undefined otherwise. + */ + get transactionId(): string | undefined { + return this._transactionId; } /** @override */ @@ -100,15 +112,12 @@ export default class DriverRemoteConnection extends RemoteConnection { requestOptions.bindings = parametersString; } - return { gremlin: gremlinLang.getGremlin(), requestOptions }; - } - - override commit() { - return this._client.submit('g.tx().commit()', null); - } + // If this connection is bound to a transaction, attach the transactionId + if (this._transactionId) { + requestOptions.transactionId = this._transactionId; + } - override rollback() { - return this._client.submit('g.tx().rollback()', null); + return { gremlin: gremlinLang.getGremlin(), requestOptions }; } override close() { diff --git a/gremlin-js/gremlin-javascript/lib/driver/remote-connection.ts b/gremlin-js/gremlin-javascript/lib/driver/remote-connection.ts index 44007e9bd4b..0880baf50b9 100644 --- a/gremlin-js/gremlin-javascript/lib/driver/remote-connection.ts +++ b/gremlin-js/gremlin-javascript/lib/driver/remote-connection.ts @@ -62,19 +62,7 @@ export abstract class RemoteConnection { abstract submit(gremlinLang: GremlinLang): Promise; /** - * Submits a commit operation to the server and closes the connection. - * @returns {Promise} - */ - abstract commit(): Promise; - - /** - * Submits a rollback operation to the server and closes the connection. - * @returns {Promise} - */ - abstract rollback(): Promise; - - /** - * Closes the connection where open transactions will close according to the features of the graph provider. + * Closes the connection and releases any associated resources. * @returns {Promise} */ abstract close(): Promise; diff --git a/gremlin-js/gremlin-javascript/lib/process/graph-traversal.ts b/gremlin-js/gremlin-javascript/lib/process/graph-traversal.ts index 48362a148b3..acde7f4f729 100644 --- a/gremlin-js/gremlin-javascript/lib/process/graph-traversal.ts +++ b/gremlin-js/gremlin-javascript/lib/process/graph-traversal.ts @@ -26,6 +26,7 @@ import { Transaction } from './transaction.js'; import { TraversalStrategies, VertexProgramStrategy, OptionsStrategy } from './traversal-strategy.js'; import {Graph, Vertex} from '../structure/graph.js'; import { RemoteConnection, RemoteStrategy } from '../driver/remote-connection.js'; +import DriverRemoteConnection from '../driver/driver-remote-connection.js'; import GremlinLang from './gremlin-lang.js'; /** @@ -33,6 +34,8 @@ import GremlinLang from './gremlin-lang.js'; */ export class GraphTraversalSource { remoteConnection?: RemoteConnection; + /** @internal - set by Transaction.begin() for gtx.tx() to return the owning transaction */ + _boundTransaction?: Transaction; /** * Creates a new instance of {@link GraphTraversalSource}. @@ -58,12 +61,24 @@ export class GraphTraversalSource { * @returns {Transaction} */ tx(): Transaction { - // you can't do g.tx().begin().tx() - no child transactions - if (this.remoteConnection) { + // If this GTS is already bound to a transaction, return that transaction. + if (this._boundTransaction) { + return this._boundTransaction; + } + + if (!this.remoteConnection) { + throw new Error('Remote connection is required for transactions'); + } + + // If the DRC has a transactionId, it was created by a Transaction.begin(). + // But we don't have the Transaction reference here (only _boundTransaction above). + if (this.remoteConnection instanceof DriverRemoteConnection && this.remoteConnection.transactionId) { throw new Error('This TraversalSource is already bound to a transaction - child transactions are not supported'); } - return new Transaction(this); + // Extract the Client from the DriverRemoteConnection. + const client = (this.remoteConnection as DriverRemoteConnection)._client; + return new Transaction(client); } /** diff --git a/gremlin-js/gremlin-javascript/lib/process/transaction.ts b/gremlin-js/gremlin-javascript/lib/process/transaction.ts index 59afeef13fc..1feda362d07 100644 --- a/gremlin-js/gremlin-javascript/lib/process/transaction.ts +++ b/gremlin-js/gremlin-javascript/lib/process/transaction.ts @@ -17,58 +17,160 @@ * under the License. */ -import { RemoteConnection, RemoteStrategy } from '../driver/remote-connection.js'; +import { RemoteStrategy } from '../driver/remote-connection.js'; +import DriverRemoteConnection from '../driver/driver-remote-connection.js'; +import Client from '../driver/client.js'; import GremlinLang from './gremlin-lang.js'; import { GraphTraversalSource } from './graph-traversal.js'; import { TraversalStrategies } from './traversal-strategy.js'; /** - * A controller for a remote transaction that is constructed from g.tx(). Calling begin() - * on this object will produce a new GraphTraversalSource that is bound to a remote transaction over which - * multiple traversals may be executed in that context. Calling commit() or rollback() will - * then close the transaction and thus, the session. This feature only works with transaction enabled graphs. + * Controls an explicit remote transaction. A thin wrapper around a Client that + * adds transaction lifecycle (begin/commit/rollback/close) and attaches a + * transactionId to every request. + * + * Created via Client.transact() or g.tx(). The traversal source (g alias) is + * inherited from the Client and cannot be changed. + * + * Transactions are short-lived and single-use. After commit or rollback, the + * transaction ID is invalid and the object cannot be reused. + * + * Each traversal on the transaction-bound source must be awaited before + * submitting the next, to ensure the server receives requests in order. + * + * This class is NOT thread-safe. Do not share a Transaction across concurrent + * async contexts without external synchronization. */ export class Transaction { - private _sessionBasedConnection?: RemoteConnection = undefined; + private _transactionId?: string; + private _isOpen = false; + private _failed = false; + private readonly _client: Client; - constructor(private readonly g: GraphTraversalSource) {} + /** + * Creates a Transaction. Accepts a Client which provides the connection + * and traversal source for all transaction requests. + */ + constructor(client: Client) { + if (!client) { + throw new Error('Client is required for transactions'); + } + this._client = client; + } /** - * Spawns a GraphTraversalSource that is bound to a remote session which enables a transaction. - * @returns {GraphTraversalSource} + * Spawns a GraphTraversalSource that is bound to a remote transaction. + * @returns {Promise} */ - begin(): GraphTraversalSource { - throw new Error ("Transactions are not yet implemented") + async begin(): Promise { + if (this._isOpen || this._failed) { + throw new Error('Transaction already started'); + } + + let result; + try { + result = await this._client.submit('g.tx().begin()', null); + } catch (e) { + this._failed = true; + throw e; + } + + const resultArray = result.toArray(); + if (!resultArray || resultArray.length === 0) { + this._failed = true; + throw new Error('Server did not return transaction ID'); + } + + const resultMap = resultArray[0]; + if (!resultMap || !(resultMap instanceof Map) || !resultMap.get('transactionId')) { + this._failed = true; + throw new Error('Server did not return transaction ID in expected format'); + } + + this._transactionId = resultMap.get('transactionId'); + this._isOpen = true; + + // Create a DriverRemoteConnection bound to this transaction. The DRC + // will automatically attach the transactionId to all requests. + const txConnection = new DriverRemoteConnection( + this._client.url, + this._client.options, + this._transactionId, + ); + + const strategies = new TraversalStrategies(); + strategies.addStrategy(new RemoteStrategy(txConnection)); + const gtx = new GraphTraversalSource(null as any, strategies, new GremlinLang()); + gtx._boundTransaction = this; + return gtx; } /** - * @returns {Promise} + * Commits the transaction. + * @returns {Promise} */ - commit(): Promise { - throw new Error ("Transactions are not yet implemented") + async commit(): Promise { + await this.#closeTransaction('g.tx().commit()'); } /** - * @returns {Promise} + * Rolls back the transaction. + * @returns {Promise} */ - rollback(): Promise { - throw new Error ("Transactions are not yet implemented") + async rollback(): Promise { + await this.#closeTransaction('g.tx().rollback()'); } /** * Returns true if transaction is open. - * @returns {Boolean} */ get isOpen(): boolean { - return this._sessionBasedConnection?.isOpen ?? false; + return this._isOpen; + } + + /** + * Returns the server-generated transaction ID, or undefined if not yet begun. + */ + get transactionId(): string | undefined { + return this._transactionId; } /** - * @returns {Promise} + * Closes the transaction. Default behavior is rollback: partial work is + * discarded rather than accidentally persisted. + * @returns {Promise} */ async close(): Promise { - if (this._sessionBasedConnection) { - this._sessionBasedConnection.close(); + if (this._isOpen) { + await this.rollback(); + } + } + + /** + * Submits a gremlin-lang string within this transaction. The transactionId + * is automatically attached. Has the same signature as Client.submit(). + */ + async submit(gremlin: string, bindings?: any, requestOptions?: any): Promise { + if (!this._isOpen) { + throw new Error('Transaction is not open'); + } + const opts = { + ...requestOptions, + transactionId: this._transactionId, + }; + return this._client.submit(gremlin, bindings || null, opts); + } + + async #closeTransaction(script: string): Promise { + if (!this._isOpen) { + throw new Error('Transaction is not open'); } + + await this._client.submit(script, null, { + transactionId: this._transactionId + }); + + this._isOpen = false; + this._failed = true; // Terminal state: transaction cannot be reused } } diff --git a/gremlin-js/gremlin-javascript/test/integration/transaction-tests.js b/gremlin-js/gremlin-javascript/test/integration/transaction-tests.js new file mode 100644 index 00000000000..e1387452aa9 --- /dev/null +++ b/gremlin-js/gremlin-javascript/test/integration/transaction-tests.js @@ -0,0 +1,374 @@ +/* + * 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. + */ + +import assert from 'assert'; +import { getClient, getConnection } from '../helper.js'; +import anon from '../../lib/process/anonymous-traversal.js'; + +let client; + +describe('Transaction', function () { + before(function () { + client = getClient('gtx'); + return client.open(); + }); + after(function () { + return client.close(); + }); + beforeEach(async function () { + // Drop all vertices before each test to prevent cross-test contamination + await client.submit("g.V().drop()"); + }); + + describe('Driver-level API (client.transact())', function () { + it('should commit transaction', async function () { + const tx = client.transact(); + await tx.begin(); + assert.strictEqual(tx.isOpen, true); + + await tx.submit("g.addV('person').property('name','alice')"); + + // Uncommitted data not visible outside the transaction + const result1 = await client.submit("g.V().hasLabel('person').count()"); + assert.strictEqual(result1.first(), 0); + + await tx.commit(); + assert.strictEqual(tx.isOpen, false); + + // Committed data visible + const result2 = await client.submit("g.V().hasLabel('person').count()"); + assert.strictEqual(result2.first(), 1); + }); + + it('should rollback transaction', async function () { + const tx = client.transact(); + await tx.begin(); + + await tx.submit("g.addV('person').property('name','bob')"); + + await tx.rollback(); + assert.strictEqual(tx.isOpen, false); + + const result = await client.submit("g.V().hasLabel('person').count()"); + assert.strictEqual(result.first(), 0); + }); + + it('should support intra-transaction consistency', async function () { + const tx = client.transact(); + await tx.begin(); + + await tx.submit("g.addV('test').property('name','A')"); + + // Read-your-own-writes + const countResult = await tx.submit("g.V().hasLabel('test').count()"); + assert.strictEqual(countResult.first(), 1); + + await tx.submit("g.addV('test').property('name','B')"); + await tx.commit(); + + const result = await client.submit("g.V().hasLabel('test').count()"); + assert.strictEqual(result.first(), 2); + }); + + it('should throw on submit after commit', async function () { + const tx = client.transact(); + await tx.begin(); + await tx.submit("g.addV()"); + await tx.commit(); + + await assert.rejects( + () => tx.submit("g.V().count()"), + /Transaction is not open/ + ); + }); + + it('should throw on submit after rollback', async function () { + const tx = client.transact(); + await tx.begin(); + await tx.submit("g.addV()"); + await tx.rollback(); + + await assert.rejects( + () => tx.submit("g.V().count()"), + /Transaction is not open/ + ); + }); + + it('should throw on double begin', async function () { + const tx = client.transact(); + await tx.begin(); + + await assert.rejects( + () => tx.begin(), + /Transaction already started/ + ); + }); + + it('should throw on commit when not open', async function () { + const tx = client.transact(); + + await assert.rejects( + () => tx.commit(), + /Transaction is not open/ + ); + }); + + it('should throw on rollback when not open', async function () { + const tx = client.transact(); + + await assert.rejects( + () => tx.rollback(), + /Transaction is not open/ + ); + }); + + it('should return undefined transactionId before begin', async function () { + const tx = client.transact(); + assert.strictEqual(tx.transactionId, undefined); + + await tx.begin(); + assert.ok(tx.transactionId); + assert.ok(tx.transactionId.length > 0); + }); + + it('should rollback on close by default', async function () { + const tx = client.transact(); + await tx.begin(); + await tx.submit("g.addV('person').property('name','close_test')"); + + await tx.close(); + assert.strictEqual(tx.isOpen, false); + + const result = await client.submit("g.V().hasLabel('person').count()"); + assert.strictEqual(result.first(), 0); + }); + + it('should isolate concurrent transactions', async function () { + const tx1 = client.transact(); + await tx1.begin(); + const tx2 = client.transact(); + await tx2.begin(); + + await tx1.submit("g.addV('tx1')"); + await tx2.submit("g.addV('tx2')"); + + // tx1 should not see tx2's data + const r1 = await tx1.submit("g.V().hasLabel('tx2').count()"); + assert.strictEqual(r1.first(), 0); + const r2 = await tx2.submit("g.V().hasLabel('tx1').count()"); + assert.strictEqual(r2.first(), 0); + + await tx1.commit(); + await tx2.commit(); + + const result1 = await client.submit("g.V().hasLabel('tx1').count()"); + assert.strictEqual(result1.first(), 1); + const result2 = await client.submit("g.V().hasLabel('tx2').count()"); + assert.strictEqual(result2.first(), 1); + }); + + it('should open and close many transactions sequentially', async function () { + const numTransactions = 50; + for (let i = 0; i < numTransactions; i++) { + const tx = client.transact(); + await tx.begin(); + await tx.submit("g.addV('stress')"); + await tx.commit(); + } + + const result = await client.submit("g.V().hasLabel('stress').count()"); + assert.strictEqual(result.first(), numTransactions); + }); + + it('should keep transaction open after traversal error', async function () { + const tx = client.transact(); + await tx.begin(); + await tx.submit("g.addV('good_vertex')"); + + try { + await tx.submit("g.V().fail()"); + } catch (e) { + // expected + } + + assert.strictEqual(tx.isOpen, true); + await tx.rollback(); + + const result = await client.submit("g.V().hasLabel('good_vertex').count()"); + assert.strictEqual(result.first(), 0); + }); + + it('should not allow begin after commit', async function () { + const tx = client.transact(); + await tx.begin(); + await tx.commit(); + + await assert.rejects( + () => tx.begin(), + /Transaction already started/ + ); + }); + + it('should not allow begin after rollback', async function () { + const tx = client.transact(); + await tx.begin(); + await tx.rollback(); + + await assert.rejects( + () => tx.begin(), + /Transaction already started/ + ); + }); + + it('should throw on double commit', async function () { + const tx = client.transact(); + await tx.begin(); + await tx.commit(); + + await assert.rejects( + () => tx.commit(), + /Transaction is not open/ + ); + }); + + it('should throw on double rollback', async function () { + const tx = client.transact(); + await tx.begin(); + await tx.rollback(); + + await assert.rejects( + () => tx.rollback(), + /Transaction is not open/ + ); + }); + + it('should isolate transactional and non-transactional requests', async function () { + const tx = client.transact(); + await tx.begin(); + await tx.submit("g.addV('tx_data')"); + + // Non-transactional read should not see uncommitted data + const result1 = await client.submit("g.V().hasLabel('tx_data').count()"); + assert.strictEqual(result1.first(), 0); + + await tx.commit(); + + const result2 = await client.submit("g.V().hasLabel('tx_data').count()"); + assert.strictEqual(result2.first(), 1); + }); + }); + + describe('Traversal API (g.tx())', function () { + it('should work with driver remote connection', async function () { + const connection = getConnection('gtx'); + const g = anon.traversal().withRemote(connection); + + const tx = g.tx(); + const gtx = await tx.begin(); + await gtx.addV('val').iterate(); + await tx.commit(); + + const count = await g.V().hasLabel('val').count().next(); + assert.strictEqual(count.value, 1); + + await connection.close(); + }); + + it('should return same transaction from gtx.tx()', async function () { + const connection = getConnection('gtx'); + const g = anon.traversal().withRemote(connection); + + const tx = g.tx(); + const gtx = await tx.begin(); + const sameTx = gtx.tx(); + assert.strictEqual(sameTx, tx); + + await tx.rollback(); + await connection.close(); + }); + + it('should throw on begin from gtx.tx()', async function () { + const connection = getConnection('gtx'); + const g = anon.traversal().withRemote(connection); + + const tx = g.tx(); + const gtx = await tx.begin(); + const sameTx = gtx.tx(); + + await assert.rejects( + () => sameTx.begin(), + /Transaction already started/ + ); + + await tx.rollback(); + await connection.close(); + }); + + it('should commit via gtx.tx().commit()', async function () { + const connection = getConnection('gtx'); + const g = anon.traversal().withRemote(connection); + + const tx = g.tx(); + const gtx = await tx.begin(); + await gtx.addV('gtx_commit_test').iterate(); + await gtx.tx().commit(); + + const count = await g.V().hasLabel('gtx_commit_test').count().next(); + assert.strictEqual(count.value, 1); + + await connection.close(); + }); + + it('should reject begin on non-transactional graph', async function () { + const nonTxClient = getClient('gclassic'); + + const tx = nonTxClient.transact(); + try { + await tx.begin(); + assert.fail('Expected error on begin for non-transactional graph'); + } catch (e) { + assert.ok(e.statusMessage.includes('does not support transactions')); + } + + await nonTxClient.close(); + }); + + it('should clean up on begin failure', async function () { + const nonTxClient = getClient('gclassic'); + + const tx = nonTxClient.transact(); + try { + await tx.begin(); + } catch (e) { + // expected + } + + assert.strictEqual(tx.isOpen, false); + assert.strictEqual(tx.transactionId, undefined); + + // Cannot begin again + await assert.rejects( + () => tx.begin(), + /Transaction already started/ + ); + + await nonTxClient.close(); + }); + }); +}); diff --git a/gremlin-js/gremlin-javascript/test/integration/traversal-test.js b/gremlin-js/gremlin-javascript/test/integration/traversal-test.js index 1ada7cabffc..2f7f5441441 100644 --- a/gremlin-js/gremlin-javascript/test/integration/traversal-test.js +++ b/gremlin-js/gremlin-javascript/test/integration/traversal-test.js @@ -35,7 +35,6 @@ import { getConnection, getDriverRemoteConnection } from '../helper.js'; const __ = statics; let connection; -let txConnection; class SocialTraversal extends GraphTraversal { constructor(graph, traversalStrategies, gremlinLang) { @@ -327,110 +326,5 @@ describe('Traversal', function () { }); }); }); - //TODO:: Re-enable after tx reimplementation - // describe("should handle tx errors if graph not support tx", function() { - // it('should throw exception on commit if graph not support tx', async function() { - // const g = anon.traversal().withRemote(connection); - // const tx = g.tx(); - // const gtx = tx.begin(); - // const result = await g.V().count().next(); - // assert.strictEqual(6, result.value); - // try { - // await tx.commit(); - // assert.fail("should throw error"); - // } catch (err) { - // assert.strictEqual(err.statusCode, 500); - // assert.ok(err.statusMessage.includes('Graph does not support transactions')); - // } - // }); - // it('should throw exception on rollback if graph not support tx', async function() { - // const g = anon.traversal().withRemote(connection); - // const tx = g.tx(); - // tx.begin(); - // try { - // await tx.rollback(); - // assert.fail("should throw error"); - // } catch (err) { - // assert.strictEqual(err.statusCode, 500); - // assert.ok(err.statusMessage.includes('Graph does not support transactions')); - // } - // }); - // }); - // describe('support remote transactions - commit', function() { - // before(function () { - // txConnection = getConnection('gtx'); - // return txConnection.open(); - // }); - // after(function () { - // const g = anon.traversal().with_(txConnection); - // return g.V().drop().iterate().then(() => { - // return txConnection.close() - // }); - // }); - // it('should commit a simple transaction', async function () { - // const g = anon.traversal().with_(txConnection); - // const tx = g.tx(); - // const gtx = tx.begin(); - // await Promise.all([ - // gtx.addV("person").property("name", "jorge").iterate(), - // gtx.addV("person").property("name", "josh").iterate() - // ]); - // - // let r = await gtx.V().count().next(); - // // assert within the transaction.... - // assert.ok(r); - // assert.strictEqual(r.value, 2); - // - // // now commit changes to test outside of the transaction - // await tx.commit(); - // - // r = await g.V().count().next(); - // assert.ok(r); - // assert.strictEqual(r.value, 2); - // // connection closing async, so need to wait - // while (tx._sessionBasedConnection.isOpen) { - // await new Promise(resolve => setTimeout(resolve, 10)); - // } - // assert.ok(!tx._sessionBasedConnection.isOpen); - // }); - // }); - // describe('support remote transactions - rollback', function() { - // before(function () { - // - // txConnection = getConnection('gtx'); - // return txConnection.open(); - // }); - // after(function () { - // const g = anon.traversal().with_(txConnection); - // return g.V().drop().iterate().then(() => { - // return txConnection.close() - // }); - // }); - // it('should rollback a simple transaction', async function() { - // const g = anon.traversal().with_(txConnection); - // const tx = g.tx(); - // const gtx = tx.begin(); - // await Promise.all([ - // gtx.addV("person").property("name", "jorge").iterate(), - // gtx.addV("person").property("name", "josh").iterate() - // ]); - // - // let r = await gtx.V().count().next(); - // // assert within the transaction.... - // assert.ok(r); - // assert.strictEqual(r.value, 2); - // - // // now rollback changes to test outside of the transaction - // await tx.rollback(); - // - // r = await g.V().count().next(); - // assert.ok(r); - // assert.strictEqual(r.value, 0); - // // connection closing async, so need to wait - // while (tx._sessionBasedConnection.isOpen) { - // await new Promise(resolve => setTimeout(resolve, 10)); - // } - // assert.ok(!tx._sessionBasedConnection.isOpen); - // }); - // }); -}); \ No newline at end of file +}); + diff --git a/gremlin-js/gremlin-javascript/test/unit/traversal-test.js b/gremlin-js/gremlin-javascript/test/unit/traversal-test.js index 3fd1857ea9a..2f2a9a78198 100644 --- a/gremlin-js/gremlin-javascript/test/unit/traversal-test.js +++ b/gremlin-js/gremlin-javascript/test/unit/traversal-test.js @@ -211,52 +211,6 @@ describe('Traversal', function () { }) }); - // TODO:: Re-enable after transactions - // describe('child transactions', function() { - // it('should not support child transactions', function() { - // const g = anon.traversal().with_(new MockRemoteConnection()); - // const tx = g.tx(); - // assert.throws(function() { - // tx.begin().tx(); - // }); - // }); - // }); - // - // describe('not opened transactions', function() { - // it('should not allow commit for not opened transactions', async function() { - // const g = anon.traversal().withRemote(new MockRemoteConnection()); - // const tx = g.tx(); - // try { - // await tx.commit(); - // assert.fail("should throw error"); - // } catch (err) { - // assert.strictEqual('Cannot commit a transaction that is not started', err.message); - // } - // }); - // it('should not allow rollback for not opened transactions', async function() { - // const g = anon.traversal().withRemote(new MockRemoteConnection()); - // const tx = g.tx(); - // try { - // await tx.rollback(); - // assert.fail("should throw error"); - // } catch (err) { - // assert.strictEqual('Cannot rollback a transaction that is not started', err.message); - // } - // }); - // }); - // - // describe('tx#begin()', function() { - // it("should not allow a transaction to begin more than once", function() { - // const g = anon.traversal().with_(new MockRemoteConnection()); - // const tx = g.tx(); - // tx.begin(); - // assert.throws(function () { - // tx.begin(); - // }); - // }); - // }); - - }); class MockRemoteConnection extends RemoteConnection { diff --git a/gremlin-python/src/main/python/gremlin_python/driver/client.py b/gremlin-python/src/main/python/gremlin_python/driver/client.py index 8b5ff1466c5..6c68c78cf36 100644 --- a/gremlin-python/src/main/python/gremlin_python/driver/client.py +++ b/gremlin-python/src/main/python/gremlin_python/driver/client.py @@ -36,7 +36,6 @@ def cpu_count(): __author__ = 'David M. Brown (davebshow@gmail.com), Lyndon Bauto (lyndonb@bitquilltech.com)' -# TODO: remove session, update connection pooling, etc. class Client: def __init__(self, url, traversal_source, pool_size=None, max_workers=None, @@ -111,6 +110,16 @@ def close(self): self._executor.shutdown() self._closed = True + def transact(self): + """Creates a new Transaction for executing operations within an explicit transaction. + + Transactions are short-lived and single-use. After commit or rollback, + create a new Transaction for the next unit of work. The traversal source + (g alias) is inherited from this Client. + """ + from gremlin_python.driver.transaction import Transaction + return Transaction(self) + def _get_connection(self): return connection.Connection( self._url, self._traversal_source, diff --git a/gremlin-python/src/main/python/gremlin_python/driver/connection.py b/gremlin-python/src/main/python/gremlin_python/driver/connection.py index 1c4bca85fd2..64a3f000818 100644 --- a/gremlin-python/src/main/python/gremlin_python/driver/connection.py +++ b/gremlin-python/src/main/python/gremlin_python/driver/connection.py @@ -87,6 +87,11 @@ def _write_request(self, request_message): if self._request_serializer is not None: content_type = str(self._request_serializer.version, encoding='utf-8') message['headers']['content-type'] = content_type + # Promote transactionId to HTTP header before interceptors run. + # The field remains in the serialized body as well (dual transmission + # per the HTTP transaction protocol specification). + if hasattr(request_message, 'fields') and 'transactionId' in request_message.fields: + message['headers']['X-Transaction-Id'] = request_message.fields['transactionId'] for interceptor in self._interceptors or []: message = interceptor(message) self._transport.write(message) diff --git a/gremlin-python/src/main/python/gremlin_python/driver/driver_remote_connection.py b/gremlin-python/src/main/python/gremlin_python/driver/driver_remote_connection.py index fdd25280cf9..ea03617de8c 100644 --- a/gremlin-python/src/main/python/gremlin_python/driver/driver_remote_connection.py +++ b/gremlin-python/src/main/python/gremlin_python/driver/driver_remote_connection.py @@ -102,15 +102,6 @@ def cb(f): def is_closed(self): return self._client.is_closed() - # TODO remove or update once HTTP transaction is implemented - # def commit(self): - # log.info("Submitting commit graph operation.") - # return self._client.submit(Bytecode.GraphOp.commit()) - # - # def rollback(self): - # log.info("Submitting rollback graph operation.") - # return self._client.submit(Bytecode.GraphOp.rollback()) - @staticmethod def extract_request_options(gremlin_lang): request_options = {} diff --git a/gremlin-python/src/main/python/gremlin_python/driver/request.py b/gremlin-python/src/main/python/gremlin_python/driver/request.py index 78cd54a60c9..119c76c0a94 100644 --- a/gremlin-python/src/main/python/gremlin_python/driver/request.py +++ b/gremlin-python/src/main/python/gremlin_python/driver/request.py @@ -24,4 +24,5 @@ 'RequestMessage', ['fields', 'gremlin']) Tokens = ['batchSize', 'bindings', 'g', 'gremlin', 'language', - 'evaluationTimeout', 'materializeProperties', 'timeoutMs', 'userAgent', 'bulkResults'] + 'evaluationTimeout', 'materializeProperties', 'timeoutMs', 'userAgent', 'bulkResults', + 'transactionId'] diff --git a/gremlin-python/src/main/python/gremlin_python/driver/transaction.py b/gremlin-python/src/main/python/gremlin_python/driver/transaction.py new file mode 100644 index 00000000000..5480e30b17e --- /dev/null +++ b/gremlin-python/src/main/python/gremlin_python/driver/transaction.py @@ -0,0 +1,170 @@ +# +# 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. +# +import logging + +from gremlin_python.driver.remote_connection import RemoteConnection, RemoteTraversal + +log = logging.getLogger("gremlinpython") + + +class Transaction: + """Controls an explicit remote transaction. A thin wrapper around a Client + that adds transaction lifecycle (begin/commit/rollback/close) and attaches + a transactionId to every request. + + Created via Client.transact() or g.tx(). The traversal source (g alias) + is inherited from the Client and cannot be changed. + + Transactions are short-lived and single-use. After commit or rollback, + the transaction ID is invalid and the object cannot be reused. + + This class is NOT thread-safe. + """ + + def __init__(self, client): + self._client = client + self._transaction_id = None + self._is_open = False + self._failed = False + + def begin(self): + """Starts the transaction and returns a transaction-bound GraphTraversalSource. + + The returned GTS can be used to submit traversals within this transaction. + Users of the driver-level API (client.transact()) may ignore the return + value and use submit() directly instead. + """ + if self._is_open or self._failed: + raise Exception("Transaction already started") + + try: + result = self._client.submit("g.tx().begin()") + results = result.all().result() + except Exception: + self._failed = True + raise + + if not results: + self._failed = True + raise Exception("Server did not return transaction ID") + + result_map = results[0] + if isinstance(result_map, dict): + self._transaction_id = result_map.get('transactionId') + else: + self._failed = True + raise Exception("Server did not return transaction ID in expected format") + + if not self._transaction_id: + self._failed = True + raise Exception("Server returned empty transaction ID") + + self._is_open = True + + # Return a GraphTraversalSource bound to this transaction via + # TransactionRemoteConnection. Inline imports avoid circular dependencies + # between driver/ and process/ packages. + from gremlin_python.process.graph_traversal import GraphTraversalSource + from gremlin_python.process.traversal import TraversalStrategies, GremlinLang + from gremlin_python.driver.remote_connection import RemoteStrategy + + tx_connection = TransactionRemoteConnection(self) + strategies = TraversalStrategies() + strategies.add_strategies([RemoteStrategy(tx_connection)]) + return GraphTraversalSource(None, strategies, GremlinLang()) + + def submit(self, gremlin, bindings=None, request_options=None): + """Submits a gremlin-lang string within this transaction. + + The transactionId is automatically attached. Has the same signature + as Client.submit() so both can be used interchangeably. + """ + if not self._is_open: + raise Exception("Transaction is not open") + opts = {'transactionId': self._transaction_id} + if request_options: + opts.update(request_options) + return self._client.submit(gremlin, bindings=bindings, request_options=opts) + + def commit(self): + """Commits the transaction.""" + self._close_transaction("g.tx().commit()") + + def rollback(self): + """Rolls back the transaction.""" + self._close_transaction("g.tx().rollback()") + + def _close_transaction(self, script): + if not self._is_open: + raise Exception("Transaction is not open") + self._client.submit(script, request_options={'transactionId': self._transaction_id}).all().result() + self._is_open = False + self._failed = True # Terminal state: transaction cannot be reused + + @property + def is_open(self): + return self._is_open + + @property + def transaction_id(self): + return self._transaction_id + + def close(self): + if self._is_open: + self.rollback() + + def __enter__(self): + return self + + def __exit__(self, exc_type, exc_val, exc_tb): + if self._is_open: + self.rollback() + return False + + +class TransactionRemoteConnection(RemoteConnection): + """A RemoteConnection that attaches transactionId to all requests submitted + through the traversal API (g.tx().begin() -> gtx.addV()...). + + This bridges the traversal machinery (which calls RemoteConnection.submit) + to the Transaction (which injects the transactionId). + """ + + def __init__(self, transaction): + super().__init__( + transaction._client._url, + transaction._client._traversal_source) + self._transaction = transaction + + def submit(self, gremlin_lang): + if not self._transaction.is_open: + raise Exception("Transaction is not open") + + gremlin_lang.add_g(self._traversal_source) + + from gremlin_python.driver.driver_remote_connection import DriverRemoteConnection + request_options = DriverRemoteConnection.extract_request_options(gremlin_lang) + request_options['transactionId'] = self._transaction.transaction_id + + result_set = self._transaction._client.submit( + gremlin_lang.get_gremlin(), request_options=request_options) + return RemoteTraversal(result_set) + + def is_closed(self): + return not self._transaction.is_open diff --git a/gremlin-python/src/main/python/gremlin_python/process/graph_traversal.py b/gremlin-python/src/main/python/gremlin_python/process/graph_traversal.py index 082df46d1ae..4e83e857ba2 100644 --- a/gremlin-python/src/main/python/gremlin_python/process/graph_traversal.py +++ b/gremlin-python/src/main/python/gremlin_python/process/graph_traversal.py @@ -143,24 +143,24 @@ def with_(self, k, v=None): return source - # TODO remove or update once HTTP transaction is implemented - # def tx(self): - # # In order to keep the constructor unchanged within 3.5.x we can try to pop the RemoteConnection out of the - # # TraversalStrategies. keeping this unchanged will allow user DSLs to not take a break. - # # This is the same strategy as gremlin-javascript. - # # TODO https://issues.apache.org/jira/browse/TINKERPOP-2664: refactor this to be nicer in 3.6.0 when - # # we can take a breaking change - # remote_connection = next((x.remote_connection for x in self.traversal_strategies.traversal_strategies if - # x.fqcn == "py:RemoteStrategy"), None) - # - # if remote_connection is None: - # raise Exception("Error, remote connection is required for transaction.") - # - # # You can't do g.tx().begin().tx() i.e child transactions are not supported. - # if remote_connection and remote_connection.is_session_bound(): - # raise Exception("This TraversalSource is already bound to a transaction - child transactions are not " - # "supported") - # return Transaction(self, remote_connection) + def tx(self): + """Spawns a Transaction or returns the existing one if this GTS is + already bound to a transaction. Requires a RemoteConnection backed by a Client. + """ + from gremlin_python.driver.remote_connection import RemoteStrategy + remote_connection = next((x.remote_connection for x in self.traversal_strategies.traversal_strategies if + x.fqcn == "py:RemoteStrategy"), None) + + if remote_connection is None: + raise Exception("Remote connection is required for transactions") + + # If this GTS is already bound to a transaction, return that transaction. + from gremlin_python.driver.transaction import TransactionRemoteConnection + if isinstance(remote_connection, TransactionRemoteConnection): + return remote_connection._transaction + + from gremlin_python.driver.transaction import Transaction + return Transaction(remote_connection._client) def withComputer(self, graph_computer=None, workers=None, result=None, persist=None, vertices=None, edges=None, configuration=None): @@ -1776,81 +1776,6 @@ def where(cls, *args): return cls.graph_traversal(None, None, GremlinLang()).where(*args) -# Class to handle transactions. -# class Transaction: -# -# def __init__(self, g, remote_connection): -# self._g = g -# self._session_based_connection = None -# self._remote_connection = remote_connection -# self.__is_open = False -# self.__mutex = Lock() -# -# # Begins transaction. -# def begin(self): -# with self.__mutex: -# # Verify transaction is not open. -# self.__verify_transaction_state(False, "Transaction already started on this object") -# -# # Create new session using the remote connection. -# self._session_based_connection = self._remote_connection.create_session() -# self.__is_open = True -# -# # Set the session as a remote strategy within the traversal strategy. -# traversal_strategy.py = TraversalStrategies() -# traversal_strategy.py.add_strategies([RemoteStrategy(self._session_based_connection)]) -# -# # Return new GraphTraversalSource. -# return GraphTraversalSource(self._g.graph, traversal_strategy.py, self._g.bytecode) -# -# # Rolls transaction back. -# def rollback(self): -# with self.__mutex: -# # Verify transaction is open, close session and return result of transaction's rollback. -# self.__verify_transaction_state(True, "Cannot rollback a transaction that is not started.") -# return self.__close_session(self._session_based_connection.rollback()) -# -# # Commits the current transaction. -# def commit(self): -# with self.__mutex: -# # Verify transaction is open, close session and return result of transaction's commit. -# self.__verify_transaction_state(True, "Cannot commit a transaction that is not started.") -# return self.__close_session(self._session_based_connection.commit()) -# -# # Closes session. -# def close(self): -# with self.__mutex: -# # Verify transaction is open. -# self.__verify_transaction_state(True, "Cannot close a transaction that has previously been closed.") -# self.__close_session(None) -# -# # Return whether or not transaction is open. -# # Allow camelcase function here to keep api consistent with other languages. -# def isOpen(self): -# warnings.warn( -# "gremlin_python.process.Transaction.isOpen will be replaced by " -# "gremlin_python.process.Transaction.is_open.", -# DeprecationWarning) -# return self.is_open() -# -# def is_open(self): -# # if the underlying DriverRemoteConnection is closed then the Transaction can't be open -# if (self._session_based_connection and self._session_based_connection.is_closed()) or \ -# self._remote_connection.is_closed(): -# self.__is_open = False -# -# return self.__is_open -# -# def __verify_transaction_state(self, state, error_message): -# if self.__is_open != state: -# raise Exception(error_message) -# -# def __close_session(self, session): -# self.__is_open = False -# self._remote_connection.remove_session(self._session_based_connection) -# return session - - def E(*args): return __.E(*args) diff --git a/gremlin-python/src/main/python/gremlin_python/process/traversal.py b/gremlin-python/src/main/python/gremlin_python/process/traversal.py index 76c91822e75..de2ce44f2eb 100644 --- a/gremlin-python/src/main/python/gremlin_python/process/traversal.py +++ b/gremlin-python/src/main/python/gremlin_python/process/traversal.py @@ -1141,22 +1141,6 @@ def __repr__(self): return (''.join(self.gremlin) if len(self.gremlin) > 0 else "") + \ (str(self.parameters) if len(self.parameters) > 0 else "") - # TODO to be removed or updated once HTTP transaction is implemented - # @staticmethod - # def _create_graph_op(name, *values): - # bc = Bytecode() - # bc.add_source(name, *values) - # return bc - # - # @staticmethod - # class GraphOp: - # @staticmethod - # def commit(): - # return Bytecode._create_graph_op("tx", "commit") - # - # @staticmethod - # def rollback(): - # return Bytecode._create_graph_op("tx", "rollback") class GValue: diff --git a/gremlin-python/src/main/python/tests/integration/driver/test_transaction.py b/gremlin-python/src/main/python/tests/integration/driver/test_transaction.py new file mode 100644 index 00000000000..4ba6283f913 --- /dev/null +++ b/gremlin-python/src/main/python/tests/integration/driver/test_transaction.py @@ -0,0 +1,345 @@ +# +# 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. +# +import os + +import pytest + +from gremlin_python.driver.client import Client +from gremlin_python.driver.driver_remote_connection import DriverRemoteConnection +from gremlin_python.process.anonymous_traversal import traversal + +gremlin_server_url = os.environ.get('GREMLIN_SERVER_URL', 'http://localhost:{}/gremlin') +test_no_auth_url = gremlin_server_url.format(45940) + + +@pytest.fixture +def client(): + c = Client(test_no_auth_url, 'gtx') + yield c + c.close() + + +@pytest.fixture +def remote_connection(): + rc = DriverRemoteConnection(test_no_auth_url, 'gtx') + yield rc + rc.close() + + +@pytest.fixture(autouse=True) +def clean_graph(): + """Drop all vertices before each test to prevent cross-test contamination.""" + c = Client(test_no_auth_url, 'gtx') + c.submit("g.V().drop()").all().result() + c.close() + yield + + +class TestTransaction(object): + + def test_should_commit_transaction(self, client): + tx = client.transact() + tx.begin() + assert tx.is_open + + tx.submit("g.addV('person').property('name','alice')") + + # Uncommitted data not visible outside the transaction + result = client.submit("g.V().hasLabel('person').count()").all().result() + assert result[0] == 0 + + tx.commit() + assert not tx.is_open + + # Committed data visible + result = client.submit("g.V().hasLabel('person').count()").all().result() + assert result[0] == 1 + + def test_should_rollback_transaction(self, client): + tx = client.transact() + tx.begin() + assert tx.is_open + + tx.submit("g.addV('person').property('name','bob')") + + tx.rollback() + assert not tx.is_open + + # Data discarded after rollback + result = client.submit("g.V().hasLabel('person').count()").all().result() + assert result[0] == 0 + + def test_should_support_intra_transaction_consistency(self, client): + tx = client.transact() + tx.begin() + + tx.submit("g.addV('test').property('name','A')") + # Read-your-own-writes + result = tx.submit("g.V().hasLabel('test').count()").all().result() + assert result[0] == 1 + + tx.submit("g.addV('test').property('name','B')") + tx.submit("g.V().has('name','A').addE('knows').to(V().has('name','B'))") + + result = tx.submit("g.V().hasLabel('test').count()").all().result() + assert result[0] == 2 + result = tx.submit("g.V().outE('knows').count()").all().result() + assert result[0] == 1 + + tx.commit() + + result = client.submit("g.V().hasLabel('test').count()").all().result() + assert result[0] == 2 + + def test_should_throw_on_submit_after_commit(self, client): + tx = client.transact() + tx.begin() + tx.submit("g.addV()") + tx.commit() + + with pytest.raises(Exception, match="Transaction is not open"): + tx.submit("g.V().count()") + + def test_should_throw_on_submit_after_rollback(self, client): + tx = client.transact() + tx.begin() + tx.submit("g.addV()") + tx.rollback() + + with pytest.raises(Exception, match="Transaction is not open"): + tx.submit("g.V().count()") + + def test_should_throw_on_double_begin(self, client): + tx = client.transact() + tx.begin() + + with pytest.raises(Exception, match="Transaction already started"): + tx.begin() + + def test_should_throw_on_commit_when_not_open(self, client): + tx = client.transact() + + with pytest.raises(Exception, match="Transaction is not open"): + tx.commit() + + def test_should_throw_on_rollback_when_not_open(self, client): + tx = client.transact() + + with pytest.raises(Exception, match="Transaction is not open"): + tx.rollback() + + def test_should_return_none_transaction_id_before_begin(self, client): + tx = client.transact() + assert tx.transaction_id is None + + tx.begin() + assert tx.transaction_id is not None + assert len(tx.transaction_id) > 0 + + def test_should_rollback_on_close_by_default(self, client): + tx = client.transact() + tx.begin() + tx.submit("g.addV('person').property('name','close_test')") + tx.close() + assert not tx.is_open + + # Data should NOT persist (rollback is default) + result = client.submit("g.V().hasLabel('person').count()").all().result() + assert result[0] == 0 + + def test_should_isolate_concurrent_transactions(self, client): + tx1 = client.transact() + tx1.begin() + tx2 = client.transact() + tx2.begin() + + tx1.submit("g.addV('tx1')") + tx2.submit("g.addV('tx2')") + + # tx1 should not see tx2's data and vice versa + result = tx1.submit("g.V().hasLabel('tx2').count()").all().result() + assert result[0] == 0 + result = tx2.submit("g.V().hasLabel('tx1').count()").all().result() + assert result[0] == 0 + + tx1.commit() + tx2.commit() + + # Both should be visible after commit + result = client.submit("g.V().hasLabel('tx1').count()").all().result() + assert result[0] == 1 + result = client.submit("g.V().hasLabel('tx2').count()").all().result() + assert result[0] == 1 + + def test_should_isolate_transactional_and_non_transactional_requests(self, client): + tx = client.transact() + tx.begin() + tx.submit("g.addV('tx_data')") + + # Non-transactional read should not see uncommitted data + result = client.submit("g.V().hasLabel('tx_data').count()").all().result() + assert result[0] == 0 + + tx.commit() + + result = client.submit("g.V().hasLabel('tx_data').count()").all().result() + assert result[0] == 1 + + def test_should_open_and_close_many_transactions_sequentially(self, client): + num_transactions = 50 + for i in range(num_transactions): + tx = client.transact() + tx.begin() + tx.submit("g.addV('stress')") + tx.commit() + + result = client.submit("g.V().hasLabel('stress').count()").all().result() + assert result[0] == num_transactions + + def test_should_keep_transaction_open_after_traversal_error(self, client): + tx = client.transact() + tx.begin() + tx.submit("g.addV('good_vertex')") + + # Submit a bad traversal that should fail + try: + tx.submit("g.V().fail()") + except Exception: + pass + + # Transaction should still be open + assert tx.is_open + tx.rollback() + + assert not tx.is_open + result = client.submit("g.V().hasLabel('good_vertex').count()").all().result() + assert result[0] == 0 + + def test_should_work_with_traversal_api(self, remote_connection): + g = traversal().with_(remote_connection) + + tx = g.tx() + gtx = tx.begin() + gtx.addV("val").iterate() + tx.commit() + + assert g.V().hasLabel("val").count().next() == 1 + + def test_context_manager_rollback_on_exception(self, client): + try: + with client.transact() as tx: + tx.begin() + tx.submit("g.addV('ctx_test')") + raise RuntimeError("simulated error") + except RuntimeError: + pass + + # Data should NOT persist (context manager rolls back) + result = client.submit("g.V().hasLabel('ctx_test').count()").all().result() + assert result[0] == 0 + + + def test_should_reject_begin_on_non_transactional_graph(self): + # gclassic is a non-transactional graph alias + c = Client(test_no_auth_url, 'gclassic') + try: + tx = c.transact() + with pytest.raises(Exception, match="Graph does not support transactions"): + tx.begin() + finally: + c.close() + + def test_should_clean_up_on_begin_failure(self): + c = Client(test_no_auth_url, 'gclassic') + try: + tx = c.transact() + try: + tx.begin() + except Exception: + pass + + # Transaction should not be open after failed begin + assert not tx.is_open + assert tx.transaction_id is None + + # Cannot begin again on a failed transaction + with pytest.raises(Exception): + tx.begin() + finally: + c.close() + + + def test_should_return_same_transaction_from_gtx_tx(self, client): + tx = client.transact() + gtx = tx.begin() + same_tx = gtx.tx() + assert same_tx is tx + + def test_should_throw_on_begin_from_gtx_tx(self, client): + tx = client.transact() + gtx = tx.begin() + same_tx = gtx.tx() + + with pytest.raises(Exception, match="Transaction already started"): + same_tx.begin() + + tx.rollback() + + def test_should_commit_via_gtx_tx(self, client): + tx = client.transact() + gtx = tx.begin() + gtx.addV("gtx_commit_test").iterate() + gtx.tx().commit() + + result = client.submit("g.V().hasLabel('gtx_commit_test').count()").all().result() + assert result[0] == 1 + + + def test_should_throw_on_double_commit(self, client): + tx = client.transact() + tx.begin() + tx.commit() + + with pytest.raises(Exception, match="Transaction is not open"): + tx.commit() + + def test_should_throw_on_double_rollback(self, client): + tx = client.transact() + tx.begin() + tx.rollback() + + with pytest.raises(Exception, match="Transaction is not open"): + tx.rollback() + + + def test_should_not_allow_begin_after_commit(self, client): + tx = client.transact() + tx.begin() + tx.commit() + + with pytest.raises(Exception, match="Transaction already started"): + tx.begin() + + def test_should_not_allow_begin_after_rollback(self, client): + tx = client.transact() + tx.begin() + tx.rollback() + + with pytest.raises(Exception, match="Transaction already started"): + tx.begin() diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverTransactionIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverTransactionIntegrateTest.java index af35cf43923..8715da8c67a 100644 --- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverTransactionIntegrateTest.java +++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverTransactionIntegrateTest.java @@ -249,31 +249,31 @@ public void shouldReturnNullTransactionIdBeforeBegin() throws Exception { } @Test - public void shouldCommitOnCloseByDefault() throws Exception { + public void shouldRollbackOnCloseByDefault() throws Exception { final RemoteTransaction tx1 = cluster.transact(GTX); tx1.begin(); tx1.submit("g.addV('person').property('name','close_test')"); - // close() should trigger default COMMIT behavior + // close() should trigger default ROLLBACK behavior tx1.close(); assertFalse(tx1.isOpen()); final RemoteTransaction tx2 = cluster.transact(GTX); tx2.begin(); - assertEquals(1L, tx2.submit("g.V().hasLabel('person').count()").one().getLong()); + assertEquals(0L, tx2.submit("g.V().hasLabel('person').count()").one().getLong()); } @Test - public void shouldRollbackOnCloseWhenConfigured() throws Exception { + public void shouldCommitOnCloseWhenConfigured() throws Exception { final RemoteTransaction tx1 = cluster.transact(GTX); - tx1.onClose(Transaction.CLOSE_BEHAVIOR.ROLLBACK); + tx1.onClose(Transaction.CLOSE_BEHAVIOR.COMMIT); tx1.begin(); - tx1.submit("g.addV('person').property('name','rollback_close_test')"); + tx1.submit("g.addV('person').property('name','commit_close_test')"); tx1.close(); assertFalse(tx1.isOpen()); final RemoteTransaction tx2 = cluster.transact(GTX); tx2.begin(); - assertEquals(0L, tx2.submit("g.V().hasLabel('person').count()").one().getLong()); + assertEquals(1L, tx2.submit("g.V().hasLabel('person').count()").one().getLong()); } @Test