Deprecate bolt URI with routing context (#655)

Amend #645 to emit a
deprecation warning instead of raising. Starting with 6.0 this will be turned
into an error.

Author: robsdedude

CHANGELOG.md

@@ -49,9 +49,9 @@
       Use `hour_minute_second_nanosecond` instead.
     - The property `second` returns an `int` instead of a `float`.  
       Use `nanosecond` to get the sub-second information.
-- Creation of a driver with `bolt[+s[sc]]://` scheme now raises an error if the
-  URI contains a query part (a routing context). Previously, the routing context
-  was silently ignored.
+- Creation of a driver with `bolt[+s[sc]]://` scheme has been deprecated and
+  will raise an error in the Future. The routing context was and will be
+  silently ignored until then.
 - `Result.single` now raises `ResultNotSingleError` if not exactly one result is
   available.
 - Bookmarks

neo4j/_async/driver.py

@@ -94,10 +94,17 @@ def driver(cls, uri, *, auth=None, **config):
 
         if driver_type == DRIVER_BOLT:
             if parse_routing_context(parsed.query):
-                raise ValueError(
-                    'Routing parameters are not supported with scheme "bolt". '
-                    'Given URI "{}".'.format(uri)
+                deprecation_warn(
+                    "Creating a direct driver (`bolt://` scheme) with routing "
+                    "context (URI parameters) is deprecated. They will be "
+                    "ignored. This will raise an error in a future release. "
+                    'Given URI "{}"'.format(uri)
                 )
+                # TODO: 6.0 - raise instead of warning
+                # raise ValueError(
+                #     'Routing parameters are not supported with scheme '
+                #     '"bolt". Given URI "{}".'.format(uri)
+                # )
             return cls.bolt_driver(parsed.netloc, auth=auth, **config)
         elif driver_type == DRIVER_NEO4j:
             routing_context = parse_routing_context(parsed.query)

neo4j/_async/work/result.py

@@ -17,7 +17,6 @@
 
 
 from collections import deque
-from warnings import warn
 
 from ..._async_compat.util import AsyncUtil
 from ...data import DataDehydrator

neo4j/_sync/driver.py

@@ -94,10 +94,17 @@ def driver(cls, uri, *, auth=None, **config):
 
         if driver_type == DRIVER_BOLT:
             if parse_routing_context(parsed.query):
-                raise ValueError(
-                    'Routing parameters are not supported with scheme "bolt". '
-                    'Given URI "{}".'.format(uri)
+                deprecation_warn(
+                    "Creating a direct driver (`bolt://` scheme) with routing "
+                    "context (URI parameters) is deprecated. They will be "
+                    "ignored. This will raise an error in a future release. "
+                    'Given URI "{}"'.format(uri)
                 )
+                # TODO: 6.0 - raise instead of warning
+                # raise ValueError(
+                #     'Routing parameters are not supported with scheme '
+                #     '"bolt". Given URI "{}".'.format(uri)
+                # )
             return cls.bolt_driver(parsed.netloc, auth=auth, **config)
         elif driver_type == DRIVER_NEO4j:
             routing_context = parse_routing_context(parsed.query)

neo4j/_sync/work/result.py

@@ -17,7 +17,6 @@
 
 
 from collections import deque
-from warnings import warn
 
 from ..._async_compat.util import Util
 from ...data import DataDehydrator

testkitbackend/test_config.json

@@ -23,7 +23,9 @@
     "stub.authorization.test_authorization.TestAuthorizationV4x1.test_should_fail_on_token_expired_on_begin_using_tx_function":
       "Flaky: test requires the driver to contact servers in a specific order",
     "stub.session_run_parameters.test_session_run_parameters.TestSessionRunParameters.test_empty_query":
-      "Driver rejects empty queries before sending it to the server"
+      "Driver rejects empty queries before sending it to the server",
+    "stub.server_side_routing.test_server_side_routing.TestServerSideRouting.test_direct_connection_with_url_params":
+      "Driver emits deprecation warning. Behavior will be unified in 6.0."
   },
   "features": {
     "Feature:API:Driver.IsEncrypted": true,

tests/unit/async_/test_driver.py

@@ -36,22 +36,32 @@
 @pytest.mark.parametrize("host", ("localhost", "127.0.0.1",
                                   "[::1]", "[0:0:0:0:0:0:0:1]"))
 @pytest.mark.parametrize("port", (":1234", "", ":7687"))
+@pytest.mark.parametrize("params", ("", "?routing_context=test"))
 @pytest.mark.parametrize("auth_token", (("test", "test"), None))
-def test_direct_driver_constructor(protocol, host, port, auth_token):
-    uri = protocol + host + port
-    driver = AsyncGraphDatabase.driver(uri, auth=auth_token)
+@mark_async_test
+async def test_direct_driver_constructor(protocol, host, port, params, auth_token):
+    uri = protocol + host + port + params
+    if params:
+        with pytest.warns(DeprecationWarning, match="routing context"):
+            driver = AsyncGraphDatabase.driver(uri, auth=auth_token)
+    else:
+        driver = AsyncGraphDatabase.driver(uri, auth=auth_token)
     assert isinstance(driver, AsyncBoltDriver)
+    await driver.close()
 
 
 @pytest.mark.parametrize("protocol", ("neo4j://", "neo4j+s://", "neo4j+ssc://"))
 @pytest.mark.parametrize("host", ("localhost", "127.0.0.1",
                                   "[::1]", "[0:0:0:0:0:0:0:1]"))
 @pytest.mark.parametrize("port", (":1234", "", ":7687"))
+@pytest.mark.parametrize("params", ("", "?routing_context=test"))
 @pytest.mark.parametrize("auth_token", (("test", "test"), None))
-def test_routing_driver_constructor(protocol, host, port, auth_token):
-    uri = protocol + host + port
+@mark_async_test
+async def test_routing_driver_constructor(protocol, host, port, params, auth_token):
+    uri = protocol + host + port + params
     driver = AsyncGraphDatabase.driver(uri, auth=auth_token)
     assert isinstance(driver, AsyncNeo4jDriver)
+    await driver.close()
 
 
 @pytest.mark.parametrize("test_uri", (
@@ -81,7 +91,8 @@ def test_routing_driver_constructor(protocol, host, port, auth_token):
         ),
     )
 )
-def test_driver_config_error(
+@mark_async_test
+async def test_driver_config_error(
     test_uri, test_config, expected_failure, expected_failure_message
 ):
     if "+" in test_uri:
@@ -90,7 +101,8 @@ def test_driver_config_error(
         with pytest.raises(expected_failure, match=expected_failure_message):
             AsyncGraphDatabase.driver(test_uri, **test_config)
     else:
-        AsyncGraphDatabase.driver(test_uri, **test_config)
+        driver = AsyncGraphDatabase.driver(test_uri, **test_config)
+        await driver.close()
 
 
 @pytest.mark.parametrize("test_uri", (
@@ -138,3 +150,5 @@ async def test_driver_opens_write_session_by_default(uri, mocker):
             mocker.ANY,
             mocker.ANY
         )
+
+    await driver.close()

tests/unit/sync/test_driver.py

@@ -36,22 +36,32 @@
 @pytest.mark.parametrize("host", ("localhost", "127.0.0.1",
                                   "[::1]", "[0:0:0:0:0:0:0:1]"))
 @pytest.mark.parametrize("port", (":1234", "", ":7687"))
+@pytest.mark.parametrize("params", ("", "?routing_context=test"))
 @pytest.mark.parametrize("auth_token", (("test", "test"), None))
-def test_direct_driver_constructor(protocol, host, port, auth_token):
-    uri = protocol + host + port
-    driver = GraphDatabase.driver(uri, auth=auth_token)
+@mark_sync_test
+def test_direct_driver_constructor(protocol, host, port, params, auth_token):
+    uri = protocol + host + port + params
+    if params:
+        with pytest.warns(DeprecationWarning, match="routing context"):
+            driver = GraphDatabase.driver(uri, auth=auth_token)
+    else:
+        driver = GraphDatabase.driver(uri, auth=auth_token)
     assert isinstance(driver, BoltDriver)
+    driver.close()
 
 
 @pytest.mark.parametrize("protocol", ("neo4j://", "neo4j+s://", "neo4j+ssc://"))
 @pytest.mark.parametrize("host", ("localhost", "127.0.0.1",
                                   "[::1]", "[0:0:0:0:0:0:0:1]"))
 @pytest.mark.parametrize("port", (":1234", "", ":7687"))
+@pytest.mark.parametrize("params", ("", "?routing_context=test"))
 @pytest.mark.parametrize("auth_token", (("test", "test"), None))
-def test_routing_driver_constructor(protocol, host, port, auth_token):
-    uri = protocol + host + port
+@mark_sync_test
+def test_routing_driver_constructor(protocol, host, port, params, auth_token):
+    uri = protocol + host + port + params
     driver = GraphDatabase.driver(uri, auth=auth_token)
     assert isinstance(driver, Neo4jDriver)
+    driver.close()
 
 
 @pytest.mark.parametrize("test_uri", (
@@ -81,6 +91,7 @@ def test_routing_driver_constructor(protocol, host, port, auth_token):
         ),
     )
 )
+@mark_sync_test
 def test_driver_config_error(
     test_uri, test_config, expected_failure, expected_failure_message
 ):
@@ -90,7 +101,8 @@ def test_driver_config_error(
         with pytest.raises(expected_failure, match=expected_failure_message):
             GraphDatabase.driver(test_uri, **test_config)
     else:
-        GraphDatabase.driver(test_uri, **test_config)
+        driver = GraphDatabase.driver(test_uri, **test_config)
+        driver.close()
 
 
 @pytest.mark.parametrize("test_uri", (
@@ -138,3 +150,5 @@ def test_driver_opens_write_session_by_default(uri, mocker):
             mocker.ANY,
             mocker.ANY
         )
+
+    driver.close()