From f97f94030a505c429063fda5cb867763c857e820 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Thu, 25 Aug 2022 18:25:36 -0700 Subject: [PATCH 01/17] bump up py version in gitactions to 3.10 --- .github/workflows/python.yml | 4 ++-- tests/base.py | 4 ---- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml index 7e17c5ff..b13b0e52 100644 --- a/.github/workflows/python.yml +++ b/.github/workflows/python.yml @@ -29,10 +29,10 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 - - name: Set up Python 3.9 + - name: Set up Python 3.10 uses: actions/setup-python@v3 with: - python-version: 3.9 + python-version: 3.10 # flake8 version should be same as the version in requirements/test.txt # to avoid lint errors on CI - name: pip install flak8 diff --git a/tests/base.py b/tests/base.py index 65ae1fe1..d4aeae8e 100644 --- a/tests/base.py +++ b/tests/base.py @@ -20,10 +20,6 @@ from optimizely import optimizely -def long(a): - raise NotImplementedError('Tests should only call `long` if running in PY2') - - class BaseTest(unittest.TestCase): def assertStrictTrue(self, to_assert): self.assertIs(to_assert, True) From 77dfec4fa2b348bf34bc3860d21f9bd89bff49ce Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Fri, 2 Sep 2022 15:34:23 -0700 Subject: [PATCH 02/17] feat: add first draft of odp_manager --- optimizely/exceptions.py | 18 ++++ optimizely/helpers/enums.py | 17 +++- optimizely/odp/odp_manager.py | 129 ++++++++++++++++++++++++++ optimizely/odp/odp_segment_manager.py | 4 +- tests/test_odp_manager.py | 89 ++++++++++++++++++ 5 files changed, 250 insertions(+), 7 deletions(-) create mode 100644 optimizely/odp/odp_manager.py create mode 100644 tests/test_odp_manager.py diff --git a/optimizely/exceptions.py b/optimizely/exceptions.py index d6003ab1..e7644064 100644 --- a/optimizely/exceptions.py +++ b/optimizely/exceptions.py @@ -64,3 +64,21 @@ class UnsupportedDatafileVersionException(Exception): """ Raised when provided version in datafile is not supported. """ pass + + +class OdpNotEnabled(Exception): + """ Raised when Optimizely Data Platform (ODP) is not enabled. """ + + pass + + +class OdpNotIntegrated(Exception): + """ Raised when Optimizely Data Platform (ODP) is not integrated. """ + + pass + + +class OdpInvalidData(Exception): + """ Raised when passing invalid ODP data. """ + + pass diff --git a/optimizely/helpers/enums.py b/optimizely/helpers/enums.py index 02bc9136..6a02593b 100644 --- a/optimizely/helpers/enums.py +++ b/optimizely/helpers/enums.py @@ -120,11 +120,12 @@ class Errors: NONE_VARIABLE_KEY_PARAMETER: Final = '"None" is an invalid value for variable key.' UNSUPPORTED_DATAFILE_VERSION: Final = ( 'This version of the Python SDK does not support the given datafile version: "{}".') - INVALID_SEGMENT_IDENTIFIER: Final = 'Audience segments fetch failed (invalid identifier).' - FETCH_SEGMENTS_FAILED: Final = 'Audience segments fetch failed ({}).' - ODP_EVENT_FAILED: Final = 'ODP event send failed ({}).' - ODP_NOT_ENABLED: Final = 'ODP is not enabled.' - ODP_NOT_INTEGRATED: Final = 'ODP is not integrated.' + INVALID_SEGMENT_IDENTIFIER = 'Audience segments fetch failed (invalid identifier).' + FETCH_SEGMENTS_FAILED = 'Audience segments fetch failed ({}).' + ODP_EVENT_FAILED = 'ODP event send failed ({}).' + ODP_NOT_INTEGRATED = 'ODP is not integrated.' + ODP_NOT_ENABLED = 'ODP is not enabled.' + ODP_INVALID_DATA = 'ODP data is not valid.' class ForcedDecisionLogs: @@ -214,3 +215,9 @@ class OdpEventManagerConfig: DEFAULT_BATCH_SIZE: Final = 10 DEFAULT_FLUSH_INTERVAL: Final = 1 DEFAULT_RETRY_COUNT: Final = 3 + + +class OdpManagerConfig: + """ODP Manager configs.""" + KEY_FOR_USER_ID = 'fs_user_id' + EVENT_TYPE = 'fullstack' diff --git a/optimizely/odp/odp_manager.py b/optimizely/odp/odp_manager.py new file mode 100644 index 00000000..8ea56979 --- /dev/null +++ b/optimizely/odp/odp_manager.py @@ -0,0 +1,129 @@ +# Copyright 2022, 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. + +from __future__ import annotations + +from typing import Optional, Any + +from optimizely import logger as optimizely_logger +from optimizely.helpers.enums import Errors, OdpManagerConfig +from optimizely.helpers.validator import are_odp_data_types_valid +from optimizely.odp.lru_cache import LRUCache +from optimizely.odp.odp_config import OdpConfig +from optimizely.odp.odp_segment_manager import OdpSegmentManager +from optimizely.odp.odp_event_manager import OdpEventManager +from optimizely.odp.zaius_graphql_api_manager import ZaiusGraphQLApiManager +from optimizely import exceptions as optimizely_exception + + +class OdpManager: + """TODO - ADD COMMENT""" + + def __init__(self, sdk_key: str, + disable: bool, + cache_size: int, + cache_timeout_in_sec: int, + segment_manager: OdpSegmentManager, + event_manager: OdpEventManager, + odp_config: OdpConfig, + logger: Optional[optimizely_logger.Logger] = None) -> None: + + self.enabled = not disable + self.cache_size = cache_size + self.cache_timeout_in_sec = cache_timeout_in_sec + self.odp_config = odp_config + self.logger = logger or optimizely_logger.NoOpLogger() + + if self.enabled: + if segment_manager: + segment_manager.odp_config = odp_config + self.segment_manager = segment_manager + else: + # TODO - careful - DO I USE self in front or not in these variables????? (ex self.opd_config or odp_config) + # - check if third param should have braces at the end + self.segment_manager = OdpSegmentManager(odp_config, + LRUCache(self.cache_size, self.cache_timeout_in_sec), + ZaiusGraphQLApiManager(), logger) + if event_manager: + event_manager.odp_config = odp_config + self.event_manager = event_manager + else: + self.event_manager = OdpEventManager(sdk_key, odp_config) # TODO NEXT - FIGURE OUT WHAT TO DO WITH THIS SDK KEY - it's not a parameter in OdpEventManager class + + + + + + + + + + + def fetch_qualified_segments(self, user_id: str, options: list[str]): + if not self.enabled: + self.logger.error(Errors.ODP_NOT_ENABLED) # TODO - check if this error is needed, should it be debug? + + user_key = OdpManagerConfig.KEY_FOR_USER_ID + user_value = user_id + + self.segment_manager.fetch_qualified_segments(user_key, user_value, options) + + def identify_user(self, user_id: str): + if not self.enabled: + self.logger.debug('ODP identify event is not dispatched (ODP disabled).') + + if not self.odp_config.odp_state().INTEGRATED: + self.logger.debug('ODP identify event is not dispatched (ODP not integrated).') + + # TODO - consider putting send_event into a separate function into OdpEventManager to have all + # event logic in there. Jae did it. But it's also fine if leave it as is. Think about it, check w Andy? + if self.event_manager: + self.event_manager.send_event(OdpManagerConfig.EVENT_TYPE, 'identified', + {OdpManagerConfig.KEY_FOR_USER_ID: user_id}, {}) + + def send_event(self, type: str, action: str, identifiers: dict[str, str], data: dict[str, Any]) -> None: + """ + Send an event to the ODP server. + + Args: + type: The event type. + action: The event action name. + identifiers: A dictionary for identifiers. + data: A dictionary for associated data. The default event data will be added to this data before sending to the ODP server. + + Raises custom exception if error is detected. + """ + if not self.enabled: + raise optimizely_exception.OdpNotEnabled(Errors.ODP_NOT_ENABLED) + + if not are_odp_data_types_valid(data): + raise optimizely_exception.OdpInvalidData(Errors.ODP_INVALID_DATA) + + self.event_manager.send_event(type, action, identifiers, data) + + def update_odp_config(self, api_key: str, api_host: str, segments_to_check: list[str]) -> None: + + if not self.enabled: + return None + + # flush old events using old odp publicKey (if exists) before updating odp key. + if self.event_manager: + self.event_manager.flush() + # wait until done flushing to avoid flushing in the middle of the batch + self.event_manager.event_queue.join() + + config_changed = self.odp_config.update(api_key, api_host, segments_to_check) + if not config_changed: + return None + + # reset segments cache when odp integration or segmentsToCheck are changed + if self.segment_manager: + self.segment_manager.reset() + + # flush events with the new integration key if events still remain in the queue + # (when we get the first datafile ready) + if self.event_manager: + self.event_manager.flush() + + # TODO - need return None at the end? diff --git a/optimizely/odp/odp_segment_manager.py b/optimizely/odp/odp_segment_manager.py index 33c829a1..7f7582e4 100644 --- a/optimizely/odp/odp_segment_manager.py +++ b/optimizely/odp/odp_segment_manager.py @@ -64,7 +64,7 @@ def fetch_qualified_segments(self, user_key: str, user_value: str, options: list reset_cache = OptimizelyOdpOption.RESET_CACHE in options if reset_cache: - self._reset() + self.reset() if not ignore_cache and not reset_cache: segments = self.segments_cache.lookup(cache_key) @@ -83,7 +83,7 @@ def fetch_qualified_segments(self, user_key: str, user_value: str, options: list return segments - def _reset(self) -> None: + def reset(self) -> None: self.segments_cache.reset() def make_cache_key(self, user_key: str, user_value: str) -> str: diff --git a/tests/test_odp_manager.py b/tests/test_odp_manager.py new file mode 100644 index 00000000..f93e39eb --- /dev/null +++ b/tests/test_odp_manager.py @@ -0,0 +1,89 @@ +# Copyright 2022, 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. + +from __future__ import annotations + +from unittest import mock +from unittest.mock import call + +from requests import exceptions as request_exception + +from optimizely.odp.lru_cache import LRUCache +from optimizely.odp.odp_config import OdpConfig +from optimizely.odp.optimizely_odp_option import OptimizelyOdpOption +from optimizely.odp.odp_segment_manager import OdpSegmentManager +from optimizely.odp.zaius_graphql_api_manager import ZaiusGraphQLApiManager +from tests import base + + +class OdpManagerTest(base.BaseTest): + # api_host = 'host' + # api_key = 'valid' + + def test_empty_list_with_no_segments_to_check(self): + odp_config = OdpConfig(self.api_key, self.api_host, []) + mock_logger = mock.MagicMock() + segments_cache = LRUCache(1000, 1000) + api = ZaiusGraphQLApiManager() + segment_manager = OdpSegmentManager(odp_config, segments_cache, api, mock_logger) + + with mock.patch.object(api, 'fetch_segments') as mock_fetch_segments: + segments = segment_manager.fetch_qualified_segments(self.user_key, self.user_value, []) + + self.assertEqual(segments, []) + mock_logger.debug.assert_called_once_with('No segments are used in the project. Returning empty list.') + mock_logger.error.assert_not_called() + mock_fetch_segments.assert_not_called() + + def test_configurations_cache(self): + pass + + def test_configurations_disable_odp(self): + pass + + def test_fetch_qualified_segments(self): + pass + + def test_identify_user_datafile_not_ready(self): + pass + + def test_identify_user_odp_integrated(self): + pass + + def test_identify_user_odp_not_integrated(self): + pass + + def test_identify_user_odp_disabled(self): + pass + + def test_send_event_datafile_not_ready(self): + pass + + def test_send_event_odp_integrated(self): + pass + + def test_send_event_odp_not_Integrated(self): + pass + + def test_send_event_odp_disabled(self): + pass + + def test_update_odp_config__reset_called(self): + pass + + def test_update_odp_config__flush_called(self): + pass + + def test_update_odp_config__odp_config_propagated_properly(self): + pass + From 05b0d706f11be823c64d9713f530adfd003e15e9 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Tue, 13 Sep 2022 12:16:06 -0700 Subject: [PATCH 03/17] add tests --- optimizely/odp/odp_event_manager.py | 21 +- optimizely/odp/odp_manager.py | 61 +++-- tests/test_odp_manager.py | 335 +++++++++++++++++++++++++--- 3 files changed, 343 insertions(+), 74 deletions(-) diff --git a/optimizely/odp/odp_event_manager.py b/optimizely/odp/odp_event_manager.py index df02e3ed..cb51beb1 100644 --- a/optimizely/odp/odp_event_manager.py +++ b/optimizely/odp/odp_event_manager.py @@ -12,17 +12,18 @@ # limitations under the License. from __future__ import annotations + +import time from enum import Enum +from queue import Empty, Queue, Full from threading import Thread from typing import Optional -import time -from queue import Empty, Queue, Full from optimizely import logger as _logging -from .odp_event import OdpEvent, OdpDataDict +from optimizely.helpers.enums import OdpEventManagerConfig, Errors, OdpManagerConfig from .odp_config import OdpConfig, OdpConfigState +from .odp_event import OdpEvent, OdpDataDict from .zaius_rest_api_manager import ZaiusRestApiManager -from optimizely.helpers.enums import OdpEventManagerConfig, Errors class Signal(Enum): @@ -41,10 +42,10 @@ class OdpEventManager: """ def __init__( - self, - odp_config: OdpConfig, - logger: Optional[_logging.Logger] = None, - api_manager: Optional[ZaiusRestApiManager] = None + self, + odp_config: OdpConfig, + logger: Optional[_logging.Logger] = None, + api_manager: Optional[ZaiusRestApiManager] = None ): """OdpEventManager init method to configure event batching. @@ -236,3 +237,7 @@ def dispatch(self, event: OdpEvent) -> None: self.event_queue.put_nowait(event) except Full: self.logger.warning(Errors.ODP_EVENT_FAILED.format("Queue is full")) + + def identify_user(self, user_id: str) -> None: + self.send_event(OdpManagerConfig.EVENT_TYPE, 'identified', + {OdpManagerConfig.KEY_FOR_USER_ID: user_id}, {}) diff --git a/optimizely/odp/odp_manager.py b/optimizely/odp/odp_manager.py index 8ea56979..910191b4 100644 --- a/optimizely/odp/odp_manager.py +++ b/optimizely/odp/odp_manager.py @@ -15,72 +15,67 @@ from typing import Optional, Any +from optimizely import exceptions as optimizely_exception from optimizely import logger as optimizely_logger from optimizely.helpers.enums import Errors, OdpManagerConfig from optimizely.helpers.validator import are_odp_data_types_valid from optimizely.odp.lru_cache import LRUCache from optimizely.odp.odp_config import OdpConfig -from optimizely.odp.odp_segment_manager import OdpSegmentManager from optimizely.odp.odp_event_manager import OdpEventManager +from optimizely.odp.odp_segment_manager import OdpSegmentManager from optimizely.odp.zaius_graphql_api_manager import ZaiusGraphQLApiManager -from optimizely import exceptions as optimizely_exception class OdpManager: - """TODO - ADD COMMENT""" + """Orchestrates segment manager, event manager and odp config.""" - def __init__(self, sdk_key: str, - disable: bool, + def __init__(self, disable: bool, cache_size: int, cache_timeout_in_sec: int, - segment_manager: OdpSegmentManager, - event_manager: OdpEventManager, - odp_config: OdpConfig, + segment_manager: Optional[OdpSegmentManager] = None, + event_manager: Optional[OdpEventManager] = None, logger: Optional[optimizely_logger.Logger] = None) -> None: self.enabled = not disable self.cache_size = cache_size self.cache_timeout_in_sec = cache_timeout_in_sec - self.odp_config = odp_config + self.odp_config = OdpConfig() self.logger = logger or optimizely_logger.NoOpLogger() + self.segment_manager = None + self.event_manager = None + if self.enabled: if segment_manager: - segment_manager.odp_config = odp_config + segment_manager.odp_config = self.odp_config self.segment_manager = segment_manager else: - # TODO - careful - DO I USE self in front or not in these variables????? (ex self.opd_config or odp_config) - # - check if third param should have braces at the end - self.segment_manager = OdpSegmentManager(odp_config, - LRUCache(self.cache_size, self.cache_timeout_in_sec), + self.segment_manager = OdpSegmentManager(self.odp_config, + LRUCache(cache_size, cache_timeout_in_sec), ZaiusGraphQLApiManager(), logger) if event_manager: - event_manager.odp_config = odp_config + event_manager.odp_config = self.odp_config self.event_manager = event_manager else: - self.event_manager = OdpEventManager(sdk_key, odp_config) # TODO NEXT - FIGURE OUT WHAT TO DO WITH THIS SDK KEY - it's not a parameter in OdpEventManager class + + + + + + + + + + self.event_manager = OdpEventManager(self.odp_config) - def fetch_qualified_segments(self, user_id: str, options: list[str]): + def fetch_qualified_segments(self, user_id: str, options: list[str]) -> None: if not self.enabled: - self.logger.error(Errors.ODP_NOT_ENABLED) # TODO - check if this error is needed, should it be debug? + self.logger.debug(Errors.ODP_NOT_ENABLED) + return None user_key = OdpManagerConfig.KEY_FOR_USER_ID user_value = user_id - self.segment_manager.fetch_qualified_segments(user_key, user_value, options) + if self.segment_manager: + self.segment_manager.fetch_qualified_segments(user_key, user_value, options) - def identify_user(self, user_id: str): + def identify_user(self, user_id: str) -> None: if not self.enabled: self.logger.debug('ODP identify event is not dispatched (ODP disabled).') - if not self.odp_config.odp_state().INTEGRATED: - self.logger.debug('ODP identify event is not dispatched (ODP not integrated).') - - # TODO - consider putting send_event into a separate function into OdpEventManager to have all - # event logic in there. Jae did it. But it's also fine if leave it as is. Think about it, check w Andy? if self.event_manager: - self.event_manager.send_event(OdpManagerConfig.EVENT_TYPE, 'identified', - {OdpManagerConfig.KEY_FOR_USER_ID: user_id}, {}) + self.event_manager.identify_user(user_id) def send_event(self, type: str, action: str, identifiers: dict[str, str], data: dict[str, Any]) -> None: """ @@ -90,7 +85,8 @@ def send_event(self, type: str, action: str, identifiers: dict[str, str], data: type: The event type. action: The event action name. identifiers: A dictionary for identifiers. - data: A dictionary for associated data. The default event data will be added to this data before sending to the ODP server. + data: A dictionary for associated data. The default event data will be added to this data + before sending to the ODP server. Raises custom exception if error is detected. """ @@ -100,10 +96,11 @@ def send_event(self, type: str, action: str, identifiers: dict[str, str], data: if not are_odp_data_types_valid(data): raise optimizely_exception.OdpInvalidData(Errors.ODP_INVALID_DATA) - self.event_manager.send_event(type, action, identifiers, data) - - def update_odp_config(self, api_key: str, api_host: str, segments_to_check: list[str]) -> None: + if self.event_manager: + self.event_manager.send_event(type, action, identifiers, data) + def update_odp_config(self, api_key: Optional[str], api_host: Optional[str], + segments_to_check: list[str]) -> None: if not self.enabled: return None @@ -125,5 +122,3 @@ def update_odp_config(self, api_key: str, api_host: str, segments_to_check: list # (when we get the first datafile ready) if self.event_manager: self.event_manager.flush() - - # TODO - need return None at the end? diff --git a/tests/test_odp_manager.py b/tests/test_odp_manager.py index f93e39eb..5fe8cf84 100644 --- a/tests/test_odp_manager.py +++ b/tests/test_odp_manager.py @@ -14,76 +14,345 @@ from __future__ import annotations from unittest import mock -from unittest.mock import call -from requests import exceptions as request_exception +import pytest +from optimizely import exceptions as optimizely_exception +from optimizely.helpers.enums import Errors from optimizely.odp.lru_cache import LRUCache from optimizely.odp.odp_config import OdpConfig -from optimizely.odp.optimizely_odp_option import OptimizelyOdpOption +from optimizely.odp.odp_event_manager import OdpEventManager +from optimizely.odp.odp_manager import OdpManager from optimizely.odp.odp_segment_manager import OdpSegmentManager from optimizely.odp.zaius_graphql_api_manager import ZaiusGraphQLApiManager +from optimizely.odp.zaius_rest_api_manager import ZaiusRestApiManager from tests import base class OdpManagerTest(base.BaseTest): - # api_host = 'host' - # api_key = 'valid' + cache_size = 10 + cache_timeout = 20 - def test_empty_list_with_no_segments_to_check(self): - odp_config = OdpConfig(self.api_key, self.api_host, []) + def test_configurations_disable_odp(self): mock_logger = mock.MagicMock() - segments_cache = LRUCache(1000, 1000) - api = ZaiusGraphQLApiManager() - segment_manager = OdpSegmentManager(odp_config, segments_cache, api, mock_logger) - with mock.patch.object(api, 'fetch_segments') as mock_fetch_segments: - segments = segment_manager.fetch_qualified_segments(self.user_key, self.user_value, []) + manager = OdpManager(disable=True, + cache_size=10, + cache_timeout_in_sec=20, + logger=mock_logger) + manager.fetch_qualified_segments('user1', []) + mock_logger.debug.assert_called_once_with(Errors.ODP_NOT_ENABLED) - self.assertEqual(segments, []) - mock_logger.debug.assert_called_once_with('No segments are used in the project. Returning empty list.') - mock_logger.error.assert_not_called() - mock_fetch_segments.assert_not_called() + manager.update_odp_config(api_key='valid', api_host='host', segments_to_check=[]) + self.assertIsNone(manager.odp_config.get_api_key()) + self.assertIsNone(manager.odp_config.get_api_host()) - def test_configurations_cache(self): - pass + # these call should be dropped gracefully with None + manager.identify_user('user1') + with pytest.raises(optimizely_exception.OdpNotEnabled): + manager.send_event(type='t1', action='a1', identifiers={}, data={}) - def test_configurations_disable_odp(self): - pass + self.assertIsNone(manager.event_manager) + self.assertIsNone(manager.segment_manager) def test_fetch_qualified_segments(self): - pass + mock_logger = mock.MagicMock() + segment_manager = OdpSegmentManager(OdpConfig(), LRUCache(1000, 1000), + ZaiusGraphQLApiManager(), mock_logger) + + manager = OdpManager(disable=False, + cache_size=10, + cache_timeout_in_sec=20, + segment_manager=segment_manager, + logger=mock_logger) + + with mock.patch.object(segment_manager, 'fetch_qualified_segments') as mock_fetch_qualif_segments: + manager.fetch_qualified_segments('user1', []) + + mock_logger.debug.assert_not_called() + mock_fetch_qualif_segments.assert_called_once_with('fs_user_id', 'user1', []) def test_identify_user_datafile_not_ready(self): - pass + mock_logger = mock.MagicMock() + event_manager = OdpEventManager(OdpConfig(), mock_logger) + event_manager.start() + + manager = OdpManager(disable=False, + cache_size=10, + cache_timeout_in_sec=20, + event_manager=event_manager, + logger=mock_logger) + + with mock.patch.object(event_manager, 'identify_user') as mock_identify_user: + manager.identify_user('user1') + + mock_identify_user.assert_called_once_with('user1') def test_identify_user_odp_integrated(self): - pass + mock_logger = mock.MagicMock() + event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) + event_manager.start() + + manager = OdpManager(disable=False, + cache_size=10, + cache_timeout_in_sec=20, + event_manager=event_manager, + logger=mock_logger) + + manager.update_odp_config(api_key='key1', api_host='host1', segments_to_check=[]) + + with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: + manager.identify_user('user1') + + mock_dispatch_event.assert_called_once() + self.assertEqual(mock_dispatch_event.call_args[0][0].type, 'fullstack') + self.assertEqual(mock_dispatch_event.call_args[0][0].action, 'identified') + self.assertEqual(mock_dispatch_event.call_args[0][0].identifiers, {'fs_user_id': 'user1'}) + self.assertGreater(len(mock_dispatch_event.call_args[0][0].data['idempotence_id']), 0) + self.assertEqual(mock_dispatch_event.call_args[0][0].data['data_source_type'], 'sdk') + self.assertEqual(mock_dispatch_event.call_args[0][0].data['data_source'], 'python-sdk') + self.assertEqual(mock_dispatch_event.call_args[0][0].data['data_source_version'], '4.1.0') def test_identify_user_odp_not_integrated(self): - pass + mock_logger = mock.MagicMock() + event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) + event_manager.start() + + manager = OdpManager(disable=False, + cache_size=10, + cache_timeout_in_sec=20, + event_manager=event_manager, + logger=mock_logger) + + manager.update_odp_config(api_key=None, api_host=None, segments_to_check=[]) + + with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: + manager.identify_user('user1') + + mock_dispatch_event.assert_not_called() + mock_logger.debug.assert_called_with(Errors.ODP_NOT_INTEGRATED) def test_identify_user_odp_disabled(self): - pass + mock_logger = mock.MagicMock() + event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) + event_manager.start() + + manager = OdpManager(disable=False, + cache_size=10, + cache_timeout_in_sec=20, + event_manager=event_manager, + logger=mock_logger) + + manager.enabled = False + + with mock.patch.object(event_manager, 'identify_user') as mock_identify_user: + manager.identify_user('user1') + + mock_identify_user.assert_called_once_with('user1') + mock_logger.debug.assert_called_with('ODP identify event is not dispatched (ODP disabled).') def test_send_event_datafile_not_ready(self): - pass + mock_logger = mock.MagicMock() + event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) + event_manager.start() + + manager = OdpManager(disable=False, + cache_size=10, + cache_timeout_in_sec=20, + event_manager=event_manager, + logger=mock_logger) + + with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: + manager.send_event('t1', 'a1', {'id-key1': 'id-val-1'}, {'key1': 'val1'}) + + mock_dispatch_event.assert_not_called() + mock_logger.debug.assert_called_with('ODP event queue: cannot send before the datafile has loaded.') def test_send_event_odp_integrated(self): - pass + mock_logger = mock.MagicMock() + event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) + event_manager.start() + + manager = OdpManager(disable=False, + cache_size=10, + cache_timeout_in_sec=20, + event_manager=event_manager, + logger=mock_logger) + + manager.update_odp_config(api_key='key1', api_host='host1', segments_to_check=[]) + + with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: + manager.send_event('t1', 'a1', {'id-key1': 'id-val-1'}, {'key1': 'val1'}) + + mock_dispatch_event.assert_called_once() + # asserting each arg individually because one of them is dynamic (idempotence_id/uuid) + # otherwise could use assert_called_once_with_args() + self.assertEqual(mock_dispatch_event.call_args[0][0].type, 't1') + self.assertEqual(mock_dispatch_event.call_args[0][0].action, 'a1') + self.assertEqual(mock_dispatch_event.call_args[0][0].identifiers, {'id-key1': 'id-val-1'}) + self.assertGreater(len(mock_dispatch_event.call_args[0][0].data['idempotence_id']), 0) + self.assertEqual(mock_dispatch_event.call_args[0][0].data['data_source_type'], 'sdk') + self.assertEqual(mock_dispatch_event.call_args[0][0].data['data_source'], 'python-sdk') + self.assertEqual(mock_dispatch_event.call_args[0][0].data['data_source_version'], '4.1.0') + + def test_send_event_odp_not_integrated(self): + mock_logger = mock.MagicMock() + event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) + event_manager.start() + + manager = OdpManager(disable=False, + cache_size=10, + cache_timeout_in_sec=20, + event_manager=event_manager, + logger=mock_logger) + + manager.update_odp_config(api_key=None, api_host=None, segments_to_check=[]) - def test_send_event_odp_not_Integrated(self): - pass + with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: + manager.send_event('t1', 'a1', {'id-key1': 'id-val-1'}, {'key1': 'val1'}) + + mock_dispatch_event.assert_not_called() + mock_logger.debug.assert_called_with(Errors.ODP_NOT_INTEGRATED) def test_send_event_odp_disabled(self): - pass + mock_logger = mock.MagicMock() + event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) + event_manager.start() + + manager = OdpManager(disable=False, + cache_size=10, + cache_timeout_in_sec=20, + event_manager=event_manager, + logger=mock_logger) + + manager.enabled = False + + with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: + with pytest.raises(optimizely_exception.OdpNotEnabled): + manager.send_event('t1', 'a1', {'id-key1': 'id-val-1'}, {'key1': 'val1'}) + + mock_dispatch_event.assert_not_called() + mock_logger.debug.assert_not_called() def test_update_odp_config__reset_called(self): - pass + # build segment manager + mock_logger = mock.MagicMock() + segment_manager = OdpSegmentManager(OdpConfig(), LRUCache(1000, 1000), + ZaiusGraphQLApiManager(), mock_logger) + # build event manager + event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) + event_manager.start() + + manager = OdpManager(disable=False, + cache_size=10, + cache_timeout_in_sec=20, + event_manager=event_manager, + segment_manager=segment_manager, + logger=mock_logger) + + with mock.patch.object(segment_manager, 'reset') as mock_reset: + manager.update_odp_config('key1', 'host1', []) + mock_reset.assert_called_once() + mock_reset.reset_mock() + + manager.update_odp_config('key1', 'host1', []) + mock_reset.assert_not_called() + + manager.update_odp_config('key2', 'host1', []) + mock_reset.assert_called_once() + mock_reset.reset_mock() + + manager.update_odp_config('key2', 'host2', []) + mock_reset.assert_called_once() + mock_reset.reset_mock() + + manager.update_odp_config('key2', 'host2', ['a']) + mock_reset.assert_called_once() + mock_reset.reset_mock() + + manager.update_odp_config('key2', 'host2', ['a', 'b']) + mock_reset.assert_called_once() + mock_reset.reset_mock() + + manager.update_odp_config('key2', 'host2', ['c']) + mock_reset.assert_called_once() + mock_reset.reset_mock() + + manager.update_odp_config('key2', 'host2', ['c']) + mock_reset.assert_not_called() + + manager.update_odp_config(None, None, []) + mock_reset.assert_called_once() def test_update_odp_config__flush_called(self): - pass + mock_logger = mock.MagicMock() + event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) + event_manager.start() + + manager = OdpManager(disable=False, + cache_size=10, + cache_timeout_in_sec=20, + event_manager=event_manager, + logger=mock_logger) + + with mock.patch.object(event_manager, 'flush') as mock_flush: + first_api_key = manager.odp_config.get_api_key() + manager.update_odp_config(api_key='key1', api_host='host1', segments_to_check=[]) + second_api_key = manager.odp_config.get_api_key() + + self.assertEqual(mock_flush.call_count, 2) + self.assertEqual(first_api_key, None) + self.assertEqual(second_api_key, 'key1') + + with mock.patch.object(event_manager, 'flush') as mock_flush: + first_api_key = manager.odp_config.get_api_key() + manager.update_odp_config(api_key='key2', api_host='host1', segments_to_check=[]) + second_api_key = manager.odp_config.get_api_key() + + self.assertEqual(mock_flush.call_count, 2) + self.assertEqual(first_api_key, 'key1') + self.assertEqual(second_api_key, 'key2') + + with mock.patch.object(event_manager, 'flush') as mock_flush: + first_api_key = manager.odp_config.get_api_key() + manager.update_odp_config(api_key='key2', api_host='host1', segments_to_check=[]) + second_api_key = manager.odp_config.get_api_key() + + self.assertEqual(mock_flush.call_count, 1) + self.assertEqual(first_api_key, 'key2') + self.assertEqual(second_api_key, 'key2') + + with mock.patch.object(event_manager, 'flush') as mock_flush: + first_api_key = manager.odp_config.get_api_key() + manager.update_odp_config(api_key=None, api_host=None, segments_to_check=[]) + second_api_key = manager.odp_config.get_api_key() + + self.assertEqual(mock_flush.call_count, 2) + self.assertEqual(first_api_key, 'key2') + self.assertEqual(second_api_key, None) def test_update_odp_config__odp_config_propagated_properly(self): - pass + mock_logger = mock.MagicMock() + event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) + event_manager.start() + + manager = OdpManager(disable=False, + cache_size=10, + cache_timeout_in_sec=20, + event_manager=event_manager, + logger=mock_logger) + + manager.update_odp_config(api_key='key1', api_host='host1', segments_to_check=[]) + + self.assertEqual(manager.segment_manager.odp_config.get_api_key(), 'key1') + self.assertEqual(manager.segment_manager.odp_config.get_api_host(), 'host1') + self.assertEqual(manager.event_manager.odp_config.get_api_key(), 'key1') + self.assertEqual(manager.event_manager.odp_config.get_api_host(), 'host1') + + # odp disabled with invalid apiKey (apiKey/apiHost propagated into submanagers) + manager.update_odp_config(None, None, []) + self.assertEqual(manager.segment_manager.odp_config.get_api_key(), None) + self.assertEqual(manager.segment_manager.odp_config.get_api_host(), None) + self.assertEqual(manager.event_manager.odp_config.get_api_key(), None) + self.assertEqual(manager.event_manager.odp_config.get_api_host(), None) From e1a2de21669ef13c73ba0f38ad93d821ee545ec1 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Tue, 13 Sep 2022 12:57:02 -0700 Subject: [PATCH 04/17] use unittest style of asserting exception --- tests/test_odp_manager.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/test_odp_manager.py b/tests/test_odp_manager.py index 5fe8cf84..a5e3897c 100644 --- a/tests/test_odp_manager.py +++ b/tests/test_odp_manager.py @@ -15,8 +15,6 @@ from unittest import mock -import pytest - from optimizely import exceptions as optimizely_exception from optimizely.helpers.enums import Errors from optimizely.odp.lru_cache import LRUCache @@ -29,6 +27,9 @@ from tests import base +# import pytest + + class OdpManagerTest(base.BaseTest): cache_size = 10 cache_timeout = 20 @@ -49,8 +50,9 @@ def test_configurations_disable_odp(self): # these call should be dropped gracefully with None manager.identify_user('user1') - with pytest.raises(optimizely_exception.OdpNotEnabled): - manager.send_event(type='t1', action='a1', identifiers={}, data={}) + + self.assertRaisesRegex(optimizely_exception.OdpNotEnabled, Errors.ODP_NOT_ENABLED, + manager.send_event, type='t1', action='a1', identifiers={}, data={}) self.assertIsNone(manager.event_manager) self.assertIsNone(manager.segment_manager) @@ -228,8 +230,8 @@ def test_send_event_odp_disabled(self): manager.enabled = False with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: - with pytest.raises(optimizely_exception.OdpNotEnabled): - manager.send_event('t1', 'a1', {'id-key1': 'id-val-1'}, {'key1': 'val1'}) + self.assertRaisesRegex(optimizely_exception.OdpNotEnabled, Errors.ODP_NOT_ENABLED, + manager.send_event, 't1', 'a1', {'id-key1': 'id-val-1'}, {'key1': 'val1'}) mock_dispatch_event.assert_not_called() mock_logger.debug.assert_not_called() From 2bb6c321e7cc8c82132f30bfefd31e50ee46f8d3 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Tue, 13 Sep 2022 13:06:14 -0700 Subject: [PATCH 05/17] remove import --- tests/test_odp_manager.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/test_odp_manager.py b/tests/test_odp_manager.py index a5e3897c..776ffe65 100644 --- a/tests/test_odp_manager.py +++ b/tests/test_odp_manager.py @@ -27,9 +27,6 @@ from tests import base -# import pytest - - class OdpManagerTest(base.BaseTest): cache_size = 10 cache_timeout = 20 From 32167fef4eb88fca69043eb54fe35d3fc866577c Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Tue, 13 Sep 2022 23:27:00 -0700 Subject: [PATCH 06/17] make tests more concise --- tests/test_odp_manager.py | 106 +++++++++----------------------------- 1 file changed, 24 insertions(+), 82 deletions(-) diff --git a/tests/test_odp_manager.py b/tests/test_odp_manager.py index 776ffe65..61524190 100644 --- a/tests/test_odp_manager.py +++ b/tests/test_odp_manager.py @@ -34,14 +34,11 @@ class OdpManagerTest(base.BaseTest): def test_configurations_disable_odp(self): mock_logger = mock.MagicMock() - manager = OdpManager(disable=True, - cache_size=10, - cache_timeout_in_sec=20, - logger=mock_logger) + manager = OdpManager(True, 10, 20, None, None, mock_logger) manager.fetch_qualified_segments('user1', []) mock_logger.debug.assert_called_once_with(Errors.ODP_NOT_ENABLED) - manager.update_odp_config(api_key='valid', api_host='host', segments_to_check=[]) + manager.update_odp_config('valid', 'host', []) self.assertIsNone(manager.odp_config.get_api_key()) self.assertIsNone(manager.odp_config.get_api_host()) @@ -49,7 +46,7 @@ def test_configurations_disable_odp(self): manager.identify_user('user1') self.assertRaisesRegex(optimizely_exception.OdpNotEnabled, Errors.ODP_NOT_ENABLED, - manager.send_event, type='t1', action='a1', identifiers={}, data={}) + manager.send_event, 't1', 'a1', {}, {}) self.assertIsNone(manager.event_manager) self.assertIsNone(manager.segment_manager) @@ -59,11 +56,7 @@ def test_fetch_qualified_segments(self): segment_manager = OdpSegmentManager(OdpConfig(), LRUCache(1000, 1000), ZaiusGraphQLApiManager(), mock_logger) - manager = OdpManager(disable=False, - cache_size=10, - cache_timeout_in_sec=20, - segment_manager=segment_manager, - logger=mock_logger) + manager = OdpManager(False, 10, 20, segment_manager, None, mock_logger) with mock.patch.object(segment_manager, 'fetch_qualified_segments') as mock_fetch_qualif_segments: manager.fetch_qualified_segments('user1', []) @@ -76,11 +69,7 @@ def test_identify_user_datafile_not_ready(self): event_manager = OdpEventManager(OdpConfig(), mock_logger) event_manager.start() - manager = OdpManager(disable=False, - cache_size=10, - cache_timeout_in_sec=20, - event_manager=event_manager, - logger=mock_logger) + manager = OdpManager(False, 10, 20, None, event_manager, mock_logger) with mock.patch.object(event_manager, 'identify_user') as mock_identify_user: manager.identify_user('user1') @@ -92,13 +81,8 @@ def test_identify_user_odp_integrated(self): event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) event_manager.start() - manager = OdpManager(disable=False, - cache_size=10, - cache_timeout_in_sec=20, - event_manager=event_manager, - logger=mock_logger) - - manager.update_odp_config(api_key='key1', api_host='host1', segments_to_check=[]) + manager = OdpManager(False, 10, 20, None, event_manager, mock_logger) + manager.update_odp_config('key1', 'host1', []) with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: manager.identify_user('user1') @@ -117,13 +101,8 @@ def test_identify_user_odp_not_integrated(self): event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) event_manager.start() - manager = OdpManager(disable=False, - cache_size=10, - cache_timeout_in_sec=20, - event_manager=event_manager, - logger=mock_logger) - - manager.update_odp_config(api_key=None, api_host=None, segments_to_check=[]) + manager = OdpManager(False, 10, 20, None, event_manager, mock_logger) + manager.update_odp_config(None, None, []) with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: manager.identify_user('user1') @@ -136,12 +115,7 @@ def test_identify_user_odp_disabled(self): event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) event_manager.start() - manager = OdpManager(disable=False, - cache_size=10, - cache_timeout_in_sec=20, - event_manager=event_manager, - logger=mock_logger) - + manager = OdpManager(False, 10, 20, None, event_manager, mock_logger) manager.enabled = False with mock.patch.object(event_manager, 'identify_user') as mock_identify_user: @@ -155,11 +129,7 @@ def test_send_event_datafile_not_ready(self): event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) event_manager.start() - manager = OdpManager(disable=False, - cache_size=10, - cache_timeout_in_sec=20, - event_manager=event_manager, - logger=mock_logger) + manager = OdpManager(False, 10, 20, None, event_manager, mock_logger) with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: manager.send_event('t1', 'a1', {'id-key1': 'id-val-1'}, {'key1': 'val1'}) @@ -172,13 +142,8 @@ def test_send_event_odp_integrated(self): event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) event_manager.start() - manager = OdpManager(disable=False, - cache_size=10, - cache_timeout_in_sec=20, - event_manager=event_manager, - logger=mock_logger) - - manager.update_odp_config(api_key='key1', api_host='host1', segments_to_check=[]) + manager = OdpManager(False, 10, 20, None, event_manager, mock_logger) + manager.update_odp_config('key1', 'host1', []) with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: manager.send_event('t1', 'a1', {'id-key1': 'id-val-1'}, {'key1': 'val1'}) @@ -199,13 +164,9 @@ def test_send_event_odp_not_integrated(self): event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) event_manager.start() - manager = OdpManager(disable=False, - cache_size=10, - cache_timeout_in_sec=20, - event_manager=event_manager, - logger=mock_logger) + manager = OdpManager(False, 10, 20, None, event_manager, mock_logger) - manager.update_odp_config(api_key=None, api_host=None, segments_to_check=[]) + manager.update_odp_config(None, None, []) with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: manager.send_event('t1', 'a1', {'id-key1': 'id-val-1'}, {'key1': 'val1'}) @@ -218,12 +179,7 @@ def test_send_event_odp_disabled(self): event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) event_manager.start() - manager = OdpManager(disable=False, - cache_size=10, - cache_timeout_in_sec=20, - event_manager=event_manager, - logger=mock_logger) - + manager = OdpManager(False, 10, 20, None, event_manager, mock_logger) manager.enabled = False with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: @@ -242,12 +198,7 @@ def test_update_odp_config__reset_called(self): event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) event_manager.start() - manager = OdpManager(disable=False, - cache_size=10, - cache_timeout_in_sec=20, - event_manager=event_manager, - segment_manager=segment_manager, - logger=mock_logger) + manager = OdpManager(False, 10, 20, segment_manager, event_manager, mock_logger) with mock.patch.object(segment_manager, 'reset') as mock_reset: manager.update_odp_config('key1', 'host1', []) @@ -288,15 +239,11 @@ def test_update_odp_config__flush_called(self): event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) event_manager.start() - manager = OdpManager(disable=False, - cache_size=10, - cache_timeout_in_sec=20, - event_manager=event_manager, - logger=mock_logger) + manager = OdpManager(False, 10, 20, None, event_manager, mock_logger) with mock.patch.object(event_manager, 'flush') as mock_flush: first_api_key = manager.odp_config.get_api_key() - manager.update_odp_config(api_key='key1', api_host='host1', segments_to_check=[]) + manager.update_odp_config('key1', 'host1', []) second_api_key = manager.odp_config.get_api_key() self.assertEqual(mock_flush.call_count, 2) @@ -305,7 +252,7 @@ def test_update_odp_config__flush_called(self): with mock.patch.object(event_manager, 'flush') as mock_flush: first_api_key = manager.odp_config.get_api_key() - manager.update_odp_config(api_key='key2', api_host='host1', segments_to_check=[]) + manager.update_odp_config('key2', 'host1', []) second_api_key = manager.odp_config.get_api_key() self.assertEqual(mock_flush.call_count, 2) @@ -314,7 +261,7 @@ def test_update_odp_config__flush_called(self): with mock.patch.object(event_manager, 'flush') as mock_flush: first_api_key = manager.odp_config.get_api_key() - manager.update_odp_config(api_key='key2', api_host='host1', segments_to_check=[]) + manager.update_odp_config('key2', 'host1', []) second_api_key = manager.odp_config.get_api_key() self.assertEqual(mock_flush.call_count, 1) @@ -323,7 +270,7 @@ def test_update_odp_config__flush_called(self): with mock.patch.object(event_manager, 'flush') as mock_flush: first_api_key = manager.odp_config.get_api_key() - manager.update_odp_config(api_key=None, api_host=None, segments_to_check=[]) + manager.update_odp_config(None, None, []) second_api_key = manager.odp_config.get_api_key() self.assertEqual(mock_flush.call_count, 2) @@ -335,13 +282,8 @@ def test_update_odp_config__odp_config_propagated_properly(self): event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) event_manager.start() - manager = OdpManager(disable=False, - cache_size=10, - cache_timeout_in_sec=20, - event_manager=event_manager, - logger=mock_logger) - - manager.update_odp_config(api_key='key1', api_host='host1', segments_to_check=[]) + manager = OdpManager(False, 10, 20, None, event_manager, mock_logger) + manager.update_odp_config('key1', 'host1', []) self.assertEqual(manager.segment_manager.odp_config.get_api_key(), 'key1') self.assertEqual(manager.segment_manager.odp_config.get_api_host(), 'host1') From 19492f9479021b91a91a7db5f89ad45c6c74856b Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Wed, 14 Sep 2022 23:41:49 -0700 Subject: [PATCH 07/17] fixed some pr comments --- optimizely/helpers/enums.py | 16 ++++---- optimizely/odp/odp_manager.py | 49 ++++++++++++----------- tests/test_odp_manager.py | 73 ++++++++++++++++++++++++----------- 3 files changed, 85 insertions(+), 53 deletions(-) diff --git a/optimizely/helpers/enums.py b/optimizely/helpers/enums.py index 6a02593b..3385e2d8 100644 --- a/optimizely/helpers/enums.py +++ b/optimizely/helpers/enums.py @@ -120,12 +120,12 @@ class Errors: NONE_VARIABLE_KEY_PARAMETER: Final = '"None" is an invalid value for variable key.' UNSUPPORTED_DATAFILE_VERSION: Final = ( 'This version of the Python SDK does not support the given datafile version: "{}".') - INVALID_SEGMENT_IDENTIFIER = 'Audience segments fetch failed (invalid identifier).' - FETCH_SEGMENTS_FAILED = 'Audience segments fetch failed ({}).' - ODP_EVENT_FAILED = 'ODP event send failed ({}).' - ODP_NOT_INTEGRATED = 'ODP is not integrated.' - ODP_NOT_ENABLED = 'ODP is not enabled.' - ODP_INVALID_DATA = 'ODP data is not valid.' + INVALID_SEGMENT_IDENTIFIER: Final = 'Audience segments fetch failed (invalid identifier).' + FETCH_SEGMENTS_FAILED: Final = 'Audience segments fetch failed ({}).' + ODP_EVENT_FAILED: Final = 'ODP event send failed ({}).' + ODP_NOT_INTEGRATED: Final = 'ODP is not integrated.' + ODP_NOT_ENABLED: Final = 'ODP is not enabled.' + ODP_INVALID_DATA: Final = 'ODP data is not valid.' class ForcedDecisionLogs: @@ -219,5 +219,5 @@ class OdpEventManagerConfig: class OdpManagerConfig: """ODP Manager configs.""" - KEY_FOR_USER_ID = 'fs_user_id' - EVENT_TYPE = 'fullstack' + KEY_FOR_USER_ID: Final = 'fs_user_id' + EVENT_TYPE: Final = 'fullstack' diff --git a/optimizely/odp/odp_manager.py b/optimizely/odp/odp_manager.py index 910191b4..fbc9bfeb 100644 --- a/optimizely/odp/odp_manager.py +++ b/optimizely/odp/odp_manager.py @@ -29,12 +29,13 @@ class OdpManager: """Orchestrates segment manager, event manager and odp config.""" - def __init__(self, disable: bool, - cache_size: int, - cache_timeout_in_sec: int, - segment_manager: Optional[OdpSegmentManager] = None, - event_manager: Optional[OdpEventManager] = None, - logger: Optional[optimizely_logger.Logger] = None) -> None: + def __init__(self, + disable: bool, + cache_size: int, + cache_timeout_in_sec: int, + segment_manager: Optional[OdpSegmentManager] = None, + event_manager: Optional[OdpEventManager] = None, + logger: Optional[optimizely_logger.Logger] = None) -> None: self.enabled = not disable self.cache_size = cache_size @@ -45,24 +46,25 @@ def __init__(self, disable: bool, self.segment_manager = None self.event_manager = None - if self.enabled: - if segment_manager: - segment_manager.odp_config = self.odp_config - self.segment_manager = segment_manager - else: - self.segment_manager = OdpSegmentManager(self.odp_config, - LRUCache(cache_size, cache_timeout_in_sec), - ZaiusGraphQLApiManager(), logger) - if event_manager: - event_manager.odp_config = self.odp_config - self.event_manager = event_manager - else: - self.event_manager = OdpEventManager(self.odp_config) + if not self.enabled: + return + + if segment_manager: + segment_manager.odp_config = self.odp_config + self.segment_manager = segment_manager + else: + self.segment_manager = OdpSegmentManager(self.odp_config, + LRUCache(cache_size, cache_timeout_in_sec), + ZaiusGraphQLApiManager(logger), logger) + if event_manager: + event_manager.odp_config = self.odp_config + self.event_manager = event_manager + else: + self.event_manager = OdpEventManager(self.odp_config, logger) def fetch_qualified_segments(self, user_id: str, options: list[str]) -> None: if not self.enabled: - self.logger.debug(Errors.ODP_NOT_ENABLED) - return None + raise optimizely_exception.OdpNotEnabled(Errors.ODP_NOT_ENABLED) user_key = OdpManagerConfig.KEY_FOR_USER_ID user_value = user_id @@ -102,7 +104,7 @@ def send_event(self, type: str, action: str, identifiers: dict[str, str], data: def update_odp_config(self, api_key: Optional[str], api_host: Optional[str], segments_to_check: list[str]) -> None: if not self.enabled: - return None + return # flush old events using old odp publicKey (if exists) before updating odp key. if self.event_manager: @@ -112,7 +114,8 @@ def update_odp_config(self, api_key: Optional[str], api_host: Optional[str], config_changed = self.odp_config.update(api_key, api_host, segments_to_check) if not config_changed: - return None + self.logger.debug('Odp config was not changed.') + return # reset segments cache when odp integration or segmentsToCheck are changed if self.segment_manager: diff --git a/tests/test_odp_manager.py b/tests/test_odp_manager.py index 61524190..581cd4b2 100644 --- a/tests/test_odp_manager.py +++ b/tests/test_odp_manager.py @@ -16,6 +16,7 @@ from unittest import mock from optimizely import exceptions as optimizely_exception +from optimizely import version from optimizely.helpers.enums import Errors from optimizely.odp.lru_cache import LRUCache from optimizely.odp.odp_config import OdpConfig @@ -35,8 +36,9 @@ def test_configurations_disable_odp(self): mock_logger = mock.MagicMock() manager = OdpManager(True, 10, 20, None, None, mock_logger) - manager.fetch_qualified_segments('user1', []) - mock_logger.debug.assert_called_once_with(Errors.ODP_NOT_ENABLED) + + self.assertRaisesRegex(optimizely_exception.OdpNotEnabled, Errors.ODP_NOT_ENABLED, + manager.fetch_qualified_segments, 'user1', []) manager.update_odp_config('valid', 'host', []) self.assertIsNone(manager.odp_config.get_api_key()) @@ -54,7 +56,7 @@ def test_configurations_disable_odp(self): def test_fetch_qualified_segments(self): mock_logger = mock.MagicMock() segment_manager = OdpSegmentManager(OdpConfig(), LRUCache(1000, 1000), - ZaiusGraphQLApiManager(), mock_logger) + ZaiusGraphQLApiManager(mock_logger), mock_logger) manager = OdpManager(False, 10, 20, segment_manager, None, mock_logger) @@ -62,6 +64,7 @@ def test_fetch_qualified_segments(self): manager.fetch_qualified_segments('user1', []) mock_logger.debug.assert_not_called() + mock_logger.error.assert_not_called() mock_fetch_qualif_segments.assert_called_once_with('fs_user_id', 'user1', []) def test_identify_user_datafile_not_ready(self): @@ -87,14 +90,16 @@ def test_identify_user_odp_integrated(self): with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: manager.identify_user('user1') - mock_dispatch_event.assert_called_once() - self.assertEqual(mock_dispatch_event.call_args[0][0].type, 'fullstack') - self.assertEqual(mock_dispatch_event.call_args[0][0].action, 'identified') - self.assertEqual(mock_dispatch_event.call_args[0][0].identifiers, {'fs_user_id': 'user1'}) - self.assertGreater(len(mock_dispatch_event.call_args[0][0].data['idempotence_id']), 0) - self.assertEqual(mock_dispatch_event.call_args[0][0].data['data_source_type'], 'sdk') - self.assertEqual(mock_dispatch_event.call_args[0][0].data['data_source'], 'python-sdk') - self.assertEqual(mock_dispatch_event.call_args[0][0].data['data_source_version'], '4.1.0') + mock_dispatch_event.assert_called_once_with({ + 'type': 'fullstack', + 'action': 'identified', + 'identifiers': {'fs_user_id': 'user1'}, + 'data': { + 'idempotence_id': mock.ANY, + 'data_source_type': 'sdk', + 'data_source': 'python-sdk', + 'data_source_version': version.__version__ + }}) def test_identify_user_odp_not_integrated(self): mock_logger = mock.MagicMock() @@ -148,16 +153,17 @@ def test_send_event_odp_integrated(self): with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: manager.send_event('t1', 'a1', {'id-key1': 'id-val-1'}, {'key1': 'val1'}) - mock_dispatch_event.assert_called_once() - # asserting each arg individually because one of them is dynamic (idempotence_id/uuid) - # otherwise could use assert_called_once_with_args() - self.assertEqual(mock_dispatch_event.call_args[0][0].type, 't1') - self.assertEqual(mock_dispatch_event.call_args[0][0].action, 'a1') - self.assertEqual(mock_dispatch_event.call_args[0][0].identifiers, {'id-key1': 'id-val-1'}) - self.assertGreater(len(mock_dispatch_event.call_args[0][0].data['idempotence_id']), 0) - self.assertEqual(mock_dispatch_event.call_args[0][0].data['data_source_type'], 'sdk') - self.assertEqual(mock_dispatch_event.call_args[0][0].data['data_source'], 'python-sdk') - self.assertEqual(mock_dispatch_event.call_args[0][0].data['data_source_version'], '4.1.0') + mock_dispatch_event.assert_called_once_with({ + 'type': 't1', + 'action': 'a1', + 'identifiers': {'id-key1': 'id-val-1'}, + 'data': { + 'idempotence_id': mock.ANY, + 'data_source_type': 'sdk', + 'data_source': 'python-sdk', + 'data_source_version': version.__version__, + 'key1': 'val1' + }}) def test_send_event_odp_not_integrated(self): mock_logger = mock.MagicMock() @@ -189,11 +195,34 @@ def test_send_event_odp_disabled(self): mock_dispatch_event.assert_not_called() mock_logger.debug.assert_not_called() + def test_send_event_invalid_data(self): + mock_logger = mock.MagicMock() + event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) + event_manager.start() + + manager = OdpManager(False, 10, 20, None, event_manager, mock_logger) + manager.update_odp_config('key1', 'host1', []) + + with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: + self.assertRaisesRegex(optimizely_exception.OdpInvalidData, Errors.ODP_INVALID_DATA, + manager.send_event, 't1', 'a1', {'id-key1': 'id-val-1'}, {'invalid-item': {}}) + + mock_dispatch_event.assert_not_called() + + def test_config_not_changed(self): + mock_logger = mock.MagicMock() + event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) + event_manager.start() + + manager = OdpManager(False, 10, 20, None, event_manager, mock_logger) + manager.update_odp_config(None, None, []) + mock_logger.debug.assert_called_with('Odp config was not changed.') + def test_update_odp_config__reset_called(self): # build segment manager mock_logger = mock.MagicMock() segment_manager = OdpSegmentManager(OdpConfig(), LRUCache(1000, 1000), - ZaiusGraphQLApiManager(), mock_logger) + ZaiusGraphQLApiManager(mock_logger), mock_logger) # build event manager event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) event_manager.start() From baff0c4d3a7d03bb395b20f3cd78eb185f4cd01d Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Wed, 14 Sep 2022 23:43:34 -0700 Subject: [PATCH 08/17] de-indent event manager init --- optimizely/odp/odp_event_manager.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/optimizely/odp/odp_event_manager.py b/optimizely/odp/odp_event_manager.py index cb51beb1..3dad2f83 100644 --- a/optimizely/odp/odp_event_manager.py +++ b/optimizely/odp/odp_event_manager.py @@ -42,10 +42,10 @@ class OdpEventManager: """ def __init__( - self, - odp_config: OdpConfig, - logger: Optional[_logging.Logger] = None, - api_manager: Optional[ZaiusRestApiManager] = None + self, + odp_config: OdpConfig, + logger: Optional[_logging.Logger] = None, + api_manager: Optional[ZaiusRestApiManager] = None ): """OdpEventManager init method to configure event batching. From 0789c7c132d961902ddb27113a352d57fac11618 Mon Sep 17 00:00:00 2001 From: Andy Leap Date: Thu, 15 Sep 2022 18:52:47 -0400 Subject: [PATCH 09/17] add update config event manager signal --- optimizely/odp/odp_event_manager.py | 33 +++++++++++++++++++++++------ tests/test_odp_event_manager.py | 17 +++++++++++---- 2 files changed, 40 insertions(+), 10 deletions(-) diff --git a/optimizely/odp/odp_event_manager.py b/optimizely/odp/odp_event_manager.py index cb51beb1..d20580cc 100644 --- a/optimizely/odp/odp_event_manager.py +++ b/optimizely/odp/odp_event_manager.py @@ -30,6 +30,7 @@ class Signal(Enum): """Enum for sending signals to the event queue.""" SHUTDOWN = 1 FLUSH = 2 + UPDATE_CONFIG = 3 class OdpEventManager: @@ -56,7 +57,11 @@ def __init__( """ self.logger = logger or _logging.NoOpLogger() self.zaius_manager = api_manager or ZaiusRestApiManager(self.logger) + self.odp_config = odp_config + self.api_key = odp_config.get_api_key() + self.api_host = odp_config.get_api_host() + self.event_queue: Queue[OdpEvent | Signal] = Queue(OdpEventManagerConfig.DEFAULT_QUEUE_CAPACITY) self.batch_size = OdpEventManagerConfig.DEFAULT_BATCH_SIZE self.flush_interval = OdpEventManagerConfig.DEFAULT_FLUSH_INTERVAL @@ -102,7 +107,11 @@ def _run(self) -> None: self.logger.debug('ODP event queue: received flush signal.') self._flush_batch() self.event_queue.task_done() - continue + + elif item == Signal.UPDATE_CONFIG: + self.logger.debug('ODP event queue: received update config signal.') + self._update_config() + self.event_queue.task_done() elif isinstance(item, OdpEvent): self._add_to_batch(item) @@ -137,10 +146,7 @@ def _flush_batch(self) -> None: self.logger.debug('ODP event queue: nothing to flush.') return - api_key = self.odp_config.get_api_key() - api_host = self.odp_config.get_api_host() - - if not api_key or not api_host: + if not self.api_key or not self.api_host: self.logger.debug(Errors.ODP_NOT_INTEGRATED) self._current_batch.clear() return @@ -150,7 +156,7 @@ def _flush_batch(self) -> None: for i in range(1 + self.retry_count): try: - should_retry = self.zaius_manager.send_odp_events(api_key, api_host, self._current_batch) + should_retry = self.zaius_manager.send_odp_events(self.api_key, self.api_host, self._current_batch) except Exception as error: should_retry = False self.logger.error(Errors.ODP_EVENT_FAILED.format(f'Error: {error} {self._current_batch}')) @@ -241,3 +247,18 @@ def dispatch(self, event: OdpEvent) -> None: def identify_user(self, user_id: str) -> None: self.send_event(OdpManagerConfig.EVENT_TYPE, 'identified', {OdpManagerConfig.KEY_FOR_USER_ID: user_id}, {}) + + def update_config(self) -> None: + """Adds update config signal to event_queue.""" + try: + self.event_queue.put_nowait(Signal.UPDATE_CONFIG) + except Full: + self.logger.error("Error updating ODP config for the event queue") + + def _update_config(self) -> None: + """Updates the configuration used to send events.""" + if len(self._current_batch) > 0: + self._flush_batch() + + self.api_host = self.odp_config.get_api_host() + self.api_key = self.odp_config.get_api_key() diff --git a/tests/test_odp_event_manager.py b/tests/test_odp_event_manager.py index ffbab40d..ea90ada5 100644 --- a/tests/test_odp_event_manager.py +++ b/tests/test_odp_event_manager.py @@ -411,6 +411,7 @@ def test_odp_event_manager_events_before_odp_ready(self, *args): event_manager.send_event(**self.events[1]) odp_config.update(self.api_key, self.api_host, []) + event_manager.update_config() event_manager.event_queue.join() event_manager.send_event(**self.events[0]) @@ -423,6 +424,7 @@ def test_odp_event_manager_events_before_odp_ready(self, *args): mock_logger.debug.assert_has_calls([ mock.call('ODP event queue: cannot send before the datafile has loaded.'), mock.call('ODP event queue: cannot send before the datafile has loaded.'), + mock.call('ODP event queue: received update config signal.'), mock.call('ODP event queue: adding event.'), mock.call('ODP event queue: adding event.'), mock.call('ODP event queue: received flush signal.'), @@ -442,6 +444,7 @@ def test_odp_event_manager_events_before_odp_disabled(self, *args): event_manager.send_event(**self.events[1]) odp_config.update(None, None, []) + event_manager.update_config() event_manager.event_queue.join() event_manager.send_event(**self.events[0]) @@ -453,6 +456,7 @@ def test_odp_event_manager_events_before_odp_disabled(self, *args): mock_logger.debug.assert_has_calls([ mock.call('ODP event queue: cannot send before the datafile has loaded.'), mock.call('ODP event queue: cannot send before the datafile has loaded.'), + mock.call('ODP event queue: received update config signal.'), mock.call(Errors.ODP_NOT_INTEGRATED), mock.call(Errors.ODP_NOT_INTEGRATED) ]) @@ -496,20 +500,25 @@ def test_odp_event_manager_disabled_after_events_in_queue(self, *args): odp_config = OdpConfig(self.api_key, self.api_host) event_manager = OdpEventManager(odp_config, mock_logger) - event_manager.batch_size = 2 + event_manager.batch_size = 3 with mock.patch('optimizely.odp.odp_event_manager.OdpEventManager.is_running', True): event_manager.send_event(**self.events[0]) event_manager.send_event(**self.events[1]) - - with mock.patch.object(event_manager.zaius_manager, 'send_odp_events') as mock_send: odp_config.update(None, None, []) + event_manager.update_config() + + with mock.patch.object( + event_manager.zaius_manager, 'send_odp_events', new_callable=CopyingMock, return_value=False + ) as mock_send: event_manager.start() + event_manager.send_event(**self.events[0]) event_manager.send_event(**self.events[1]) + event_manager.send_event(**self.events[0]) event_manager.event_queue.join() self.assertEqual(len(event_manager._current_batch), 0) mock_logger.debug.assert_any_call(Errors.ODP_NOT_INTEGRATED) mock_logger.error.assert_not_called() - mock_send.assert_not_called() + mock_send.assert_called_once_with(self.api_key, self.api_host, self.processed_events) event_manager.stop() From 4b0b33843c5a70807899b25a274ecd1ef42d1612 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Fri, 16 Sep 2022 15:37:34 -0700 Subject: [PATCH 10/17] updated tests --- tests/test_odp_manager.py | 156 ++++++++++++++--------- tests/test_odp_zaius_rest_api_manager.py | 1 + 2 files changed, 100 insertions(+), 57 deletions(-) diff --git a/tests/test_odp_manager.py b/tests/test_odp_manager.py index 581cd4b2..87db41f3 100644 --- a/tests/test_odp_manager.py +++ b/tests/test_odp_manager.py @@ -18,7 +18,7 @@ from optimizely import exceptions as optimizely_exception from optimizely import version from optimizely.helpers.enums import Errors -from optimizely.odp.lru_cache import LRUCache +from optimizely.odp.lru_cache import OptimizelySegmentsCache, LRUCache from optimizely.odp.odp_config import OdpConfig from optimizely.odp.odp_event_manager import OdpEventManager from optimizely.odp.odp_manager import OdpManager @@ -29,13 +29,11 @@ class OdpManagerTest(base.BaseTest): - cache_size = 10 - cache_timeout = 20 def test_configurations_disable_odp(self): mock_logger = mock.MagicMock() - manager = OdpManager(True, 10, 20, None, None, mock_logger) + manager = OdpManager(True, OptimizelySegmentsCache, None, None, mock_logger) self.assertRaisesRegex(optimizely_exception.OdpNotEnabled, Errors.ODP_NOT_ENABLED, manager.fetch_qualified_segments, 'user1', []) @@ -55,10 +53,10 @@ def test_configurations_disable_odp(self): def test_fetch_qualified_segments(self): mock_logger = mock.MagicMock() - segment_manager = OdpSegmentManager(OdpConfig(), LRUCache(1000, 1000), + segment_manager = OdpSegmentManager(OdpConfig(), OptimizelySegmentsCache, ZaiusGraphQLApiManager(mock_logger), mock_logger) - manager = OdpManager(False, 10, 20, segment_manager, None, mock_logger) + manager = OdpManager(False, OptimizelySegmentsCache, segment_manager, None, mock_logger) with mock.patch.object(segment_manager, 'fetch_qualified_segments') as mock_fetch_qualif_segments: manager.fetch_qualified_segments('user1', []) @@ -67,12 +65,48 @@ def test_fetch_qualified_segments(self): mock_logger.error.assert_not_called() mock_fetch_qualif_segments.assert_called_once_with('fs_user_id', 'user1', []) + with mock.patch.object(segment_manager, 'fetch_qualified_segments') as mock_fetch_qualif_segments: + manager.fetch_qualified_segments('user1', ['IGNORE_CACHE']) + + mock_logger.debug.assert_not_called() + mock_logger.error.assert_not_called() + mock_fetch_qualif_segments.assert_called_once_with('fs_user_id', 'user1', ['IGNORE_CACHE']) + + def test_fetch_qualified_segments__not_enabled(self): + mock_logger = mock.MagicMock() + segment_manager = OdpSegmentManager(OdpConfig(), OptimizelySegmentsCache, + ZaiusGraphQLApiManager(mock_logger), mock_logger) + + manager = OdpManager(False, OptimizelySegmentsCache, segment_manager, None, mock_logger) + manager.enabled = False + + with mock.patch.object(segment_manager, 'fetch_qualified_segments') as mock_fetch_qualif_segments: + self.assertRaisesRegex(optimizely_exception.OdpNotEnabled, Errors.ODP_NOT_ENABLED, + manager.fetch_qualified_segments, 'user1', []) + + # verify segment manager did not try to fetch segments + mock_fetch_qualif_segments.assert_not_called() + + def test_fetch_qualified_segments__segment_mgr_not_available(self): + mock_logger = mock.MagicMock() + segment_manager = OdpSegmentManager(OdpConfig(), OptimizelySegmentsCache, + ZaiusGraphQLApiManager(mock_logger), mock_logger) + + manager = OdpManager(False, OptimizelySegmentsCache, segment_manager, None, mock_logger) + manager.segment_manager = False + + with mock.patch.object(segment_manager, 'fetch_qualified_segments') as mock_fetch_qualif_segments: + self.assertRaisesRegex(optimizely_exception.OdpNotEnabled, Errors.ODP_NOT_ENABLED, + manager.fetch_qualified_segments, 'user1', []) + + # verify segment manager did not try to fetch segments + mock_fetch_qualif_segments.assert_not_called() + def test_identify_user_datafile_not_ready(self): mock_logger = mock.MagicMock() event_manager = OdpEventManager(OdpConfig(), mock_logger) - event_manager.start() - manager = OdpManager(False, 10, 20, None, event_manager, mock_logger) + manager = OdpManager(False, OptimizelySegmentsCache, None, event_manager, mock_logger) with mock.patch.object(event_manager, 'identify_user') as mock_identify_user: manager.identify_user('user1') @@ -82,9 +116,8 @@ def test_identify_user_datafile_not_ready(self): def test_identify_user_odp_integrated(self): mock_logger = mock.MagicMock() event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) - event_manager.start() - manager = OdpManager(False, 10, 20, None, event_manager, mock_logger) + manager = OdpManager(False, LRUCache(10, 20), None, event_manager, mock_logger) manager.update_odp_config('key1', 'host1', []) with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: @@ -104,37 +137,35 @@ def test_identify_user_odp_integrated(self): def test_identify_user_odp_not_integrated(self): mock_logger = mock.MagicMock() event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) - event_manager.start() - manager = OdpManager(False, 10, 20, None, event_manager, mock_logger) + manager = OdpManager(False, OptimizelySegmentsCache, None, event_manager, mock_logger) manager.update_odp_config(None, None, []) with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: manager.identify_user('user1') mock_dispatch_event.assert_not_called() - mock_logger.debug.assert_called_with(Errors.ODP_NOT_INTEGRATED) + mock_logger.debug.assert_any_call('Odp config was not changed.') + mock_logger.debug.assert_any_call('ODP Identify event is not dispatched (ODP not integrated).') def test_identify_user_odp_disabled(self): mock_logger = mock.MagicMock() event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) - event_manager.start() - manager = OdpManager(False, 10, 20, None, event_manager, mock_logger) + manager = OdpManager(False, OptimizelySegmentsCache, None, event_manager, mock_logger) manager.enabled = False with mock.patch.object(event_manager, 'identify_user') as mock_identify_user: manager.identify_user('user1') - mock_identify_user.assert_called_once_with('user1') + mock_identify_user.assert_not_called() mock_logger.debug.assert_called_with('ODP identify event is not dispatched (ODP disabled).') def test_send_event_datafile_not_ready(self): mock_logger = mock.MagicMock() event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) - event_manager.start() - manager = OdpManager(False, 10, 20, None, event_manager, mock_logger) + manager = OdpManager(False, OptimizelySegmentsCache, None, event_manager, mock_logger) with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: manager.send_event('t1', 'a1', {'id-key1': 'id-val-1'}, {'key1': 'val1'}) @@ -145,9 +176,8 @@ def test_send_event_datafile_not_ready(self): def test_send_event_odp_integrated(self): mock_logger = mock.MagicMock() event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) - event_manager.start() - manager = OdpManager(False, 10, 20, None, event_manager, mock_logger) + manager = OdpManager(False, LRUCache(10, 20), None, event_manager, mock_logger) manager.update_odp_config('key1', 'host1', []) with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: @@ -168,24 +198,22 @@ def test_send_event_odp_integrated(self): def test_send_event_odp_not_integrated(self): mock_logger = mock.MagicMock() event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) - event_manager.start() - - manager = OdpManager(False, 10, 20, None, event_manager, mock_logger) + manager = OdpManager(False, OptimizelySegmentsCache, None, event_manager, mock_logger) manager.update_odp_config(None, None, []) with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: - manager.send_event('t1', 'a1', {'id-key1': 'id-val-1'}, {'key1': 'val1'}) + self.assertRaisesRegex(optimizely_exception.OdpNotIntegrated, Errors.ODP_NOT_INTEGRATED, + manager.send_event, 't1', 'a1', {'id-key1': 'id-val-1'}, {'key1': 'val1'}) mock_dispatch_event.assert_not_called() - mock_logger.debug.assert_called_with(Errors.ODP_NOT_INTEGRATED) + mock_logger.debug.assert_any_call('Odp config was not changed.') def test_send_event_odp_disabled(self): mock_logger = mock.MagicMock() event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) - event_manager.start() - manager = OdpManager(False, 10, 20, None, event_manager, mock_logger) + manager = OdpManager(False, OptimizelySegmentsCache, None, event_manager, mock_logger) manager.enabled = False with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: @@ -195,12 +223,25 @@ def test_send_event_odp_disabled(self): mock_dispatch_event.assert_not_called() mock_logger.debug.assert_not_called() + def test_send_event_odp_disabled__event_manager_not_available(self): + mock_logger = mock.MagicMock() + event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) + + manager = OdpManager(False, OptimizelySegmentsCache, None, event_manager, mock_logger) + manager.event_manager = False + + with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: + self.assertRaisesRegex(optimizely_exception.OdpNotEnabled, Errors.ODP_NOT_ENABLED, + manager.send_event, 't1', 'a1', {'id-key1': 'id-val-1'}, {'key1': 'val1'}) + + mock_dispatch_event.assert_not_called() + mock_logger.debug.assert_not_called() + def test_send_event_invalid_data(self): mock_logger = mock.MagicMock() event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) - event_manager.start() - manager = OdpManager(False, 10, 20, None, event_manager, mock_logger) + manager = OdpManager(False, LRUCache(10, 20), None, event_manager, mock_logger) manager.update_odp_config('key1', 'host1', []) with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: @@ -212,22 +253,20 @@ def test_send_event_invalid_data(self): def test_config_not_changed(self): mock_logger = mock.MagicMock() event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) - event_manager.start() - manager = OdpManager(False, 10, 20, None, event_manager, mock_logger) + manager = OdpManager(False, OptimizelySegmentsCache, None, event_manager, mock_logger) manager.update_odp_config(None, None, []) mock_logger.debug.assert_called_with('Odp config was not changed.') def test_update_odp_config__reset_called(self): # build segment manager mock_logger = mock.MagicMock() - segment_manager = OdpSegmentManager(OdpConfig(), LRUCache(1000, 1000), + segment_manager = OdpSegmentManager(OdpConfig(), OptimizelySegmentsCache, ZaiusGraphQLApiManager(mock_logger), mock_logger) # build event manager event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) - event_manager.start() - manager = OdpManager(False, 10, 20, segment_manager, event_manager, mock_logger) + manager = OdpManager(False, OptimizelySegmentsCache, segment_manager, event_manager, mock_logger) with mock.patch.object(segment_manager, 'reset') as mock_reset: manager.update_odp_config('key1', 'host1', []) @@ -263,66 +302,69 @@ def test_update_odp_config__reset_called(self): manager.update_odp_config(None, None, []) mock_reset.assert_called_once() - def test_update_odp_config__flush_called(self): + def test_update_odp_config__update_config_called(self): + """ + Test if event_manager.update_config is called when change + to odp_config is made or not in OdpManager. + """ mock_logger = mock.MagicMock() event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) - event_manager.start() + manager = OdpManager(False, LRUCache(10, 20), None, event_manager, mock_logger) - manager = OdpManager(False, 10, 20, None, event_manager, mock_logger) - - with mock.patch.object(event_manager, 'flush') as mock_flush: + with mock.patch.object(event_manager, 'update_config') as mock_update: first_api_key = manager.odp_config.get_api_key() manager.update_odp_config('key1', 'host1', []) second_api_key = manager.odp_config.get_api_key() - self.assertEqual(mock_flush.call_count, 2) + mock_update.assert_called_once() + mock_logger.debug.assert_not_called() self.assertEqual(first_api_key, None) self.assertEqual(second_api_key, 'key1') - with mock.patch.object(event_manager, 'flush') as mock_flush: + with mock.patch.object(event_manager, 'update_config') as mock_update: first_api_key = manager.odp_config.get_api_key() manager.update_odp_config('key2', 'host1', []) second_api_key = manager.odp_config.get_api_key() - self.assertEqual(mock_flush.call_count, 2) + mock_update.assert_called_once() + mock_logger.debug.assert_not_called() self.assertEqual(first_api_key, 'key1') self.assertEqual(second_api_key, 'key2') - with mock.patch.object(event_manager, 'flush') as mock_flush: + with mock.patch.object(event_manager, 'update_config') as mock_update: first_api_key = manager.odp_config.get_api_key() manager.update_odp_config('key2', 'host1', []) second_api_key = manager.odp_config.get_api_key() - self.assertEqual(mock_flush.call_count, 1) + # event_manager.update_config not called when no change to odp_config + mock_update.assert_not_called() + mock_logger.debug('Odp config was not changed.') self.assertEqual(first_api_key, 'key2') self.assertEqual(second_api_key, 'key2') - with mock.patch.object(event_manager, 'flush') as mock_flush: - first_api_key = manager.odp_config.get_api_key() - manager.update_odp_config(None, None, []) - second_api_key = manager.odp_config.get_api_key() - - self.assertEqual(mock_flush.call_count, 2) - self.assertEqual(first_api_key, 'key2') - self.assertEqual(second_api_key, None) - def test_update_odp_config__odp_config_propagated_properly(self): mock_logger = mock.MagicMock() event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) - event_manager.start() - - manager = OdpManager(False, 10, 20, None, event_manager, mock_logger) - manager.update_odp_config('key1', 'host1', []) + manager = OdpManager(False, LRUCache(10, 20), None, event_manager, mock_logger) + manager.update_odp_config('key1', 'host1', ['a', 'b']) self.assertEqual(manager.segment_manager.odp_config.get_api_key(), 'key1') self.assertEqual(manager.segment_manager.odp_config.get_api_host(), 'host1') + self.assertEqual(manager.segment_manager.odp_config.get_segments_to_check(), ['a', 'b']) self.assertEqual(manager.event_manager.odp_config.get_api_key(), 'key1') self.assertEqual(manager.event_manager.odp_config.get_api_host(), 'host1') + self.assertEqual(manager.event_manager.odp_config.get_segments_to_check(), ['a', 'b']) # odp disabled with invalid apiKey (apiKey/apiHost propagated into submanagers) manager.update_odp_config(None, None, []) self.assertEqual(manager.segment_manager.odp_config.get_api_key(), None) self.assertEqual(manager.segment_manager.odp_config.get_api_host(), None) + self.assertEqual(manager.segment_manager.odp_config.get_segments_to_check(), []) self.assertEqual(manager.event_manager.odp_config.get_api_key(), None) self.assertEqual(manager.event_manager.odp_config.get_api_host(), None) + self.assertEqual(manager.event_manager.odp_config.get_segments_to_check(), []) + + manager.update_odp_config(None, None, ['a', 'b']) + self.assertEqual(manager.segment_manager.odp_config.get_segments_to_check(), ['a', 'b']) + self.assertEqual(manager.event_manager.odp_config.get_segments_to_check(), ['a', 'b']) diff --git a/tests/test_odp_zaius_rest_api_manager.py b/tests/test_odp_zaius_rest_api_manager.py index e7327d6f..b6400604 100644 --- a/tests/test_odp_zaius_rest_api_manager.py +++ b/tests/test_odp_zaius_rest_api_manager.py @@ -27,6 +27,7 @@ class ZaiusRestApiManagerTest(base.BaseTest): api_key = "test-api-key" api_host = "test-host" + # TODO - UPDATE THESE EVENTS TO BE OdpEvent class not dict (see Andy's comment and change) events = [ {"type": "t1", "action": "a1", "identifiers": {"id-key-1": "id-value-1"}, "data": {"key-1": "value1"}}, {"type": "t2", "action": "a2", "identifiers": {"id-key-2": "id-value-2"}, "data": {"key-2": "value2"}}, From ec96869d72c4f8980cedff37b3edb4ae6004be51 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Fri, 16 Sep 2022 15:52:06 -0700 Subject: [PATCH 11/17] commit odp_manager updates and cleanup --- optimizely/odp/odp_manager.py | 56 ++++++++++++------------ tests/test_odp_manager.py | 2 +- tests/test_odp_zaius_rest_api_manager.py | 1 - 3 files changed, 28 insertions(+), 31 deletions(-) diff --git a/optimizely/odp/odp_manager.py b/optimizely/odp/odp_manager.py index fbc9bfeb..690ec7bf 100644 --- a/optimizely/odp/odp_manager.py +++ b/optimizely/odp/odp_manager.py @@ -19,8 +19,8 @@ from optimizely import logger as optimizely_logger from optimizely.helpers.enums import Errors, OdpManagerConfig from optimizely.helpers.validator import are_odp_data_types_valid -from optimizely.odp.lru_cache import LRUCache -from optimizely.odp.odp_config import OdpConfig +from optimizely.odp.lru_cache import OptimizelySegmentsCache +from optimizely.odp.odp_config import OdpConfig, OdpConfigState from optimizely.odp.odp_event_manager import OdpEventManager from optimizely.odp.odp_segment_manager import OdpSegmentManager from optimizely.odp.zaius_graphql_api_manager import ZaiusGraphQLApiManager @@ -29,17 +29,16 @@ class OdpManager: """Orchestrates segment manager, event manager and odp config.""" - def __init__(self, + def __init__( + self, disable: bool, - cache_size: int, - cache_timeout_in_sec: int, + segments_cache: OptimizelySegmentsCache, segment_manager: Optional[OdpSegmentManager] = None, event_manager: Optional[OdpEventManager] = None, - logger: Optional[optimizely_logger.Logger] = None) -> None: + logger: Optional[optimizely_logger.Logger] = None + ) -> None: self.enabled = not disable - self.cache_size = cache_size - self.cache_timeout_in_sec = cache_timeout_in_sec self.odp_config = OdpConfig() self.logger = logger or optimizely_logger.NoOpLogger() @@ -47,6 +46,7 @@ def __init__(self, self.event_manager = None if not self.enabled: + self.logger.info('ODP is disabled.') return if segment_manager: @@ -54,7 +54,7 @@ def __init__(self, self.segment_manager = segment_manager else: self.segment_manager = OdpSegmentManager(self.odp_config, - LRUCache(cache_size, cache_timeout_in_sec), + segments_cache, ZaiusGraphQLApiManager(logger), logger) if event_manager: event_manager.odp_config = self.odp_config @@ -62,22 +62,26 @@ def __init__(self, else: self.event_manager = OdpEventManager(self.odp_config, logger) - def fetch_qualified_segments(self, user_id: str, options: list[str]) -> None: - if not self.enabled: + self.event_manager.start() + + def fetch_qualified_segments(self, user_id: str, options: list[str]) -> Optional[list[str]]: + if not self.enabled or not self.segment_manager: raise optimizely_exception.OdpNotEnabled(Errors.ODP_NOT_ENABLED) user_key = OdpManagerConfig.KEY_FOR_USER_ID user_value = user_id - if self.segment_manager: - self.segment_manager.fetch_qualified_segments(user_key, user_value, options) + return self.segment_manager.fetch_qualified_segments(user_key, user_value, options) def identify_user(self, user_id: str) -> None: - if not self.enabled: + if not self.enabled or not self.event_manager: self.logger.debug('ODP identify event is not dispatched (ODP disabled).') + return + if self.odp_config.odp_state() == OdpConfigState.NOT_INTEGRATED: + self.logger.debug('ODP Identify event is not dispatched (ODP not integrated).') + return - if self.event_manager: - self.event_manager.identify_user(user_id) + self.event_manager.identify_user(user_id) def send_event(self, type: str, action: str, identifiers: dict[str, str], data: dict[str, Any]) -> None: """ @@ -92,36 +96,30 @@ def send_event(self, type: str, action: str, identifiers: dict[str, str], data: Raises custom exception if error is detected. """ - if not self.enabled: + if not self.enabled or not self.event_manager: raise optimizely_exception.OdpNotEnabled(Errors.ODP_NOT_ENABLED) + if self.odp_config.odp_state() == OdpConfigState.NOT_INTEGRATED: + raise optimizely_exception.OdpNotIntegrated(Errors.ODP_NOT_INTEGRATED) + if not are_odp_data_types_valid(data): raise optimizely_exception.OdpInvalidData(Errors.ODP_INVALID_DATA) - if self.event_manager: - self.event_manager.send_event(type, action, identifiers, data) + self.event_manager.send_event(type, action, identifiers, data) def update_odp_config(self, api_key: Optional[str], api_host: Optional[str], segments_to_check: list[str]) -> None: if not self.enabled: return - # flush old events using old odp publicKey (if exists) before updating odp key. - if self.event_manager: - self.event_manager.flush() - # wait until done flushing to avoid flushing in the middle of the batch - self.event_manager.event_queue.join() - config_changed = self.odp_config.update(api_key, api_host, segments_to_check) if not config_changed: self.logger.debug('Odp config was not changed.') return - # reset segments cache when odp integration or segmentsToCheck are changed + # reset segments cache when odp integration or segments to check are changed if self.segment_manager: self.segment_manager.reset() - # flush events with the new integration key if events still remain in the queue - # (when we get the first datafile ready) if self.event_manager: - self.event_manager.flush() + self.event_manager.update_config() diff --git a/tests/test_odp_manager.py b/tests/test_odp_manager.py index 87db41f3..4fd28397 100644 --- a/tests/test_odp_manager.py +++ b/tests/test_odp_manager.py @@ -32,12 +32,12 @@ class OdpManagerTest(base.BaseTest): def test_configurations_disable_odp(self): mock_logger = mock.MagicMock() - manager = OdpManager(True, OptimizelySegmentsCache, None, None, mock_logger) self.assertRaisesRegex(optimizely_exception.OdpNotEnabled, Errors.ODP_NOT_ENABLED, manager.fetch_qualified_segments, 'user1', []) + mock_logger.info.assert_called_once_with('ODP is disabled.') manager.update_odp_config('valid', 'host', []) self.assertIsNone(manager.odp_config.get_api_key()) self.assertIsNone(manager.odp_config.get_api_host()) diff --git a/tests/test_odp_zaius_rest_api_manager.py b/tests/test_odp_zaius_rest_api_manager.py index b6400604..e7327d6f 100644 --- a/tests/test_odp_zaius_rest_api_manager.py +++ b/tests/test_odp_zaius_rest_api_manager.py @@ -27,7 +27,6 @@ class ZaiusRestApiManagerTest(base.BaseTest): api_key = "test-api-key" api_host = "test-host" - # TODO - UPDATE THESE EVENTS TO BE OdpEvent class not dict (see Andy's comment and change) events = [ {"type": "t1", "action": "a1", "identifiers": {"id-key-1": "id-value-1"}, "data": {"key-1": "value1"}}, {"type": "t2", "action": "a2", "identifiers": {"id-key-2": "id-value-2"}, "data": {"key-2": "value2"}}, From e53eaa3135f821b1ae8fceee63cebe06534de468 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Tue, 20 Sep 2022 00:10:19 -0700 Subject: [PATCH 12/17] addres PR comments --- optimizely/helpers/enums.py | 6 ++++ optimizely/odp/odp_manager.py | 36 +++++++++++++--------- optimizely/odp/odp_segment_manager.py | 14 ++++++--- tests/test_odp_manager.py | 44 ++++++++++++++++++--------- 4 files changed, 67 insertions(+), 33 deletions(-) diff --git a/optimizely/helpers/enums.py b/optimizely/helpers/enums.py index 3385e2d8..c47a89d0 100644 --- a/optimizely/helpers/enums.py +++ b/optimizely/helpers/enums.py @@ -221,3 +221,9 @@ class OdpManagerConfig: """ODP Manager configs.""" KEY_FOR_USER_ID: Final = 'fs_user_id' EVENT_TYPE: Final = 'fullstack' + + +class OdpSegmentsCacheConfig: + """ODP Segment Cache configs.""" + DEFAULT_CAPACITY: Final = 10_000 + DEFAULT_TIMEOUT_SECS: Final = 10 diff --git a/optimizely/odp/odp_manager.py b/optimizely/odp/odp_manager.py index 690ec7bf..9fd352f7 100644 --- a/optimizely/odp/odp_manager.py +++ b/optimizely/odp/odp_manager.py @@ -17,9 +17,9 @@ from optimizely import exceptions as optimizely_exception from optimizely import logger as optimizely_logger -from optimizely.helpers.enums import Errors, OdpManagerConfig +from optimizely.helpers.enums import Errors, OdpManagerConfig, OdpSegmentsCacheConfig from optimizely.helpers.validator import are_odp_data_types_valid -from optimizely.odp.lru_cache import OptimizelySegmentsCache +from optimizely.odp.lru_cache import OptimizelySegmentsCache, LRUCache from optimizely.odp.odp_config import OdpConfig, OdpConfigState from optimizely.odp.odp_event_manager import OdpEventManager from optimizely.odp.odp_segment_manager import OdpSegmentManager @@ -32,7 +32,7 @@ class OdpManager: def __init__( self, disable: bool, - segments_cache: OptimizelySegmentsCache, + segments_cache: Optional[OptimizelySegmentsCache] = None, segment_manager: Optional[OdpSegmentManager] = None, event_manager: Optional[OdpEventManager] = None, logger: Optional[optimizely_logger.Logger] = None @@ -42,20 +42,27 @@ def __init__( self.odp_config = OdpConfig() self.logger = logger or optimizely_logger.NoOpLogger() - self.segment_manager = None - self.event_manager = None + self.segment_manager = segment_manager + self.event_manager = event_manager if not self.enabled: self.logger.info('ODP is disabled.') return - if segment_manager: - segment_manager.odp_config = self.odp_config - self.segment_manager = segment_manager - else: - self.segment_manager = OdpSegmentManager(self.odp_config, - segments_cache, - ZaiusGraphQLApiManager(logger), logger) + if not self.segment_manager: + if not segments_cache: + segments_cache = LRUCache( + OdpSegmentsCacheConfig.DEFAULT_CAPACITY, + OdpSegmentsCacheConfig.DEFAULT_TIMEOUT_SECS + ) + self.segment_manager = OdpSegmentManager( + self.odp_config, + segments_cache, + ZaiusGraphQLApiManager(logger), logger + ) + + self.segment_manager.odp_config = self.odp_config + if event_manager: event_manager.odp_config = self.odp_config self.event_manager = event_manager @@ -66,7 +73,8 @@ def __init__( def fetch_qualified_segments(self, user_id: str, options: list[str]) -> Optional[list[str]]: if not self.enabled or not self.segment_manager: - raise optimizely_exception.OdpNotEnabled(Errors.ODP_NOT_ENABLED) + self.logger.error(Errors.ODP_NOT_ENABLED) + return None user_key = OdpManagerConfig.KEY_FOR_USER_ID user_value = user_id @@ -78,7 +86,7 @@ def identify_user(self, user_id: str) -> None: self.logger.debug('ODP identify event is not dispatched (ODP disabled).') return if self.odp_config.odp_state() == OdpConfigState.NOT_INTEGRATED: - self.logger.debug('ODP Identify event is not dispatched (ODP not integrated).') + self.logger.debug('ODP identify event is not dispatched (ODP not integrated).') return self.event_manager.identify_user(user_id) diff --git a/optimizely/odp/odp_segment_manager.py b/optimizely/odp/odp_segment_manager.py index 7f7582e4..a5d363fd 100644 --- a/optimizely/odp/odp_segment_manager.py +++ b/optimizely/odp/odp_segment_manager.py @@ -26,17 +26,21 @@ class OdpSegmentManager: """Schedules connections to ODP for audience segmentation and caches the results.""" - def __init__(self, odp_config: OdpConfig, segments_cache: OptimizelySegmentsCache, - zaius_manager: ZaiusGraphQLApiManager, - logger: Optional[optimizely_logger.Logger] = None) -> None: + def __init__( + self, + odp_config: OdpConfig, + segments_cache: OptimizelySegmentsCache, + zaius_manager: ZaiusGraphQLApiManager, + logger: Optional[optimizely_logger.Logger] = None + ) -> None: self.odp_config = odp_config self.segments_cache = segments_cache self.zaius_manager = zaius_manager self.logger = logger or optimizely_logger.NoOpLogger() - def fetch_qualified_segments(self, user_key: str, user_value: str, options: list[str]) -> \ - Optional[list[str]]: + def fetch_qualified_segments(self, user_key: str, user_value: str, options: list[str] + ) -> Optional[list[str]]: """ Args: user_key: The key for identifying the id type. diff --git a/tests/test_odp_manager.py b/tests/test_odp_manager.py index 4fd28397..5d316bf4 100644 --- a/tests/test_odp_manager.py +++ b/tests/test_odp_manager.py @@ -34,14 +34,14 @@ def test_configurations_disable_odp(self): mock_logger = mock.MagicMock() manager = OdpManager(True, OptimizelySegmentsCache, None, None, mock_logger) - self.assertRaisesRegex(optimizely_exception.OdpNotEnabled, Errors.ODP_NOT_ENABLED, - manager.fetch_qualified_segments, 'user1', []) - mock_logger.info.assert_called_once_with('ODP is disabled.') manager.update_odp_config('valid', 'host', []) self.assertIsNone(manager.odp_config.get_api_key()) self.assertIsNone(manager.odp_config.get_api_host()) + manager.fetch_qualified_segments('user1', []) + mock_logger.error.assert_called_once_with(Errors.ODP_NOT_ENABLED) + # these call should be dropped gracefully with None manager.identify_user('user1') @@ -72,21 +72,37 @@ def test_fetch_qualified_segments(self): mock_logger.error.assert_not_called() mock_fetch_qualif_segments.assert_called_once_with('fs_user_id', 'user1', ['IGNORE_CACHE']) - def test_fetch_qualified_segments__not_enabled(self): + def test_fetch_qualified_segments__seg_mgr_and_seg_cache_are_none(self): mock_logger = mock.MagicMock() segment_manager = OdpSegmentManager(OdpConfig(), OptimizelySegmentsCache, ZaiusGraphQLApiManager(mock_logger), mock_logger) manager = OdpManager(False, OptimizelySegmentsCache, segment_manager, None, mock_logger) - manager.enabled = False + manager.OptimizelySegmentsCache = None + manager.segment_manager = None with mock.patch.object(segment_manager, 'fetch_qualified_segments') as mock_fetch_qualif_segments: - self.assertRaisesRegex(optimizely_exception.OdpNotEnabled, Errors.ODP_NOT_ENABLED, - manager.fetch_qualified_segments, 'user1', []) + manager.fetch_qualified_segments('user1', []) - # verify segment manager did not try to fetch segments + mock_logger.debug.assert_not_called() + mock_logger.error.assert_called_once_with('ODP is not enabled.') mock_fetch_qualif_segments.assert_not_called() + def test_fetch_qualified_segments__not_enabled(self): + mock_logger = mock.MagicMock() + segment_manager = OdpSegmentManager(OdpConfig(), OptimizelySegmentsCache, + ZaiusGraphQLApiManager(mock_logger), mock_logger) + + manager = OdpManager(False, OptimizelySegmentsCache, segment_manager, None, mock_logger) + manager.enabled = False + + manager.fetch_qualified_segments('user1', []) + mock_logger.error.assert_called_once_with(Errors.ODP_NOT_ENABLED) + + with mock.patch.object(segment_manager, 'fetch_qualified_segments') as mock_fetch_qualif_segments: + # verify segment manager did not try to fetch segments + mock_fetch_qualif_segments.assert_not_called() + def test_fetch_qualified_segments__segment_mgr_not_available(self): mock_logger = mock.MagicMock() segment_manager = OdpSegmentManager(OdpConfig(), OptimizelySegmentsCache, @@ -95,12 +111,12 @@ def test_fetch_qualified_segments__segment_mgr_not_available(self): manager = OdpManager(False, OptimizelySegmentsCache, segment_manager, None, mock_logger) manager.segment_manager = False - with mock.patch.object(segment_manager, 'fetch_qualified_segments') as mock_fetch_qualif_segments: - self.assertRaisesRegex(optimizely_exception.OdpNotEnabled, Errors.ODP_NOT_ENABLED, - manager.fetch_qualified_segments, 'user1', []) + manager.fetch_qualified_segments('user1', []) + mock_logger.error.assert_called_once_with(Errors.ODP_NOT_ENABLED) - # verify segment manager did not try to fetch segments - mock_fetch_qualif_segments.assert_not_called() + with mock.patch.object(segment_manager, 'fetch_qualified_segments') as mock_fetch_qualif_segments: + # verify segment manager did not try to fetch segments + mock_fetch_qualif_segments.assert_not_called() def test_identify_user_datafile_not_ready(self): mock_logger = mock.MagicMock() @@ -146,7 +162,7 @@ def test_identify_user_odp_not_integrated(self): mock_dispatch_event.assert_not_called() mock_logger.debug.assert_any_call('Odp config was not changed.') - mock_logger.debug.assert_any_call('ODP Identify event is not dispatched (ODP not integrated).') + mock_logger.debug.assert_any_call('ODP identify event is not dispatched (ODP not integrated).') def test_identify_user_odp_disabled(self): mock_logger = mock.MagicMock() From 7c7535b95fdb7ba010cb9081f854aceecc0cf4ca Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Tue, 20 Sep 2022 10:52:15 -0700 Subject: [PATCH 13/17] refactored tests for fetch segments --- optimizely/odp/odp_manager.py | 4 +- tests/test_odp_manager.py | 88 +++++++++++++++++------------------ 2 files changed, 45 insertions(+), 47 deletions(-) diff --git a/optimizely/odp/odp_manager.py b/optimizely/odp/odp_manager.py index 9fd352f7..72c61514 100644 --- a/optimizely/odp/odp_manager.py +++ b/optimizely/odp/odp_manager.py @@ -60,8 +60,8 @@ def __init__( segments_cache, ZaiusGraphQLApiManager(logger), logger ) - - self.segment_manager.odp_config = self.odp_config + else: + self.segment_manager.odp_config = self.odp_config if event_manager: event_manager.odp_config = self.odp_config diff --git a/tests/test_odp_manager.py b/tests/test_odp_manager.py index 5d316bf4..5f0609f4 100644 --- a/tests/test_odp_manager.py +++ b/tests/test_odp_manager.py @@ -32,7 +32,7 @@ class OdpManagerTest(base.BaseTest): def test_configurations_disable_odp(self): mock_logger = mock.MagicMock() - manager = OdpManager(True, OptimizelySegmentsCache, None, None, mock_logger) + manager = OdpManager(True, OptimizelySegmentsCache, logger=mock_logger) mock_logger.info.assert_called_once_with('ODP is disabled.') manager.update_odp_config('valid', 'host', []) @@ -56,7 +56,7 @@ def test_fetch_qualified_segments(self): segment_manager = OdpSegmentManager(OdpConfig(), OptimizelySegmentsCache, ZaiusGraphQLApiManager(mock_logger), mock_logger) - manager = OdpManager(False, OptimizelySegmentsCache, segment_manager, None, mock_logger) + manager = OdpManager(False, OptimizelySegmentsCache, segment_manager, logger=mock_logger) with mock.patch.object(segment_manager, 'fetch_qualified_segments') as mock_fetch_qualif_segments: manager.fetch_qualified_segments('user1', []) @@ -72,57 +72,55 @@ def test_fetch_qualified_segments(self): mock_logger.error.assert_not_called() mock_fetch_qualif_segments.assert_called_once_with('fs_user_id', 'user1', ['IGNORE_CACHE']) - def test_fetch_qualified_segments__seg_mgr_and_seg_cache_are_none(self): + def test_fetch_qualified_segments__disabled(self): mock_logger = mock.MagicMock() segment_manager = OdpSegmentManager(OdpConfig(), OptimizelySegmentsCache, ZaiusGraphQLApiManager(mock_logger), mock_logger) - manager = OdpManager(False, OptimizelySegmentsCache, segment_manager, None, mock_logger) - manager.OptimizelySegmentsCache = None - manager.segment_manager = None + manager = OdpManager(True, OptimizelySegmentsCache, segment_manager, logger=mock_logger) with mock.patch.object(segment_manager, 'fetch_qualified_segments') as mock_fetch_qualif_segments: manager.fetch_qualified_segments('user1', []) + mock_logger.error.assert_called_once_with(Errors.ODP_NOT_ENABLED) + mock_fetch_qualif_segments.assert_not_called() - mock_logger.debug.assert_not_called() - mock_logger.error.assert_called_once_with('ODP is not enabled.') - mock_fetch_qualif_segments.assert_not_called() - - def test_fetch_qualified_segments__not_enabled(self): + def test_fetch_qualified_segments__segment_mgr_is_none(self): + """ + When segment manager is None, then fetching segment + should take place using the default segment manager. + """ mock_logger = mock.MagicMock() - segment_manager = OdpSegmentManager(OdpConfig(), OptimizelySegmentsCache, - ZaiusGraphQLApiManager(mock_logger), mock_logger) + manager = OdpManager(False, LRUCache(10, 20), logger=mock_logger) + manager.update_odp_config('api_key', 'api_host', []) - manager = OdpManager(False, OptimizelySegmentsCache, segment_manager, None, mock_logger) - manager.enabled = False - - manager.fetch_qualified_segments('user1', []) - mock_logger.error.assert_called_once_with(Errors.ODP_NOT_ENABLED) + with mock.patch.object(manager.segment_manager, 'fetch_qualified_segments') as mock_fetch_qualif_segments: + manager.fetch_qualified_segments('user1', []) - with mock.patch.object(segment_manager, 'fetch_qualified_segments') as mock_fetch_qualif_segments: - # verify segment manager did not try to fetch segments - mock_fetch_qualif_segments.assert_not_called() + mock_logger.debug.assert_not_called() + mock_logger.error.assert_not_called() + mock_fetch_qualif_segments.assert_called_once_with('fs_user_id', 'user1', []) - def test_fetch_qualified_segments__segment_mgr_not_available(self): + def test_fetch_qualified_segments__seg_cache_and_seg_mgr_are_none(self): + """ + When segment cache and segment manager are None, then fetching segment + should take place using the default managers. + """ mock_logger = mock.MagicMock() - segment_manager = OdpSegmentManager(OdpConfig(), OptimizelySegmentsCache, - ZaiusGraphQLApiManager(mock_logger), mock_logger) + manager = OdpManager(False, mock_logger) + manager.update_odp_config('api_key', 'api_host', []) - manager = OdpManager(False, OptimizelySegmentsCache, segment_manager, None, mock_logger) - manager.segment_manager = False - - manager.fetch_qualified_segments('user1', []) - mock_logger.error.assert_called_once_with(Errors.ODP_NOT_ENABLED) + with mock.patch.object(manager.segment_manager, 'fetch_qualified_segments') as mock_fetch_qualif_segments: + manager.fetch_qualified_segments('user1', []) - with mock.patch.object(segment_manager, 'fetch_qualified_segments') as mock_fetch_qualif_segments: - # verify segment manager did not try to fetch segments - mock_fetch_qualif_segments.assert_not_called() + mock_logger.debug.assert_not_called() + mock_logger.error.assert_not_called() + mock_fetch_qualif_segments.assert_called_once_with('fs_user_id', 'user1', []) def test_identify_user_datafile_not_ready(self): mock_logger = mock.MagicMock() event_manager = OdpEventManager(OdpConfig(), mock_logger) - manager = OdpManager(False, OptimizelySegmentsCache, None, event_manager, mock_logger) + manager = OdpManager(False, OptimizelySegmentsCache, event_manager=event_manager, logger=mock_logger) with mock.patch.object(event_manager, 'identify_user') as mock_identify_user: manager.identify_user('user1') @@ -133,7 +131,7 @@ def test_identify_user_odp_integrated(self): mock_logger = mock.MagicMock() event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) - manager = OdpManager(False, LRUCache(10, 20), None, event_manager, mock_logger) + manager = OdpManager(False, LRUCache(10, 20), event_manager=event_manager, logger=mock_logger) manager.update_odp_config('key1', 'host1', []) with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: @@ -154,7 +152,7 @@ def test_identify_user_odp_not_integrated(self): mock_logger = mock.MagicMock() event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) - manager = OdpManager(False, OptimizelySegmentsCache, None, event_manager, mock_logger) + manager = OdpManager(False, OptimizelySegmentsCache, event_manager=event_manager, logger=mock_logger) manager.update_odp_config(None, None, []) with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: @@ -168,7 +166,7 @@ def test_identify_user_odp_disabled(self): mock_logger = mock.MagicMock() event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) - manager = OdpManager(False, OptimizelySegmentsCache, None, event_manager, mock_logger) + manager = OdpManager(False, OptimizelySegmentsCache, event_manager=event_manager, logger=mock_logger) manager.enabled = False with mock.patch.object(event_manager, 'identify_user') as mock_identify_user: @@ -181,7 +179,7 @@ def test_send_event_datafile_not_ready(self): mock_logger = mock.MagicMock() event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) - manager = OdpManager(False, OptimizelySegmentsCache, None, event_manager, mock_logger) + manager = OdpManager(False, OptimizelySegmentsCache, event_manager=event_manager, logger=mock_logger) with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: manager.send_event('t1', 'a1', {'id-key1': 'id-val-1'}, {'key1': 'val1'}) @@ -193,7 +191,7 @@ def test_send_event_odp_integrated(self): mock_logger = mock.MagicMock() event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) - manager = OdpManager(False, LRUCache(10, 20), None, event_manager, mock_logger) + manager = OdpManager(False, LRUCache(10, 20), event_manager=event_manager, logger=mock_logger) manager.update_odp_config('key1', 'host1', []) with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: @@ -215,7 +213,7 @@ def test_send_event_odp_not_integrated(self): mock_logger = mock.MagicMock() event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) - manager = OdpManager(False, OptimizelySegmentsCache, None, event_manager, mock_logger) + manager = OdpManager(False, OptimizelySegmentsCache, event_manager=event_manager, logger=mock_logger) manager.update_odp_config(None, None, []) with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: @@ -229,7 +227,7 @@ def test_send_event_odp_disabled(self): mock_logger = mock.MagicMock() event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) - manager = OdpManager(False, OptimizelySegmentsCache, None, event_manager, mock_logger) + manager = OdpManager(False, OptimizelySegmentsCache, event_manager=event_manager, logger=mock_logger) manager.enabled = False with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: @@ -243,7 +241,7 @@ def test_send_event_odp_disabled__event_manager_not_available(self): mock_logger = mock.MagicMock() event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) - manager = OdpManager(False, OptimizelySegmentsCache, None, event_manager, mock_logger) + manager = OdpManager(False, OptimizelySegmentsCache, event_manager=event_manager, logger=mock_logger) manager.event_manager = False with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: @@ -257,7 +255,7 @@ def test_send_event_invalid_data(self): mock_logger = mock.MagicMock() event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) - manager = OdpManager(False, LRUCache(10, 20), None, event_manager, mock_logger) + manager = OdpManager(False, LRUCache(10, 20), event_manager=event_manager, logger=mock_logger) manager.update_odp_config('key1', 'host1', []) with mock.patch.object(event_manager, 'dispatch') as mock_dispatch_event: @@ -270,7 +268,7 @@ def test_config_not_changed(self): mock_logger = mock.MagicMock() event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) - manager = OdpManager(False, OptimizelySegmentsCache, None, event_manager, mock_logger) + manager = OdpManager(False, OptimizelySegmentsCache, event_manager=event_manager, logger=mock_logger) manager.update_odp_config(None, None, []) mock_logger.debug.assert_called_with('Odp config was not changed.') @@ -325,7 +323,7 @@ def test_update_odp_config__update_config_called(self): """ mock_logger = mock.MagicMock() event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) - manager = OdpManager(False, LRUCache(10, 20), None, event_manager, mock_logger) + manager = OdpManager(False, LRUCache(10, 20), event_manager=event_manager, logger=mock_logger) with mock.patch.object(event_manager, 'update_config') as mock_update: first_api_key = manager.odp_config.get_api_key() @@ -361,7 +359,7 @@ def test_update_odp_config__update_config_called(self): def test_update_odp_config__odp_config_propagated_properly(self): mock_logger = mock.MagicMock() event_manager = OdpEventManager(OdpConfig(), mock_logger, ZaiusRestApiManager()) - manager = OdpManager(False, LRUCache(10, 20), None, event_manager, mock_logger) + manager = OdpManager(False, LRUCache(10, 20), event_manager=event_manager, logger=mock_logger) manager.update_odp_config('key1', 'host1', ['a', 'b']) self.assertEqual(manager.segment_manager.odp_config.get_api_key(), 'key1') From 0a2be6d00a9160588a98c442ad6be5993fec8c88 Mon Sep 17 00:00:00 2001 From: Matjaz Pirnovar Date: Tue, 20 Sep 2022 11:27:14 -0700 Subject: [PATCH 14/17] update cache default timeout --- optimizely/helpers/enums.py | 2 +- tests/test_odp_manager.py | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/optimizely/helpers/enums.py b/optimizely/helpers/enums.py index c47a89d0..886d269a 100644 --- a/optimizely/helpers/enums.py +++ b/optimizely/helpers/enums.py @@ -226,4 +226,4 @@ class OdpManagerConfig: class OdpSegmentsCacheConfig: """ODP Segment Cache configs.""" DEFAULT_CAPACITY: Final = 10_000 - DEFAULT_TIMEOUT_SECS: Final = 10 + DEFAULT_TIMEOUT_SECS: Final = 600 diff --git a/tests/test_odp_manager.py b/tests/test_odp_manager.py index 5f0609f4..47c64de2 100644 --- a/tests/test_odp_manager.py +++ b/tests/test_odp_manager.py @@ -17,7 +17,7 @@ from optimizely import exceptions as optimizely_exception from optimizely import version -from optimizely.helpers.enums import Errors +from optimizely.helpers.enums import Errors, OdpSegmentsCacheConfig from optimizely.odp.lru_cache import OptimizelySegmentsCache, LRUCache from optimizely.odp.odp_config import OdpConfig from optimizely.odp.odp_event_manager import OdpEventManager @@ -382,3 +382,8 @@ def test_update_odp_config__odp_config_propagated_properly(self): manager.update_odp_config(None, None, ['a', 'b']) self.assertEqual(manager.segment_manager.odp_config.get_segments_to_check(), ['a', 'b']) self.assertEqual(manager.event_manager.odp_config.get_segments_to_check(), ['a', 'b']) + + def test_segments_cache_default_settings(self): + manager = OdpManager(False) + self.assertEqual(manager.segment_manager.segments_cache.capacity, OdpSegmentsCacheConfig.DEFAULT_CAPACITY) + self.assertEqual(manager.segment_manager.segments_cache.timeout, OdpSegmentsCacheConfig.DEFAULT_TIMEOUT_SECS) From de1850854721c5ce7c9a336b8edb4ecacb11c9bf Mon Sep 17 00:00:00 2001 From: Andy Leap Date: Wed, 21 Sep 2022 10:09:11 -0400 Subject: [PATCH 15/17] fix pypy failed test --- tests/test_odp_manager.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_odp_manager.py b/tests/test_odp_manager.py index 47c64de2..7b63cfdb 100644 --- a/tests/test_odp_manager.py +++ b/tests/test_odp_manager.py @@ -96,7 +96,6 @@ def test_fetch_qualified_segments__segment_mgr_is_none(self): with mock.patch.object(manager.segment_manager, 'fetch_qualified_segments') as mock_fetch_qualif_segments: manager.fetch_qualified_segments('user1', []) - mock_logger.debug.assert_not_called() mock_logger.error.assert_not_called() mock_fetch_qualif_segments.assert_called_once_with('fs_user_id', 'user1', []) From 6b7385f419da53fd44bbea3eb1dbebc4b77f3951 Mon Sep 17 00:00:00 2001 From: Andy Leap Date: Wed, 21 Sep 2022 10:10:49 -0400 Subject: [PATCH 16/17] hardcode cache settings --- tests/test_odp_manager.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/test_odp_manager.py b/tests/test_odp_manager.py index 7b63cfdb..6c7cf263 100644 --- a/tests/test_odp_manager.py +++ b/tests/test_odp_manager.py @@ -17,7 +17,7 @@ from optimizely import exceptions as optimizely_exception from optimizely import version -from optimizely.helpers.enums import Errors, OdpSegmentsCacheConfig +from optimizely.helpers.enums import Errors from optimizely.odp.lru_cache import OptimizelySegmentsCache, LRUCache from optimizely.odp.odp_config import OdpConfig from optimizely.odp.odp_event_manager import OdpEventManager @@ -384,5 +384,6 @@ def test_update_odp_config__odp_config_propagated_properly(self): def test_segments_cache_default_settings(self): manager = OdpManager(False) - self.assertEqual(manager.segment_manager.segments_cache.capacity, OdpSegmentsCacheConfig.DEFAULT_CAPACITY) - self.assertEqual(manager.segment_manager.segments_cache.timeout, OdpSegmentsCacheConfig.DEFAULT_TIMEOUT_SECS) + segments_cache = manager.segment_manager.segments_cache + self.assertEqual(segments_cache.capacity, 10_000) + self.assertEqual(segments_cache.timeout, 600) From 18fbf642e5af9967f2f0dcfbd560804d4989cbfc Mon Sep 17 00:00:00 2001 From: Andy Leap Date: Wed, 21 Sep 2022 10:11:16 -0400 Subject: [PATCH 17/17] assert error log not called --- tests/test_odp_manager.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tests/test_odp_manager.py b/tests/test_odp_manager.py index 6c7cf263..d60d40c9 100644 --- a/tests/test_odp_manager.py +++ b/tests/test_odp_manager.py @@ -125,6 +125,7 @@ def test_identify_user_datafile_not_ready(self): manager.identify_user('user1') mock_identify_user.assert_called_once_with('user1') + mock_logger.error.assert_not_called() def test_identify_user_odp_integrated(self): mock_logger = mock.MagicMock() @@ -146,6 +147,7 @@ def test_identify_user_odp_integrated(self): 'data_source': 'python-sdk', 'data_source_version': version.__version__ }}) + mock_logger.error.assert_not_called() def test_identify_user_odp_not_integrated(self): mock_logger = mock.MagicMock() @@ -158,6 +160,7 @@ def test_identify_user_odp_not_integrated(self): manager.identify_user('user1') mock_dispatch_event.assert_not_called() + mock_logger.error.assert_not_called() mock_logger.debug.assert_any_call('Odp config was not changed.') mock_logger.debug.assert_any_call('ODP identify event is not dispatched (ODP not integrated).') @@ -172,6 +175,7 @@ def test_identify_user_odp_disabled(self): manager.identify_user('user1') mock_identify_user.assert_not_called() + mock_logger.error.assert_not_called() mock_logger.debug.assert_called_with('ODP identify event is not dispatched (ODP disabled).') def test_send_event_datafile_not_ready(self): @@ -184,6 +188,7 @@ def test_send_event_datafile_not_ready(self): manager.send_event('t1', 'a1', {'id-key1': 'id-val-1'}, {'key1': 'val1'}) mock_dispatch_event.assert_not_called() + mock_logger.error.assert_not_called() mock_logger.debug.assert_called_with('ODP event queue: cannot send before the datafile has loaded.') def test_send_event_odp_integrated(self): @@ -221,6 +226,7 @@ def test_send_event_odp_not_integrated(self): mock_dispatch_event.assert_not_called() mock_logger.debug.assert_any_call('Odp config was not changed.') + mock_logger.error.assert_not_called() def test_send_event_odp_disabled(self): mock_logger = mock.MagicMock() @@ -235,6 +241,7 @@ def test_send_event_odp_disabled(self): mock_dispatch_event.assert_not_called() mock_logger.debug.assert_not_called() + mock_logger.error.assert_not_called() def test_send_event_odp_disabled__event_manager_not_available(self): mock_logger = mock.MagicMock() @@ -249,6 +256,7 @@ def test_send_event_odp_disabled__event_manager_not_available(self): mock_dispatch_event.assert_not_called() mock_logger.debug.assert_not_called() + mock_logger.error.assert_not_called() def test_send_event_invalid_data(self): mock_logger = mock.MagicMock() @@ -262,6 +270,7 @@ def test_send_event_invalid_data(self): manager.send_event, 't1', 'a1', {'id-key1': 'id-val-1'}, {'invalid-item': {}}) mock_dispatch_event.assert_not_called() + mock_logger.error.assert_not_called() def test_config_not_changed(self): mock_logger = mock.MagicMock() @@ -270,6 +279,7 @@ def test_config_not_changed(self): manager = OdpManager(False, OptimizelySegmentsCache, event_manager=event_manager, logger=mock_logger) manager.update_odp_config(None, None, []) mock_logger.debug.assert_called_with('Odp config was not changed.') + mock_logger.error.assert_not_called() def test_update_odp_config__reset_called(self): # build segment manager @@ -314,6 +324,7 @@ def test_update_odp_config__reset_called(self): manager.update_odp_config(None, None, []) mock_reset.assert_called_once() + mock_logger.error.assert_not_called() def test_update_odp_config__update_config_called(self): """ @@ -351,7 +362,8 @@ def test_update_odp_config__update_config_called(self): # event_manager.update_config not called when no change to odp_config mock_update.assert_not_called() - mock_logger.debug('Odp config was not changed.') + mock_logger.error.assert_not_called() + mock_logger.debug.assert_called_with('Odp config was not changed.') self.assertEqual(first_api_key, 'key2') self.assertEqual(second_api_key, 'key2') @@ -381,6 +393,7 @@ def test_update_odp_config__odp_config_propagated_properly(self): manager.update_odp_config(None, None, ['a', 'b']) self.assertEqual(manager.segment_manager.odp_config.get_segments_to_check(), ['a', 'b']) self.assertEqual(manager.event_manager.odp_config.get_segments_to_check(), ['a', 'b']) + mock_logger.error.assert_not_called() def test_segments_cache_default_settings(self): manager = OdpManager(False)