Enable autocommit in GremlinServer

docs/src/dev/provider/index.asciidoc

@@ -1260,8 +1260,25 @@ Graph transactions are typically `ThreadLocal`-bound. The server maintains a sin
 to ensure all operations execute on the same thread. This is an important implementation detail for graph system
 providers whose `Transaction` implementation relies on thread-local state.
 
-NOTE: Non-transactional requests (those without a `transactionId`) are not affected by any of the transaction-specific
-behavior described above. They continue to operate exactly as before.
+==== Auto-Commit for Non-Transactional Requests
+
+Non-transactional requests (those without a `transactionId`) are not affected by any of the transaction-specific
+behavior described above. However, if the underlying graph supports transactions, the server must automatically manage
+transactions for these requests:
+
+* Before processing: roll back any stale open transaction to prevent state leakage between requests.
+* On success: commit the transaction after the traversal has been fully iterated and serialized.
+* On error: roll back the transaction so that partial mutations are not persisted.
+
+This auto-commit behavior ensures that users who do not use explicit transactions still get durable writes on success
+and clean rollback on failure. Graph system providers implementing their own server or HTTP endpoint must replicate
+this behavior if their graph implementation supports transactions. The commit occurs after serialization is complete
+but before the final response is flushed to the client, so the client only receives a success response after the
+data is committed.
+
+IMPORTANT: This auto-commit/rollback logic must only apply to non-transactional requests. Requests that carry a
+`transactionId` are part of a client-managed transaction and must not be auto-committed or auto-rolled-back by the
+server.
 
 === HTTP Request Interceptor
 

docs/src/reference/gremlin-applications.asciidoc

@@ -2242,10 +2242,14 @@ above with the use of the `maximumSize`.
 [[considering-transactions]]
 ==== Considering Transactions
 
-Non-transactional requests (those without a `transactionId`) behave as self-contained units of work where the graph's
-own transaction semantics apply. Each traversal executes within its own transaction as managed by the graph
-implementation itself. Transactional requests participate in a transaction opened via `g.tx().begin()`, where the
-client explicitly controls the lifecycle through `g.tx().commit()` and `g.tx().rollback()`.
+Non-transactional requests (those without a `transactionId`) behave as self-contained units of work. If the underlying
+graph supports transactions, the server automatically commits after successfully iterating the traversal and
+automatically rolls back on any error. This means that a simple `g.addV('person')` will be persisted immediately upon
+success without requiring an explicit `commit()`. Before processing each non-transactional request, the server also
+rolls back any stale open transaction to prevent state leakage between requests.
+
+Transactional requests participate in a transaction opened via `g.tx().begin()`, where the client explicitly controls
+the lifecycle through `g.tx().commit()` and `g.tx().rollback()`.
 
 IMPORTANT: Understand the transactional capabilities of the graph configured in Gremlin Server. For example, a basic
 `TinkerGraph` does not support transactions. Use `TinkerTransactionGraph` or another transaction-capable graph

docs/src/reference/the-traversal.asciidoc

@@ -89,9 +89,11 @@ in relation to the usage convention and graph provider caveats alluded to earlie
 
 Focusing on remote contexts first, note that it is still possible to issue traversals from `g`, but those will have a
 transaction scope outside of `gtx` and will simply behave as self-contained units of work where the graph's own
-transaction semantics apply (i.e. one traversal is one transaction). Each isolated transaction will require its own
-`Transaction` object. Multiple `begin()` calls on the same `Transaction` object are not permitted and will throw an
-`IllegalStateException`:
+transaction semantics apply (i.e. one traversal is one transaction). For graphs that support transactions, the server
+automatically commits on successful completion of the traversal and rolls back on any error, so a simple
+`g.addV('person')` will be persisted immediately upon success without requiring an explicit `commit()`. Each isolated
+transaction will require its own `Transaction` object. Multiple `begin()` calls on the same `Transaction` object are
+not permitted and will throw an `IllegalStateException`:
 
 [source,java]
 ----

gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpGremlinEndpointHandler.java

@@ -418,6 +418,17 @@ private void iterateScriptEvalResult(final Context context, MessageSerializer<?>
         }
 
         final Bindings mergedBindings = mergeBindingsFromRequest(context, new SimpleBindings(graphManager.getAsBindings()));
+
+        // resolve the graph for auto-transaction management on non-transactional requests
+        final String g = message.getField(Tokens.ARGS_G);
+        final TraversalSource ts = g != null ? graphManager.getTraversalSource(g) : null;
+        final Graph graph = ts != null ? ts.getGraph() : null;
+        final boolean autoCommit = (context.getTransactionId() == null) && (graph != null) &&
+                graph.features().graph().supportsTransactions();
+
+        // rollback any stale open transaction before processing
+        if (autoCommit && graph.tx().isOpen()) graph.tx().rollback();
+
         final Object result = scriptEngine.eval(message.getGremlin(), mergedBindings);
 
         final String bulkingSetting = context.getChannelHandlerContext().channel().attr(StateKey.REQUEST_HEADERS).get().get(Tokens.BULK_RESULTS);
@@ -440,15 +451,19 @@ private void iterateScriptEvalResult(final Context context, MessageSerializer<?>
                 itty = IteratorUtils.asIterator(result);
                 handleIterator(context, itty, serializer, false);
             }
-        } catch (Exception ex) {
+
+            if (autoCommit && graph.tx().isOpen()) graph.tx().commit();
+        } catch (Throwable t) {
+            if (autoCommit && graph.tx().isOpen()) graph.tx().rollback();
+
             // TINKERPOP-3144 ensure Traversals are closed when exception thrown.
             if (itty instanceof TraverserIterator) {
                 CloseableIterator.closeIterator(((TraverserIterator) itty).getTraversal());
             } else if (itty != null) {
                 CloseableIterator.closeIterator(itty);
             }
 
-            throw ex;
+            throw t;
         }
     }
 

gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverTransactionIntegrateTest.java

@@ -647,4 +647,31 @@ public void shouldRejectTransactionWithUnsupportedTypeInGValue() throws Exceptio
             gtx.tx().rollback();
         }
     }
+
+    @Test
+    public void shouldAutoCommitNonTransactionalMutatingTraversal() throws Exception {
+        final Client client = cluster.connect().alias(GTX);
+
+        // add a vertex without explicit transaction management - should be auto-committed
+        client.submit("g.addV('person').property('name','alice')").all().get();
+
+        // verify persisted on a subsequent request
+        assertEquals(1L, client.submit("g.V().hasLabel('person').count()").all().get().get(0).getLong());
+    }
+
+    @Test
+    public void shouldAutoRollbackOnFailedNonTransactionalMutatingTraversal() throws Exception {
+        final Client client = cluster.connect().alias(GTX);
+
+        // submit a traversal that mutates then fails - should be rolled back
+        try {
+            client.submit("g.addV('person').fail()").all().get();
+            fail("Expected an exception");
+        } catch (Exception ex) {
+            // expected
+        }
+
+        // verify nothing was persisted
+        assertEquals(0L, client.submit("g.V().hasLabel('person').count()").all().get().get(0).getLong());
+    }
 }

gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerHttpIntegrateTest.java

@@ -111,6 +111,8 @@ public Settings overrideSettings(final Settings settings) {
                 settings.maxRequestContentLength = 31;
                 break;
             case "should200OnPOSTTransactionalGraph":
+            case "shouldRollbackOnFailedMutatingTraversal":
+            case "shouldCommitMutatingTraversalWithEmptyResult":
                 useTinkerTransactionGraph(settings);
                 break;
             case "should200OnPOSTTransactionalGraphInStrictMode":
@@ -489,36 +491,97 @@ public void should200OnPOSTWithGremlinJsonEndcodedBodyForJavaTime() throws Excep
         }
     }
 
-    /*@Test disabled for now as current implementation doesn't support implicit transactions.
+    @Test
     public void should200OnPOSTTransactionalGraph() throws Exception {
         final CloseableHttpClient httpclient = HttpClients.createDefault();
+
+        // add a vertex without an explicit transaction - should be auto-committed
         final HttpPost httppost = new HttpPost(TestClientFactory.createURLString());
         httppost.addHeader("Content-Type", "application/json");
-        httppost.setEntity(new StringEntity("{\"gremlin\":\"graph.addVertex('name','stephen');g.V().count()\"}", Consts.UTF_8));
+        httppost.setEntity(new StringEntity(
+                "{\"gremlin\":\"g.addV('person').property('name','stephen')\",\"g\":\"g\"}", Consts.UTF_8));
 
         try (final CloseableHttpResponse response = httpclient.execute(httppost)) {
             assertEquals(200, response.getStatusLine().getStatusCode());
-            assertEquals("application/json", response.getEntity().getContentType().getValue());
-            final String json = EntityUtils.toString(response.getEntity());
-            final JsonNode node = mapper.readTree(json);
-            assertEquals(1, node.get("result").get(TOKEN_DATA).get(GraphSONTokens.VALUEPROP).get(0).get(GraphSONTokens.VALUEPROP).intValue());
+            EntityUtils.consume(response.getEntity());
         }
 
-        final HttpGet httpget = new HttpGet(TestClientFactory.createURLString("?gremlin=g.V().count()"));
-        httpget.addHeader("Accept", "application/json");
+        // verify the vertex is visible on subsequent requests from potentially different threads
+        final HttpPost countPost = new HttpPost(TestClientFactory.createURLString());
+        countPost.addHeader("Content-Type", "application/json");
+        countPost.setEntity(new StringEntity("{\"gremlin\":\"g.V().count()\",\"g\":\"g\"}", Consts.UTF_8));
 
-        // execute this a bunch of times so that there's a good chance a different thread on the server processes
-        // the request
         for (int ix = 0; ix < 100; ix++) {
-            try (final CloseableHttpResponse response = httpclient.execute(httpget)) {
+            try (final CloseableHttpResponse response = httpclient.execute(countPost)) {
                 assertEquals(200, response.getStatusLine().getStatusCode());
-                assertEquals("application/json", response.getEntity().getContentType().getValue());
                 final String json = EntityUtils.toString(response.getEntity());
                 final JsonNode node = mapper.readTree(json);
-                assertEquals(1, node.get("result").get(TOKEN_DATA).get(GraphSONTokens.VALUEPROP).get(0).get(GraphSONTokens.VALUEPROP).intValue());
+                assertEquals(1, node.get("result").get(TOKEN_DATA)
+                        .get(GraphSONTokens.VALUEPROP).get(0)
+                        .get(GraphSONTokens.VALUEPROP).intValue());
             }
         }
-    } */
+    }
+
+    @Test
+    public void shouldRollbackOnFailedMutatingTraversal() throws Exception {
+        final CloseableHttpClient httpclient = HttpClients.createDefault();
+
+        // submit a traversal that adds a vertex then fails - should be rolled back
+        final HttpPost httppost = new HttpPost(TestClientFactory.createURLString());
+        httppost.addHeader("Content-Type", "application/json");
+        httppost.setEntity(new StringEntity("{\"gremlin\":\"g.addV('person').fail()\",\"g\":\"g\"}", Consts.UTF_8));
+
+        try (final CloseableHttpResponse response = httpclient.execute(httppost)) {
+            // the fail() error appears in the response body, not the HTTP status
+        }
+
+        // verify the vertex was not persisted
+        final HttpPost countPost = new HttpPost(TestClientFactory.createURLString());
+        countPost.addHeader("Content-Type", "application/json");
+        countPost.setEntity(new StringEntity(
+                "{\"gremlin\":\"g.V().hasLabel('person').count()\",\"g\":\"g\"}", Consts.UTF_8));
+
+        try (final CloseableHttpResponse response = httpclient.execute(countPost)) {
+            assertEquals(200, response.getStatusLine().getStatusCode());
+            final String json = EntityUtils.toString(response.getEntity());
+            final JsonNode node = mapper.readTree(json);
+            assertEquals(0, node.get("result").get(TOKEN_DATA)
+                    .get(GraphSONTokens.VALUEPROP).get(0)
+                    .get(GraphSONTokens.VALUEPROP).intValue());
+        }
+    }
+
+    @Test
+    public void shouldCommitMutatingTraversalWithEmptyResult() throws Exception {
+        final CloseableHttpClient httpclient = HttpClients.createDefault();
+
+        // g.addV() followed by iterate-style consumption returns no results but should still commit
+        final HttpPost httppost = new HttpPost(TestClientFactory.createURLString());
+        httppost.addHeader("Content-Type", "application/json");
+        httppost.setEntity(new StringEntity(
+                "{\"gremlin\":\"g.addV('person').property('name','p').iterate()\",\"g\":\"g\"}", Consts.UTF_8));
+
+        try (final CloseableHttpResponse response = httpclient.execute(httppost)) {
+            assertEquals(200, response.getStatusLine().getStatusCode());
+            EntityUtils.consume(response.getEntity());
+        }
+
+        // verify the vertex was committed despite the empty result set
+        final HttpPost countPost2 = new HttpPost(TestClientFactory.createURLString());
+        countPost2.addHeader("Content-Type", "application/json");
+        countPost2.setEntity(new StringEntity(
+                "{\"gremlin\":\"g.V().hasLabel('person').count()\",\"g\":\"g\"}", Consts.UTF_8));
+
+        try (final CloseableHttpResponse response = httpclient.execute(countPost2)) {
+            assertEquals(200, response.getStatusLine().getStatusCode());
+            final String json = EntityUtils.toString(response.getEntity());
+            final JsonNode node = mapper.readTree(json);
+            assertEquals(1, node.get("result").get(TOKEN_DATA)
+                    .get(GraphSONTokens.VALUEPROP).get(0)
+                    .get(GraphSONTokens.VALUEPROP).intValue());
+        }
+    }
 
     @Test
     public void should200OnPOSTTransactionalGraphInStrictMode() throws Exception {