Add tests for eager BEGIN on tx run (before tx func call)

nutkit/protocol/feature.py

@@ -11,14 +11,19 @@ class Feature(Enum):
     # a started job. All other connections should be re-established before
     # running the next job with them.
     OPT_AUTHORIZATION_EXPIRED_TREATMENT = 'AuthorizationExpiredTreatment'
+    # The driver caches connections (e.g., in a pool) and doesn't start a new
+    # one (with hand-shake, HELLO, etc.) for each query.
+    OPT_CONNECTION_REUSE = "Optimization:ConnectionReuse"
+    # The driver first tries to SUCCESSfully BEGIN a transaction before calling
+    # the user-defined transaction function. This way, the (potentially costly)
+    # transaction function is not started until a working transaction has been
+    # established.
+    OPT_EAGER_TX_BEGIN = "Optimization:EagerTransactionBegin"
     # Driver doesn't explicitly send message data that is the default value.
     # This conserves bandwidth.
     OPT_IMPLICIT_DEFAULT_ARGUMENTS = "Optimization:ImplicitDefaultArguments"
     # The driver sends no more than the strictly necessary RESET messages.
     OPT_MINIMAL_RESETS = "Optimization:MinimalResets"
-    # The driver caches connections (e.g., in a pool) and doesn't start a new
-    # one (with hand-shake, HELLO, etc.) for each query.
-    OPT_CONNECTION_REUSE = "Optimization:ConnectionReuse"
     # The driver doesn't wait for a SUCCESS after calling RUN but pipelines a
     # PULL right afterwards and consumes two messages after that. This saves a
     # full round-trip.
@@ -33,21 +38,9 @@ class Feature(Enum):
     # as well.
     CONF_HINT_CON_RECV_TIMEOUT = "ConfHint:connection.recv_timeout_seconds"
 
-    # Temporary driver feature that will be removed when all official drivers
-    # have been unified in their behaviour of when they return a Result object.
-    # We aim for drivers to not providing a Result until the server replied with
-    # SUCCESS so that the result keys are already known and attached to the
-    # Result object without further waiting or communication with the server.
-    TMP_RESULT_KEYS = "Temporary:ResultKeys"
-    # Temporary driver feature that will be removed when all official driver
-    # backends have implemented all summary response fields.
-    TMP_FULL_SUMMARY = "Temporary:FullSummary"
     # Temporary driver feature that will be removed when all official driver
     # backends have implemented path and relationship types
     TMP_CYPHER_PATH_AND_RELATIONSHIP = "Temporary:CypherPathAndRelationship"
-    # Temporary driver feature that will be removed when all official driver
-    # backends have implemented the TransactionClose request
-    TMP_TRANSACTION_CLOSE = "Temporary:TransactionClose"
     # TODO Update this once the decision has been made.
     # Temporary driver feature. There is a pending decision on whether it should
     # be supported in all drivers or be removed from all of them.
@@ -56,5 +49,17 @@ class Feature(Enum):
     # backends have implemented it.
     TMP_DRIVER_MAX_TX_RETRY_TIME = "Temporary:DriverMaxTxRetryTime"
     # Temporary driver feature that will be removed when all official driver
+    # backends have implemented all summary response fields.
+    TMP_FULL_SUMMARY = "Temporary:FullSummary"
+    # Temporary driver feature that will be removed when all official drivers
+    # have been unified in their behaviour of when they return a Result object.
+    # We aim for drivers to not providing a Result until the server replied with
+    # SUCCESS so that the result keys are already known and attached to the
+    # Result object without further waiting or communication with the server.
+    TMP_RESULT_KEYS = "Temporary:ResultKeys"
+    # Temporary driver feature that will be removed when all official driver
     # backends have implemented it.
     TMP_RESULT_LIST = "Temporary:ResultList"
+    # Temporary driver feature that will be removed when all official driver
+    # backends have implemented the TransactionClose request
+    TMP_TRANSACTION_CLOSE = "Temporary:TransactionClose"

tests/stub/authorization/test_authorization.py

@@ -158,11 +158,7 @@ def test_should_fail_with_auth_expired_on_begin_using_tx_run(self):
         try:
             tx = session.beginTransaction()
             # TODO: remove block when all drivers behave the same way
-            if get_driver_name() in ["python"]:
-                # driver waits with sending BEGIN until .run is called
-                # this pipelining saves time but exposes issues later
-                tx.run("cypher")
-            if get_driver_name() in ["javascript", "dotnet"]:
+            if get_driver_name() in ["javascript"]:
                 tx.run("cypher").next()
         except types.DriverError as e:
             self.assert_is_authorization_error(error=e)

tests/stub/disconnects/test_disconnects.py

@@ -96,9 +96,9 @@ def test_disconnect_on_hello(self):
         self._driver.close()
         self._server.done()
 
-        expected_step = "after first next"
-        if self._driverName in ["go", "java", "dotnet", "python"]:
-            expected_step = "after run"
+        expected_step = "after run"
+        if self._driverName in ["javascript"]:
+            expected_step = "after first next"
         self.assertEqual(step, expected_step)
 
     def test_disconnect_after_hello(self):
@@ -115,9 +115,9 @@ def test_disconnect_after_hello(self):
         self._driver.close()
         self._server.done()
 
-        expected_step = "after first next"
-        if self._driverName in ["go", "python", "java"]:
-            expected_step = "after run"
+        expected_step = "after run"
+        if self._driverName in ["dotnet", "javascript"]:
+            expected_step = "after first next"
         self.assertEqual(step, expected_step)
 
     def test_disconnect_session_on_run(self):
@@ -130,10 +130,9 @@ def test_disconnect_session_on_run(self):
         self._driver.close()
         self._server.done()
 
-        expected_step = "after first next"
-        if self._driverName in ["go", "python", "java"]:
-            # Go reports this error earlier
-            expected_step = "after run"
+        expected_step = "after run"
+        if self._driverName in ["dotnet", "javascript"]:
+            expected_step = "after first next"
         self.assertEqual(step, expected_step)
 
     def test_disconnect_on_pull(self):
@@ -175,7 +174,7 @@ def test_disconnect_on_tx_begin(self):
         self._server.done()
 
         expected_step = "after begin"
-        if self._driverName in ["python", "go"]:
+        if self._driverName in ["go"]:
             expected_step = "after run"
         elif self._driverName in ["javascript"]:
             expected_step = "after first next"

tests/stub/routing/test_routing_v4x3.py

@@ -521,17 +521,14 @@ def work(tx):
     @driver_feature(types.Feature.OPT_PULL_PIPELINING)
     def test_should_retry_write_until_success_with_leader_change_using_tx_function(
             self):
-        # TODO remove this block once all languages work
-        if get_driver_name() in ['python']:
-            self.skipTest("requires investigation")
         self._should_retry_write_until_success_with_leader_change_using_tx_function(
             "writer_tx_with_unexpected_interruption_on_pipelined_pull.script"
         )
 
     def test_should_retry_write_until_success_with_leader_change_on_run_using_tx_function(
             self):
         # TODO remove this block once all languages work
-        if get_driver_name() in ['javascript', 'python']:
+        if get_driver_name() in ['javascript']:
             self.skipTest("requires investigation")
         self._should_retry_write_until_success_with_leader_change_using_tx_function(
             "writer_tx_with_unexpected_interruption_on_run.script"

tests/stub/tx_run/scripts/router_switch_server.script

@@ -0,0 +1,11 @@
+!: BOLT 4.4
+!: AUTO RESET
+
+A: HELLO {"{}": "*"}
+C: ROUTE "*" "*" "*"
+S: SUCCESS { "rt": { "ttl": 1000, "servers": [{"addresses": ["#HOST#:9000"], "role":"ROUTE"}, {"addresses": ["#HOST#:9010"], "role":"READ"}, {"addresses": ["#HOST#:9010"], "role":"WRITE"}]}}
+{*
+    C: ROUTE "*" "*" "*"
+    S: SUCCESS { "rt": { "ttl": 1000, "servers": [{"addresses": ["#HOST#:9000"], "role":"ROUTE"}, {"addresses": ["#HOST#:9011"], "role":"READ"}, {"addresses": ["#HOST#:9011"], "role":"WRITE"}]}}
+*}
+?: GOODBYE

tests/stub/tx_run/scripts/tx_commit.script

@@ -0,0 +1,20 @@
+!: BOLT 4.4
+
+A: HELLO {"{}": "*"}
+*: RESET
+C: BEGIN {"{}": "*"}
+S: SUCCESS {}
+C: RUN {"U": "*"} {"{}": "*"} {"{}": "*"}
+S: SUCCESS {"fields": ["n"], "qid": 1}
+{{
+    C: PULL {"n": {"Z": "*"}, "[qid]": -1}
+----
+    C: PULL {"n": {"Z": "*"}, "qid": 1}
+}}
+S: RECORD [1]
+   RECORD [2]
+   SUCCESS {"type": "r"}
+C: COMMIT
+S: SUCCESS {"bookmark": "neo4j:bookmark:v1:tx424242"}
+*: RESET
+?: GOODBYE

tests/stub/tx_run/scripts/tx_disconnect_on_begin.script

@@ -0,0 +1,6 @@
+!: BOLT 4.4
+
+A: HELLO {"{}": "*"}
+*: RESET
+C: BEGIN {"{}": "*"}
+S: <EXIT>

tests/stub/tx_run/scripts/tx_error_on_begin.script

@@ -0,0 +1,8 @@
+!: BOLT 4.4
+
+A: HELLO {"{}": "*"}
+*: RESET
+C: BEGIN {"{}": "*"}
+S: FAILURE {"code": "Neo.ClientError.MadeUp.Code", "message": "Something went wrong..."}
++: RESET
+?: GOODBYE

tests/stub/tx_run/scripts/tx_pull_then_rollback.script

@@ -4,7 +4,7 @@ A: HELLO {"{}": "*"}
 *: RESET
 C: BEGIN {"{}": "*"}
 S: SUCCESS {}
-C: RUN {"U": "*"}{"{}": "*"} {"{}": "*"}
+C: RUN {"U": "*"} {"{}": "*"} {"{}": "*"}
 S: SUCCESS {"fields": ["n"]}
 C: PULL {"n": {"Z": "*"}, "[qid]": -1}
 S: RECORD [1]