From f0e1713c8efda8d979d68a8c1e0e39351d701ae9 Mon Sep 17 00:00:00 2001
From: Alistair Coles <alistairncoles@gmail.com>
Date: Tue, 2 Apr 2024 15:13:07 +0100
Subject: [PATCH] Clarify behaviour of InternalClient.make_request params arg

Update make_request docstring to clarify that if the params argument
is used it will override any query_string component of the path
argument.

Add a unit test to verify this.

No intended behavioural change.

Change-Id: I4b3f525fdbe6c67132384901071d6824c2964011
---
 swift/common/internal_client.py          |  6 ++++--
 test/unit/common/test_internal_client.py | 19 +++++++++++++++++++
 test/unit/common/test_swob.py            |  6 ++++++
 3 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/swift/common/internal_client.py b/swift/common/internal_client.py
index fc5242ae8a..885c585bd9 100644
--- a/swift/common/internal_client.py
+++ b/swift/common/internal_client.py
@@ -199,8 +199,10 @@ class InternalClient(object):
         :param acceptable_statuses: List of acceptable statuses for request.
         :param body_file: Body file to be passed along with request,
                           defaults to None.
-        :param params: A dict of params to be set in request query string,
-                       defaults to None.
+        :param params: A dict of params to be set in request query string. If
+                       the ``path`` argument includes a query string, that
+                       query string will be replaced by any params set by the
+                       ``params`` argument. Defaults to None.
 
         :returns: Response object on success.
 
diff --git a/test/unit/common/test_internal_client.py b/test/unit/common/test_internal_client.py
index f5580f0ed8..a457ee6cad 100644
--- a/test/unit/common/test_internal_client.py
+++ b/test/unit/common/test_internal_client.py
@@ -720,6 +720,25 @@ class TestInternalClient(unittest.TestCase):
         self.assertEqual(client.app._pipeline_final_app.backend_user_agent,
                          'some_agent')
 
+    def test_make_request_query_string(self):
+        fake_swift = FakeSwift()
+        fake_swift.register('PUT', '/path', swob.HTTPOk, {})
+        client = internal_client.InternalClient(
+            None, 'some_agent', 3, use_replication_network=False,
+            app=fake_swift)
+
+        client.make_request('PUT', '/path?query=string', {}, (200,))
+        self.assertEqual([('PUT', '/path?query=string')], fake_swift.calls)
+
+        fake_swift.clear_calls()
+        client.make_request('PUT', '/path?query=string', {}, (200,), params={})
+        self.assertEqual([('PUT', '/path?query=string')], fake_swift.calls)
+
+        fake_swift.clear_calls()
+        client.make_request('PUT', '/path?query=string', {}, (200,),
+                            params={'param1': 'one'})
+        self.assertEqual([('PUT', '/path?param1=one')], fake_swift.calls)
+
     def test_make_request_error_case(self):
         class FakeApp(FakeSwift):
             def __call__(self, env, start_response):
diff --git a/test/unit/common/test_swob.py b/test/unit/common/test_swob.py
index 8768bc5e61..2a63d26d4e 100644
--- a/test/unit/common/test_swob.py
+++ b/test/unit/common/test_swob.py
@@ -610,6 +610,12 @@ class TestRequest(unittest.TestCase):
         req.params = new_params
         self.assertDictEqual(dict(new_params), req.params)
 
+        req = swob.Request.blank('/?a=b&c=d', params={'x': 'y'})
+        self.assertEqual(req.params, {'x': 'y'})
+
+        req = swob.Request.blank('/?a=b&c=d', params={})
+        self.assertFalse(req.params)
+
     def test_unicode_params(self):
         # NB: all of these strings are WSGI strings
         req = swob.Request.blank(