Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: odp segment manager #402

Merged
merged 10 commits into from
Aug 26, 2022
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions optimizely/odp/odp_options.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# 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 sys import version_info

if version_info < (3, 8):
from typing_extensions import Final
else:
from typing import Final # type: ignore


class OptimizelySegmentOption:
Mat001 marked this conversation as resolved.
Show resolved Hide resolved
"""Options for the OdpSegmentManager."""
IGNORE_CACHE: Final = 'IGNORE_CACHE'
RESET_CACHE: Final = 'RESET_CACHE'
89 changes: 89 additions & 0 deletions optimizely/odp/odp_segment_manager.py
Original file line number Diff line number Diff line change
@@ -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 typing import Optional

from optimizely import logger as optimizely_logger
from optimizely.helpers.enums import Errors
from optimizely.odp.odp_options import OptimizelySegmentOption
from optimizely.odp.lru_cache import OptimizelySegmentsCache
from optimizely.odp.odp_config import OdpConfig
from optimizely.odp.zaius_graphql_api_manager import ZaiusGraphQLApiManager


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:

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]]:
"""
Args:
user_key: The key for identifying the id type.
user_value: The id itself.
options: An array of OptimizelySegmentOptions used to ignore and/or reset the cache.

Returns:
Qualified segments for the user from the cache or the ODP server if not in the cache.
"""
odp_api_key = self.odp_config.get_api_key()
odp_api_host = self.odp_config.get_api_host()
odp_segments_to_check = self.odp_config.get_segments_to_check()

if not (odp_api_key and odp_api_host):
self.logger.error(Errors.FETCH_SEGMENTS_FAILED.format('apiKey/apiHost not defined'))
Mat001 marked this conversation as resolved.
Show resolved Hide resolved
return None

if not odp_segments_to_check:
self.logger.debug('No segments are used in the project. Returning empty list.')
return []

cache_key = self.make_cache_key(user_key, user_value)

ignore_cache = OptimizelySegmentOption.IGNORE_CACHE in options
reset_cache = OptimizelySegmentOption.RESET_CACHE in options

if reset_cache:
self._reset()

if not ignore_cache and not reset_cache:
segments = self.segments_cache.lookup(cache_key)
if segments:
self.logger.debug('ODP cache hit. Returning segments from cache.')
return segments

self.logger.debug('ODP cache miss. Making a call to ODP server.')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not necessarily a cache miss... We might want to split this into two debug logs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andrewleap-optimizely Not sure what you mean, can you clarify? If we don't hit the cache we miss it no?
U mean if we don't hit the cache there is the third option that is not a cache miss?

Rb segment manager has the same btw

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean that the cache might not have been checked, because ignore_cache was set.

Yea I should change it in ruby and probably Java where I pulled it from 😀

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like this:

if not ignore_cache and not reset_cache:
    segments = self.segments_cache.lookup(cache_key)
    if segments:
        self.logger.debug('ODP cache hit. Returning segments from cache.')
        return segments
    self.logger.debug('ODP cache miss.')

self.logger.debug('Making a call to ODP server.')

Otherwise looking at the logs with a disabled cache may confuse the reader into thinking the cache is being used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, yeah, that makes total sense. good catch


segments = self.zaius_manager.fetch_segments(odp_api_key, odp_api_host, user_key, user_value,
odp_segments_to_check)

if segments and not ignore_cache:
self.segments_cache.save(cache_key, segments)

return segments

def _reset(self) -> None:
self.segments_cache.reset()

def make_cache_key(self, user_key: str, user_value: str) -> str:
return f'{user_key}-$-{user_value}'
208 changes: 208 additions & 0 deletions tests/test_odp_segment_manager.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,208 @@
# 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 requests import exceptions as request_exception

from optimizely.odp.lru_cache import LRUCache
from optimizely.odp.odp_config import OdpConfig
from optimizely.odp.odp_options import OptimizelySegmentOption
from optimizely.odp.odp_segment_manager import OdpSegmentManager
from optimizely.odp.zaius_graphql_api_manager import ZaiusGraphQLApiManager
from tests import base


class OdpSegmentManagerTest(base.BaseTest):
api_host = 'host'
api_key = 'valid'
user_key = 'fs_user_id'
user_value = 'test-user-value'

def test_empty_list_with_no_segments_to_check(self):
Mat001 marked this conversation as resolved.
Show resolved Hide resolved
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()
andrewleap-optimizely marked this conversation as resolved.
Show resolved Hide resolved
mock_fetch_segments.assert_not_called()
Mat001 marked this conversation as resolved.
Show resolved Hide resolved

def test_fetch_segments_success_cache_miss(self):
"""
we are fetching user key/value 'fs_user_id'/'test-user-value'
which is different from what we have passed to cache (fs_user_id-$-123/['d'])
---> hence we trigger a cache miss
"""
odp_config = OdpConfig(self.api_key, self.api_host, ["a", "b", "c"])
mock_logger = mock.MagicMock()
segments_cache = LRUCache(1000, 1000)
api = ZaiusGraphQLApiManager()

segment_manager = OdpSegmentManager(odp_config, segments_cache, api, mock_logger)
cache_key = segment_manager.make_cache_key(self.user_key, '123')
segment_manager.segments_cache.save(cache_key, ["d"])

with mock.patch('requests.post') as mock_request_post:
mock_request_post.return_value = self.fake_server_response(status_code=200,
content=self.good_response_data)

segments = segment_manager.fetch_qualified_segments(self.user_key, self.user_value, [])

self.assertEqual(segments, ["a", "b"])
actual_cache_key = segment_manager.make_cache_key(self.user_key, self.user_value)
self.assertEqual(segment_manager.segments_cache.lookup(actual_cache_key), ["a", "b"])
mock_logger.debug.assert_called_once_with('ODP cache miss. Making a call to ODP server.')
mock_logger.error.assert_not_called()

def test_fetch_segments_success_cache_hit(self):
odp_config = OdpConfig()
odp_config.update(self.api_key, self.api_host, ['c'])
mock_logger = mock.MagicMock()
api = ZaiusGraphQLApiManager()
segments_cache = LRUCache(1000, 1000)

segment_manager = OdpSegmentManager(odp_config, segments_cache, None, mock_logger)
cache_key = segment_manager.make_cache_key(self.user_key, self.user_value)
segment_manager.segments_cache.save(cache_key, ['c'])

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, ['c'])
mock_logger.debug.assert_called_once_with('ODP cache hit. Returning segments from cache.')
mock_logger.error.assert_not_called()
mock_fetch_segments.assert_not_called()

Mat001 marked this conversation as resolved.
Show resolved Hide resolved
def test_fetch_segments_missing_api_host_api_key(self):
with mock.patch('optimizely.logger') as mock_logger:
segment_manager = OdpSegmentManager(OdpConfig(), LRUCache(1000, 1000), None, mock_logger)
segments = segment_manager.fetch_qualified_segments(self.user_key, self.user_value, [])

self.assertEqual(segments, None)
mock_logger.error.assert_called_once_with('Audience segments fetch failed (apiKey/apiHost not defined).')

def test_fetch_segments_network_error(self):
"""
Trigger connection error with mock side_effect. Note that Python's requests don't
have a status code for connection error, that's why we need to trigger the exception
instead of returning a fake server response with status code 500.
The error log should come form the GraphQL API manager, not from ODP Segment Manager.
The active mock logger should be placed as parameter in ZaiusGraphQLApiManager object.
"""
odp_config = OdpConfig(self.api_key, self.api_host, ["a", "b", "c"])
mock_logger = mock.MagicMock()
segments_cache = LRUCache(1000, 1000)
api = ZaiusGraphQLApiManager(mock_logger)
segment_manager = OdpSegmentManager(odp_config, segments_cache, api, None)

with mock.patch('requests.post',
side_effect=request_exception.ConnectionError('Connection error')):
segments = segment_manager.fetch_qualified_segments(self.user_key, self.user_value, [])

self.assertEqual(segments, None)
mock_logger.error.assert_called_once_with('Audience segments fetch failed (network error).')
Mat001 marked this conversation as resolved.
Show resolved Hide resolved

def test_options_ignore_cache(self):
odp_config = OdpConfig(self.api_key, self.api_host, ["a", "b", "c"])
mock_logger = mock.MagicMock()
segments_cache = LRUCache(1000, 1000)
api = ZaiusGraphQLApiManager()

segment_manager = OdpSegmentManager(odp_config, segments_cache, api, mock_logger)
cache_key = segment_manager.make_cache_key(self.user_key, self.user_value)
segment_manager.segments_cache.save(cache_key, ['d'])

with mock.patch('requests.post') as mock_request_post:
mock_request_post.return_value = self.fake_server_response(status_code=200,
content=self.good_response_data)

segments = segment_manager.fetch_qualified_segments(self.user_key, self.user_value,
[OptimizelySegmentOption.IGNORE_CACHE])

self.assertEqual(segments, ["a", "b"])
self.assertEqual(segment_manager.segments_cache.lookup(cache_key), ['d'])
mock_logger.debug.assert_called_once_with('ODP cache miss. Making a call to ODP server.')
mock_logger.error.assert_not_called()

def test_options_reset_cache(self):
odp_config = OdpConfig(self.api_key, self.api_host, ["a", "b", "c"])
mock_logger = mock.MagicMock()
segments_cache = LRUCache(1000, 1000)
api = ZaiusGraphQLApiManager()

segment_manager = OdpSegmentManager(odp_config, segments_cache, api, mock_logger)
cache_key = segment_manager.make_cache_key(self.user_key, self.user_value)
segment_manager.segments_cache.save(cache_key, ['d'])
segment_manager.segments_cache.save('123', ['c', 'd'])

with mock.patch('requests.post') as mock_request_post:
mock_request_post.return_value = self.fake_server_response(status_code=200,
content=self.good_response_data)

segments = segment_manager.fetch_qualified_segments(self.user_key, self.user_value,
[OptimizelySegmentOption.RESET_CACHE])

self.assertEqual(segments, ["a", "b"])
self.assertEqual(segment_manager.segments_cache.lookup(cache_key), ['a', 'b'])
self.assertTrue(len(segment_manager.segments_cache.map) == 1)
mock_logger.debug.assert_called_once_with('ODP cache miss. Making a call to ODP server.')
mock_logger.error.assert_not_called()

def test_make_correct_cache_key(self):
segment_manager = OdpSegmentManager(None, None, None, None)
cache_key = segment_manager.make_cache_key(self.user_key, self.user_value)
self.assertEqual(cache_key, 'fs_user_id-$-test-user-value')

# test json response
good_response_data = """
{
"data": {
"customer": {
"audiences": {
"edges": [
{
"node": {
"name": "a",
"state": "qualified",
"description": "qualifed sample 1"
}
},
{
"node": {
"name": "b",
"state": "qualified",
"description": "qualifed sample 2"
}
},
{
"node": {
"name": "c",
"state": "not_qualified",
"description": "not-qualified sample"
}
}
]
}
}
}
}
"""