From 4edd3b93b9bed88f45a3487f5233279f937b7783 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Tue, 6 May 2025 19:25:29 +0600 Subject: [PATCH 1/8] Implement CMAB client with retry logic for fetching predictions --- optimizely/cmab/cmab_client.py | 119 +++++++++++++++++++++++++++++++++ 1 file changed, 119 insertions(+) create mode 100644 optimizely/cmab/cmab_client.py diff --git a/optimizely/cmab/cmab_client.py b/optimizely/cmab/cmab_client.py new file mode 100644 index 00000000..5d6e6fe6 --- /dev/null +++ b/optimizely/cmab/cmab_client.py @@ -0,0 +1,119 @@ +import json +import time +import requests +import math +from typing import Dict, Any, Optional, List +from optimizely import logger as _logging +from optimizely.helpers.enums import Errors + +# CMAB_PREDICTION_ENDPOINT is the endpoint for CMAB predictions +CMAB_PREDICTION_ENDPOINT = "https://prediction.cmab.optimizely.com/predict/%s" + +# Default constants for CMAB requests +DEFAULT_MAX_RETRIES = 3 +DEFAULT_INITIAL_BACKOFF = 0.1 # in seconds (100 ms) +DEFAULT_MAX_BACKOFF = 10 # in seconds +DEFAULT_BACKOFF_MULTIPLIER = 2.0 +MAX_WAIT_TIME = 10.0 + +class CmabRetryConfig: + def __init__( + self, + max_retries: int = DEFAULT_MAX_RETRIES, + initial_backoff: float = DEFAULT_INITIAL_BACKOFF, + max_backoff: float = DEFAULT_MAX_BACKOFF, + backoff_multiplier: float = DEFAULT_BACKOFF_MULTIPLIER, + ): + self.max_retries = max_retries + self.initial_backoff = initial_backoff + self.max_backoff = max_backoff + self.backoff_multiplier = backoff_multiplier + +class DefaultCmabClient: + def __init__(self, http_client: Optional[requests.Session] = None, + retry_config: Optional[CmabRetryConfig] = None, + logger: Optional[_logging.Logger] = None): + self.http_client = http_client or requests.Session() + self.retry_config = retry_config + self.logger = _logging.adapt_logger(logger or _logging.NoOpLogger()) + + def fetch_decision( + self, + rule_id: str, + user_id: str, + attributes: Dict[str, Any], + cmab_uuid: str + ) -> Optional[str]: + + url = CMAB_PREDICTION_ENDPOINT % rule_id + cmab_attributes = [ + {"id": key, "value": value, "type": "custom_attribute"} + for key, value in attributes.items() + ] + + request_body = { + "instances": [{ + "visitorId": user_id, + "experimentId": rule_id, + "attributes": cmab_attributes, + "cmabUUID": cmab_uuid, + }] + } + + try: + if self.retry_config: + variation_id = self._do_fetch_with_retry(url, request_body, self.retry_config) + else: + variation_id = self._do_fetch(url, request_body) + return variation_id + + except requests.RequestException as e: + self.logger.error(f"Error fetching CMAB decision: {e}") + pass + + def _do_fetch(self, url: str, request_body: str) -> Optional[str]: + headers = {'Content-Type': 'application/json'} + try: + response = self.http_client.post(url, data=json.dumps(request_body), headers=headers, timeout=MAX_WAIT_TIME) + except requests.exceptions.RequestException as e: + self.logger.exception(str(e)) + return None + + if not 200 <= response.status_code < 300: + self.logger.exception(f'CMAB Request failed with status code: {response.status_code}') + return None + + try: + body = response.json() + except json.JSONDecodeError as e: + self.logger.exception(str(e)) + return None + + if not self.validate_response(body): + self.logger.exception('Invalid response') + return None + + return str(body['predictions'][0]['variation_id']) + + def validate_response(self, body: dict) -> bool: + return ( + isinstance(body, dict) + and 'predictions' in body + and isinstance(body['predictions'], list) + and len(body['predictions']) > 0 + and isinstance(body['predictions'][0], dict) + and "variation_id" in body["predictions"][0] + ) + + def _do_fetch_with_retry(self, url: str, request_body: dict, retry_config: CmabRetryConfig) -> Optional[str]: + backoff = retry_config.initial_backoff + for attempt in range(retry_config.max_retries + 1): + variation_id = self._do_fetch(url, request_body) + if variation_id: + return variation_id + if attempt < retry_config.max_retries: + self.logger.info(f"Retrying CMAB request (attempt: {attempt + 1}) after {backoff} seconds...") + time.sleep(backoff) + backoff = min(backoff * math.pow(retry_config.backoff_multiplier, attempt + 1), retry_config.max_backoff) + self.logger.error("Exhausted all retries for CMAB request.") + return None From fb32e0b06c1bdb34d12f704942a3a33e0e4e7229 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Tue, 6 May 2025 20:07:51 +0600 Subject: [PATCH 2/8] Enhance CMAB client error handling and logging; add unit tests for fetch methods --- optimizely/cmab/cmab_client.py | 14 +-- optimizely/helpers/enums.py | 2 + tests/test_cmab_client.py | 157 +++++++++++++++++++++++++++++++++ 3 files changed, 166 insertions(+), 7 deletions(-) create mode 100644 tests/test_cmab_client.py diff --git a/optimizely/cmab/cmab_client.py b/optimizely/cmab/cmab_client.py index 5d6e6fe6..013d604b 100644 --- a/optimizely/cmab/cmab_client.py +++ b/optimizely/cmab/cmab_client.py @@ -68,29 +68,29 @@ def fetch_decision( return variation_id except requests.RequestException as e: - self.logger.error(f"Error fetching CMAB decision: {e}") - pass + self.logger.error(Errors.CMAB_FETCH_FAILED.format(str(e))) + return None def _do_fetch(self, url: str, request_body: str) -> Optional[str]: headers = {'Content-Type': 'application/json'} try: response = self.http_client.post(url, data=json.dumps(request_body), headers=headers, timeout=MAX_WAIT_TIME) except requests.exceptions.RequestException as e: - self.logger.exception(str(e)) + self.logger.exception(Errors.CMAB_FETCH_FAILED.format(str(e))) return None if not 200 <= response.status_code < 300: - self.logger.exception(f'CMAB Request failed with status code: {response.status_code}') + self.logger.exception(Errors.CMAB_FETCH_FAILED.format(str(response.status_code))) return None try: body = response.json() except json.JSONDecodeError as e: - self.logger.exception(str(e)) + self.logger.exception(Errors.INVALID_CMAB_FETCH_RESPONSE) return None if not self.validate_response(body): - self.logger.exception('Invalid response') + self.logger.exception(Errors.INVALID_CMAB_FETCH_RESPONSE) return None return str(body['predictions'][0]['variation_id']) @@ -115,5 +115,5 @@ def _do_fetch_with_retry(self, url: str, request_body: dict, retry_config: CmabR self.logger.info(f"Retrying CMAB request (attempt: {attempt + 1}) after {backoff} seconds...") time.sleep(backoff) backoff = min(backoff * math.pow(retry_config.backoff_multiplier, attempt + 1), retry_config.max_backoff) - self.logger.error("Exhausted all retries for CMAB request.") + self.logger.error(Errors.CMAB_FETCH_FAILED.format('Exhausted all retries for CMAB request.')) return None diff --git a/optimizely/helpers/enums.py b/optimizely/helpers/enums.py index fe90946e..2d6febab 100644 --- a/optimizely/helpers/enums.py +++ b/optimizely/helpers/enums.py @@ -127,6 +127,8 @@ class Errors: ODP_INVALID_DATA: Final = 'ODP data is not valid.' ODP_INVALID_ACTION: Final = 'ODP action is not valid (cannot be empty).' MISSING_SDK_KEY: Final = 'SDK key not provided/cannot be found in the datafile.' + CMAB_FETCH_FAILED: Final = 'CMAB decision fetch failed with status: {}' + INVALID_CMAB_FETCH_RESPONSE = 'Invalid CMAB fetch response' class ForcedDecisionLogs: diff --git a/tests/test_cmab_client.py b/tests/test_cmab_client.py new file mode 100644 index 00000000..90dd8aee --- /dev/null +++ b/tests/test_cmab_client.py @@ -0,0 +1,157 @@ +import unittest +import json +from unittest.mock import MagicMock, patch, call +from optimizely.cmab.cmab_client import DefaultCmabClient, CmabRetryConfig +from requests.exceptions import RequestException +from optimizely.helpers.enums import Errors + +class TestDefaultCmabClient_do_fetch(unittest.TestCase): + def setUp(self): + self.mock_http_client = MagicMock() + self.mock_logger = MagicMock() + self.client = DefaultCmabClient(http_client=self.mock_http_client, logger=self.mock_logger) + + def test_do_fetch_success(self): + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.json.return_value = { + 'predictions': [{'variation_id': 'abc123'}] + } + self.mock_http_client.post.return_value = mock_response + + result = self.client._do_fetch('http://fake-url', {'some': 'data'}) + self.assertEqual(result, 'abc123') + + def test_do_fetch_http_exception(self): + self.mock_http_client.post.side_effect = RequestException('Connection error') + result = self.client._do_fetch('http://fake-url', {'some': 'data'}) + self.assertIsNone(result) + self.mock_logger.exception.assert_called_with(Errors.CMAB_FETCH_FAILED.format('Connection error')) + + def test_do_fetch_non_2xx_status(self): + mock_response = MagicMock() + mock_response.status_code = 500 + self.mock_http_client.post.return_value = mock_response + result = self.client._do_fetch('http://fake-url', {'some': 'data'}) + self.assertIsNone(result) + self.mock_logger.exception.assert_called_with(Errors.CMAB_FETCH_FAILED.format(str(mock_response.status_code))) + + def test_do_fetch_invalid_json(self): + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.json.side_effect = json.JSONDecodeError("Expecting value", "", 0) + self.mock_http_client.post.return_value = mock_response + result = self.client._do_fetch('http://fake-url', {'some': 'data'}) + self.assertIsNone(result) + self.mock_logger.exception.assert_called_with(Errors.INVALID_CMAB_FETCH_RESPONSE) + + def test_do_fetch_invalid_response_structure(self): + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.json.return_value = {'no_predictions': []} + self.mock_http_client.post.return_value = mock_response + result = self.client._do_fetch('http://fake-url', {'some': 'data'}) + self.assertIsNone(result) + self.mock_logger.exception.assert_called_with(Errors.INVALID_CMAB_FETCH_RESPONSE) + +class TestDefaultCmabClientWithRetry(unittest.TestCase): + def setUp(self): + self.mock_http_client = MagicMock() + self.mock_logger = MagicMock() + self.retry_config = CmabRetryConfig(max_retries=2, initial_backoff=0.01, max_backoff=1, backoff_multiplier=2) + self.client = DefaultCmabClient( + http_client=self.mock_http_client, + logger=self.mock_logger, + retry_config=self.retry_config + ) + + @patch("time.sleep", return_value=None) + def test_do_fetch_with_retry_success_on_first_try(self, _): + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.json.return_value = { + "predictions": [{"variation_id": "abc123"}] + } + self.mock_http_client.post.return_value = mock_response + + result = self.client._do_fetch_with_retry("http://fake-url", {}, self.retry_config) + self.assertEqual(result, "abc123") + self.assertEqual(self.mock_http_client.post.call_count, 1) + + @patch("time.sleep", return_value=None) + def test_do_fetch_with_retry_success_on_retry(self, _): + # First call fails, second call succeeds + failure_response = MagicMock() + failure_response.status_code = 500 + + success_response = MagicMock() + success_response.status_code = 200 + success_response.json.return_value = { + "predictions": [{"variation_id": "xyz456"}] + } + + self.mock_http_client.post.side_effect = [ + failure_response, + success_response + ] + + result = self.client._do_fetch_with_retry("http://fake-url", {}, self.retry_config) + self.assertEqual(result, "xyz456") + self.assertEqual(self.mock_http_client.post.call_count, 2) + self.mock_logger.info.assert_called_with("Retrying CMAB request (attempt: 1) after 0.01 seconds...") + + @patch("time.sleep", return_value=None) + def test_do_fetch_with_retry_exhausts_all_attempts(self, _): + failure_response = MagicMock() + failure_response.status_code = 500 + + self.mock_http_client.post.return_value = failure_response + + result = self.client._do_fetch_with_retry("http://fake-url", {}, self.retry_config) + self.assertIsNone(result) + self.assertEqual(self.mock_http_client.post.call_count, 3) # 1 original + 2 retries + self.mock_logger.error.assert_called_with(Errors.CMAB_FETCH_FAILED.format("Exhausted all retries for CMAB request.")) + +class TestDefaultCmabClientFetchDecision(unittest.TestCase): + def setUp(self): + self.mock_http_client = MagicMock() + self.mock_logger = MagicMock() + self.retry_config = CmabRetryConfig(max_retries=2, initial_backoff=0.01, max_backoff=1, backoff_multiplier=2) + self.client = DefaultCmabClient( + http_client=self.mock_http_client, + logger=self.mock_logger, + retry_config=self.retry_config + ) + self.rule_id = 'test_rule' + self.user_id = 'user123' + self.attributes = {'attr1': 'value1'} + self.cmab_uuid = 'uuid-1234' + + @patch.object(DefaultCmabClient, '_do_fetch', return_value='var-abc') + def test_fetch_decision_success_no_retry(self, mock_do_fetch): + result = self.client.fetch_decision(self.rule_id, self.user_id, self.attributes, self.cmab_uuid) + self.assertEqual(result, 'var-abc') + mock_do_fetch.assert_called_once() + + @patch.object(DefaultCmabClient, '_do_fetch_with_retry', return_value='var-xyz') + def test_fetch_decision_success_with_retry(self, mock_do_fetch_with_retry): + client_with_retry = DefaultCmabClient( + http_client=self.mock_http_client, + logger=self.mock_logger, + retry_config=self.retry_config + ) + result = client_with_retry.fetch_decision(self.rule_id, self.user_id, self.attributes, self.cmab_uuid) + self.assertEqual(result, 'var-xyz') + mock_do_fetch_with_retry.assert_called_once() + + @patch.object(DefaultCmabClient, '_do_fetch', side_effect=RequestException("Network error")) + def test_fetch_decision_request_exception(self, mock_do_fetch): + result = self.client.fetch_decision(self.rule_id, self.user_id, self.attributes, self.cmab_uuid) + self.assertIsNone(result) + self.mock_logger.error.assert_called_with(Errors.CMAB_FETCH_FAILED.format("Network error")) + + @patch.object(DefaultCmabClient, '_do_fetch', return_value=None) + def test_fetch_decision_invalid_response(self, mock_do_fetch): + result = self.client.fetch_decision(self.rule_id, self.user_id, self.attributes, self.cmab_uuid) + self.assertIsNone(result) + self.mock_logger.error.assert_called_once() From 5e6bbde228935e26bde7a7ab59c9033d3f54f289 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Tue, 6 May 2025 20:31:26 +0600 Subject: [PATCH 3/8] Refactor CMAB client: enhance docstrings for classes and methods, improve formatting, and clean up imports --- optimizely/cmab/cmab_client.py | 108 +++++++++++++++++++++++++++------ tests/test_cmab_client.py | 8 ++- 2 files changed, 96 insertions(+), 20 deletions(-) diff --git a/optimizely/cmab/cmab_client.py b/optimizely/cmab/cmab_client.py index 013d604b..bd9161f3 100644 --- a/optimizely/cmab/cmab_client.py +++ b/optimizely/cmab/cmab_client.py @@ -1,22 +1,39 @@ +# Copyright 2025 Optimizely +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. import json import time import requests import math -from typing import Dict, Any, Optional, List -from optimizely import logger as _logging +from typing import Dict, Any, Optional +from optimizely import logger as _logging from optimizely.helpers.enums import Errors # CMAB_PREDICTION_ENDPOINT is the endpoint for CMAB predictions -CMAB_PREDICTION_ENDPOINT = "https://prediction.cmab.optimizely.com/predict/%s" +CMAB_PREDICTION_ENDPOINT = "https://prediction.cmab.optimizely.com/predict/%s" # Default constants for CMAB requests DEFAULT_MAX_RETRIES = 3 DEFAULT_INITIAL_BACKOFF = 0.1 # in seconds (100 ms) -DEFAULT_MAX_BACKOFF = 10 # in seconds +DEFAULT_MAX_BACKOFF = 10 # in seconds DEFAULT_BACKOFF_MULTIPLIER = 2.0 MAX_WAIT_TIME = 10.0 + class CmabRetryConfig: + """Configuration for retrying CMAB requests. + + Contains parameters for maximum retries, backoff intervals, and multipliers. + """ def __init__( self, max_retries: int = DEFAULT_MAX_RETRIES, @@ -29,14 +46,26 @@ def __init__( self.max_backoff = max_backoff self.backoff_multiplier = backoff_multiplier + class DefaultCmabClient: + """Client for interacting with the CMAB service. + + Provides methods to fetch decisions with optional retry logic. + """ def __init__(self, http_client: Optional[requests.Session] = None, retry_config: Optional[CmabRetryConfig] = None, logger: Optional[_logging.Logger] = None): + """Initialize the CMAB client. + + Args: + http_client (Optional[requests.Session]): HTTP client for making requests. + retry_config (Optional[CmabRetryConfig]): Configuration for retry logic. + logger (Optional[_logging.Logger]): Logger for logging messages. + """ self.http_client = http_client or requests.Session() self.retry_config = retry_config self.logger = _logging.adapt_logger(logger or _logging.NoOpLogger()) - + def fetch_decision( self, rule_id: str, @@ -44,7 +73,17 @@ def fetch_decision( attributes: Dict[str, Any], cmab_uuid: str ) -> Optional[str]: + """Fetch a decision from the CMAB prediction service. + + Args: + rule_id (str): The rule ID for the experiment. + user_id (str): The user ID for the request. + attributes (Dict[str, Any]): User attributes for the request. + cmab_uuid (str): Unique identifier for the CMAB request. + Returns: + Optional[str]: The variation ID if successful, None otherwise. + """ url = CMAB_PREDICTION_ENDPOINT % rule_id cmab_attributes = [ {"id": key, "value": value, "type": "custom_attribute"} @@ -59,7 +98,7 @@ def fetch_decision( "cmabUUID": cmab_uuid, }] } - + try: if self.retry_config: variation_id = self._do_fetch_with_retry(url, request_body, self.retry_config) @@ -71,7 +110,16 @@ def fetch_decision( self.logger.error(Errors.CMAB_FETCH_FAILED.format(str(e))) return None - def _do_fetch(self, url: str, request_body: str) -> Optional[str]: + def _do_fetch(self, url: str, request_body: Dict[str, Any]) -> Optional[str]: + """Perform a single fetch request to the CMAB prediction service. + + Args: + url (str): The endpoint URL. + request_body (Dict[str, Any]): The request payload. + + Returns: + Optional[str]: The variation ID if successful, None otherwise. + """ headers = {'Content-Type': 'application/json'} try: response = self.http_client.post(url, data=json.dumps(request_body), headers=headers, timeout=MAX_WAIT_TIME) @@ -85,7 +133,7 @@ def _do_fetch(self, url: str, request_body: str) -> Optional[str]: try: body = response.json() - except json.JSONDecodeError as e: + except json.JSONDecodeError: self.logger.exception(Errors.INVALID_CMAB_FETCH_RESPONSE) return None @@ -94,18 +142,41 @@ def _do_fetch(self, url: str, request_body: str) -> Optional[str]: return None return str(body['predictions'][0]['variation_id']) - - def validate_response(self, body: dict) -> bool: + + def validate_response(self, body: Dict[str, Any]) -> bool: + """Validate the response structure from the CMAB service. + + Args: + body (Dict[str, Any]): The response body to validate. + + Returns: + bool: True if the response is valid, False otherwise. + """ return ( - isinstance(body, dict) - and 'predictions' in body - and isinstance(body['predictions'], list) - and len(body['predictions']) > 0 - and isinstance(body['predictions'][0], dict) - and "variation_id" in body["predictions"][0] + isinstance(body, dict) and + 'predictions' in body and + isinstance(body['predictions'], list) and + len(body['predictions']) > 0 and + isinstance(body['predictions'][0], dict) and + "variation_id" in body["predictions"][0] ) - def _do_fetch_with_retry(self, url: str, request_body: dict, retry_config: CmabRetryConfig) -> Optional[str]: + def _do_fetch_with_retry( + self, + url: str, + request_body: Dict[str, Any], + retry_config: CmabRetryConfig + ) -> Optional[str]: + """Perform a fetch request with retry logic. + + Args: + url (str): The endpoint URL. + request_body (Dict[str, Any]): The request payload. + retry_config (CmabRetryConfig): Configuration for retry logic. + + Returns: + Optional[str]: The variation ID if successful, None otherwise. + """ backoff = retry_config.initial_backoff for attempt in range(retry_config.max_retries + 1): variation_id = self._do_fetch(url, request_body) @@ -114,6 +185,7 @@ def _do_fetch_with_retry(self, url: str, request_body: dict, retry_config: CmabR if attempt < retry_config.max_retries: self.logger.info(f"Retrying CMAB request (attempt: {attempt + 1}) after {backoff} seconds...") time.sleep(backoff) - backoff = min(backoff * math.pow(retry_config.backoff_multiplier, attempt + 1), retry_config.max_backoff) + backoff = min(backoff * math.pow(retry_config.backoff_multiplier, attempt + 1), + retry_config.max_backoff) self.logger.error(Errors.CMAB_FETCH_FAILED.format('Exhausted all retries for CMAB request.')) return None diff --git a/tests/test_cmab_client.py b/tests/test_cmab_client.py index 90dd8aee..90d09645 100644 --- a/tests/test_cmab_client.py +++ b/tests/test_cmab_client.py @@ -1,10 +1,11 @@ import unittest import json -from unittest.mock import MagicMock, patch, call +from unittest.mock import MagicMock, patch from optimizely.cmab.cmab_client import DefaultCmabClient, CmabRetryConfig from requests.exceptions import RequestException from optimizely.helpers.enums import Errors + class TestDefaultCmabClient_do_fetch(unittest.TestCase): def setUp(self): self.mock_http_client = MagicMock() @@ -54,6 +55,7 @@ def test_do_fetch_invalid_response_structure(self): self.assertIsNone(result) self.mock_logger.exception.assert_called_with(Errors.INVALID_CMAB_FETCH_RESPONSE) + class TestDefaultCmabClientWithRetry(unittest.TestCase): def setUp(self): self.mock_http_client = MagicMock() @@ -110,7 +112,9 @@ def test_do_fetch_with_retry_exhausts_all_attempts(self, _): result = self.client._do_fetch_with_retry("http://fake-url", {}, self.retry_config) self.assertIsNone(result) self.assertEqual(self.mock_http_client.post.call_count, 3) # 1 original + 2 retries - self.mock_logger.error.assert_called_with(Errors.CMAB_FETCH_FAILED.format("Exhausted all retries for CMAB request.")) + self.mock_logger.error.assert_called_with( + Errors.CMAB_FETCH_FAILED.format("Exhausted all retries for CMAB request.")) + class TestDefaultCmabClientFetchDecision(unittest.TestCase): def setUp(self): From 58755f1cdf62bd61293aafb9c34efd67af267dff Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 7 May 2025 20:29:48 +0600 Subject: [PATCH 4/8] Add custom exceptions for CMAB client errors and enhance error handling in fetch methods --- optimizely/cmab/cmab_client.py | 77 ++++++++++++++++++++-------------- optimizely/exceptions.py | 18 ++++++++ tests/test_cmab_client.py | 60 ++++++++++++++------------ 3 files changed, 97 insertions(+), 58 deletions(-) diff --git a/optimizely/cmab/cmab_client.py b/optimizely/cmab/cmab_client.py index bd9161f3..8d4d5d64 100644 --- a/optimizely/cmab/cmab_client.py +++ b/optimizely/cmab/cmab_client.py @@ -17,6 +17,7 @@ from typing import Dict, Any, Optional from optimizely import logger as _logging from optimizely.helpers.enums import Errors +from optimizely.exceptions import CmabFetchError, CmabInvalidResponseError # CMAB_PREDICTION_ENDPOINT is the endpoint for CMAB predictions CMAB_PREDICTION_ENDPOINT = "https://prediction.cmab.optimizely.com/predict/%s" @@ -71,8 +72,9 @@ def fetch_decision( rule_id: str, user_id: str, attributes: Dict[str, Any], - cmab_uuid: str - ) -> Optional[str]: + cmab_uuid: str, + timeout: Optional[float] = None + ) -> str: """Fetch a decision from the CMAB prediction service. Args: @@ -80,11 +82,13 @@ def fetch_decision( user_id (str): The user ID for the request. attributes (Dict[str, Any]): User attributes for the request. cmab_uuid (str): Unique identifier for the CMAB request. + timeout (float): Maximum wait time for request to respond in seconds. Returns: - Optional[str]: The variation ID if successful, None otherwise. + str: The variation ID. """ url = CMAB_PREDICTION_ENDPOINT % rule_id + timeout = timeout or MAX_WAIT_TIME cmab_attributes = [ {"id": key, "value": value, "type": "custom_attribute"} for key, value in attributes.items() @@ -101,45 +105,50 @@ def fetch_decision( try: if self.retry_config: - variation_id = self._do_fetch_with_retry(url, request_body, self.retry_config) + variation_id = self._do_fetch_with_retry(url, request_body, self.retry_config, timeout) else: - variation_id = self._do_fetch(url, request_body) + variation_id = self._do_fetch(url, request_body, timeout) return variation_id except requests.RequestException as e: - self.logger.error(Errors.CMAB_FETCH_FAILED.format(str(e))) - return None + error_message = Errors.CMAB_FETCH_FAILED.format(str(e)) + self.logger.error(error_message) + raise CmabFetchError(error_message) - def _do_fetch(self, url: str, request_body: Dict[str, Any]) -> Optional[str]: + def _do_fetch(self, url: str, request_body: Dict[str, Any], timeout: float) -> str: """Perform a single fetch request to the CMAB prediction service. Args: url (str): The endpoint URL. request_body (Dict[str, Any]): The request payload. - + timeout (float): Maximum wait time for request to respond in seconds. Returns: - Optional[str]: The variation ID if successful, None otherwise. + str: The variation ID """ headers = {'Content-Type': 'application/json'} try: - response = self.http_client.post(url, data=json.dumps(request_body), headers=headers, timeout=MAX_WAIT_TIME) + response = self.http_client.post(url, data=json.dumps(request_body), headers=headers, timeout=timeout) except requests.exceptions.RequestException as e: - self.logger.exception(Errors.CMAB_FETCH_FAILED.format(str(e))) - return None + error_message = Errors.CMAB_FETCH_FAILED.format(str(e)) + self.logger.error(error_message) + raise CmabFetchError(error_message) if not 200 <= response.status_code < 300: - self.logger.exception(Errors.CMAB_FETCH_FAILED.format(str(response.status_code))) - return None + error_message = Errors.CMAB_FETCH_FAILED.format(str(response.status_code)) + self.logger.error(error_message) + raise CmabFetchError(error_message) try: body = response.json() except json.JSONDecodeError: - self.logger.exception(Errors.INVALID_CMAB_FETCH_RESPONSE) - return None + error_message = Errors.INVALID_CMAB_FETCH_RESPONSE + self.logger.error(error_message) + raise CmabInvalidResponseError(error_message) if not self.validate_response(body): - self.logger.exception(Errors.INVALID_CMAB_FETCH_RESPONSE) - return None + error_message = Errors.INVALID_CMAB_FETCH_RESPONSE + self.logger.error(error_message) + raise CmabInvalidResponseError(error_message) return str(body['predictions'][0]['variation_id']) @@ -165,27 +174,31 @@ def _do_fetch_with_retry( self, url: str, request_body: Dict[str, Any], - retry_config: CmabRetryConfig - ) -> Optional[str]: + retry_config: CmabRetryConfig, + timeout: float + ) -> str: """Perform a fetch request with retry logic. Args: url (str): The endpoint URL. request_body (Dict[str, Any]): The request payload. retry_config (CmabRetryConfig): Configuration for retry logic. - + timeout (float): Maximum wait time for request to respond in seconds. Returns: - Optional[str]: The variation ID if successful, None otherwise. + str: The variation ID """ backoff = retry_config.initial_backoff for attempt in range(retry_config.max_retries + 1): - variation_id = self._do_fetch(url, request_body) - if variation_id: + try: + variation_id = self._do_fetch(url, request_body, timeout) return variation_id - if attempt < retry_config.max_retries: - self.logger.info(f"Retrying CMAB request (attempt: {attempt + 1}) after {backoff} seconds...") - time.sleep(backoff) - backoff = min(backoff * math.pow(retry_config.backoff_multiplier, attempt + 1), - retry_config.max_backoff) - self.logger.error(Errors.CMAB_FETCH_FAILED.format('Exhausted all retries for CMAB request.')) - return None + except: + if attempt < retry_config.max_retries: + self.logger.info(f"Retrying CMAB request (attempt: {attempt + 1}) after {backoff} seconds...") + time.sleep(backoff) + backoff = min(backoff * math.pow(retry_config.backoff_multiplier, attempt + 1), + retry_config.max_backoff) + + error_message = Errors.CMAB_FETCH_FAILED.format('Exhausted all retries for CMAB request.') + self.logger.error(error_message) + raise CmabFetchError(error_message) diff --git a/optimizely/exceptions.py b/optimizely/exceptions.py index e7644064..b17b1397 100644 --- a/optimizely/exceptions.py +++ b/optimizely/exceptions.py @@ -82,3 +82,21 @@ class OdpInvalidData(Exception): """ Raised when passing invalid ODP data. """ pass + + +class CmabError(Exception): + """Base exception for CMAB client errors.""" + + pass + + +class CmabFetchError(CmabError): + """Exception raised when CMAB fetch fails.""" + + pass + + +class CmabInvalidResponseError(CmabError): + """Exception raised when CMAB response is invalid.""" + + pass diff --git a/tests/test_cmab_client.py b/tests/test_cmab_client.py index 90d09645..8dff8f18 100644 --- a/tests/test_cmab_client.py +++ b/tests/test_cmab_client.py @@ -4,6 +4,7 @@ from optimizely.cmab.cmab_client import DefaultCmabClient, CmabRetryConfig from requests.exceptions import RequestException from optimizely.helpers.enums import Errors +from optimizely.exceptions import CmabFetchError, CmabInvalidResponseError class TestDefaultCmabClient_do_fetch(unittest.TestCase): @@ -20,40 +21,52 @@ def test_do_fetch_success(self): } self.mock_http_client.post.return_value = mock_response - result = self.client._do_fetch('http://fake-url', {'some': 'data'}) + result = self.client._do_fetch('http://fake-url', {'some': 'data'}, 1.0) self.assertEqual(result, 'abc123') def test_do_fetch_http_exception(self): self.mock_http_client.post.side_effect = RequestException('Connection error') - result = self.client._do_fetch('http://fake-url', {'some': 'data'}) - self.assertIsNone(result) - self.mock_logger.exception.assert_called_with(Errors.CMAB_FETCH_FAILED.format('Connection error')) + + with self.assertRaises(CmabFetchError) as context: + self.client._do_fetch('http://fake-url', {'some': 'data'}, 1.0) + + self.mock_logger.error.assert_called_with(Errors.CMAB_FETCH_FAILED.format('Connection error')) + self.assertIn('Connection error', str(context.exception)) def test_do_fetch_non_2xx_status(self): mock_response = MagicMock() mock_response.status_code = 500 self.mock_http_client.post.return_value = mock_response - result = self.client._do_fetch('http://fake-url', {'some': 'data'}) - self.assertIsNone(result) - self.mock_logger.exception.assert_called_with(Errors.CMAB_FETCH_FAILED.format(str(mock_response.status_code))) + + with self.assertRaises(CmabFetchError) as context: + self.client._do_fetch('http://fake-url', {'some': 'data'}, 1.0) + + self.mock_logger.error.assert_called_with(Errors.CMAB_FETCH_FAILED.format(str(mock_response.status_code))) + self.assertIn(str(mock_response.status_code), str(context.exception)) def test_do_fetch_invalid_json(self): mock_response = MagicMock() mock_response.status_code = 200 mock_response.json.side_effect = json.JSONDecodeError("Expecting value", "", 0) self.mock_http_client.post.return_value = mock_response - result = self.client._do_fetch('http://fake-url', {'some': 'data'}) - self.assertIsNone(result) - self.mock_logger.exception.assert_called_with(Errors.INVALID_CMAB_FETCH_RESPONSE) + + with self.assertRaises(CmabInvalidResponseError) as context: + self.client._do_fetch('http://fake-url', {'some': 'data'}, 1.0) + + self.mock_logger.error.assert_called_with(Errors.INVALID_CMAB_FETCH_RESPONSE) + self.assertIn(Errors.INVALID_CMAB_FETCH_RESPONSE, str(context.exception)) def test_do_fetch_invalid_response_structure(self): mock_response = MagicMock() mock_response.status_code = 200 mock_response.json.return_value = {'no_predictions': []} self.mock_http_client.post.return_value = mock_response - result = self.client._do_fetch('http://fake-url', {'some': 'data'}) - self.assertIsNone(result) - self.mock_logger.exception.assert_called_with(Errors.INVALID_CMAB_FETCH_RESPONSE) + + with self.assertRaises(CmabInvalidResponseError) as context: + self.client._do_fetch('http://fake-url', {'some': 'data'}, 1.0) + + self.mock_logger.error.assert_called_with(Errors.INVALID_CMAB_FETCH_RESPONSE) + self.assertIn(Errors.INVALID_CMAB_FETCH_RESPONSE, str(context.exception)) class TestDefaultCmabClientWithRetry(unittest.TestCase): @@ -76,7 +89,7 @@ def test_do_fetch_with_retry_success_on_first_try(self, _): } self.mock_http_client.post.return_value = mock_response - result = self.client._do_fetch_with_retry("http://fake-url", {}, self.retry_config) + result = self.client._do_fetch_with_retry("http://fake-url", {}, self.retry_config, 1.0) self.assertEqual(result, "abc123") self.assertEqual(self.mock_http_client.post.call_count, 1) @@ -97,7 +110,7 @@ def test_do_fetch_with_retry_success_on_retry(self, _): success_response ] - result = self.client._do_fetch_with_retry("http://fake-url", {}, self.retry_config) + result = self.client._do_fetch_with_retry("http://fake-url", {}, self.retry_config, 1.0) self.assertEqual(result, "xyz456") self.assertEqual(self.mock_http_client.post.call_count, 2) self.mock_logger.info.assert_called_with("Retrying CMAB request (attempt: 1) after 0.01 seconds...") @@ -109,8 +122,9 @@ def test_do_fetch_with_retry_exhausts_all_attempts(self, _): self.mock_http_client.post.return_value = failure_response - result = self.client._do_fetch_with_retry("http://fake-url", {}, self.retry_config) - self.assertIsNone(result) + with self.assertRaises(CmabFetchError): + self.client._do_fetch_with_retry("http://fake-url", {}, self.retry_config, 1.0) + self.assertEqual(self.mock_http_client.post.call_count, 3) # 1 original + 2 retries self.mock_logger.error.assert_called_with( Errors.CMAB_FETCH_FAILED.format("Exhausted all retries for CMAB request.")) @@ -124,7 +138,7 @@ def setUp(self): self.client = DefaultCmabClient( http_client=self.mock_http_client, logger=self.mock_logger, - retry_config=self.retry_config + retry_config=None ) self.rule_id = 'test_rule' self.user_id = 'user123' @@ -150,12 +164,6 @@ def test_fetch_decision_success_with_retry(self, mock_do_fetch_with_retry): @patch.object(DefaultCmabClient, '_do_fetch', side_effect=RequestException("Network error")) def test_fetch_decision_request_exception(self, mock_do_fetch): - result = self.client.fetch_decision(self.rule_id, self.user_id, self.attributes, self.cmab_uuid) - self.assertIsNone(result) + with self.assertRaises(CmabFetchError): + self.client.fetch_decision(self.rule_id, self.user_id, self.attributes, self.cmab_uuid) self.mock_logger.error.assert_called_with(Errors.CMAB_FETCH_FAILED.format("Network error")) - - @patch.object(DefaultCmabClient, '_do_fetch', return_value=None) - def test_fetch_decision_invalid_response(self, mock_do_fetch): - result = self.client.fetch_decision(self.rule_id, self.user_id, self.attributes, self.cmab_uuid) - self.assertIsNone(result) - self.mock_logger.error.assert_called_once() From afb4d50e7c0b297a0a9bba84758bb70f69d8d357 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 7 May 2025 20:35:00 +0600 Subject: [PATCH 5/8] Update fetch_decision method to set default timeout value to 10 seconds --- optimizely/cmab/cmab_client.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/optimizely/cmab/cmab_client.py b/optimizely/cmab/cmab_client.py index 8d4d5d64..806f847d 100644 --- a/optimizely/cmab/cmab_client.py +++ b/optimizely/cmab/cmab_client.py @@ -73,7 +73,7 @@ def fetch_decision( user_id: str, attributes: Dict[str, Any], cmab_uuid: str, - timeout: Optional[float] = None + timeout: float = MAX_WAIT_TIME ) -> str: """Fetch a decision from the CMAB prediction service. @@ -82,13 +82,12 @@ def fetch_decision( user_id (str): The user ID for the request. attributes (Dict[str, Any]): User attributes for the request. cmab_uuid (str): Unique identifier for the CMAB request. - timeout (float): Maximum wait time for request to respond in seconds. + timeout (float): Maximum wait time for request to respond in seconds. Defaults to 10 seconds. Returns: str: The variation ID. """ url = CMAB_PREDICTION_ENDPOINT % rule_id - timeout = timeout or MAX_WAIT_TIME cmab_attributes = [ {"id": key, "value": value, "type": "custom_attribute"} for key, value in attributes.items() From a3bce83f22d7f97d1fbb024bfe0425f0920afd09 Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Wed, 7 May 2025 23:00:32 +0600 Subject: [PATCH 6/8] replace constant endpoint with formatted string in fetch_decision method --- optimizely/cmab/cmab_client.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/optimizely/cmab/cmab_client.py b/optimizely/cmab/cmab_client.py index 806f847d..a4389f96 100644 --- a/optimizely/cmab/cmab_client.py +++ b/optimizely/cmab/cmab_client.py @@ -19,9 +19,6 @@ from optimizely.helpers.enums import Errors from optimizely.exceptions import CmabFetchError, CmabInvalidResponseError -# CMAB_PREDICTION_ENDPOINT is the endpoint for CMAB predictions -CMAB_PREDICTION_ENDPOINT = "https://prediction.cmab.optimizely.com/predict/%s" - # Default constants for CMAB requests DEFAULT_MAX_RETRIES = 3 DEFAULT_INITIAL_BACKOFF = 0.1 # in seconds (100 ms) @@ -87,7 +84,7 @@ def fetch_decision( Returns: str: The variation ID. """ - url = CMAB_PREDICTION_ENDPOINT % rule_id + url = f"https://prediction.cmab.optimizely.com/predict/{rule_id}" cmab_attributes = [ {"id": key, "value": value, "type": "custom_attribute"} for key, value in attributes.items() From 8c8573a6fe995bf74bdb25cafb0fc5fe01757f7e Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Mon, 12 May 2025 22:23:03 +0600 Subject: [PATCH 7/8] chore: trigger CI From fd81a295daa7b0ca28a161080324900791aa22ee Mon Sep 17 00:00:00 2001 From: FarhanAnjum-opti Date: Tue, 13 May 2025 05:13:49 +0600 Subject: [PATCH 8/8] refactor: streamline fetch_decision method and enhance test cases for improved clarity and functionality --- optimizely/cmab/cmab_client.py | 17 +-- tests/test_cmab_client.py | 216 +++++++++++++++++++++------------ 2 files changed, 146 insertions(+), 87 deletions(-) diff --git a/optimizely/cmab/cmab_client.py b/optimizely/cmab/cmab_client.py index a4389f96..dfcffa78 100644 --- a/optimizely/cmab/cmab_client.py +++ b/optimizely/cmab/cmab_client.py @@ -98,18 +98,11 @@ def fetch_decision( "cmabUUID": cmab_uuid, }] } - - try: - if self.retry_config: - variation_id = self._do_fetch_with_retry(url, request_body, self.retry_config, timeout) - else: - variation_id = self._do_fetch(url, request_body, timeout) - return variation_id - - except requests.RequestException as e: - error_message = Errors.CMAB_FETCH_FAILED.format(str(e)) - self.logger.error(error_message) - raise CmabFetchError(error_message) + if self.retry_config: + variation_id = self._do_fetch_with_retry(url, request_body, self.retry_config, timeout) + else: + variation_id = self._do_fetch(url, request_body, timeout) + return variation_id def _do_fetch(self, url: str, request_body: Dict[str, Any], timeout: float) -> str: """Perform a single fetch request to the CMAB prediction service. diff --git a/tests/test_cmab_client.py b/tests/test_cmab_client.py index 8dff8f18..0e15b3f4 100644 --- a/tests/test_cmab_client.py +++ b/tests/test_cmab_client.py @@ -1,169 +1,235 @@ import unittest import json -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock, patch, call from optimizely.cmab.cmab_client import DefaultCmabClient, CmabRetryConfig from requests.exceptions import RequestException from optimizely.helpers.enums import Errors from optimizely.exceptions import CmabFetchError, CmabInvalidResponseError -class TestDefaultCmabClient_do_fetch(unittest.TestCase): +class TestDefaultCmabClient(unittest.TestCase): def setUp(self): self.mock_http_client = MagicMock() self.mock_logger = MagicMock() - self.client = DefaultCmabClient(http_client=self.mock_http_client, logger=self.mock_logger) + self.retry_config = CmabRetryConfig(max_retries=3, initial_backoff=0.01, max_backoff=1, backoff_multiplier=2) + self.client = DefaultCmabClient( + http_client=self.mock_http_client, + logger=self.mock_logger, + retry_config=None + ) + self.rule_id = 'test_rule' + self.user_id = 'user123' + self.attributes = {'attr1': 'value1', 'attr2': 'value2'} + self.cmab_uuid = 'uuid-1234' + self.expected_url = f"https://prediction.cmab.optimizely.com/predict/{self.rule_id}" + self.expected_body = { + "instances": [{ + "visitorId": self.user_id, + "experimentId": self.rule_id, + "attributes": [ + {"id": "attr1", "value": "value1", "type": "custom_attribute"}, + {"id": "attr2", "value": "value2", "type": "custom_attribute"} + ], + "cmabUUID": self.cmab_uuid, + }] + } + self.expected_headers = {'Content-Type': 'application/json'} - def test_do_fetch_success(self): + def test_fetch_decision_returns_success_no_retry(self): mock_response = MagicMock() mock_response.status_code = 200 mock_response.json.return_value = { 'predictions': [{'variation_id': 'abc123'}] } self.mock_http_client.post.return_value = mock_response - - result = self.client._do_fetch('http://fake-url', {'some': 'data'}, 1.0) + result = self.client.fetch_decision(self.rule_id, self.user_id, self.attributes, self.cmab_uuid) self.assertEqual(result, 'abc123') + self.mock_http_client.post.assert_called_once_with( + self.expected_url, + data=json.dumps(self.expected_body), + headers=self.expected_headers, + timeout=10.0 + ) - def test_do_fetch_http_exception(self): + def test_fetch_decision_returns_http_exception_no_retry(self): self.mock_http_client.post.side_effect = RequestException('Connection error') with self.assertRaises(CmabFetchError) as context: - self.client._do_fetch('http://fake-url', {'some': 'data'}, 1.0) + self.client.fetch_decision(self.rule_id, self.user_id, self.attributes, self.cmab_uuid) + self.mock_http_client.post.assert_called_once() self.mock_logger.error.assert_called_with(Errors.CMAB_FETCH_FAILED.format('Connection error')) self.assertIn('Connection error', str(context.exception)) - def test_do_fetch_non_2xx_status(self): + def test_fetch_decision_returns_non_2xx_status_no_retry(self): mock_response = MagicMock() mock_response.status_code = 500 self.mock_http_client.post.return_value = mock_response with self.assertRaises(CmabFetchError) as context: - self.client._do_fetch('http://fake-url', {'some': 'data'}, 1.0) + self.client.fetch_decision(self.rule_id, self.user_id, self.attributes, self.cmab_uuid) + self.mock_http_client.post.assert_called_once_with( + self.expected_url, + data=json.dumps(self.expected_body), + headers=self.expected_headers, + timeout=10.0 + ) self.mock_logger.error.assert_called_with(Errors.CMAB_FETCH_FAILED.format(str(mock_response.status_code))) self.assertIn(str(mock_response.status_code), str(context.exception)) - def test_do_fetch_invalid_json(self): + def test_fetch_decision_returns_invalid_json_no_retry(self): mock_response = MagicMock() mock_response.status_code = 200 mock_response.json.side_effect = json.JSONDecodeError("Expecting value", "", 0) self.mock_http_client.post.return_value = mock_response with self.assertRaises(CmabInvalidResponseError) as context: - self.client._do_fetch('http://fake-url', {'some': 'data'}, 1.0) + self.client.fetch_decision(self.rule_id, self.user_id, self.attributes, self.cmab_uuid) + self.mock_http_client.post.assert_called_once_with( + self.expected_url, + data=json.dumps(self.expected_body), + headers=self.expected_headers, + timeout=10.0 + ) self.mock_logger.error.assert_called_with(Errors.INVALID_CMAB_FETCH_RESPONSE) self.assertIn(Errors.INVALID_CMAB_FETCH_RESPONSE, str(context.exception)) - def test_do_fetch_invalid_response_structure(self): + def test_fetch_decision_returns_invalid_response_structure_no_retry(self): mock_response = MagicMock() mock_response.status_code = 200 mock_response.json.return_value = {'no_predictions': []} self.mock_http_client.post.return_value = mock_response with self.assertRaises(CmabInvalidResponseError) as context: - self.client._do_fetch('http://fake-url', {'some': 'data'}, 1.0) + self.client.fetch_decision(self.rule_id, self.user_id, self.attributes, self.cmab_uuid) + self.mock_http_client.post.assert_called_once_with( + self.expected_url, + data=json.dumps(self.expected_body), + headers=self.expected_headers, + timeout=10.0 + ) self.mock_logger.error.assert_called_with(Errors.INVALID_CMAB_FETCH_RESPONSE) self.assertIn(Errors.INVALID_CMAB_FETCH_RESPONSE, str(context.exception)) - -class TestDefaultCmabClientWithRetry(unittest.TestCase): - def setUp(self): - self.mock_http_client = MagicMock() - self.mock_logger = MagicMock() - self.retry_config = CmabRetryConfig(max_retries=2, initial_backoff=0.01, max_backoff=1, backoff_multiplier=2) - self.client = DefaultCmabClient( + @patch('time.sleep', return_value=None) + def test_fetch_decision_returns_success_with_retry_on_first_try(self, mock_sleep): + # Create client with retry + client_with_retry = DefaultCmabClient( http_client=self.mock_http_client, logger=self.mock_logger, retry_config=self.retry_config ) - @patch("time.sleep", return_value=None) - def test_do_fetch_with_retry_success_on_first_try(self, _): + # Mock successful response mock_response = MagicMock() mock_response.status_code = 200 mock_response.json.return_value = { - "predictions": [{"variation_id": "abc123"}] + 'predictions': [{'variation_id': 'abc123'}] } self.mock_http_client.post.return_value = mock_response - result = self.client._do_fetch_with_retry("http://fake-url", {}, self.retry_config, 1.0) - self.assertEqual(result, "abc123") + result = client_with_retry.fetch_decision(self.rule_id, self.user_id, self.attributes, self.cmab_uuid) + + # Verify result and request parameters + self.assertEqual(result, 'abc123') + self.mock_http_client.post.assert_called_once_with( + self.expected_url, + data=json.dumps(self.expected_body), + headers=self.expected_headers, + timeout=10.0 + ) self.assertEqual(self.mock_http_client.post.call_count, 1) + mock_sleep.assert_not_called() + + @patch('time.sleep', return_value=None) + def test_fetch_decision_returns_success_with_retry_on_third_try(self, mock_sleep): + client_with_retry = DefaultCmabClient( + http_client=self.mock_http_client, + logger=self.mock_logger, + retry_config=self.retry_config + ) - @patch("time.sleep", return_value=None) - def test_do_fetch_with_retry_success_on_retry(self, _): - # First call fails, second call succeeds + # Create failure and success responses failure_response = MagicMock() failure_response.status_code = 500 success_response = MagicMock() success_response.status_code = 200 success_response.json.return_value = { - "predictions": [{"variation_id": "xyz456"}] + 'predictions': [{'variation_id': 'xyz456'}] } + # First two calls fail, third succeeds self.mock_http_client.post.side_effect = [ + failure_response, failure_response, success_response ] - result = self.client._do_fetch_with_retry("http://fake-url", {}, self.retry_config, 1.0) - self.assertEqual(result, "xyz456") - self.assertEqual(self.mock_http_client.post.call_count, 2) - self.mock_logger.info.assert_called_with("Retrying CMAB request (attempt: 1) after 0.01 seconds...") - - @patch("time.sleep", return_value=None) - def test_do_fetch_with_retry_exhausts_all_attempts(self, _): - failure_response = MagicMock() - failure_response.status_code = 500 - - self.mock_http_client.post.return_value = failure_response - - with self.assertRaises(CmabFetchError): - self.client._do_fetch_with_retry("http://fake-url", {}, self.retry_config, 1.0) - - self.assertEqual(self.mock_http_client.post.call_count, 3) # 1 original + 2 retries - self.mock_logger.error.assert_called_with( - Errors.CMAB_FETCH_FAILED.format("Exhausted all retries for CMAB request.")) + result = client_with_retry.fetch_decision(self.rule_id, self.user_id, self.attributes, self.cmab_uuid) + self.assertEqual(result, 'xyz456') + self.assertEqual(self.mock_http_client.post.call_count, 3) -class TestDefaultCmabClientFetchDecision(unittest.TestCase): - def setUp(self): - self.mock_http_client = MagicMock() - self.mock_logger = MagicMock() - self.retry_config = CmabRetryConfig(max_retries=2, initial_backoff=0.01, max_backoff=1, backoff_multiplier=2) - self.client = DefaultCmabClient( - http_client=self.mock_http_client, - logger=self.mock_logger, - retry_config=None + # Verify all HTTP calls used correct parameters + self.mock_http_client.post.assert_called_with( + self.expected_url, + data=json.dumps(self.expected_body), + headers=self.expected_headers, + timeout=10.0 ) - self.rule_id = 'test_rule' - self.user_id = 'user123' - self.attributes = {'attr1': 'value1'} - self.cmab_uuid = 'uuid-1234' - @patch.object(DefaultCmabClient, '_do_fetch', return_value='var-abc') - def test_fetch_decision_success_no_retry(self, mock_do_fetch): - result = self.client.fetch_decision(self.rule_id, self.user_id, self.attributes, self.cmab_uuid) - self.assertEqual(result, 'var-abc') - mock_do_fetch.assert_called_once() + # Verify retry logging + self.mock_logger.info.assert_has_calls([ + call("Retrying CMAB request (attempt: 1) after 0.01 seconds..."), + call("Retrying CMAB request (attempt: 2) after 0.02 seconds...") + ]) - @patch.object(DefaultCmabClient, '_do_fetch_with_retry', return_value='var-xyz') - def test_fetch_decision_success_with_retry(self, mock_do_fetch_with_retry): + # Verify sleep was called with correct backoff times + mock_sleep.assert_has_calls([ + call(0.01), + call(0.02) + ]) + + @patch('time.sleep', return_value=None) + def test_fetch_decision_exhausts_all_retry_attempts(self, mock_sleep): client_with_retry = DefaultCmabClient( http_client=self.mock_http_client, logger=self.mock_logger, retry_config=self.retry_config ) - result = client_with_retry.fetch_decision(self.rule_id, self.user_id, self.attributes, self.cmab_uuid) - self.assertEqual(result, 'var-xyz') - mock_do_fetch_with_retry.assert_called_once() - @patch.object(DefaultCmabClient, '_do_fetch', side_effect=RequestException("Network error")) - def test_fetch_decision_request_exception(self, mock_do_fetch): + # Create failure response + failure_response = MagicMock() + failure_response.status_code = 500 + + # All attempts fail + self.mock_http_client.post.return_value = failure_response + with self.assertRaises(CmabFetchError): - self.client.fetch_decision(self.rule_id, self.user_id, self.attributes, self.cmab_uuid) - self.mock_logger.error.assert_called_with(Errors.CMAB_FETCH_FAILED.format("Network error")) + client_with_retry.fetch_decision(self.rule_id, self.user_id, self.attributes, self.cmab_uuid) + + # Verify all attempts were made (1 initial + 3 retries) + self.assertEqual(self.mock_http_client.post.call_count, 4) + + # Verify retry logging + self.mock_logger.info.assert_has_calls([ + call("Retrying CMAB request (attempt: 1) after 0.01 seconds..."), + call("Retrying CMAB request (attempt: 2) after 0.02 seconds..."), + call("Retrying CMAB request (attempt: 3) after 0.08 seconds...") + ]) + + # Verify sleep was called for each retry + mock_sleep.assert_has_calls([ + call(0.01), + call(0.02), + call(0.08) + ]) + + # Verify final error + self.mock_logger.error.assert_called_with( + Errors.CMAB_FETCH_FAILED.format('Exhausted all retries for CMAB request.') + )