Fix after review

Author: Zhen

neo4j/addressing.py

@@ -25,6 +25,11 @@
 from neo4j.compat import urlparse
 from neo4j.exceptions import AddressError
 
+try:
+    from urllib.parse import parse_qs
+except ImportError:
+    from urllib import parse_qs
+
 
 VALID_IPv4_SEGMENTS = [str(i).encode("latin1") for i in range(0x100)]
 VALID_IPv6_SEGMENT_CHARS = b"0123456789abcdef"
@@ -86,15 +91,14 @@ def parse_routing_context(cls, uri):
             return {}
 
         context = {}
-        parameters = [x for x in query.split('&') if x]
-        for keyValue in parameters:
-            pair = keyValue.split('=')
-            if len(pair) != 2 or not pair[0] or not pair[1]:
-                raise ValueError("Invalid parameters: '%s' in URI '%s'." % (keyValue, uri))
-            key = pair[0]
-            value = pair[1]
-            if key in context:
-                raise ValueError("Duplicated query parameters with key '%s' found in URL '%s'" % (key, uri))
+        parameters = parse_qs(query, True)
+        for key in parameters:
+            value_list = parameters[key]
+            if len(value_list) != 1:
+                raise ValueError("Duplicated query parameters with key '%s', value '%s' found in URL '%s'" % (key, value_list, uri))
+            value = value_list[0]
+            if not value:
+                raise ValueError("Invalid parameters: key '%s', value '%s' in URI '%s'." % (key, parameters[key], uri))
             context[key] = value
         return context
 

neo4j/v1/direct.py

@@ -57,7 +57,7 @@ def __init__(self, uri, **config):
         # the connection pool may contain multiple IP address keys, one for
         # an old address and one for a new address.
         if SocketAddress.parse_routing_context(uri):
-            raise ValueError("Routing parameters are not supported with scheme 'bolt'. Given URI: '%s'." % uri)
+            raise ValueError("Parameters are not supported with scheme 'bolt'. Given URI: '%s'." % uri)
         self.address = SocketAddress.from_uri(uri, DEFAULT_PORT)
         self.security_plan = security_plan = SecurityPlan.build(**config)
         self.encrypted = security_plan.encrypted

neo4j/v1/routing.py

@@ -137,7 +137,7 @@ def is_fresh(self, access_mode):
         """
         expired = self.last_updated_time + self.ttl <= self.timer()
         has_server_for_mode = (access_mode == READ_ACCESS and self.readers) or (access_mode == WRITE_ACCESS and self.writers)
-        return not expired and len(self.routers) >= 1 and has_server_for_mode
+        return not expired and self.routers and has_server_for_mode
 
     def update(self, new_routing_table):
         """ Update the current routing table with new routing information
@@ -170,7 +170,7 @@ def routing_info_procedure(self, connection):
         if ServerVersion.from_str(connection.server.version).at_least_version(3, 2):
             return self.call_get_routing_table, {self.get_routing_table_param: self.routing_context}
         else:
-            return self.call_get_servers
+            return self.call_get_servers, {}
 
     def fetch_routing_info(self, address):
         """ Fetch raw routing info from a given router address.
@@ -182,9 +182,15 @@ def fetch_routing_info(self, address):
                                    if routing support is broken
         """
         try:
-            connection = self.acquire_direct(address)
-            with BoltSession(lambda _: connection) as session:
-                return list(session.run(*self.routing_info_procedure(connection)))
+            connections = [None]
+
+            def connector(_):
+                connection = self.acquire_direct(address)
+                connections[0] = connection
+                return connection
+
+            with BoltSession(lambda _: connector) as session:
+                return list(session.run(*self.routing_info_procedure(connections[0])))
         except CypherError as error:
             if error.code == "Neo.ClientError.Procedure.ProcedureNotFound":
                 raise ServiceUnavailable("Server {!r} does not support routing".format(address))
@@ -269,7 +275,7 @@ def update_routing_table(self):
         # None of the routers have been successful, so just fail
         raise ServiceUnavailable("Unable to retrieve routing information")
 
-    def ensure_routing_table(self, access_mode):
+    def ensure_routing_table_is_fresh(self, access_mode):
         """ Update the routing table if stale.
 
         This method performs two freshness checks, before and after acquiring
@@ -303,7 +309,7 @@ def acquire(self, access_mode=None):
         else:
             raise ValueError("Unsupported access mode {}".format(access_mode))
 
-        self.ensure_routing_table(access_mode)
+        self.ensure_routing_table_is_fresh(access_mode)
         while True:
             address = next(server_list)
             if address is None:

test/stub/test_routing.py

@@ -278,7 +278,7 @@ def test_should_update_if_stale(self):
             with RoutingPool(address) as pool:
                 first_updated_time = pool.routing_table.last_updated_time
                 pool.routing_table.ttl = 0
-                pool.ensure_routing_table(WRITE_ACCESS)
+                pool.ensure_routing_table_is_fresh(WRITE_ACCESS)
                 second_updated_time = pool.routing_table.last_updated_time
                 assert second_updated_time != first_updated_time
                 assert not pool.missing_writer
@@ -287,9 +287,9 @@ def test_should_not_update_if_fresh(self):
         with StubCluster({9001: "router.script"}):
             address = ("127.0.0.1", 9001)
             with RoutingPool(address) as pool:
-                pool.ensure_routing_table(WRITE_ACCESS)
+                pool.ensure_routing_table_is_fresh(WRITE_ACCESS)
                 first_updated_time = pool.routing_table.last_updated_time
-                pool.ensure_routing_table(WRITE_ACCESS)
+                pool.ensure_routing_table_is_fresh(WRITE_ACCESS)
                 second_updated_time = pool.routing_table.last_updated_time
                 assert second_updated_time == first_updated_time
                 assert not pool.missing_writer
@@ -300,7 +300,7 @@ def test_should_flag_reading_without_writer(self):
             with RoutingPool(address) as pool:
                 assert not pool.routing_table.is_fresh(READ_ACCESS)
                 assert not pool.routing_table.is_fresh(WRITE_ACCESS)
-                pool.ensure_routing_table(READ_ACCESS)
+                pool.ensure_routing_table_is_fresh(READ_ACCESS)
                 assert pool.missing_writer
 
     # TODO: fix flaky test
@@ -486,7 +486,7 @@ def test_should_remove_router_from_routing_table_if_present(self):
         with StubCluster({9001: "router.script"}):
             address = ("127.0.0.1", 9001)
             with RoutingPool(address) as pool:
-                pool.ensure_routing_table(WRITE_ACCESS)
+                pool.ensure_routing_table_is_fresh(WRITE_ACCESS)
                 target = ("127.0.0.1", 9001)
                 assert target in pool.routing_table.routers
                 pool.remove(target)
@@ -496,7 +496,7 @@ def test_should_remove_reader_from_routing_table_if_present(self):
         with StubCluster({9001: "router.script"}):
             address = ("127.0.0.1", 9001)
             with RoutingPool(address) as pool:
-                pool.ensure_routing_table(WRITE_ACCESS)
+                pool.ensure_routing_table_is_fresh(WRITE_ACCESS)
                 target = ("127.0.0.1", 9004)
                 assert target in pool.routing_table.readers
                 pool.remove(target)
@@ -506,7 +506,7 @@ def test_should_remove_writer_from_routing_table_if_present(self):
         with StubCluster({9001: "router.script"}):
             address = ("127.0.0.1", 9001)
             with RoutingPool(address) as pool:
-                pool.ensure_routing_table(WRITE_ACCESS)
+                pool.ensure_routing_table_is_fresh(WRITE_ACCESS)
                 target = ("127.0.0.1", 9006)
                 assert target in pool.routing_table.writers
                 pool.remove(target)
@@ -516,6 +516,6 @@ def test_should_not_fail_if_absent(self):
         with StubCluster({9001: "router.script"}):
             address = ("127.0.0.1", 9001)
             with RoutingPool(address) as pool:
-                pool.ensure_routing_table(WRITE_ACCESS)
+                pool.ensure_routing_table_is_fresh(WRITE_ACCESS)
                 target = ("127.0.0.1", 9007)
                 pool.remove(target)