Migrate tests from java driver (#211)

* Migrate tests from java driver (#206)

* Migrate tests from java driver

This updated also includes:
- TransactionClose request support (`Temporary:TransactionClose` feature)
- Driver fetch size configuration (`Temporary:DriverFetchSize` feature)

Migrated tests:
- shouldThrowRollbackErrorWhenTransactionRollback -> test_should_error_on_rollback_failure_using_tx_rollback
- shouldThrowRollbackErrorWhenTransactionClose -> test_should_error_on_rollback_failure_using_tx_close
- shouldPropagateTransactionRollbackErrorWhenSessionClosed -> test_should_error_on_rollback_failure_using_session_close
- shouldStreamingRecordsInBatches -> test_should_accept_custom_fetch_size_using_driver_configuration (protocol bump from 4 to 4.1)
- shouldChangeFetchSize -> test_should_accept_custom_fetch_size_using_session_configuration (protocol bump from 4 to 4.1)

* Update TMP_DRIVER_FETCH_SIZE feature comment

* Update _assert_is_transient_exception check for java driver

* Remove pipelining from direct scripts and update fetchSize handling

* Add optional RESET to writer_with_custom_fetch_size.script

* Skip direct rollback tests for go and javascript

* Skip direct rollback test for javascript

* Skip test_should_error_on_rollback_failure_using_session_close for dotnet

* Skip test_should_accept_custom_fetch_size_using_driver_configuration in v3 (#210)

Author: injectives

nutkit/frontend/driver.py

@@ -3,14 +3,18 @@
 
 
 class Driver:
-    def __init__(self, backend, uri, authToken, userAgent=None, resolverFn=None, domainNameResolverFn=None,
-                 connectionTimeoutMs=None):
+    def __init__(self, backend, uri, authToken, userAgent=None, resolverFn=None,
+                 domainNameResolverFn=None,
+                 connectionTimeoutMs=None, fetchSize=None):
         self._backend = backend
         self._resolverFn = resolverFn
         self._domainNameResolverFn = domainNameResolverFn
-        req = protocol.NewDriver(uri, authToken, userAgent=userAgent, resolverRegistered=resolverFn is not None,
-                                 domainNameResolverRegistered=domainNameResolverFn is not None,
-                                 connectionTimeoutMs=connectionTimeoutMs)
+        req = protocol.NewDriver(
+            uri, authToken, userAgent=userAgent,
+            resolverRegistered=resolverFn is not None,
+            domainNameResolverRegistered=domainNameResolverFn is not None,
+            connectionTimeoutMs=connectionTimeoutMs,
+            fetchSize=fetchSize)
         res = backend.sendAndReceive(req)
         if not isinstance(res, protocol.Driver):
             raise Exception("Should be Driver but was %s" % res)

nutkit/frontend/transaction.py

@@ -25,3 +25,9 @@ def rollback(self):
         res = self._backend.sendAndReceive(req)
         if not isinstance(res, protocol.Transaction):
             raise Exception("Should be transaction but was: %s" % res)
+
+    def close(self):
+        req = protocol.TransactionClose(self._id)
+        res = self._backend.sendAndReceive(req)
+        if not isinstance(res, protocol.Transaction):
+            raise Exception("Should be transaction but was: %s" % res)

nutkit/protocol/feature.py

@@ -45,3 +45,10 @@ class Feature(Enum):
     # 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.
+    TMP_DRIVER_FETCH_SIZE = "Temporary:DriverFetchSize"

nutkit/protocol/requests.py

@@ -39,8 +39,9 @@ class NewDriver:
     Backend should respond with a Driver response or an Error response.
     """
 
-    def __init__(self, uri, authToken, userAgent=None, resolverRegistered=False, domainNameResolverRegistered=False,
-                 connectionTimeoutMs=None):
+    def __init__(self, uri, authToken, userAgent=None, resolverRegistered=False,
+                 domainNameResolverRegistered=False,
+                 connectionTimeoutMs=None, fetchSize=None):
         # Neo4j URI to connect to
         self.uri = uri
         # Authorization token used by driver when connecting to Neo4j
@@ -50,6 +51,12 @@ def __init__(self, uri, authToken, userAgent=None, resolverRegistered=False, dom
         self.resolverRegistered = resolverRegistered
         self.domainNameResolverRegistered = domainNameResolverRegistered
         self.connectionTimeoutMs = connectionTimeoutMs
+        # TODO: remove assertion and condition as soon as all drivers support
+        #       driver-scoped fetch-size config
+        from .feature import Feature
+        assert hasattr(Feature, "TMP_DRIVER_FETCH_SIZE")
+        if fetchSize is not None:
+            self.fetchSize = fetchSize
 
 
 class AuthorizationToken:
@@ -231,6 +238,16 @@ def __init__(self, txId):
         self.txId = txId
 
 
+class TransactionClose:
+    """ Request to close the transaction instance on the backend.
+    Backend should respond with a transaction response representing the closed
+    transaction or an error response.
+    """
+
+    def __init__(self, txId):
+        self.txId = txId
+
+
 class ResultNext:
     """ Request to retrieve the next record on a result living on the backend.
     Backend should respond with a Record if there is a record, an Error if an

tests/neo4j/test_direct_driver.py

@@ -122,10 +122,6 @@ def test_multi_db_non_existing(self):
             return
         self.assertEqual(exc.code,
                          "Neo.ClientError.Database.DatabaseNotFound")
-        # TODO remove this block once all languages work
-        if get_driver_name() in ["java"]:
-            # does not set exception message
-            return
         self.assertIn("test-database", exc.msg)
         self.assertIn("exist", exc.msg)
         if get_driver_name() in ["python"]:

tests/stub/routing/scripts/v3_no_routing/writer_yielding_error_on_rollback.script

@@ -0,0 +1,20 @@
+!: BOLT #VERSION#
+!: AUTO GOODBYE
+
+C: HELLO {"scheme": "basic", "credentials": "c", "principal": "p", "user_agent": "#USER_AGENT#" #ROUTING# #EXTRA_HELLO_PROPS#}
+S: SUCCESS {"server": "#SERVER_AGENT#", "connection_id": "bolt-123456789"}
+C: BEGIN {}
+S: SUCCESS {}
+C: RUN "RETURN 1 as n" {} {}
+S: SUCCESS {"fields": ["n"]}
+C: PULL_ALL
+S: RECORD [1]
+   SUCCESS {"type": "r"}
+C: ROLLBACK
+S: FAILURE {"code": "Neo.TransientError.General.DatabaseUnavailable", "message": "Unable to rollback"}
+C: RESET
+S: SUCCESS {}
+{?
+    C: RESET
+    S: SUCCESS {}
+?}

tests/stub/routing/scripts/v4x1_no_routing/writer_with_custom_fetch_size.script

@@ -0,0 +1,25 @@
+!: BOLT #VERSION#
+!: AUTO GOODBYE
+
+C: HELLO {"scheme": "basic", "credentials": "c", "principal": "p", "user_agent": "#USER_AGENT#" #ROUTING# #EXTRA_HELLO_PROPS#}
+S: SUCCESS {"server": "#SERVER_AGENT#", "connection_id": "bolt-123456789"}
+# Added this block for GO
+{?
+    C: RESET
+    S: SUCCESS {}
+?}
+C: RUN "RETURN 1 as n" {} {"db": "adb"}
+S: SUCCESS {"fields": ["n"]}
+C: PULL {"n": 2}
+S: RECORD [1]
+   RECORD [5]
+   SUCCESS {"has_more": true}
+C: PULL {"n": 2}
+S: RECORD [7]
+   SUCCESS {}
+C: RESET
+S: SUCCESS {}
+{?
+    C: RESET
+    S: SUCCESS {}
+?}

tests/stub/routing/scripts/v4x1_no_routing/writer_yielding_error_on_rollback.script

@@ -0,0 +1,20 @@
+!: BOLT #VERSION#
+!: AUTO GOODBYE
+
+C: HELLO {"scheme": "basic", "credentials": "c", "principal": "p", "user_agent": "#USER_AGENT#" #ROUTING# #EXTRA_HELLO_PROPS#}
+S: SUCCESS {"server": "#SERVER_AGENT#", "connection_id": "bolt-123456789"}
+C: BEGIN {"db": "adb"}
+S: SUCCESS {}
+C: RUN "RETURN 1 as n" {} {}
+S: SUCCESS {"fields": ["n"]}
+C: PULL {"n": 1000}
+S: RECORD [1]
+   SUCCESS {"type": "r"}
+C: ROLLBACK
+S: FAILURE {"code": "Neo.TransientError.General.DatabaseUnavailable", "message": "Unable to rollback"}
+C: RESET
+S: SUCCESS {}
+{?
+    C: RESET
+    S: SUCCESS {}
+?}

tests/stub/routing/test_no_routing_v3.py

@@ -19,3 +19,10 @@ def get_vars(self):
             "#USER_AGENT#": '007',
             "#ROUTING#": ''
         }
+
+    def test_should_accept_custom_fetch_size_using_driver_configuration(self):
+        pass
+
+    def test_should_accept_custom_fetch_size_using_session_configuration(
+            self):
+        pass

tests/stub/routing/test_no_routing_v4x1.py

@@ -4,6 +4,7 @@
     get_dns_resolved_server_address,
     get_driver_name,
     TestkitTestCase,
+    driver_feature,
 )
 from tests.stub.shared import StubServer
 from ._routing import get_extra_hello_props
@@ -137,3 +138,158 @@ def test_should_send_custom_user_agent_using_write_session_run(self):
         self.assertEqual(summary.server_info.address,
                          get_dns_resolved_server_address(self._server))
         self._server.done()
+
+    def test_should_error_on_rollback_failure_using_tx_rollback(self):
+        # TODO There is a pending unification task to fix this.
+        # Once fixed, this block should be removed.
+        if get_driver_name() in ["javascript", "go"]:
+            self.skipTest("There is a pending unification task to fix this.")
+        uri = "bolt://%s" % self._server.address
+        self._server.start(
+            path=self.script_path(self.version_dir,
+                                  "writer_yielding_error_on_rollback.script"),
+            vars=self.get_vars()
+        )
+        driver = Driver(self._backend, uri,
+                        types.AuthorizationToken(scheme="basic", principal="p",
+                                                 credentials="c"),
+                        userAgent="007")
+
+        session = driver.session('w', database=self.adb)
+        tx = session.beginTransaction()
+        res = tx.run("RETURN 1 as n")
+        summary = res.consume()
+
+        with self.assertRaises(types.DriverError) as exc:
+            tx.rollback()
+
+        session.close()
+        driver.close()
+
+        self._assert_is_transient_exception(exc.exception)
+        self.assertEqual(summary.server_info.address,
+                         get_dns_resolved_server_address(self._server))
+        self._server.done()
+
+    @driver_feature(types.Feature.TMP_TRANSACTION_CLOSE)
+    def test_should_error_on_rollback_failure_using_tx_close(self):
+        uri = "bolt://%s" % self._server.address
+        self._server.start(
+            path=self.script_path(self.version_dir,
+                                  "writer_yielding_error_on_rollback.script"),
+            vars=self.get_vars()
+        )
+        driver = Driver(self._backend, uri,
+                        types.AuthorizationToken(scheme="basic", principal="p",
+                                                 credentials="c"),
+                        userAgent="007")
+
+        session = driver.session('w', database=self.adb)
+        tx = session.beginTransaction()
+        res = tx.run("RETURN 1 as n")
+        summary = res.consume()
+
+        with self.assertRaises(types.DriverError) as exc:
+            tx.close()
+
+        session.close()
+        driver.close()
+
+        self._assert_is_transient_exception(exc.exception)
+        self.assertEqual(summary.server_info.address,
+                         get_dns_resolved_server_address(self._server))
+        self._server.done()
+
+    def test_should_error_on_rollback_failure_using_session_close(
+            self):
+        # TODO There is a pending unification task to fix this.
+        # Once fixed, this block should be removed.
+        if get_driver_name() in ["javascript", "go"]:
+            self.skipTest("There is a pending unification task to fix this.")
+        # TODO This needs investigation
+        if get_driver_name() in ["dotnet"]:
+            self.skipTest("Throws client exception instead.")
+        uri = "bolt://%s" % self._server.address
+        self._server.start(
+            path=self.script_path(self.version_dir,
+                                  "writer_yielding_error_on_rollback.script"),
+            vars=self.get_vars()
+        )
+        driver = Driver(self._backend, uri,
+                        types.AuthorizationToken(scheme="basic", principal="p",
+                                                 credentials="c"),
+                        userAgent="007")
+
+        session = driver.session('w', database=self.adb)
+        tx = session.beginTransaction()
+        res = tx.run("RETURN 1 as n")
+        summary = res.consume()
+
+        with self.assertRaises(types.DriverError) as exc:
+            session.close()
+
+        driver.close()
+
+        self._assert_is_transient_exception(exc.exception)
+        self.assertEqual(summary.server_info.address,
+                         get_dns_resolved_server_address(self._server))
+        self._server.done()
+
+    @driver_feature(types.Feature.TMP_DRIVER_FETCH_SIZE)
+    def test_should_accept_custom_fetch_size_using_driver_configuration(
+            self):
+        uri = "bolt://%s" % self._server.address
+        self._server.start(
+            path=self.script_path(self.version_dir,
+                                  "writer_with_custom_fetch_size.script"),
+            vars=self.get_vars()
+        )
+        driver = Driver(self._backend, uri,
+                        types.AuthorizationToken(scheme="basic", principal="p",
+                                                 credentials="c"),
+                        userAgent="007", fetchSize=2)
+
+        session = driver.session('w', database=self.adb)
+        res = session.run("RETURN 1 as n")
+        records = list(res)
+
+        session.close()
+        driver.close()
+
+        self.assertEqual([types.Record(values=[types.CypherInt(1)]),
+                          types.Record(values=[types.CypherInt(5)]),
+                          types.Record(values=[types.CypherInt(7)])], records)
+        self._server.done()
+
+    def test_should_accept_custom_fetch_size_using_session_configuration(
+            self):
+        uri = "bolt://%s" % self._server.address
+        self._server.start(
+            path=self.script_path(self.version_dir,
+                                  "writer_with_custom_fetch_size.script"),
+            vars=self.get_vars()
+        )
+        driver = Driver(self._backend, uri,
+                        types.AuthorizationToken(scheme="basic", principal="p",
+                                                 credentials="c"),
+                        userAgent="007")
+
+        session = driver.session('w', database=self.adb, fetchSize=2)
+        res = session.run("RETURN 1 as n")
+        records = list(res)
+
+        session.close()
+        driver.close()
+
+        self.assertEqual([types.Record(values=[types.CypherInt(1)]),
+                          types.Record(values=[types.CypherInt(5)]),
+                          types.Record(values=[types.CypherInt(7)])], records)
+        self._server.done()
+
+    def _assert_is_transient_exception(self, e):
+        if get_driver_name() in ["java"]:
+            self.assertEqual("org.neo4j.driver.exceptions.TransientException",
+                             e.errorType)
+        self.assertEqual("Unable to rollback", e.msg)
+        self.assertEqual("Neo.TransientError.General.DatabaseUnavailable",
+                         e.code)