Native transactions: no JSON decode for a status the caller does not read

src/arcadedb/_native/database.py

@@ -65,6 +65,14 @@ def _call(prefix: str, name: str, *args) -> dict:
     return reply
 
 
+# Every successful ``arcadedb_n_tx`` reply is ``{"ok":true,"active":<bool>,"depth":<n>,"commits":<n>}`` (``ArcadeNative.txState``). A caller
+# that reads none of the state (begin, commit, rollback), or only ``active``, tells success and that bool from the text it already holds
+# instead of decoding a JSON object. An error reply is ``{"ok":false,...}``: it fails the prefix test and is decoded, as before.
+_TX_OK = '{"ok":true,'
+_TX_ACTIVE = '{"ok":true,"active":true,'
+_TX_IDLE = '{"ok":true,"active":false,'
+
+
 class NativeResult:
     """One row. Values are already Python types (datetime, Decimal, lists, dicts, RID text)."""
 
@@ -576,24 +584,36 @@ def command(self, language: str, command: str, *args) -> NativeResultSet:
 
     # -- transactions
 
-    def _tx(self, op: int, prefix: str) -> dict:
-        self._check_not_closed()
+    def _tx_text(self, op: int, prefix: str) -> str:
+        """One ``arcadedb_n_tx`` crossing: the reply text of a success (not decoded), an ArcadeDBError for a failure."""
+        if self._closed:
+            raise ArcadeDBError("Database is closed")
         if (op == 1 or op == 2) and lib.CLOSE_BATCH:
             lib.flush_closes()
-        return _call(prefix, "arcadedb_n_tx", self._h, op)
+        text = lib.call("arcadedb_n_tx", self._h, op)
+        if text.startswith(_TX_OK):
+            return text
+        reply = json.loads(text)
+        if not reply.get("ok"):
+            raise _error(prefix, reply)
+        return text
+
+    def _tx(self, op: int, prefix: str) -> dict:
+        """The decoded state reply: for a caller that reads ``depth`` or ``commits``."""
+        return json.loads(self._tx_text(op, prefix))
 
     def begin(self):
-        self._begin_state()
+        self._tx_text(0, "Failed to begin transaction")
 
     def _begin_state(self) -> dict:
         """begin(), and the state right after it: the reply of ``arcadedb_n_tx`` carries ``active``, ``depth`` and ``commits``."""
         return self._tx(0, "Failed to begin transaction")
 
     def commit(self):
-        self._tx(1, "Failed to commit transaction")
+        self._tx_text(1, "Failed to commit transaction")
 
     def rollback(self):
-        self._rollback_state()
+        self._tx_text(2, "Failed to rollback transaction")
 
     def _rollback_state(self) -> dict:
         """rollback(), and the state right after it (the reply carries it, as begin's does): the unwind reads it instead of asking."""
@@ -603,7 +623,12 @@ def transaction(self) -> TransactionContext:
         return TransactionContext(self)
 
     def is_transaction_active(self) -> bool:
-        return self._tx(3, "Failed to check transaction status")["active"]
+        text = self._tx_text(3, "Failed to check transaction status")
+        if text.startswith(_TX_ACTIVE):
+            return True
+        if text.startswith(_TX_IDLE):
+            return False
+        return json.loads(text)["active"]
 
     def _transaction_stack_mark(self):
         state = self._tx(3, "Failed to read the transaction state")

tests/test_native_tx_crossings.py

@@ -11,8 +11,11 @@
 scripts/check_test_skips.py); it runs in the native parity run.
 """
 
-import arcadedb as arcadedb
+import json
+
 import pytest
+
+import arcadedb as arcadedb
 from arcadedb import ArcadeDBError
 from arcadedb._native import lib
 
@@ -188,3 +191,95 @@ def test_begin_commit_rollback_return_nothing(db):
     assert db.rollback() is None
     assert db.begin() is None
     assert db.commit() is None
+
+
+# -- decodes: a status the caller does not read is not a JSON decode
+
+
+class _Decodes:
+    """Counts every ``json.loads`` while it is installed (the transaction replies are decoded with it)."""
+
+    def __init__(self, monkeypatch):
+        self.n = 0
+        real = json.loads
+
+        def counting(*args, **kwargs):
+            self.n += 1
+            return real(*args, **kwargs)
+
+        monkeypatch.setattr(json, "loads", counting)
+
+
+@pytest.fixture
+def decodes(db, monkeypatch):
+    return _Decodes(monkeypatch)
+
+
+def test_begin_commit_rollback_decode_nothing(db, decodes):
+    db.begin()
+    db.commit()
+    db.begin()
+    db.rollback()
+    assert decodes.n == 0
+
+
+def test_is_transaction_active_decodes_nothing_and_reports_the_state(db, decodes):
+    assert db.is_transaction_active() is False
+    db.begin()
+    assert db.is_transaction_active() is True
+    db.commit()
+    assert db.is_transaction_active() is False
+    assert decodes.n == 0
+
+
+def test_with_transaction_decodes_only_the_mark(db, decodes):
+    # the mark reads two fields (active, depth) and is decoded; begin and commit read none
+    with db.transaction():
+        pass
+    assert decodes.n == 1
+
+
+def test_a_state_reply_still_carries_depth_and_commits(db):
+    db.begin()
+    db.begin()
+    assert db._transaction_stack_mark() == (True, 2)
+    db.rollback()
+    db.rollback()
+    assert db._transaction_stack_mark()[0] is False
+    assert db._begin_state()["depth"] == 1
+    assert db._rollback_state()["active"] is False
+
+
+def test_an_error_reply_is_decoded_into_the_same_error(db, monkeypatch):
+    reply = (
+        '{"ok":false,"error":"boom","cls":"x.Y","chain":[["x.Y","boom"]],'
+        '"retry":"need_retry","retry_ms":3}'
+    )
+    real = lib.call
+    monkeypatch.setattr(
+        lib,
+        "call",
+        lambda name, *a: reply if name == "arcadedb_n_tx" else real(name, *a),
+    )
+    for op, prefix in (
+        (db.begin, "Failed to begin transaction"),
+        (db.commit, "Failed to commit transaction"),
+        (db.rollback, "Failed to rollback transaction"),
+        (db.is_transaction_active, "Failed to check transaction status"),
+    ):
+        with pytest.raises(ArcadeDBError) as caught:
+            op()
+        assert str(caught.value) == f"{prefix}: x.Y: boom"
+        assert caught.value.native["retry"] == "need_retry"
+        assert caught.value.java_chain == [["x.Y", "boom"]]
+
+
+def test_a_differently_spaced_ok_reply_is_still_read(db, monkeypatch):
+    monkeypatch.setattr(
+        lib,
+        "call",
+        lambda name, *a: '{"ok": true, "active": true, "depth": 1, "commits": 0}',
+    )
+    assert db.is_transaction_active() is True
+    db.begin()
+    db.commit()