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 3 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
6 changes: 6 additions & 0 deletions optimizely/helpers/enums.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,3 +205,9 @@ class OdpRestApiConfig:
class OdpGraphQLApiConfig:
"""ODP GraphQL API configs."""
REQUEST_TIMEOUT: Final = 10


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

from optimizely import logger as optimizely_logger
from optimizely.helpers.enums import Errors
from optimizely.helpers.enums import OptimizelySegmentOption
from optimizely.odp.lru_cache import LRUCache
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."""

IGNORE_CACHE = OptimizelySegmentOption.IGNORE_CACHE
RESET_CACHE = OptimizelySegmentOption.RESET_CACHE

def __init__(self, odp_config: Optional[OdpConfig], segments_cache: Optional[LRUCache[str, List[str]](1000, 1000)],
zaius_manager: Optional[ZaiusGraphQLApiManager],
Mat001 marked this conversation as resolved.
Show resolved Hide resolved
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[OptimizelySegmentOption]):
Mat001 marked this conversation as resolved.
Show resolved Hide resolved
if not self.odp_config.odp_integrated():
self.logger.error(Errors.FETCH_SEGMENTS_FAILED.format('apiKey/apiHost not defined'))
Mat001 marked this conversation as resolved.
Show resolved Hide resolved
return None

odp_api_key: Optional[str] = self.odp_config.get_api_key()
Mat001 marked this conversation as resolved.
Show resolved Hide resolved
odp_api_host: Optional[str] = self.odp_config.get_api_host()
odp_segments_to_check: Optional[list[str]] = self.odp_config.get_segments_to_check()

if not odp_segments_to_check and not len(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 = self.IGNORE_CACHE if self.IGNORE_CACHE in options else None
Mat001 marked this conversation as resolved.
Show resolved Hide resolved
reset_cache = self.RESET_CACHE if self.RESET_CACHE in options else None

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):
self.segments_cache.reset()

def make_cache_key(self, user_key: str, user_value: str) -> str:
return user_key + '-$-' + user_value
Mat001 marked this conversation as resolved.
Show resolved Hide resolved
210 changes: 210 additions & 0 deletions tests/test_odp_segment_manager.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,210 @@
# 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.helpers.enums import OptimizelySegmentOption
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.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_array_with_no_segments_to_check(self):
with mock.patch('requests.post') as mock_request_post, \
mock.patch('optimizely.logger') as mock_logger:
mock_request_post.return_value = self.fake_server_response(status_code=200,
content=self.good_response_data)

odp_config = OdpConfig(self.api_key, self.api_host, [])
segments_cache = LRUCache(1000, 1000)
api = ZaiusGraphQLApiManager()

segment_manager = OdpSegmentManager(odp_config, segments_cache, api, mock_logger)
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

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
"""
with mock.patch('requests.post') as mock_request_post, \
mock.patch('optimizely.logger') as mock_logger:
mock_request_post.return_value = self.fake_server_response(status_code=200,
content=self.good_response_data)

odp_config = OdpConfig(self.api_key, self.api_host, ["a", "b", "c"])
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"])

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):
with mock.patch('optimizely.logger') as mock_logger:
odp_config = OdpConfig()
odp_config.update(self.api_key, self.api_host, ['c'])
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'])

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()

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.
"""
with mock.patch('requests.post',
side_effect=request_exception.ConnectionError('Connection error')), \
mock.patch('optimizely.logger') as mock_logger:
odp_config = OdpConfig(self.api_key, self.api_host, ["a", "b", "c"])
segments_cache = LRUCache(1000, 1000)
api = ZaiusGraphQLApiManager(mock_logger)

segment_manager = OdpSegmentManager(odp_config, segments_cache, api, None)
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):
with mock.patch('requests.post') as mock_request_post, \
mock.patch('optimizely.logger') as mock_logger:
mock_request_post.return_value = self.fake_server_response(status_code=200,
content=self.good_response_data)

odp_config = OdpConfig(self.api_key, self.api_host, ["a", "b", "c"])
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'])

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):
with mock.patch('requests.post') as mock_request_post, \
mock.patch('optimizely.logger') as mock_logger:
mock_request_post.return_value = self.fake_server_response(status_code=200,
content=self.good_response_data)

odp_config = OdpConfig(self.api_key, self.api_host, ["a", "b", "c"])
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'])

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"
}
}
]
}
}
}
}
"""