Fixed bug in ConnectParams.set() where parameters found in a connect

string (like "host" and "service_name") would be ignored.

Author: anthony-tuininga

doc/src/api_manual/connect_params.rst

@@ -61,11 +61,8 @@ ConnectParams Methods
         connection_id_prefix=None, ssl_context=None, sdu=None, \
         pool_boundary=None, use_tcp_fast_open=False, handle=None)
 
-    Sets the default values for one or more of the parameters of an empty
-    ConnectParams object.  A default will be overriden when a connection string
-    with that attribute is parsed.  After a ConnectParams object has been
-    populated by parsing a connection string, ``ConnectParams.set()`` will not
-    override any values.
+    Sets the values for one or more of the parameters of a ConnectParams
+    object.
 
     .. versionchanged:: 2.1.0
 

doc/src/release_notes.rst

@@ -85,6 +85,8 @@ Common Changes
 #)  If both the ``sid`` and ``service_name`` parameters are specified to
     :meth:`oracledb.makedsn()`, now only the ``service_name`` parameter is
     used and the ``sid`` parameter is ignored.
+#)  Fixed bug in :meth:`ConnectParams.set()` where parameters found in a
+    connect string (like ``host`` and ``service_name``) would be ignored.
 #)  Fixed bug in :meth:`Connection.tpc_recover()` where the returned items were
     not of the type returned by :meth:`Connection.xid()` as documented.
 #)  Internal changes to ensure that no circular imports occur.

src/oracledb/impl/base/connect_params.pyx

@@ -145,6 +145,11 @@ cdef class ConnectParamsImpl:
         Sets the property values based on the supplied arguments. All values
         not supplied will be left unchanged.
         """
+        cdef:
+            Description description
+            Address address
+
+        # set parameters found directly on the ConnectParamsImpl object
         self._external_handle = args.get("handle", self._external_handle)
         _set_str_param(args, "user", self)
         _set_str_param(args, "proxy_user", self)
@@ -166,13 +171,21 @@ cdef class ConnectParamsImpl:
         _set_obj_param(args, "appcontext", self)
         _set_obj_param(args, "shardingkey", self)
         _set_obj_param(args, "supershardingkey", self)
-        self._default_description.set_from_connect_data_args(args)
-        self._default_description.set_from_description_args(args)
-        self._default_description.set_from_security_args(args)
-        self._default_address.set_from_args(args)
         _set_bool_param(args, "externalauth", &self.externalauth)
         self._set_access_token_param(args.get("access_token"))
 
+        # set parameters found on Description instances
+        self._default_description.set_from_args(args)
+        for description in self.description_list.children:
+            if description is not self._default_description:
+                description.set_from_args(args)
+
+        # set parameters found on Address instances
+        self._default_address.set_from_args(args)
+        for address in self.description_list.get_addresses():
+            if address is not self._default_address:
+                address.set_from_args(args)
+
     cdef int _check_credentials(self) except -1:
         """
         Check to see that credentials have been supplied: either a password or
@@ -901,25 +914,32 @@ cdef class Description(ConnectParamsNode):
         """
         cdef Description description = Description.__new__(Description)
         description._copy(self)
-        description.service_name = self.service_name
-        description.sid = self.sid
-        description.server_type = self.server_type
-        description.cclass = self.cclass
-        description.purity = self.purity
         description.expire_time = self.expire_time
-        description.load_balance = self.load_balance
-        description.source_route = self.source_route
         description.retry_count = self.retry_count
         description.retry_delay = self.retry_delay
         description.sdu = self.sdu
         description.tcp_connect_timeout = self.tcp_connect_timeout
+        description.service_name = self.service_name
+        description.server_type = self.server_type
+        description.sid = self.sid
+        description.cclass = self.cclass
+        description.connection_id_prefix = self.connection_id_prefix
+        description.pool_boundary = self.pool_boundary
+        description.purity = self.purity
         description.ssl_server_dn_match = self.ssl_server_dn_match
+        description.use_tcp_fast_open = self.use_tcp_fast_open
         description.ssl_server_cert_dn = self.ssl_server_cert_dn
         description.wallet_location = self.wallet_location
-        description.connection_id_prefix = self.connection_id_prefix
-        description.use_tcp_fast_open = self.use_tcp_fast_open
         return description
 
+    def set_from_args(self, dict args):
+        """
+        Set parameter values from an argument dictionary.
+        """
+        self.set_from_connect_data_args(args)
+        self.set_from_description_args(args)
+        self.set_from_security_args(args)
+
     def set_from_connect_data_args(self, dict args):
         """
         Set parameter values from an argument dictionary or a (CONNECT_DATA)

tests/test_4500_connect_params.py

@@ -596,18 +596,20 @@ def test_4537(self):
         self.assertEqual(params.connection_id_prefix, "prefix4562b")
 
     def test_4538(self):
-        "4538 - test overriding parameters with parse_connection_string"
+        "4538 - test overriding parameters"
         params = oracledb.ConnectParams()
-        params.parse_connect_string("my_host:3578/my_service_name")
-        params.set(service_name="new_service_name", port=613)
-        self.assertEqual(params.service_name, "my_service_name")
-        self.assertEqual(params.port, 3578)
-
-        params = oracledb.ConnectParams()
-        params.set(service_name="my_service_name", port=613)
-        params.parse_connect_string("my_host:3578/new_service_name")
-        self.assertEqual(params.service_name, "new_service_name")
-        self.assertEqual(params.port, 3578)
+        host = "my_host_4538"
+        port = 3578
+        service_name = "my_service_name_4538"
+        connect_string = f"{host}:{port}/{service_name}"
+        params.parse_connect_string(connect_string)
+        self.assertEqual(params.service_name, service_name)
+        self.assertEqual(params.port, port)
+        new_service_name = "new_service_name_4538"
+        new_port = 613
+        params.set(service_name=new_service_name, port=new_port)
+        self.assertEqual(params.service_name, new_service_name)
+        self.assertEqual(params.port, new_port)
 
     def test_4539(self):
         "4539 - test ConnectParams repr()"
@@ -656,6 +658,59 @@ def test_4539(self):
         self.assertEqual(repr(params), expected_value)
         self.assertIs(params.purity, oracledb.Purity.SELF)
         self.assertIs(params.mode, oracledb.AuthMode.SYSDBA)
+        new_values = [
+            ("user", "USER_NEW"),
+            ("proxy_user", "PROXY_USER_NEW"),
+            ("host", "my_host_new"),
+            ("port", 1621),
+            ("protocol", "tcps"),
+            ("https_proxy", "proxy_b"),
+            ("https_proxy_port", 4529),
+            ("service_name", "my_service_name_new"),
+            ("sid", "my_sid_new"),
+            ("server_type", "pooled"),
+            ("cclass", "cclass_new"),
+            ("purity", oracledb.PURITY_NEW),
+            ("expire_time", 90),
+            ("retry_count", 8),
+            ("retry_delay", 15),
+            ("tcp_connect_timeout", 15.0),
+            ("ssl_server_dn_match", True),
+            ("ssl_server_cert_dn", "CN=unknown19_new"),
+            ("wallet_location", "/tmp/wallet_loc1_new"),
+            ("events", False),
+            ("externalauth", False),
+            ("mode", oracledb.AUTH_MODE_SYSDGD),
+            ("disable_oob", False),
+            ("stmtcachesize", 35),
+            ("edition", "edition_new"),
+            ("tag", "tag_new"),
+            ("matchanytag", False),
+            ("config_dir", "config_dir_new"),
+            ("appcontext", [("a", "b", "c", "new")]),
+            ("shardingkey", [1, 2, 3, 4]),
+            ("supershardingkey", [6]),
+            ("debug_jdwp", "host=host;port=4638"),
+            ("connection_id_prefix", "prefix4664"),
+            ("ssl_context", ssl.create_default_context()),
+            ("sdu", 32768),
+            ("pool_boundary", "transaction"),
+            ("use_tcp_fast_open", False),
+        ]
+        params.set(**dict(new_values))
+        parts = [f"{name}={value!r}" for name, value in new_values]
+        expected_value = f"ConnectParams({', '.join(parts)})"
+        self.assertEqual(repr(params), expected_value)
+        cs_values = dict(
+            host="my_host_final",
+            service_name="my_service_final",
+        )
+        connect_string = f"{cs_values['host']}/{cs_values['service_name']}"
+        params.parse_connect_string(connect_string)
+        final_values = [(n, cs_values.get(n, v)) for n, v in new_values]
+        parts = [f"{name}={value!r}" for name, value in final_values]
+        expected_value = f"ConnectParams({', '.join(parts)})"
+        self.assertEqual(repr(params), expected_value)
 
     def test_4540(self):
         "4540 - connect descriptor with SDU"