Dont Refresh NewMode Oauth (#1517)

* dont try to auto refresh

* dont try to auto refresh

* use the new client

* slight refactor

* fix default client

* dont pass around multiple clients

* cleanup test

* Kasiah/newmode hotfix (#1518)

* Update newmode.py

* Update newmode.py

---------

Co-authored-by: Kasia Hinkson <52927664+KasiaHinkson@users.noreply.github.com>

Author: codybraun

parsons/newmode/newmode.py

@@ -431,11 +431,23 @@ def __init__(
         self.client_secret: str = check_env.check("NEWMODE_API_CLIENT_SECRET", client_secret)
         self.headers: Dict[str, str] = {"content-type": "application/json"}
         self.default_client: OAuth2APIConnector = self.get_default_oauth_client()
+        self.campaigns_client: OAuth2APIConnector = self.get_campaigns_oauth_client()
+
+    def get_campaigns_oauth_client(self) -> OAuth2APIConnector:
+        return OAuth2APIConnector(
+            uri=V2_API_CAMPAIGNS_URL,
+            auto_refresh_url=None,
+            client_id=self.client_id,
+            client_secret=self.client_secret,
+            headers=V2_API_CAMPAIGNS_HEADERS,
+            token_url=V2_API_AUTH_URL,
+            grant_type="client_credentials",
+        )
 
     def get_default_oauth_client(self) -> OAuth2APIConnector:
         return OAuth2APIConnector(
             uri=self.base_url,
-            auto_refresh_url=V2_API_AUTH_URL,
+            auto_refresh_url=None,
             client_id=self.client_id,
             client_secret=self.client_secret,
             headers=self.headers,
@@ -461,7 +473,7 @@ def base_request(
         self,
         method: str,
         url: str,
-        client: OAuth2APIConnector,
+        use_campaigns_client: bool = False,
         data: Optional[Dict[str, Any]] = None,
         json: Optional[Dict[str, Any]] = None,
         params: Optional[Dict[str, Any]] = None,
@@ -474,6 +486,8 @@ def base_request(
         if params is None:
             params = {}
 
+        client = self.default_client if not use_campaigns_client else self.campaigns_client
+
         for attempt in range(retries + 1):
             try:
                 try:
@@ -484,6 +498,10 @@ def base_request(
                 except TokenExpiredError as e:
                     logger.warning(f"Token expired: {e}. Refreshing it...")
                     self.default_client = self.get_default_oauth_client()
+                    self.campaigns_client = self.get_campaigns_oauth_client()
+                    client = (
+                        self.default_client if not use_campaigns_client else self.campaigns_client
+                    )
             except Exception as e:
                 if attempt < retries:
                     logger.warning(f"Request failed (attempt {attempt + 1}/{retries}). Retrying...")
@@ -496,7 +514,7 @@ def paginate_request(
         self,
         method: str,
         endpoint: str,
-        client: OAuth2APIConnector,
+        use_campaigns_client: bool = False,
         data_key: str = RESPONSE_DATA_KEY,
         data: Optional[Dict[str, Any]] = None,
         json: Optional[Dict[str, Any]] = None,
@@ -517,7 +535,7 @@ def paginate_request(
             response = self.base_request(
                 method=method,
                 url=url,
-                client=client,
+                use_campaigns_client=use_campaigns_client,
                 data=data,
                 json=json,
                 params=params,
@@ -531,7 +549,12 @@ def paginate_request(
             else:
                 results.append(response)
             # Check for pagination
-            url = response.get(RESPONSE_LINKS_KEY, {}).get(PAGINATION_NEXT) if response else None
+            url = None
+            if response:
+                url = response.get(RESPONSE_LINKS_KEY, {}).get(PAGINATION_NEXT, {})
+                if isinstance(url, dict):
+                    url = url.get("href")
+
         return results
 
     def converted_request(
@@ -544,14 +567,13 @@ def converted_request(
         params: Optional[Dict[str, Any]] = None,
         convert_to_table: bool = True,
         data_key: Optional[str] = None,
-        client: Optional[OAuth2APIConnector] = None,
+        use_campaigns_client: bool = False,
         override_api_version: Optional[str] = None,
     ) -> Union[Table, Dict[str, Any]]:
         """Internal method to make a call to the Newmode API and convert the result to a Parsons table."""
 
         if params is None:
             params = {}
-        client = client if client else self.default_client
         response = self.paginate_request(
             method=method,
             json=json,
@@ -560,12 +582,12 @@ def converted_request(
             data_key=data_key,
             supports_version=supports_version,
             endpoint=endpoint,
-            client=client,
+            use_campaigns_client=use_campaigns_client,
             override_api_version=override_api_version,
         )
         if response:
             if convert_to_table:
-                return client.convert_to_table(data=response)
+                return self.default_client.convert_to_table(data=response)
             else:
                 return response
 
@@ -607,22 +629,13 @@ def get_campaign_ids(self, params: Optional[Dict[str, Any]] = None) -> List[str]
         if params is None:
             params = {}
         endpoint = "node/action"
-        campaigns_client = OAuth2APIConnector(
-            uri=V2_API_CAMPAIGNS_URL,
-            auto_refresh_url=V2_API_AUTH_URL,
-            client_id=self.client_id,
-            client_secret=self.client_secret,
-            headers=V2_API_CAMPAIGNS_HEADERS,
-            token_url=V2_API_AUTH_URL,
-            grant_type="client_credentials",
-        )
 
         data = self.converted_request(
             endpoint=endpoint,
             method="GET",
             params=params,
             data_key=RESPONSE_DATA_KEY,
-            client=campaigns_client,
+            use_campaigns_client=True,
             override_api_version=V2_API_CAMPAIGNS_VERSION,
         )
         return data["id"]
@@ -726,7 +739,10 @@ def get_submissions(self, campaign_id: str, params: Optional[Dict[str, Any]] = N
             params = {}
         params = {"action": campaign_id}
         response = self.converted_request(
-            endpoint="submission", method="GET", params=params, data_key=RESPONSE_DATA_KEY
+            endpoint="submission",
+            method="GET",
+            params=params,
+            data_key=RESPONSE_DATA_KEY,
         )
         return response
 
@@ -765,7 +781,9 @@ def __new__(
         api_version = check_env.check("NEWMODE_API_VERSION", api_version)
         if api_version.startswith("v2"):
             return NewmodeV2(
-                client_id=client_id, client_secret=client_secret, api_version=api_version
+                client_id=client_id,
+                client_secret=client_secret,
+                api_version=api_version,
             )
         if api_version.startswith("v1"):
             return NewmodeV1(api_user=api_user, api_password=api_password, api_version=api_version)

test/test_newmode/test_newmode.py

@@ -283,7 +283,6 @@ def test_base_request_retries(self, m, mock_logger):
             self.nm.base_request(
                 method="GET",
                 url=f"{V2_API_URL}v2.1/test-endpoint",
-                client=self.nm.default_client,
                 retries=2,
             )
 
@@ -343,18 +342,18 @@ def test_checked_response_http_error(self, m):
     @patch("parsons.newmode.newmode.NewmodeV2.get_default_oauth_client")
     def test_token_refresh_on_expired_token(self, m, mock_get_default_oauth_client):
         m.post(V2_API_AUTH_URL, json={"access_token": "fakeAccessToken"})
-        m.get(f"{V2_API_URL}v2.1/test-endpoint", status_code=401)
 
         mock_new_client = mock.MagicMock()
         mock_get_default_oauth_client.return_value = mock_new_client
-        self.nm.default_client.request = mock.MagicMock()
 
         mock_response = mock.MagicMock()
         mock_response.raise_for_status = mock.MagicMock()
         mock_response.status_code = 200
         mock_response.json.return_value = {"data": "success"}
+        mock_new_client.request.return_value = mock_response
+
+        mock_new_client.json_check.return_value = True
 
-        # Simulate token expiration and successful response
         def oauth_side_effect(*args, **kwargs):
             if not hasattr(self, "call_count"):
                 self.call_count = 0
@@ -363,13 +362,12 @@ def oauth_side_effect(*args, **kwargs):
                 raise TokenExpiredError()
             return mock_response
 
-        self.nm.default_client.request.side_effect = oauth_side_effect
-
-        response = self.nm.base_request(
-            method="GET",
-            url=f"{V2_API_URL}v2.1/test-endpoint",
-            client=self.nm.default_client,
-        )
+        with patch.object(self.nm.default_client, "request", side_effect=oauth_side_effect):
+            m.get(f"{V2_API_URL}v2.1/test-endpoint", json={"data": "success"}, status_code=200)
+            response = self.nm.base_request(method="GET", url=f"{V2_API_URL}v2.1/test-endpoint")
 
         mock_get_default_oauth_client.assert_called_once()
         self.assertEqual(response, {"data": "success"})
+        mock_new_client.request.assert_called_with(
+            url=f"{V2_API_URL}v2.1/test-endpoint", req_type="GET", json=None, data=None, params={}
+        )