From 49458bac23cf226b64279aab16537b952c4d4ab9 Mon Sep 17 00:00:00 2001 From: Andy Leap Date: Mon, 8 Aug 2022 14:43:49 -0400 Subject: [PATCH 1/5] add odp segment manager --- lib/optimizely/odp/odp_segment_manager.rb | 120 ++++++++ .../odp/zaius_graphql_api_manager.rb | 2 +- spec/odp/odp_segment_manager_spec.rb | 264 ++++++++++++++++++ spec/odp/zaius_graphql_api_manager_spec.rb | 6 +- 4 files changed, 388 insertions(+), 4 deletions(-) create mode 100644 lib/optimizely/odp/odp_segment_manager.rb create mode 100644 spec/odp/odp_segment_manager_spec.rb diff --git a/lib/optimizely/odp/odp_segment_manager.rb b/lib/optimizely/odp/odp_segment_manager.rb new file mode 100644 index 00000000..88c9df71 --- /dev/null +++ b/lib/optimizely/odp/odp_segment_manager.rb @@ -0,0 +1,120 @@ +# frozen_string_literal: true + +# +# Copyright 2022, Optimizely and contributors +# +# 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. +# + +require 'optimizely/logger' +require_relative 'zaius_graphql_api_manager' + +module Optimizely + class OdpSegmentManager + # Schedules connections to ODP for audience segmentation and caches the results + attr_reader :odp_config, :segments_cache, :zaius_manager, :logger + + def initialize(cache_size, cache_timeout_in_secs, odp_config, api_manager = nil, logger = nil, proxy_config = nil) + @odp_config = odp_config + @logger = logger || NoOpLogger.new + @zaius_manager = api_manager || ZaiusGraphQLApiManager.new(logger: @logger, proxy_config: proxy_config) + @segments_cache = Optimizely::LRUCache.new(cache_size, cache_timeout_in_secs) + end + + def fetch_qualified_segments(segment_request) + odp_api_key = @odp_config&.api_key + odp_api_host = @odp_config&.api_host + + unless odp_api_host && odp_api_key + @logger.log(Logger::ERROR, 'api_key/api_host not defined') + segment_request.segments = nil + return + end + segments_to_check = @odp_config&.segments_to_check + + unless segments_to_check&.size&.positive? + segment_request.segments = [] + return + end + + cache_key = make_cache_key(segment_request.user_key, segment_request.user_value) + + ignore_cache = segment_request.options.include?(OptimizelySegmentOption::IGNORE_CACHE) + reset_cache = segment_request.options.include?(OptimizelySegmentOption::RESET_CACHE) + + reset if reset_cache + + unless ignore_cache || reset_cache + segments = @segments_cache.lookup(cache_key) + unless segments.nil? + segment_request.segments = segments + return + end + end + + Thread.new do + segments = @zaius_manager.fetch_segments(odp_api_key, odp_api_host, segment_request.user_key, segment_request.user_value, segments_to_check) + @segments_cache.save(cache_key, segments) unless segments.nil? || ignore_cache + segment_request.segments = segments + end + + nil + end + + def reset + @segments_cache.reset + nil + end + + private + + def make_cache_key(user_key, user_value) + "#{user_key}-$-#{user_value}" + end + end + + class OptimizelySegmentOption + IGNORE_CACHE = :IGNORE_CACHE + RESET_CACHE = :RESET_CACHE + end + + class OdpSegmentRequest + # Allows asynchronous communication between OptimizelyUserContext and OdpSegmentManger + attr_reader :user_key, :user_value, :options + + def initialize(user_key, user_value, options) + @user_key = user_key + @user_value = user_value + @options = options + @queue = Thread::SizedQueue.new(1) + @segments = nil + end + + # If this method is called without a corresponding call to segments=, it will wait indefinitely + def wait_for_segments + return @segments if @queue.closed? + + @segments = @queue.pop + @queue.close + @segments + end + + def segments=(segments) + if @queue.closed? + @segments = segments + return + end + @queue.push(segments, non_block: true) + end + end +end diff --git a/lib/optimizely/odp/zaius_graphql_api_manager.rb b/lib/optimizely/odp/zaius_graphql_api_manager.rb index 8123b69e..ccc9afc3 100644 --- a/lib/optimizely/odp/zaius_graphql_api_manager.rb +++ b/lib/optimizely/odp/zaius_graphql_api_manager.rb @@ -19,7 +19,7 @@ require 'json' module Optimizely - class ZaiusGraphQlApiManager + class ZaiusGraphQLApiManager # Interface that handles fetching audience segments. def initialize(logger: nil, proxy_config: nil) diff --git a/spec/odp/odp_segment_manager_spec.rb b/spec/odp/odp_segment_manager_spec.rb new file mode 100644 index 00000000..a104a877 --- /dev/null +++ b/spec/odp/odp_segment_manager_spec.rb @@ -0,0 +1,264 @@ +# frozen_string_literal: true + +# 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. +require 'spec_helper' +require 'optimizely/odp/odp_segment_manager' +require 'optimizely/odp/lru_cache' +require 'optimizely/odp/odp_config' +require 'optimizely/odp/zaius_graphql_api_manager' +require 'optimizely/logger' + +describe Optimizely::OdpSegmentManager do + let(:config_body_JSON) { OptimizelySpec::VALID_CONFIG_BODY_JSON } + let(:error_handler) { Optimizely::NoOpErrorHandler.new } + let(:spy_logger) { spy('logger') } + let(:datafile_project_config) { Optimizely::DatafileProjectConfig.new(config_body_JSON, spy_logger, error_handler) } + let(:api_host) { 'https://test-host' } + let(:user_key) { 'fs_user_id' } + let(:user_value) { 'test-user-value' } + let(:api_key) { 'test-api-key' } + let(:segments_to_check) { %w[a b c] } + let(:good_response_data) do + { + 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' + } + } + ] + } + } + } + }.to_json + end + + describe '#initialize' do + it 'should return OdpSegmentManager instance' do + config = Optimizely::OdpConfig.new + + api_manager = Optimizely::ZaiusGraphQLApiManager.new + segment_manager = Optimizely::OdpSegmentManager.new(0, 0, config, api_manager, spy_logger) + + expect(segment_manager.segments_cache).to be_a Optimizely::LRUCache + expect(segment_manager.odp_config).to be config + expect(segment_manager.zaius_manager).to be api_manager + expect(segment_manager.logger).to be spy_logger + + segment_manager = Optimizely::OdpSegmentManager.new(0, 0, config) + expect(segment_manager.logger).to be_a Optimizely::NoOpLogger + expect(segment_manager.zaius_manager).to be_a Optimizely::ZaiusGraphQLApiManager + end + end + + describe '#fetch_qualified_segments' do + it 'should return segments successfully' do + stub_request(:post, "#{api_host}/v3/graphql") + .with({headers: {'x-api-key': api_key}, body: { + 'query' => %'query {customer(#{user_key}: "#{user_value}")' \ + "{audiences(subset:#{segments_to_check}) {edges {node {name state}}}}}" + }}) + .to_return(status: 200, body: good_response_data) + + odp_config = Optimizely::OdpConfig.new(api_key, api_host, segments_to_check) + segment_manager = Optimizely::OdpSegmentManager.new(1000, 1000, odp_config, nil, spy_logger) + segment_request = Optimizely::OdpSegmentRequest.new(user_key, user_value, []) + + segment_manager.fetch_qualified_segments(segment_request) + + expect(segment_request.wait_for_segments).to match_array(%w[a b]) + expect(spy_logger).not_to have_received(:log) + end + + it 'should return empty array with no segments to check' do + stub_request(:post, "#{api_host}/v3/graphql") + .to_return(status: 200, body: good_response_data) + + odp_config = Optimizely::OdpConfig.new(api_key, api_host, []) + segment_manager = Optimizely::OdpSegmentManager.new(1000, 1000, odp_config, nil, spy_logger) + segment_request = Optimizely::OdpSegmentRequest.new(user_key, user_value, []) + + segment_manager.fetch_qualified_segments(segment_request) + + expect(segment_request.wait_for_segments).to match_array([]) + expect(spy_logger).not_to have_received(:log) + end + + it 'should return success with cache miss' do + stub_request(:post, "#{api_host}/v3/graphql") + .to_return(status: 200, body: good_response_data) + + odp_config = Optimizely::OdpConfig.new(api_key, api_host, %w[a b c]) + segment_manager = Optimizely::OdpSegmentManager.new(1000, 1000, odp_config, nil, spy_logger) + + cache_key = segment_manager.send(:make_cache_key, user_key, '123') + segment_manager.segments_cache.save(cache_key, %w[d]) + + segment_request = Optimizely::OdpSegmentRequest.new(user_key, user_value, []) + + segment_manager.fetch_qualified_segments(segment_request) + + expect(segment_request.wait_for_segments).to match_array(%w[a b]) + actual_cache_key = segment_manager.send(:make_cache_key, user_key, user_value) + expect(segment_manager.segments_cache.lookup(actual_cache_key)).to match_array(%w[a b]) + expect(spy_logger).not_to have_received(:log) + end + + it 'should return success with cache hit' do + odp_config = Optimizely::OdpConfig.new + odp_config.update(api_key, api_host, %w[a b c]) + segment_manager = Optimizely::OdpSegmentManager.new(1000, 1000, odp_config, nil, spy_logger) + + cache_key = segment_manager.send(:make_cache_key, user_key, user_value) + segment_manager.segments_cache.save(cache_key, %w[c]) + + segment_request = Optimizely::OdpSegmentRequest.new(user_key, user_value, []) + + segment_manager.fetch_qualified_segments(segment_request) + + expect(segment_request.wait_for_segments).to match_array(%w[c]) + expect(spy_logger).not_to have_received(:log) + end + + it 'should return nil and log error with missing api_host/api_key' do + odp_config = Optimizely::OdpConfig.new + + segment_manager = Optimizely::OdpSegmentManager.new(1000, 1000, odp_config, nil, spy_logger) + segment_request = Optimizely::OdpSegmentRequest.new(user_key, user_value, []) + + segment_manager.fetch_qualified_segments(segment_request) + + expect(segment_request.wait_for_segments).to be_nil + expect(spy_logger).to have_received(:log).with(Logger::ERROR, 'api_key/api_host not defined') + end + + it 'should return nil with network error' do + stub_request(:post, "#{api_host}/v3/graphql") + .to_return(status: 500, body: '{}') + + odp_config = Optimizely::OdpConfig.new(api_key, api_host, segments_to_check) + segment_manager = Optimizely::OdpSegmentManager.new(1000, 1000, odp_config) + segment_request = Optimizely::OdpSegmentRequest.new(user_key, user_value, []) + + segment_manager.fetch_qualified_segments(segment_request) + + expect(segment_request.wait_for_segments).to be_nil + end + + it 'should return success with ignore cache' do + stub_request(:post, "#{api_host}/v3/graphql") + .to_return(status: 200, body: good_response_data) + + odp_config = Optimizely::OdpConfig.new(api_key, api_host, %w[a b c]) + segment_manager = Optimizely::OdpSegmentManager.new(1000, 1000, odp_config, nil, spy_logger) + + cache_key = segment_manager.send(:make_cache_key, user_key, user_value) + segment_manager.segments_cache.save(cache_key, %w[d]) + + segment_request = Optimizely::OdpSegmentRequest.new(user_key, user_value, [Optimizely::OptimizelySegmentOption::IGNORE_CACHE]) + + segment_manager.fetch_qualified_segments(segment_request) + + expect(segment_request.wait_for_segments).to match_array(%w[a b]) + expect(segment_manager.segments_cache.lookup(cache_key)).to match_array(%w[d]) + expect(spy_logger).not_to have_received(:log) + end + + it 'should return success with reset cache' do + stub_request(:post, "#{api_host}/v3/graphql") + .to_return(status: 200, body: good_response_data) + + odp_config = Optimizely::OdpConfig.new(api_key, api_host, %w[a b c]) + segment_manager = Optimizely::OdpSegmentManager.new(1000, 1000, odp_config, nil, spy_logger) + + cache_key = segment_manager.send(:make_cache_key, user_key, user_value) + segment_manager.segments_cache.save(cache_key, %w[d]) + + segment_request = Optimizely::OdpSegmentRequest.new(user_key, user_value, [Optimizely::OptimizelySegmentOption::RESET_CACHE]) + + segment_manager.fetch_qualified_segments(segment_request) + + expect(segment_request.wait_for_segments).to match_array(%w[a b]) + expect(segment_manager.segments_cache.lookup(cache_key)).to match_array(%w[a b]) + expect(spy_logger).not_to have_received(:log) + end + + it 'should make correct cache key' do + segment_manager = Optimizely::OdpSegmentManager.new(1000, 1000, nil) + cache_key = segment_manager.send(:make_cache_key, user_key, user_value) + expect(cache_key).to be == "#{user_key}-$-#{user_value}" + end + + it 'should process all segment requests quickly' do + mutex = Mutex.new + results = [] + odp_config = Optimizely::OdpConfig.new(api_key, api_host, segments_to_check) + + segment_manager = Optimizely::OdpSegmentManager.new(0, 0, odp_config, nil) + + allow(segment_manager.zaius_manager).to receive(:fetch_segments) do + # simulate slow REST query + sleep(1) + %w[a b] + end + + expect(segment_manager.logger).not_to receive(:log) + + thread_count = 1000 + threads = [] + + started = Time.now + thread_count.times do + threads << Thread.new do + user_key = rand(1..1000).to_s + user_value = rand(1..1000).to_s + segment_request = Optimizely::OdpSegmentRequest.new(user_key, user_value, [Optimizely::OptimizelySegmentOption::IGNORE_CACHE]) + + segment_manager.fetch_qualified_segments(segment_request) + response = segment_request.wait_for_segments + expect(response).to match_array(%w[a b]) + + mutex.synchronize do + results.push(response) + end + end + end + + threads.each(&:join) + + expect(results.length).to be == thread_count + expect(results).to all(match_array(%w[a b])) + expect(Time.now - started).to be < 10 + end + end +end diff --git a/spec/odp/zaius_graphql_api_manager_spec.rb b/spec/odp/zaius_graphql_api_manager_spec.rb index 91c3a14b..1112fc5c 100644 --- a/spec/odp/zaius_graphql_api_manager_spec.rb +++ b/spec/odp/zaius_graphql_api_manager_spec.rb @@ -18,14 +18,14 @@ require 'spec_helper' require 'optimizely/odp/zaius_graphql_api_manager' -describe Optimizely::ZaiusGraphQlApiManager do +describe Optimizely::ZaiusGraphQLApiManager do let(:user_key) { 'vuid' } let(:user_value) { 'test-user-value' } let(:api_key) { 'test-api-key' } let(:api_host) { 'https://test-host' } let(:error_handler) { Optimizely::RaiseErrorHandler.new } let(:spy_logger) { spy('logger') } - let(:zaius_manager) { Optimizely::ZaiusGraphQlApiManager.new(logger: spy_logger) } + let(:zaius_manager) { Optimizely::ZaiusGraphQLApiManager.new(logger: spy_logger) } let(:good_response_data) do { data: { @@ -430,7 +430,7 @@ allow(Optimizely::Helpers::HttpUtils).to receive(:make_request).and_raise(SocketError) stub_request(:post, "#{api_host}/v3/graphql") - zaius_manager = Optimizely::ZaiusGraphQlApiManager.new(logger: spy_logger, proxy_config: :proxy_config) + zaius_manager = Optimizely::ZaiusGraphQLApiManager.new(logger: spy_logger, proxy_config: :proxy_config) zaius_manager.fetch_segments(api_key, api_host, user_key, user_value, []) expect(Optimizely::Helpers::HttpUtils).to have_received(:make_request).with(anything, anything, anything, anything, anything, :proxy_config) end From 0f7457878578272854a362285cb29f0dd0a6f5ec Mon Sep 17 00:00:00 2001 From: Andy Leap Date: Tue, 16 Aug 2022 09:17:47 -0400 Subject: [PATCH 2/5] return nil on graphql error --- .../odp/zaius_graphql_api_manager.rb | 14 +++--- spec/odp/zaius_graphql_api_manager_spec.rb | 44 +++++++++---------- 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/lib/optimizely/odp/zaius_graphql_api_manager.rb b/lib/optimizely/odp/zaius_graphql_api_manager.rb index ccc9afc3..cabcaefd 100644 --- a/lib/optimizely/odp/zaius_graphql_api_manager.rb +++ b/lib/optimizely/odp/zaius_graphql_api_manager.rb @@ -52,23 +52,23 @@ def fetch_segments(api_key, api_host, user_key, user_value, segments_to_check) rescue SocketError, Timeout::Error, Net::ProtocolError, Errno::ECONNRESET => e @logger.log(Logger::DEBUG, "GraphQL download failed: #{e}") log_failure('network error') - return [] + return nil rescue Errno::EINVAL, Net::HTTPBadResponse, Net::HTTPHeaderSyntaxError => e log_failure(e) - return [] + return nil end status = response.code.to_i if status >= 400 log_failure(status) - return [] + return nil end begin response = JSON.parse(response.body) rescue JSON::ParserError log_failure('JSON decode error') - return [] + return nil end if response.include?('errors') @@ -78,13 +78,13 @@ def fetch_segments(api_key, api_host, user_key, user_value, segments_to_check) else log_failure(error_class) end - return [] + return nil end audiences = response.dig('data', 'customer', 'audiences', 'edges') unless audiences log_failure('decode error') - return [] + return nil end audiences.filter_map do |edge| @@ -92,7 +92,7 @@ def fetch_segments(api_key, api_host, user_key, user_value, segments_to_check) state = edge.dig('node', 'state') unless name && state log_failure('decode error') - return [] + return nil end state == 'qualified' ? name : nil end diff --git a/spec/odp/zaius_graphql_api_manager_spec.rb b/spec/odp/zaius_graphql_api_manager_spec.rb index 1112fc5c..72ae67e1 100644 --- a/spec/odp/zaius_graphql_api_manager_spec.rb +++ b/spec/odp/zaius_graphql_api_manager_spec.rb @@ -240,12 +240,12 @@ expect(segments).to match_array [] end - it 'should log error and return empty array when response is missing node' do + it 'should log error and return nil when response is missing node' do stub_request(:post, "#{api_host}/v3/graphql") .to_return(status: 200, body: node_missing_response_data.to_json) segments = zaius_manager.fetch_segments(api_key, api_host, user_key, user_value, %w[a b]) - expect(segments).to match_array([]) + expect(segments).to be_nil expect(spy_logger).to have_received(:log).once.with( Logger::ERROR, @@ -253,12 +253,12 @@ ) end - it 'should log error and return empty array when response keys are incorrect' do + it 'should log error and return nil when response keys are incorrect' do stub_request(:post, "#{api_host}/v3/graphql") .to_return(status: 200, body: mixed_missing_keys_response_data.to_json) segments = zaius_manager.fetch_segments(api_key, api_host, user_key, user_value, %w[a b]) - expect(segments).to match_array([]) + expect(segments).to be_nil expect(spy_logger).to have_received(:log).once.with( Logger::ERROR, @@ -266,12 +266,12 @@ ) end - it 'should log warning and return empty array with invalid identifier exception' do + it 'should log warning and return nil with invalid identifier exception' do stub_request(:post, "#{api_host}/v3/graphql") .to_return(status: 200, body: invalid_identifier_response_data.to_json) segments = zaius_manager.fetch_segments(api_key, api_host, user_key, user_value, %w[a b]) - expect(segments).to match_array([]) + expect(segments).to be_nil expect(spy_logger).to have_received(:log).once.with( Logger::WARN, @@ -279,12 +279,12 @@ ) end - it 'should log error and return empty array with other exception' do + it 'should log error and return nil with other exception' do stub_request(:post, "#{api_host}/v3/graphql") .to_return(status: 200, body: other_exception_response_data.to_json) segments = zaius_manager.fetch_segments(api_key, api_host, user_key, user_value, %w[a b]) - expect(segments).to match_array([]) + expect(segments).to be_nil expect(spy_logger).to have_received(:log).once.with( Logger::ERROR, @@ -292,12 +292,12 @@ ) end - it 'should log error and return empty array with bad response data' do + it 'should log error and return nil with bad response data' do stub_request(:post, "#{api_host}/v3/graphql") .to_return(status: 200, body: bad_response_data.to_json) segments = zaius_manager.fetch_segments(api_key, api_host, user_key, user_value, %w[a b]) - expect(segments).to match_array([]) + expect(segments).to be_nil expect(spy_logger).to have_received(:log).once.with( Logger::ERROR, @@ -305,12 +305,12 @@ ) end - it 'should log error and return empty array with invalid name' do + it 'should log error and return nil with invalid name' do stub_request(:post, "#{api_host}/v3/graphql") .to_return(status: 200, body: name_invalid_response_data) segments = zaius_manager.fetch_segments(api_key, api_host, user_key, user_value, %w[a b]) - expect(segments).to match_array([]) + expect(segments).to be_nil expect(spy_logger).to have_received(:log).once.with( Logger::ERROR, @@ -318,12 +318,12 @@ ) end - it 'should log error and return empty array with invalid key' do + it 'should log error and return nil with invalid key' do stub_request(:post, "#{api_host}/v3/graphql") .to_return(status: 200, body: invalid_edges_key_response_data.to_json) segments = zaius_manager.fetch_segments(api_key, api_host, user_key, user_value, %w[a b]) - expect(segments).to match_array([]) + expect(segments).to be_nil expect(spy_logger).to have_received(:log).once.with( Logger::ERROR, @@ -331,12 +331,12 @@ ) end - it 'should log error and return empty array with invalid key in error body' do + it 'should log error and return nil with invalid key in error body' do stub_request(:post, "#{api_host}/v3/graphql") .to_return(status: 200, body: invalid_key_for_error_response_data.to_json) segments = zaius_manager.fetch_segments(api_key, api_host, user_key, user_value, %w[a b]) - expect(segments).to match_array([]) + expect(segments).to be_nil expect(spy_logger).to have_received(:log).once.with( Logger::ERROR, @@ -344,12 +344,12 @@ ) end - it 'should log error and return empty array with network error' do + it 'should log error and return nil with network error' do stub_request(:post, "#{api_host}/v3/graphql") .and_raise(SocketError) segments = zaius_manager.fetch_segments(api_key, api_host, user_key, user_value, %w[a b]) - expect(segments).to match_array([]) + expect(segments).to be_nil expect(spy_logger).to have_received(:log).once.with( Logger::ERROR, @@ -362,12 +362,12 @@ ) end - it 'should log error and return empty array with http status 400' do + it 'should log error and return nil with http status 400' do stub_request(:post, "#{api_host}/v3/graphql") .to_return(status: 400) segments = zaius_manager.fetch_segments(api_key, api_host, user_key, user_value, %w[a b]) - expect(segments).to match_array([]) + expect(segments).to be_nil expect(spy_logger).to have_received(:log).once.with( Logger::ERROR, @@ -375,12 +375,12 @@ ) end - it 'should log error and return empty array with http status 500' do + it 'should log error and return nil with http status 500' do stub_request(:post, "#{api_host}/v3/graphql") .to_return(status: 500) segments = zaius_manager.fetch_segments(api_key, api_host, user_key, user_value, %w[a b]) - expect(segments).to match_array([]) + expect(segments).to be_nil expect(spy_logger).to have_received(:log).once.with( Logger::ERROR, From 48d5b020cde7a5a71038bfa2b57a9a7343ea2902 Mon Sep 17 00:00:00 2001 From: Andy Leap Date: Tue, 16 Aug 2022 09:52:47 -0400 Subject: [PATCH 3/5] add odp config --- lib/optimizely/odp/odp_config.rb | 122 +++++++++++++++++++++++++++++++ 1 file changed, 122 insertions(+) create mode 100644 lib/optimizely/odp/odp_config.rb diff --git a/lib/optimizely/odp/odp_config.rb b/lib/optimizely/odp/odp_config.rb new file mode 100644 index 00000000..c40b6f43 --- /dev/null +++ b/lib/optimizely/odp/odp_config.rb @@ -0,0 +1,122 @@ +# frozen_string_literal: true + +# +# Copyright 2022, Optimizely and contributors +# +# 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. +# + +require 'optimizely/logger' + +module Optimizely + class OdpConfig + # Contains configuration used for ODP integration. + # + # @param api_host - The host URL for the ODP audience segments API (optional). If not provided, SDK will use the default host in datafile. + # @param api_key - The public API key for the ODP account from which the audience segments will be fetched (optional). If not provided, SDK will use the default publicKey in datafile. + # @param segments_to_check - An array of all ODP segments used in the current datafile (associated with api_host/api_key). + # @param odp_enabled - A boolean value indicating whether odp is enabled for the project or not. + def initialize(api_key = nil, api_host = nil, segments_to_check = []) + @api_key = api_key + @api_host = api_host + @segments_to_check = segments_to_check + @odp_enabled = !api_key.nil? && !api_host.nil? ? true : false + @mutex = Mutex.new + end + + # Replaces the existing configuration + # + # @param api_host - The host URL for the ODP audience segments API (optional). If not provided, SDK will use the default host in datafile. + # @param api_key - The public API key for the ODP account from which the audience segments will be fetched (optional). If not provided, SDK will use the default publicKey in datafile. + # @param segments_to_check - An array of all ODP segments used in the current datafile (associated with api_host/api_key). + # + # @return - True if the provided values were different than the existing values. + + def update(api_key = nil, api_host = nil, segments_to_check = []) + @mutex.synchronize do + @odp_enabled = !api_key.nil? && !api_host.nil? ? true : false + + return false if @api_key == api_key && @api_host == api_host && @segments_to_check == segments_to_check + + @api_key = api_key + @api_host = api_host + @segments_to_check = segments_to_check + true + end + end + + # Returns the api host for odp connections + # + # @return - The api host. + + def api_host + @mutex.synchronize { @api_host.clone } + end + + # Returns the api host for odp connections + # + # @return - The api host. + + def api_host=(api_host) + @mutex.synchronize { @api_host = api_host.clone } + end + + # Returns the api key for odp connections + # + # @return - The api key. + + def api_key + @mutex.synchronize { @api_key.clone } + end + + # Replace the api key with the provided string + # + # @param api_key - An api key + + def api_key=(api_key) + @mutex.synchronize { @api_key = api_key.clone } + end + + # Returns An array of qualified segments for this user + # + # @return - An array of segments names. + + def segments_to_check + @mutex.synchronize { @segments_to_check.clone } + end + + # Replace qualified segments with provided segments + # + # @param segments - An array of segment names + + def segments_to_check=(segments_to_check) + @mutex.synchronize { @segments_to_check = segments_to_check.clone } + end + + # Returns True if odp is enabled and ready + # + # @return - bool + + def odp_enabled + @mutex.synchronize { @odp_enabled.clone } + end + + # Enable/disable the odp integration + # + # @param odp_enabled - bool + + def odp_enabled=(odp_enabled) + @mutex.synchronize { @odp_enabled = odp_enabled.clone } + end + end +end From a96ade443547324004e184d6f5f70f890aade39e Mon Sep 17 00:00:00 2001 From: Andy Leap Date: Wed, 17 Aug 2022 11:19:42 -0400 Subject: [PATCH 4/5] cleanup tests --- spec/odp/odp_segment_manager_spec.rb | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/spec/odp/odp_segment_manager_spec.rb b/spec/odp/odp_segment_manager_spec.rb index a104a877..21a74cdf 100644 --- a/spec/odp/odp_segment_manager_spec.rb +++ b/spec/odp/odp_segment_manager_spec.rb @@ -20,10 +20,7 @@ require 'optimizely/logger' describe Optimizely::OdpSegmentManager do - let(:config_body_JSON) { OptimizelySpec::VALID_CONFIG_BODY_JSON } - let(:error_handler) { Optimizely::NoOpErrorHandler.new } let(:spy_logger) { spy('logger') } - let(:datafile_project_config) { Optimizely::DatafileProjectConfig.new(config_body_JSON, spy_logger, error_handler) } let(:api_host) { 'https://test-host' } let(:user_key) { 'fs_user_id' } let(:user_value) { 'test-user-value' } @@ -94,6 +91,8 @@ segment_manager = Optimizely::OdpSegmentManager.new(1000, 1000, odp_config, nil, spy_logger) segment_request = Optimizely::OdpSegmentRequest.new(user_key, user_value, []) + expect(segment_manager.zaius_manager).not_to receive(:log_failure) + segment_manager.fetch_qualified_segments(segment_request) expect(segment_request.wait_for_segments).to match_array(%w[a b]) @@ -108,6 +107,8 @@ segment_manager = Optimizely::OdpSegmentManager.new(1000, 1000, odp_config, nil, spy_logger) segment_request = Optimizely::OdpSegmentRequest.new(user_key, user_value, []) + expect(segment_manager.zaius_manager).not_to receive(:log_failure) + segment_manager.fetch_qualified_segments(segment_request) expect(segment_request.wait_for_segments).to match_array([]) @@ -121,6 +122,8 @@ odp_config = Optimizely::OdpConfig.new(api_key, api_host, %w[a b c]) segment_manager = Optimizely::OdpSegmentManager.new(1000, 1000, odp_config, nil, spy_logger) + expect(segment_manager.zaius_manager).not_to receive(:log_failure) + cache_key = segment_manager.send(:make_cache_key, user_key, '123') segment_manager.segments_cache.save(cache_key, %w[d]) @@ -139,6 +142,8 @@ odp_config.update(api_key, api_host, %w[a b c]) segment_manager = Optimizely::OdpSegmentManager.new(1000, 1000, odp_config, nil, spy_logger) + expect(segment_manager.zaius_manager).not_to receive(:log_failure) + cache_key = segment_manager.send(:make_cache_key, user_key, user_value) segment_manager.segments_cache.save(cache_key, %w[c]) @@ -170,18 +175,22 @@ segment_manager = Optimizely::OdpSegmentManager.new(1000, 1000, odp_config) segment_request = Optimizely::OdpSegmentRequest.new(user_key, user_value, []) + expect(segment_manager.zaius_manager).to receive(:log_failure) + segment_manager.fetch_qualified_segments(segment_request) expect(segment_request.wait_for_segments).to be_nil end - it 'should return success with ignore cache' do + it 'should return non cached value with ignore cache' do stub_request(:post, "#{api_host}/v3/graphql") .to_return(status: 200, body: good_response_data) odp_config = Optimizely::OdpConfig.new(api_key, api_host, %w[a b c]) segment_manager = Optimizely::OdpSegmentManager.new(1000, 1000, odp_config, nil, spy_logger) + expect(segment_manager.zaius_manager).not_to receive(:log_failure) + cache_key = segment_manager.send(:make_cache_key, user_key, user_value) segment_manager.segments_cache.save(cache_key, %w[d]) @@ -194,15 +203,18 @@ expect(spy_logger).not_to have_received(:log) end - it 'should return success with reset cache' do + it 'should reset cache and return non cached value with reset cache' do stub_request(:post, "#{api_host}/v3/graphql") .to_return(status: 200, body: good_response_data) odp_config = Optimizely::OdpConfig.new(api_key, api_host, %w[a b c]) segment_manager = Optimizely::OdpSegmentManager.new(1000, 1000, odp_config, nil, spy_logger) + expect(segment_manager.zaius_manager).not_to receive(:log_failure) + cache_key = segment_manager.send(:make_cache_key, user_key, user_value) segment_manager.segments_cache.save(cache_key, %w[d]) + segment_manager.segments_cache.save('123', %w[c d]) segment_request = Optimizely::OdpSegmentRequest.new(user_key, user_value, [Optimizely::OptimizelySegmentOption::RESET_CACHE]) @@ -210,7 +222,7 @@ expect(segment_request.wait_for_segments).to match_array(%w[a b]) expect(segment_manager.segments_cache.lookup(cache_key)).to match_array(%w[a b]) - expect(spy_logger).not_to have_received(:log) + expect(segment_manager.segments_cache.instance_variable_get('@map').length).to be 1 end it 'should make correct cache key' do @@ -231,8 +243,7 @@ sleep(1) %w[a b] end - - expect(segment_manager.logger).not_to receive(:log) + expect(segment_manager.zaius_manager).not_to receive(:log_failure) thread_count = 1000 threads = [] From f1dfef8cd696698b166d80f47e5ce3d315fa1ae0 Mon Sep 17 00:00:00 2001 From: Andy Leap Date: Wed, 17 Aug 2022 18:00:53 -0400 Subject: [PATCH 5/5] address comments --- lib/optimizely/odp/odp_config.rb | 30 ++--- lib/optimizely/odp/odp_segment_manager.rb | 81 +++++-------- spec/odp/odp_segment_manager_spec.rb | 136 ++++++---------------- 3 files changed, 73 insertions(+), 174 deletions(-) diff --git a/lib/optimizely/odp/odp_config.rb b/lib/optimizely/odp/odp_config.rb index c40b6f43..862b7608 100644 --- a/lib/optimizely/odp/odp_config.rb +++ b/lib/optimizely/odp/odp_config.rb @@ -22,36 +22,32 @@ module Optimizely class OdpConfig # Contains configuration used for ODP integration. # - # @param api_host - The host URL for the ODP audience segments API (optional). If not provided, SDK will use the default host in datafile. - # @param api_key - The public API key for the ODP account from which the audience segments will be fetched (optional). If not provided, SDK will use the default publicKey in datafile. + # @param api_host - The host URL for the ODP audience segments API (optional). + # @param api_key - The public API key for the ODP account from which the audience segments will be fetched (optional). # @param segments_to_check - An array of all ODP segments used in the current datafile (associated with api_host/api_key). - # @param odp_enabled - A boolean value indicating whether odp is enabled for the project or not. def initialize(api_key = nil, api_host = nil, segments_to_check = []) @api_key = api_key @api_host = api_host @segments_to_check = segments_to_check - @odp_enabled = !api_key.nil? && !api_host.nil? ? true : false @mutex = Mutex.new end # Replaces the existing configuration # - # @param api_host - The host URL for the ODP audience segments API (optional). If not provided, SDK will use the default host in datafile. - # @param api_key - The public API key for the ODP account from which the audience segments will be fetched (optional). If not provided, SDK will use the default publicKey in datafile. + # @param api_host - The host URL for the ODP audience segments API (optional). + # @param api_key - The public API key for the ODP account from which the audience segments will be fetched (optional). # @param segments_to_check - An array of all ODP segments used in the current datafile (associated with api_host/api_key). # # @return - True if the provided values were different than the existing values. def update(api_key = nil, api_host = nil, segments_to_check = []) @mutex.synchronize do - @odp_enabled = !api_key.nil? && !api_host.nil? ? true : false - - return false if @api_key == api_key && @api_host == api_host && @segments_to_check == segments_to_check + break false if @api_key == api_key && @api_host == api_host && @segments_to_check == segments_to_check @api_key = api_key @api_host = api_host @segments_to_check = segments_to_check - true + break true end end @@ -103,20 +99,12 @@ def segments_to_check=(segments_to_check) @mutex.synchronize { @segments_to_check = segments_to_check.clone } end - # Returns True if odp is enabled and ready + # Returns True if odp is integrated # # @return - bool - def odp_enabled - @mutex.synchronize { @odp_enabled.clone } - end - - # Enable/disable the odp integration - # - # @param odp_enabled - bool - - def odp_enabled=(odp_enabled) - @mutex.synchronize { @odp_enabled = odp_enabled.clone } + def odp_integrated? + @mutex.synchronize { !@api_key.nil? && !@api_host.nil? } end end end diff --git a/lib/optimizely/odp/odp_segment_manager.rb b/lib/optimizely/odp/odp_segment_manager.rb index 88c9df71..345ee64c 100644 --- a/lib/optimizely/odp/odp_segment_manager.rb +++ b/lib/optimizely/odp/odp_segment_manager.rb @@ -24,51 +24,55 @@ class OdpSegmentManager # Schedules connections to ODP for audience segmentation and caches the results attr_reader :odp_config, :segments_cache, :zaius_manager, :logger - def initialize(cache_size, cache_timeout_in_secs, odp_config, api_manager = nil, logger = nil, proxy_config = nil) + def initialize(odp_config, segments_cache, api_manager = nil, logger = nil, proxy_config = nil) @odp_config = odp_config @logger = logger || NoOpLogger.new @zaius_manager = api_manager || ZaiusGraphQLApiManager.new(logger: @logger, proxy_config: proxy_config) - @segments_cache = Optimizely::LRUCache.new(cache_size, cache_timeout_in_secs) + @segments_cache = segments_cache end - def fetch_qualified_segments(segment_request) - odp_api_key = @odp_config&.api_key - odp_api_host = @odp_config&.api_host - - unless odp_api_host && odp_api_key - @logger.log(Logger::ERROR, 'api_key/api_host not defined') - segment_request.segments = nil - return + # Returns qualified segments for the user from the cache or the ODP server if not in the cache. + # + # @param user_key - The key for identifying the id type. + # @param user_value - The id itself. + # @param options - An array of OptimizelySegmentOptions used to ignore and/or reset the cache. + # + # @return - Array of qualified segments. + def fetch_qualified_segments(user_key, user_value, options) + unless @odp_config.odp_integrated? + @logger.log(Logger::ERROR, format(Optimizely::Helpers::Constants::ODP_LOGS[:FETCH_SEGMENTS_FAILED], 'ODP is not enabled')) + return nil end + + odp_api_key = @odp_config.api_key + odp_api_host = @odp_config.api_host segments_to_check = @odp_config&.segments_to_check unless segments_to_check&.size&.positive? - segment_request.segments = [] - return + @logger.log(Logger::DEBUG, 'No segments are used in the project. Returning empty list') + return [] end - cache_key = make_cache_key(segment_request.user_key, segment_request.user_value) + cache_key = make_cache_key(user_key, user_value) - ignore_cache = segment_request.options.include?(OptimizelySegmentOption::IGNORE_CACHE) - reset_cache = segment_request.options.include?(OptimizelySegmentOption::RESET_CACHE) + ignore_cache = options.include?(OptimizelySegmentOption::IGNORE_CACHE) + reset_cache = options.include?(OptimizelySegmentOption::RESET_CACHE) reset if reset_cache unless ignore_cache || reset_cache segments = @segments_cache.lookup(cache_key) unless segments.nil? - segment_request.segments = segments - return + @logger.log(Logger::DEBUG, 'ODP cache hit. Returning segments from cache.') + return segments end end - Thread.new do - segments = @zaius_manager.fetch_segments(odp_api_key, odp_api_host, segment_request.user_key, segment_request.user_value, segments_to_check) - @segments_cache.save(cache_key, segments) unless segments.nil? || ignore_cache - segment_request.segments = segments - end + @logger.log(Logger::DEBUG, 'ODP cache miss. Making a call to ODP server.') - nil + segments = @zaius_manager.fetch_segments(odp_api_key, odp_api_host, user_key, user_value, segments_to_check) + @segments_cache.save(cache_key, segments) unless segments.nil? || ignore_cache + segments end def reset @@ -84,37 +88,8 @@ def make_cache_key(user_key, user_value) end class OptimizelySegmentOption + # Options for the OdpSegmentManager IGNORE_CACHE = :IGNORE_CACHE RESET_CACHE = :RESET_CACHE end - - class OdpSegmentRequest - # Allows asynchronous communication between OptimizelyUserContext and OdpSegmentManger - attr_reader :user_key, :user_value, :options - - def initialize(user_key, user_value, options) - @user_key = user_key - @user_value = user_value - @options = options - @queue = Thread::SizedQueue.new(1) - @segments = nil - end - - # If this method is called without a corresponding call to segments=, it will wait indefinitely - def wait_for_segments - return @segments if @queue.closed? - - @segments = @queue.pop - @queue.close - @segments - end - - def segments=(segments) - if @queue.closed? - @segments = segments - return - end - @queue.push(segments, non_block: true) - end - end end diff --git a/spec/odp/odp_segment_manager_spec.rb b/spec/odp/odp_segment_manager_spec.rb index 21a74cdf..7de2598e 100644 --- a/spec/odp/odp_segment_manager_spec.rb +++ b/spec/odp/odp_segment_manager_spec.rb @@ -26,6 +26,7 @@ let(:user_value) { 'test-user-value' } let(:api_key) { 'test-api-key' } let(:segments_to_check) { %w[a b c] } + let(:segments_cache) { Optimizely::LRUCache.new(1000, 1000) } let(:good_response_data) do { data: { @@ -65,14 +66,14 @@ config = Optimizely::OdpConfig.new api_manager = Optimizely::ZaiusGraphQLApiManager.new - segment_manager = Optimizely::OdpSegmentManager.new(0, 0, config, api_manager, spy_logger) + segment_manager = Optimizely::OdpSegmentManager.new(config, segments_cache, api_manager, spy_logger) expect(segment_manager.segments_cache).to be_a Optimizely::LRUCache expect(segment_manager.odp_config).to be config expect(segment_manager.zaius_manager).to be api_manager expect(segment_manager.logger).to be spy_logger - segment_manager = Optimizely::OdpSegmentManager.new(0, 0, config) + segment_manager = Optimizely::OdpSegmentManager.new(config, segments_cache) expect(segment_manager.logger).to be_a Optimizely::NoOpLogger expect(segment_manager.zaius_manager).to be_a Optimizely::ZaiusGraphQLApiManager end @@ -88,15 +89,12 @@ .to_return(status: 200, body: good_response_data) odp_config = Optimizely::OdpConfig.new(api_key, api_host, segments_to_check) - segment_manager = Optimizely::OdpSegmentManager.new(1000, 1000, odp_config, nil, spy_logger) - segment_request = Optimizely::OdpSegmentRequest.new(user_key, user_value, []) + segment_manager = Optimizely::OdpSegmentManager.new(odp_config, segments_cache, nil, spy_logger) - expect(segment_manager.zaius_manager).not_to receive(:log_failure) + segments = segment_manager.fetch_qualified_segments(user_key, user_value, []) - segment_manager.fetch_qualified_segments(segment_request) - - expect(segment_request.wait_for_segments).to match_array(%w[a b]) - expect(spy_logger).not_to have_received(:log) + expect(segments).to match_array(%w[a b]) + expect(spy_logger).not_to have_received(:log).with(Logger::ERROR, anything) end it 'should return empty array with no segments to check' do @@ -104,15 +102,12 @@ .to_return(status: 200, body: good_response_data) odp_config = Optimizely::OdpConfig.new(api_key, api_host, []) - segment_manager = Optimizely::OdpSegmentManager.new(1000, 1000, odp_config, nil, spy_logger) - segment_request = Optimizely::OdpSegmentRequest.new(user_key, user_value, []) - - expect(segment_manager.zaius_manager).not_to receive(:log_failure) + segment_manager = Optimizely::OdpSegmentManager.new(odp_config, segments_cache, nil, spy_logger) - segment_manager.fetch_qualified_segments(segment_request) + segments = segment_manager.fetch_qualified_segments(user_key, user_value, []) - expect(segment_request.wait_for_segments).to match_array([]) - expect(spy_logger).not_to have_received(:log) + expect(segments).to match_array([]) + expect(spy_logger).not_to have_received(:log).with(Logger::ERROR, anything) end it 'should return success with cache miss' do @@ -120,51 +115,42 @@ .to_return(status: 200, body: good_response_data) odp_config = Optimizely::OdpConfig.new(api_key, api_host, %w[a b c]) - segment_manager = Optimizely::OdpSegmentManager.new(1000, 1000, odp_config, nil, spy_logger) - - expect(segment_manager.zaius_manager).not_to receive(:log_failure) + segment_manager = Optimizely::OdpSegmentManager.new(odp_config, segments_cache, nil, spy_logger) cache_key = segment_manager.send(:make_cache_key, user_key, '123') segment_manager.segments_cache.save(cache_key, %w[d]) - segment_request = Optimizely::OdpSegmentRequest.new(user_key, user_value, []) + segments = segment_manager.fetch_qualified_segments(user_key, user_value, []) - segment_manager.fetch_qualified_segments(segment_request) - - expect(segment_request.wait_for_segments).to match_array(%w[a b]) + expect(segments).to match_array(%w[a b]) actual_cache_key = segment_manager.send(:make_cache_key, user_key, user_value) expect(segment_manager.segments_cache.lookup(actual_cache_key)).to match_array(%w[a b]) - expect(spy_logger).not_to have_received(:log) + expect(spy_logger).not_to have_received(:log).with(Logger::ERROR, anything) end it 'should return success with cache hit' do odp_config = Optimizely::OdpConfig.new odp_config.update(api_key, api_host, %w[a b c]) - segment_manager = Optimizely::OdpSegmentManager.new(1000, 1000, odp_config, nil, spy_logger) - - expect(segment_manager.zaius_manager).not_to receive(:log_failure) + segment_manager = Optimizely::OdpSegmentManager.new(odp_config, segments_cache, nil, spy_logger) cache_key = segment_manager.send(:make_cache_key, user_key, user_value) segment_manager.segments_cache.save(cache_key, %w[c]) - segment_request = Optimizely::OdpSegmentRequest.new(user_key, user_value, []) - - segment_manager.fetch_qualified_segments(segment_request) + segments = segment_manager.fetch_qualified_segments(user_key, user_value, []) - expect(segment_request.wait_for_segments).to match_array(%w[c]) - expect(spy_logger).not_to have_received(:log) + expect(segments).to match_array(%w[c]) + expect(spy_logger).not_to have_received(:log).with(Logger::ERROR, anything) end it 'should return nil and log error with missing api_host/api_key' do odp_config = Optimizely::OdpConfig.new - segment_manager = Optimizely::OdpSegmentManager.new(1000, 1000, odp_config, nil, spy_logger) - segment_request = Optimizely::OdpSegmentRequest.new(user_key, user_value, []) + segment_manager = Optimizely::OdpSegmentManager.new(odp_config, segments_cache, nil, spy_logger) - segment_manager.fetch_qualified_segments(segment_request) + segments = segment_manager.fetch_qualified_segments(user_key, user_value, []) - expect(segment_request.wait_for_segments).to be_nil - expect(spy_logger).to have_received(:log).with(Logger::ERROR, 'api_key/api_host not defined') + expect(segments).to be_nil + expect(spy_logger).to have_received(:log).with(Logger::ERROR, 'Audience segments fetch failed (ODP is not enabled).') end it 'should return nil with network error' do @@ -172,14 +158,12 @@ .to_return(status: 500, body: '{}') odp_config = Optimizely::OdpConfig.new(api_key, api_host, segments_to_check) - segment_manager = Optimizely::OdpSegmentManager.new(1000, 1000, odp_config) - segment_request = Optimizely::OdpSegmentRequest.new(user_key, user_value, []) + segment_manager = Optimizely::OdpSegmentManager.new(odp_config, segments_cache, nil, spy_logger) - expect(segment_manager.zaius_manager).to receive(:log_failure) + segments = segment_manager.fetch_qualified_segments(user_key, user_value, []) - segment_manager.fetch_qualified_segments(segment_request) - - expect(segment_request.wait_for_segments).to be_nil + expect(segments).to be_nil + expect(spy_logger).to have_received(:log).with(Logger::ERROR, 'Audience segments fetch failed (500).') end it 'should return non cached value with ignore cache' do @@ -187,20 +171,16 @@ .to_return(status: 200, body: good_response_data) odp_config = Optimizely::OdpConfig.new(api_key, api_host, %w[a b c]) - segment_manager = Optimizely::OdpSegmentManager.new(1000, 1000, odp_config, nil, spy_logger) - - expect(segment_manager.zaius_manager).not_to receive(:log_failure) + segment_manager = Optimizely::OdpSegmentManager.new(odp_config, segments_cache, nil, spy_logger) cache_key = segment_manager.send(:make_cache_key, user_key, user_value) segment_manager.segments_cache.save(cache_key, %w[d]) - segment_request = Optimizely::OdpSegmentRequest.new(user_key, user_value, [Optimizely::OptimizelySegmentOption::IGNORE_CACHE]) - - segment_manager.fetch_qualified_segments(segment_request) + segments = segment_manager.fetch_qualified_segments(user_key, user_value, [Optimizely::OptimizelySegmentOption::IGNORE_CACHE]) - expect(segment_request.wait_for_segments).to match_array(%w[a b]) + expect(segments).to match_array(%w[a b]) expect(segment_manager.segments_cache.lookup(cache_key)).to match_array(%w[d]) - expect(spy_logger).not_to have_received(:log) + expect(spy_logger).not_to have_received(:log).with(Logger::ERROR, anything) end it 'should reset cache and return non cached value with reset cache' do @@ -208,68 +188,24 @@ .to_return(status: 200, body: good_response_data) odp_config = Optimizely::OdpConfig.new(api_key, api_host, %w[a b c]) - segment_manager = Optimizely::OdpSegmentManager.new(1000, 1000, odp_config, nil, spy_logger) - - expect(segment_manager.zaius_manager).not_to receive(:log_failure) + segment_manager = Optimizely::OdpSegmentManager.new(odp_config, segments_cache, nil, spy_logger) cache_key = segment_manager.send(:make_cache_key, user_key, user_value) segment_manager.segments_cache.save(cache_key, %w[d]) segment_manager.segments_cache.save('123', %w[c d]) - segment_request = Optimizely::OdpSegmentRequest.new(user_key, user_value, [Optimizely::OptimizelySegmentOption::RESET_CACHE]) - - segment_manager.fetch_qualified_segments(segment_request) + segments = segment_manager.fetch_qualified_segments(user_key, user_value, [Optimizely::OptimizelySegmentOption::RESET_CACHE]) - expect(segment_request.wait_for_segments).to match_array(%w[a b]) + expect(segments).to match_array(%w[a b]) expect(segment_manager.segments_cache.lookup(cache_key)).to match_array(%w[a b]) expect(segment_manager.segments_cache.instance_variable_get('@map').length).to be 1 + expect(spy_logger).not_to have_received(:log).with(Logger::ERROR, anything) end it 'should make correct cache key' do - segment_manager = Optimizely::OdpSegmentManager.new(1000, 1000, nil) + segment_manager = Optimizely::OdpSegmentManager.new(nil, nil) cache_key = segment_manager.send(:make_cache_key, user_key, user_value) expect(cache_key).to be == "#{user_key}-$-#{user_value}" end - - it 'should process all segment requests quickly' do - mutex = Mutex.new - results = [] - odp_config = Optimizely::OdpConfig.new(api_key, api_host, segments_to_check) - - segment_manager = Optimizely::OdpSegmentManager.new(0, 0, odp_config, nil) - - allow(segment_manager.zaius_manager).to receive(:fetch_segments) do - # simulate slow REST query - sleep(1) - %w[a b] - end - expect(segment_manager.zaius_manager).not_to receive(:log_failure) - - thread_count = 1000 - threads = [] - - started = Time.now - thread_count.times do - threads << Thread.new do - user_key = rand(1..1000).to_s - user_value = rand(1..1000).to_s - segment_request = Optimizely::OdpSegmentRequest.new(user_key, user_value, [Optimizely::OptimizelySegmentOption::IGNORE_CACHE]) - - segment_manager.fetch_qualified_segments(segment_request) - response = segment_request.wait_for_segments - expect(response).to match_array(%w[a b]) - - mutex.synchronize do - results.push(response) - end - end - end - - threads.each(&:join) - - expect(results.length).to be == thread_count - expect(results).to all(match_array(%w[a b])) - expect(Time.now - started).to be < 10 - end end end