Skip to content

Commit b630ba0

Browse files
robfrankclaude
andcommitted
fix: include transactionId in gRPC CRUD requests to support external transactions (#3524)
saveRecord() and deleteRecord() in RemoteGrpcDatabase did not include the active transactionId in gRPC requests, and the server-side handlers did not look up external transactions from the activeTransactions map. This caused "Transaction not active" errors when using begin()/commit() with vertex save or delete operations. Refactored server handlers to follow the same pattern as executeCommand() — dispatching to the transaction's dedicated executor thread when an external transaction is active. Co-Authored-By: Claude Opus 4.6 <[email protected]>
1 parent dd6d413 commit b630ba0

3 files changed

Lines changed: 307 additions & 170 deletions

File tree

grpc-client/src/main/java/com/arcadedb/remote/grpc/RemoteGrpcDatabase.java

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -372,9 +372,14 @@ public void deleteRecord(final Record record) {
372372
if (record.getIdentity() == null)
373373
throw new IllegalArgumentException("Cannot delete a non persistent record");
374374

375-
final DeleteRecordRequest req =
375+
final DeleteRecordRequest.Builder deleteBuilder =
376376
DeleteRecordRequest.newBuilder().setDatabase(getName()).setRid(record.getIdentity().toString())
377-
.setCredentials(buildCredentials()).build();
377+
.setCredentials(buildCredentials());
378+
379+
if (transactionId != null)
380+
deleteBuilder.setTransaction(TransactionContext.newBuilder().setTransactionId(transactionId).setDatabase(getName()).build());
381+
382+
final DeleteRecordRequest req = deleteBuilder.build();
378383

379384
try {
380385
if (LogManager.instance().isDebugEnabled()) {
@@ -402,9 +407,13 @@ public boolean deleteRecord(final String rid, final long timeoutMs) {
402407
checkDatabaseIsOpen();
403408
stats.deleteRecord.incrementAndGet();
404409

405-
final DeleteRecordRequest req = DeleteRecordRequest.newBuilder().setDatabase(getName()).setRid(rid)
406-
.setCredentials(buildCredentials())
407-
.build();
410+
final DeleteRecordRequest.Builder deleteBuilder = DeleteRecordRequest.newBuilder().setDatabase(getName()).setRid(rid)
411+
.setCredentials(buildCredentials());
412+
413+
if (transactionId != null)
414+
deleteBuilder.setTransaction(TransactionContext.newBuilder().setTransactionId(transactionId).setDatabase(getName()).build());
415+
416+
final DeleteRecordRequest req = deleteBuilder.build();
408417

409418
try {
410419
if (LogManager.instance().isDebugEnabled()) {
@@ -654,15 +663,17 @@ protected RID saveRecord(final MutableDocument record) {
654663
PropertiesUpdate.newBuilder().putAllProperties(convertParamsToGrpcValue(record.toMap(false)))
655664
.build();
656665

657-
UpdateRecordRequest request = UpdateRecordRequest.newBuilder().setDatabase(getName()).setRid(rid.toString())
658-
.setPartial(partial)
659-
.setDatabase(databaseName).setCredentials(buildCredentials()).build();
666+
UpdateRecordRequest.Builder updateBuilder = UpdateRecordRequest.newBuilder().setDatabase(getName())
667+
.setRid(rid.toString()).setPartial(partial).setDatabase(databaseName).setCredentials(buildCredentials());
668+
669+
if (transactionId != null)
670+
updateBuilder.setTransaction(TransactionContext.newBuilder().setTransactionId(transactionId).setDatabase(getName()).build());
660671

661672
try {
662673

663674
@SuppressWarnings("unused")
664675
UpdateRecordResponse response =
665-
blockingStub.withDeadlineAfter(getTimeout(), TimeUnit.MILLISECONDS).updateRecord(request);
676+
blockingStub.withDeadlineAfter(getTimeout(), TimeUnit.MILLISECONDS).updateRecord(updateBuilder.build());
666677

667678
// If your proto has flags, you can check response.getSuccess()/getUpdated()
668679
// Otherwise, treat non-exception as success.
@@ -679,12 +690,14 @@ protected RID saveRecord(final MutableDocument record) {
679690
GrpcRecord recMsg =
680691
GrpcRecord.newBuilder().putAllProperties(convertParamsToGrpcValue(record.toMap(false))).build();
681692

682-
CreateRecordRequest request =
693+
CreateRecordRequest.Builder createBuilder =
683694
CreateRecordRequest.newBuilder().setDatabase(getName()).setType(record.getTypeName())
684-
.setRecord(recMsg) // nested
685-
// GrpcRecord
686-
// payload
687-
.setCredentials(buildCredentials()).build();
695+
.setRecord(recMsg).setCredentials(buildCredentials());
696+
697+
if (transactionId != null)
698+
createBuilder.setTransaction(TransactionContext.newBuilder().setTransactionId(transactionId).setDatabase(getName()).build());
699+
700+
final CreateRecordRequest request = createBuilder.build();
688701

689702
try {
690703
CreateRecordResponse response =

grpc-client/src/test/java/com/arcadedb/remote/grpc/RemoteGrpcDatabaseRegressionTest.java

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
package com.arcadedb.remote.grpc;
2020

2121
import com.arcadedb.GlobalConfiguration;
22+
import com.arcadedb.graph.MutableVertex;
2223
import com.arcadedb.query.sql.executor.Result;
2324
import com.arcadedb.query.sql.executor.ResultSet;
2425
import com.arcadedb.test.BaseGraphServerTest;
@@ -305,6 +306,96 @@ void embeddedAuditMetadataRoundTrip() {
305306
}
306307
}
307308

309+
@Test
310+
@DisplayName("Issue #3524: newVertex().save() and newEdge() within a transaction should not throw 'Transaction not active'")
311+
void vertexSaveAndEdgeCreationWithinTransaction() {
312+
final String VERTEX_TYPE = "SVEx3524";
313+
final String EDGE_TYPE = "SVEx3524_Edge";
314+
315+
grpc.command("sql", "CREATE VERTEX TYPE `" + VERTEX_TYPE + "` IF NOT EXISTS", Map.of());
316+
grpc.command("sql", "CREATE PROPERTY `" + VERTEX_TYPE + "`.svex IF NOT EXISTS STRING", Map.of());
317+
grpc.command("sql", "CREATE EDGE TYPE `" + EDGE_TYPE + "` IF NOT EXISTS", Map.of());
318+
319+
try {
320+
grpc.begin();
321+
322+
MutableVertex svt1 = grpc.newVertex(VERTEX_TYPE);
323+
svt1.set("svex", "svt1");
324+
svt1.save();
325+
326+
MutableVertex svt2 = grpc.newVertex(VERTEX_TYPE);
327+
svt2.set("svex", "svt2");
328+
svt2.save();
329+
330+
// This should not throw 'Transaction not active'
331+
svt1.newEdge(EDGE_TYPE, svt2);
332+
333+
// This save (updating the vertex after edge creation) should also work within the transaction
334+
svt1.save();
335+
336+
grpc.commit();
337+
338+
// Verify both vertices exist
339+
try (ResultSet rs = grpc.query("sql", "SELECT count(*) AS c FROM `" + VERTEX_TYPE + "`", Map.of())) {
340+
assertThat(rs.hasNext()).isTrue();
341+
Number count = rs.next().getProperty("c");
342+
assertThat(count.longValue()).as("both vertices should be persisted").isEqualTo(2L);
343+
}
344+
345+
// Verify the edge exists
346+
try (ResultSet rs = grpc.query("sql",
347+
"SELECT count(*) AS c FROM `" + EDGE_TYPE + "`", Map.of())) {
348+
assertThat(rs.hasNext()).isTrue();
349+
Number count = rs.next().getProperty("c");
350+
assertThat(count.longValue()).as("edge should be persisted").isEqualTo(1L);
351+
}
352+
} finally {
353+
grpc.command("sql", "DELETE FROM `" + EDGE_TYPE + "`", Map.of());
354+
grpc.command("sql", "DELETE FROM `" + VERTEX_TYPE + "`", Map.of());
355+
}
356+
}
357+
358+
@Test
359+
@DisplayName("Issue #3524: newVertex().save() within a transaction should persist only on commit and rollback on abort")
360+
void vertexSaveRollbackWithinTransaction() {
361+
final String VERTEX_TYPE = "SVEx3524Roll";
362+
363+
grpc.command("sql", "CREATE VERTEX TYPE `" + VERTEX_TYPE + "` IF NOT EXISTS", Map.of());
364+
grpc.command("sql", "CREATE PROPERTY `" + VERTEX_TYPE + "`.svex IF NOT EXISTS STRING", Map.of());
365+
366+
try {
367+
// Verify rollback: creates vertices and then rolls back
368+
grpc.begin();
369+
370+
MutableVertex svt1 = grpc.newVertex(VERTEX_TYPE);
371+
svt1.set("svex", "rollback1");
372+
svt1.save();
373+
374+
grpc.rollback();
375+
376+
try (ResultSet rs = grpc.query("sql", "SELECT count(*) AS c FROM `" + VERTEX_TYPE + "`", Map.of())) {
377+
Number count = rs.next().getProperty("c");
378+
assertThat(count.longValue()).as("vertex should NOT be persisted after rollback").isEqualTo(0L);
379+
}
380+
381+
// Verify commit: creates a vertex and commits
382+
grpc.begin();
383+
384+
MutableVertex svt2 = grpc.newVertex(VERTEX_TYPE);
385+
svt2.set("svex", "committed");
386+
svt2.save();
387+
388+
grpc.commit();
389+
390+
try (ResultSet rs = grpc.query("sql", "SELECT count(*) AS c FROM `" + VERTEX_TYPE + "`", Map.of())) {
391+
Number count = rs.next().getProperty("c");
392+
assertThat(count.longValue()).as("vertex should be persisted after commit").isEqualTo(1L);
393+
}
394+
} finally {
395+
grpc.command("sql", "DELETE FROM `" + VERTEX_TYPE + "`", Map.of());
396+
}
397+
}
398+
308399
@Test
309400
@DisplayName("Issue #2854: gRPC ResultSet should preserve SQL aliases")
310401
void sqlAliasesArePreservedInGrpcResultSet() {

0 commit comments

Comments
 (0)