From 997dc00c0180c9d6f73c6e66ad32a8def424c90a Mon Sep 17 00:00:00 2001 From: Mauro Sanz Date: Mon, 22 Nov 2021 14:45:52 -0300 Subject: [PATCH 1/3] removing sdk_blocker and using status manger --- lib/splitclient-rb.rb | 3 +- .../cache/fetchers/segment_fetcher.rb | 12 +-- .../cache/fetchers/split_fetcher.rb | 9 +-- .../cache/stores/localhost_split_store.rb | 9 +-- .../cache/stores/sdk_blocker.rb | 64 ---------------- lib/splitclient-rb/clients/split_client.rb | 10 +-- lib/splitclient-rb/engine/status_manager.rb | 10 ++- lib/splitclient-rb/engine/sync_manager.rb | 74 +++++++------------ lib/splitclient-rb/engine/synchronizer.rb | 29 ++++---- lib/splitclient-rb/managers/split_manager.rb | 8 +- lib/splitclient-rb/split_factory.rb | 38 +++++----- spec/cache/fetchers/segment_fetch_spec.rb | 9 +-- spec/cache/fetchers/split_fetch_spec.rb | 4 +- spec/cache/stores/sdk_blocker_spec.rb | 46 ------------ spec/engine/matchers/between_matcher_spec.rb | 16 +++- spec/engine/matchers/equal_to_matcher_spec.rb | 8 ++ .../greater_than_or_equal_to_matcher_spec.rb | 7 ++ .../less_than_or_equal_to_matcher_spec.rb | 7 ++ .../engine/matchers/whitelist_matcher_spec.rb | 2 + spec/engine/push_manager_spec.rb | 7 +- spec/engine/status_manager_spec.rb | 4 +- spec/engine/sync_manager_spec.rb | 25 +++---- spec/engine/synchronizer_spec.rb | 36 +++++++-- spec/engine_spec.rb | 17 +++++ spec/integrations/dedupe_impression_spec.rb | 1 + spec/splitclient/split_manager_spec.rb | 25 +++---- spec/splitclient_rb_corner_cases_spec.rb | 2 +- spec/sse/sse_handler_spec.rb | 7 +- spec/sse/workers/segments_worker_spec.rb | 7 +- spec/sse/workers/splits_worker_spec.rb | 7 +- 30 files changed, 217 insertions(+), 286 deletions(-) delete mode 100644 lib/splitclient-rb/cache/stores/sdk_blocker.rb delete mode 100644 spec/cache/stores/sdk_blocker_spec.rb diff --git a/lib/splitclient-rb.rb b/lib/splitclient-rb.rb index d61008e65..f12280772 100644 --- a/lib/splitclient-rb.rb +++ b/lib/splitclient-rb.rb @@ -28,10 +28,9 @@ require 'splitclient-rb/cache/senders/events_sender' require 'splitclient-rb/cache/senders/impressions_count_sender' require 'splitclient-rb/cache/senders/localhost_repo_cleaner' -require 'splitclient-rb/cache/stores/store_utils' require 'splitclient-rb/cache/stores/localhost_split_builder' -require 'splitclient-rb/cache/stores/sdk_blocker' require 'splitclient-rb/cache/stores/localhost_split_store' +require 'splitclient-rb/cache/stores/store_utils' require 'splitclient-rb/clients/split_client' require 'splitclient-rb/managers/split_manager' diff --git a/lib/splitclient-rb/cache/fetchers/segment_fetcher.rb b/lib/splitclient-rb/cache/fetchers/segment_fetcher.rb index efd24bd2b..8c618e893 100644 --- a/lib/splitclient-rb/cache/fetchers/segment_fetcher.rb +++ b/lib/splitclient-rb/cache/fetchers/segment_fetcher.rb @@ -4,11 +4,10 @@ module Fetchers class SegmentFetcher attr_reader :segments_repository - def initialize(segments_repository, api_key, config, sdk_blocker, telemetry_runtime_producer) + def initialize(segments_repository, api_key, config, telemetry_runtime_producer) @segments_repository = segments_repository @api_key = api_key @config = config - @sdk_blocker = sdk_blocker @semaphore = Mutex.new @telemetry_runtime_producer = telemetry_runtime_producer end @@ -52,11 +51,11 @@ def fetch_segments @semaphore.synchronize do segments_api.fetch_segments_by_names(@segments_repository.used_segment_names) - @sdk_blocker.segments_ready! - @sdk_blocker.sdk_internal_ready + true end rescue StandardError => error @config.log_found_exception(__method__.to_s, error) + false end def stop_segments_thread @@ -70,11 +69,6 @@ def segments_thread @config.logger.info('Starting segments fetcher service') if @config.debug_enabled loop do - unless @sdk_blocker.splits_repository.ready? - sleep 0.2 - next - end - fetch_segments @config.logger.debug("Segment names: #{@segments_repository.used_segment_names.to_a}") if @config.debug_enabled diff --git a/lib/splitclient-rb/cache/fetchers/split_fetcher.rb b/lib/splitclient-rb/cache/fetchers/split_fetcher.rb index 51534bab8..92c33b4d6 100644 --- a/lib/splitclient-rb/cache/fetchers/split_fetcher.rb +++ b/lib/splitclient-rb/cache/fetchers/split_fetcher.rb @@ -4,11 +4,10 @@ module Fetchers class SplitFetcher attr_reader :splits_repository - def initialize(splits_repository, api_key, config, sdk_blocker, telemetry_runtime_producer) + def initialize(splits_repository, api_key, config, telemetry_runtime_producer) @splits_repository = splits_repository @api_key = api_key @config = config - @sdk_blocker = sdk_blocker @semaphore = Mutex.new @telemetry_runtime_producer = telemetry_runtime_producer end @@ -40,13 +39,11 @@ def fetch_splits(fetch_options = { cache_control_headers: false, till: nil }) @config.logger.debug("segments seen(#{data[:segment_names].length}): #{data[:segment_names].to_a}") if @config.debug_enabled - @sdk_blocker.splits_ready! - - data[:segment_names] + { segment_names: data[:segment_names], success: true } end rescue StandardError => error @config.log_found_exception(__method__.to_s, error) - [] + { segment_names: [], success: false } end def stop_splits_thread diff --git a/lib/splitclient-rb/cache/stores/localhost_split_store.rb b/lib/splitclient-rb/cache/stores/localhost_split_store.rb index 4c1cde560..1b7071bf7 100644 --- a/lib/splitclient-rb/cache/stores/localhost_split_store.rb +++ b/lib/splitclient-rb/cache/stores/localhost_split_store.rb @@ -7,10 +7,10 @@ class LocalhostSplitStore require 'yaml' attr_reader :splits_repository - def initialize(splits_repository, config, sdk_blocker = nil) + def initialize(splits_repository, config, status_manager = nil) @splits_repository = splits_repository @config = config - @sdk_blocker = sdk_blocker + @status_manager = status_manager end def call @@ -45,10 +45,7 @@ def store_splits store_split(split) end - if @sdk_blocker - @sdk_blocker.splits_ready! - @sdk_blocker.segments_ready! - end + @status_manager.ready! if @status_manager rescue StandardError => error @config.logger.error('Error while parsing the split file. ' \ 'Check that the input file matches the expected format') diff --git a/lib/splitclient-rb/cache/stores/sdk_blocker.rb b/lib/splitclient-rb/cache/stores/sdk_blocker.rb deleted file mode 100644 index 7a71dbc05..000000000 --- a/lib/splitclient-rb/cache/stores/sdk_blocker.rb +++ /dev/null @@ -1,64 +0,0 @@ -require 'thread' -require 'timeout' - -module SplitIoClient - module Cache - module Stores - class SDKBlocker - attr_reader :splits_repository - - def initialize(splits_repository, segments_repository, config) - @splits_repository = splits_repository - @segments_repository = segments_repository - @config = config - @internal_ready = Concurrent::CountDownLatch.new(1) - - if @config.standalone? - @splits_repository.not_ready! - @segments_repository.not_ready! - end - end - - def splits_ready! - if !ready? - @splits_repository.ready! - @config.logger.info('splits are ready') - end - end - - def segments_ready! - if !ready? - @segments_repository.ready! - @config.logger.info('segments are ready') - end - end - - def block(time = nil) - begin - timeout = time || @config.block_until_ready - Timeout::timeout(timeout) do - sleep 0.1 until ready? - end - rescue Timeout::Error - fail SDKBlockerTimeoutExpiredException, 'SDK start up timeout expired' - end - - @config.logger.info('SplitIO SDK is ready') - end - - def ready? - return true if @config.consumer? - @splits_repository.ready? && @segments_repository.ready? - end - - def sdk_internal_ready - @internal_ready.count_down - end - - def wait_unitil_internal_ready - @internal_ready.wait - end - end - end - end -end diff --git a/lib/splitclient-rb/clients/split_client.rb b/lib/splitclient-rb/clients/split_client.rb index e72661b5b..43ed6f3ae 100644 --- a/lib/splitclient-rb/clients/split_client.rb +++ b/lib/splitclient-rb/clients/split_client.rb @@ -14,13 +14,13 @@ class SplitClient # @param api_key [String] the API key for your split account # # @return [SplitIoClient] split.io client instance - def initialize(api_key, repositories, sdk_blocker, config, impressions_manager, telemetry_evaluation_producer) + def initialize(api_key, repositories, status_manager, config, impressions_manager, telemetry_evaluation_producer) @api_key = api_key @splits_repository = repositories[:splits] @segments_repository = repositories[:segments] @impressions_repository = repositories[:impressions] @events_repository = repositories[:events] - @sdk_blocker = sdk_blocker + @status_manager = status_manager @destroyed = false @config = config @impressions_manager = impressions_manager @@ -137,7 +137,7 @@ def parsed_treatment(multiple, treatment_data) else { treatment: treatment_data[:treatment], - config: treatment_data[:config] + config: treatment_data[:config], } end end @@ -157,7 +157,7 @@ def sanitize_split_names(calling_method, split_names) end def block_until_ready(time = nil) - @sdk_blocker.block(time) if @sdk_blocker && !@sdk_blocker.ready? + @status_manager.wait_until_ready(time) if @status_manager end private @@ -310,7 +310,7 @@ def variable_size(value) end def ready? - return @sdk_blocker.ready? if @sdk_blocker + return @status_manager.ready? if @status_manager true end diff --git a/lib/splitclient-rb/engine/status_manager.rb b/lib/splitclient-rb/engine/status_manager.rb index ed4dbdb01..c0115ffe0 100644 --- a/lib/splitclient-rb/engine/status_manager.rb +++ b/lib/splitclient-rb/engine/status_manager.rb @@ -9,6 +9,8 @@ def initialize(config) end def ready? + return true if @config.consumer? + @sdk_ready.wait(0) end @@ -19,8 +21,12 @@ def ready! @config.logger.info('SplitIO SDK is ready') end - def wait_until_ready(seconds) - @sdk_ready.wait(seconds) + def wait_until_ready(seconds = nil) + return if @config.consumer? + + timeout = seconds || @config.block_until_ready + + raise SDKBlockerTimeoutExpiredException, 'SDK start up timeout expired' unless @sdk_ready.wait(timeout) end end end diff --git a/lib/splitclient-rb/engine/sync_manager.rb b/lib/splitclient-rb/engine/sync_manager.rb index 2af9789bd..eab05728b 100644 --- a/lib/splitclient-rb/engine/sync_manager.rb +++ b/lib/splitclient-rb/engine/sync_manager.rb @@ -12,8 +12,8 @@ def initialize( config, synchronizer, telemetry_runtime_producer, - sdk_blocker, - telemetry_synchronizer + telemetry_synchronizer, + status_manager ) @synchronizer = synchronizer notification_manager_keeper = SSE::NotificationManagerKeeper.new(config, telemetry_runtime_producer) do |manager| @@ -33,56 +33,39 @@ def initialize( @sse_connected = Concurrent::AtomicBoolean.new(false) @config = config @telemetry_runtime_producer = telemetry_runtime_producer - @sdk_blocker = sdk_blocker @telemetry_synchronizer = telemetry_synchronizer + @status_manager = status_manager end def start - if @config.streaming_enabled - start_stream - start_stream_forked if defined?(PhusionPassenger) - elsif @config.standalone? - start_poll - end + @config.threads[:start_sdk] = Thread.new do + sleep(0.5) until @synchronizer.sync_all(false) - synchronize_telemetry_config - end + @status_manager.ready! + @telemetry_synchronizer.synchronize_config + @synchronizer.start_periodic_data_recording - private + if @config.streaming_enabled + @config.logger.debug('Starting Straming mode ...') - # Starts tasks if stream is enabled. - def start_stream - @config.logger.debug('Starting push mode ...') - @synchronizer.sync_all - @synchronizer.start_periodic_data_recording + connected = @push_manager.start_sse - start_sse_connection_thread - end + if defined?(PhusionPassenger) + PhusionPassenger.on_event(:starting_worker_process) do |forked| + sse_thread_forked if forked + end + end - def start_poll - @config.logger.debug('Starting polling mode ...') - @synchronizer.start_periodic_fetch - @synchronizer.start_periodic_data_recording - record_telemetry(Telemetry::Domain::Constants::SYNC_MODE, SYNC_MODE_POLLING) - rescue StandardError => e - @config.logger.error("start_poll error : #{e.inspect}") - end - - # Starts thread which connect to sse and after that fetch splits and segments once. - def start_sse_connection_thread - @config.threads[:sync_manager_start_sse] = Thread.new do - begin - connected = @push_manager.start_sse - @synchronizer.start_periodic_fetch unless connected - rescue StandardError => e - @config.logger.error("start_sse_connection_thread error : #{e.inspect}") + return if connected end + + @config.logger.debug('Starting polling mode ...') + @synchronizer.start_periodic_fetch + record_telemetry(Telemetry::Domain::Constants::SYNC_MODE, SYNC_MODE_POLLING) end end - def start_stream_forked - PhusionPassenger.on_event(:starting_worker_process) { |forked| start_stream if forked } - end + private def process_action(action) case action @@ -165,16 +148,9 @@ def record_telemetry(type, data) @telemetry_runtime_producer.record_streaming_event(type, data) end - def synchronize_telemetry_config - @config.threads[:telemetry_config_sender] = Thread.new do - begin - @sdk_blocker.wait_unitil_internal_ready unless @config.consumer? - @telemetry_synchronizer.synchronize_config - rescue SplitIoClient::SDKShutdownException - @telemetry_synchronizer.synchronize_config - @config.logger.info('Posting Telemetry config due to shutdown') - end - end + def sse_thread_forked + connected = @push_manager.start_sse + @synchronizer.start_periodic_fetch unless connected end end end diff --git a/lib/splitclient-rb/engine/synchronizer.rb b/lib/splitclient-rb/engine/synchronizer.rb index bbaae13ae..56e107ab8 100644 --- a/lib/splitclient-rb/engine/synchronizer.rb +++ b/lib/splitclient-rb/engine/synchronizer.rb @@ -14,7 +14,6 @@ def initialize( repositories, api_key, config, - sdk_blocker, params ) @splits_repository = repositories[:splits] @@ -23,7 +22,6 @@ def initialize( @events_repository = repositories[:events] @api_key = api_key @config = config - @sdk_blocker = sdk_blocker @split_fetcher = params[:split_fetcher] @segment_fetcher = params[:segment_fetcher] @impressions_api = SplitIoClient::Api::Impressions.new(@api_key, @config, params[:telemetry_runtime_producer]) @@ -31,12 +29,14 @@ def initialize( @telemetry_synchronizer = params[:telemetry_synchronizer] end - def sync_all + def sync_all(asynchronous = true) + return sync_splits_and_segments unless asynchronous + @config.threads[:sync_all_thread] = Thread.new do - @config.logger.debug('Synchronizing Splits and Segments ...') if @config.debug_enabled - @split_fetcher.fetch_splits - @segment_fetcher.fetch_segments + sync_splits_and_segments end + + true end def start_periodic_data_recording @@ -156,20 +156,16 @@ def attempt_splits_sync(target_cn, fetch_options, max_retries, retry_delay_secon loop do remaining_attempts -= 1 - segment_names = @split_fetcher.fetch_splits(fetch_options) + result = @split_fetcher.fetch_splits(fetch_options) - return sync_result(true, remaining_attempts, segment_names) if target_cn <= @splits_repository.get_change_number - return sync_result(false, remaining_attempts, segment_names) if remaining_attempts <= 0 + return sync_result(true, remaining_attempts, result[:segment_names]) if target_cn <= @splits_repository.get_change_number + return sync_result(false, remaining_attempts, result[:segment_names]) if remaining_attempts <= 0 delay = with_backoff ? backoff.interval : retry_delay_seconds sleep(delay) end end - def fetch_segments - @segment_fetcher.fetch_segments - end - # Starts thread which loops constantly and sends impressions to the Split API def impressions_sender ImpressionsSender.new(@impressions_repository, @config, @impressions_api).call @@ -192,6 +188,13 @@ def start_telemetry_sync_task def sync_result(success, remaining_attempts, segment_names = nil) { success: success, remaining_attempts: remaining_attempts, segment_names: segment_names } end + + def sync_splits_and_segments + @config.logger.debug('Synchronizing Splits and Segments ...') if @config.debug_enabled + splits_result = @split_fetcher.fetch_splits + + splits_result[:success] && @segment_fetcher.fetch_segments + end end end end diff --git a/lib/splitclient-rb/managers/split_manager.rb b/lib/splitclient-rb/managers/split_manager.rb index 841ea3438..ea79ab6d5 100644 --- a/lib/splitclient-rb/managers/split_manager.rb +++ b/lib/splitclient-rb/managers/split_manager.rb @@ -4,9 +4,9 @@ class SplitManager # Creates a new split manager instance that connects to split.io API. # # @return [SplitIoManager] split.io client instance - def initialize(splits_repository = nil, sdk_blocker, config) + def initialize(splits_repository = nil, status_manager, config) @splits_repository = splits_repository - @sdk_blocker = sdk_blocker + @status_manager = status_manager @config = config end @@ -78,7 +78,7 @@ def split(split_name) end def block_until_ready(time = nil) - @sdk_blocker.block(time) if @sdk_blocker && !@sdk_blocker.ready? + @status_manager.wait_until_ready(time) if @status_manager end private @@ -111,7 +111,7 @@ def build_split_view(name, split) # move to blocker, alongside block until ready to avoid duplication def ready? - return @sdk_blocker.ready? if @sdk_blocker + return @status_manager.ready? if @status_manager true end end diff --git a/lib/splitclient-rb/split_factory.rb b/lib/splitclient-rb/split_factory.rb index 23003e1eb..72771950b 100644 --- a/lib/splitclient-rb/split_factory.rb +++ b/lib/splitclient-rb/split_factory.rb @@ -34,16 +34,16 @@ def initialize(api_key, config_hash = {}) @segments_repository = SegmentsRepository.new(@config) @impressions_repository = ImpressionsRepository.new(@config) @events_repository = EventsRepository.new(@config, @api_key, @runtime_producer) - @sdk_blocker = SDKBlocker.new(@splits_repository, @segments_repository, @config) @impression_counter = SplitIoClient::Engine::Common::ImpressionCounter.new @impressions_manager = SplitIoClient::Engine::Common::ImpressionManager.new(@config, @impressions_repository, @impression_counter, @runtime_producer) @telemetry_api = SplitIoClient::Api::TelemetryApi.new(@config, @api_key, @runtime_producer) @telemetry_synchronizer = Telemetry::Synchronizer.new(@config, @telemetry_consumers, @init_producer, repositories, @telemetry_api) + @status_manager = Engine::StatusManager.new(@config) start! - @client = SplitClient.new(@api_key, repositories, @sdk_blocker, @config, @impressions_manager, @evaluation_producer) - @manager = SplitManager.new(@splits_repository, @sdk_blocker, @config) + @client = SplitClient.new(@api_key, repositories, @status_manager, @config, @impressions_manager, @evaluation_producer) + @manager = SplitManager.new(@splits_repository, @status_manager, @config) validate_api_key @@ -51,22 +51,20 @@ def initialize(api_key, config_hash = {}) end def start! - if @config.localhost_mode - start_localhost_components - else - split_fetcher = SplitFetcher.new(@splits_repository, @api_key, config, @sdk_blocker, @runtime_producer) - segment_fetcher = SegmentFetcher.new(@segments_repository, @api_key, config, @sdk_blocker, @runtime_producer) - params = { - split_fetcher: split_fetcher, - segment_fetcher: segment_fetcher, - imp_counter: @impression_counter, - telemetry_runtime_producer: @runtime_producer, - telemetry_synchronizer: @telemetry_synchronizer - } - - synchronizer = SplitIoClient::Engine::Synchronizer.new(repositories, @api_key, @config, @sdk_blocker, params) - SplitIoClient::Engine::SyncManager.new(repositories, @api_key, @config, synchronizer, @runtime_producer, @sdk_blocker, @telemetry_synchronizer).start - end + return start_localhost_components if @config.localhost_mode + + split_fetcher = SplitFetcher.new(@splits_repository, @api_key, config, @runtime_producer) + segment_fetcher = SegmentFetcher.new(@segments_repository, @api_key, config, @runtime_producer) + params = { + split_fetcher: split_fetcher, + segment_fetcher: segment_fetcher, + imp_counter: @impression_counter, + telemetry_runtime_producer: @runtime_producer, + telemetry_synchronizer: @telemetry_synchronizer + } + + synchronizer = SplitIoClient::Engine::Synchronizer.new(repositories, @api_key, @config, params) + SplitIoClient::Engine::SyncManager.new(repositories, @api_key, @config, synchronizer, @runtime_producer, @telemetry_synchronizer, @status_manager).start end def stop! @@ -145,7 +143,7 @@ def repositories end def start_localhost_components - LocalhostSplitStore.new(@splits_repository, @config, @sdk_blocker).call + LocalhostSplitStore.new(@splits_repository, @config, @status_manager).call # Starts thread which loops constantly and cleans up repositories to avoid memory issues in localhost mode LocalhostRepoCleaner.new(@impressions_repository, @events_repository, @config).call diff --git a/spec/cache/fetchers/segment_fetch_spec.rb b/spec/cache/fetchers/segment_fetch_spec.rb index fcf47d70e..a8016075d 100644 --- a/spec/cache/fetchers/segment_fetch_spec.rb +++ b/spec/cache/fetchers/segment_fetch_spec.rb @@ -35,10 +35,9 @@ let(:segments_repository) { SplitIoClient::Cache::Repositories::SegmentsRepository.new(config) } let(:splits_repository) { SplitIoClient::Cache::Repositories::SplitsRepository.new(config) } let(:telemetry_runtime_producer) { SplitIoClient::Telemetry::RuntimeProducer.new(config) } - let(:sdk_blocker) { SplitIoClient::Cache::Stores::SDKBlocker.new(splits_repository, segments_repository, config) } - let(:segment_fetcher) { described_class.new(segments_repository, '', config, sdk_blocker, telemetry_runtime_producer) } + let(:segment_fetcher) { described_class.new(segments_repository, '', config, telemetry_runtime_producer) } let(:split_fetcher) do - SplitIoClient::Cache::Fetchers::SplitFetcher.new(splits_repository, '', config, sdk_blocker, telemetry_runtime_producer) + SplitIoClient::Cache::Fetchers::SplitFetcher.new(splits_repository, '', config, telemetry_runtime_producer) end it 'fetch segments' do @@ -70,9 +69,9 @@ let(:segments_repository) { SplitIoClient::Cache::Repositories::SegmentsRepository.new(config) } let(:splits_repository) { SplitIoClient::Cache::Repositories::SplitsRepository.new(config) } let(:telemetry_runtime_producer) { SplitIoClient::Telemetry::RuntimeProducer.new(config) } - let(:segment_fetcher) { described_class.new(segments_repository, '', config, nil, telemetry_runtime_producer) } + let(:segment_fetcher) { described_class.new(segments_repository, '', config, telemetry_runtime_producer) } let(:split_fetcher) do - SplitIoClient::Cache::Fetchers::SplitFetcher.new(splits_repository, '', config, nil, telemetry_runtime_producer) + SplitIoClient::Cache::Fetchers::SplitFetcher.new(splits_repository, '', config, telemetry_runtime_producer) end it 'fetch segments' do diff --git a/spec/cache/fetchers/split_fetch_spec.rb b/spec/cache/fetchers/split_fetch_spec.rb index fcf8657ee..d605cfd35 100644 --- a/spec/cache/fetchers/split_fetch_spec.rb +++ b/spec/cache/fetchers/split_fetch_spec.rb @@ -25,7 +25,7 @@ end let(:splits_repository) { SplitIoClient::Cache::Repositories::SplitsRepository.new(config) } let(:telemetry_runtime_producer) { SplitIoClient::Telemetry::RuntimeProducer.new(config) } - let(:store) { described_class.new(splits_repository, '', config, nil, telemetry_runtime_producer) } + let(:store) { described_class.new(splits_repository, '', config, telemetry_runtime_producer) } it 'returns splits since' do splits = store.send(:splits_since, -1) @@ -73,7 +73,7 @@ end let(:splits_repository) { SplitIoClient::Cache::Repositories::SplitsRepository.new(config) } let(:telemetry_runtime_producer) { SplitIoClient::Telemetry::RuntimeProducer.new(config) } - let(:store) { described_class.new(splits_repository, '', config, nil, telemetry_runtime_producer) } + let(:store) { described_class.new(splits_repository, '', config, telemetry_runtime_producer) } it 'returns splits since' do splits = store.send(:splits_since, -1) diff --git a/spec/cache/stores/sdk_blocker_spec.rb b/spec/cache/stores/sdk_blocker_spec.rb deleted file mode 100644 index 46c95559e..000000000 --- a/spec/cache/stores/sdk_blocker_spec.rb +++ /dev/null @@ -1,46 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe SplitIoClient::Cache::Stores::SDKBlocker do - RSpec.shared_examples 'SDK Blocker' do |cache_adapter| - let(:config) do - config = SplitIoClient::SplitConfig.new(cache_adapter: cache_adapter) - config.block_until_ready = 0.1 - config - end - let(:splits_repository) { SplitIoClient::Cache::Repositories::SplitsRepository.new(config) } - let(:segments_repository) { SplitIoClient::Cache::Repositories::SegmentsRepository.new(config) } - let(:sdk_blocker) { described_class.new(splits_repository, segments_repository, config) } - - before :each do - Redis.new.flushall - end - - it 'is not ready after initialization' do - sdk_blocker - - expect(splits_repository.ready?).to be(false) - end - - it 'is ready when both splits and segments are ready' do - sdk_blocker.splits_ready! - sdk_blocker.segments_ready! - - expect(sdk_blocker.ready?).to be true - end - - it 'throws exception if not ready' do - allow_any_instance_of(described_class).to receive(:ready?).and_return(false) - expect { sdk_blocker.block }.to raise_error(SplitIoClient::SDKBlockerTimeoutExpiredException) - end - end - - describe 'with Memory Adapter' do - it_behaves_like 'SDK Blocker', :memory - end - - describe 'with Redis Adapter' do - it_behaves_like 'SDK Blocker', :redis - end -end diff --git a/spec/engine/matchers/between_matcher_spec.rb b/spec/engine/matchers/between_matcher_spec.rb index d6052c1f2..4e58871ab 100644 --- a/spec/engine/matchers/between_matcher_spec.rb +++ b/spec/engine/matchers/between_matcher_spec.rb @@ -24,7 +24,6 @@ let(:feature) { 'test_feature' } let(:matching_attributes) { { income: 110 } } let(:non_matching_high_value_attributes) { { income: 121 } } - let(:missing_key_attributes) { {} } let(:nil_attributes) { nil } @@ -36,6 +35,11 @@ before do stub_request(:get, 'https://sdk.split.io/api/splitChanges?since=-1') .to_return(status: 200, body: number_matcher_splits) + + stub_request(:post, 'https://telemetry.split.io/api/v1/metrics/config') + .to_return(status: 200, body: 'ok') + + subject.block_until_ready end it 'validates the treatment is ON for correct number attribute value' do @@ -60,6 +64,11 @@ before do stub_request(:get, 'https://sdk.split.io/api/splitChanges?since=-1') .to_return(status: 200, body: negative_number_matcher_splits) + + stub_request(:post, 'https://telemetry.split.io/api/v1/metrics/config') + .to_return(status: 200, body: 'ok') + + subject.block_until_ready end it 'validates the treatment is ON for correct negative numbers attribute value' do @@ -86,6 +95,11 @@ before do stub_request(:get, 'https://sdk.split.io/api/splitChanges?since=-1') .to_return(status: 200, body: datetime_matcher_splits) + + stub_request(:post, 'https://telemetry.split.io/api/v1/metrics/config') + .to_return(status: 200, body: 'ok') + + subject.block_until_ready end it 'validates the treatment is ON for correct number attribute value' do diff --git a/spec/engine/matchers/equal_to_matcher_spec.rb b/spec/engine/matchers/equal_to_matcher_spec.rb index 3a3cbe941..8c19e9c37 100644 --- a/spec/engine/matchers/equal_to_matcher_spec.rb +++ b/spec/engine/matchers/equal_to_matcher_spec.rb @@ -40,10 +40,12 @@ end it 'validates the treatment is ON for correct attribute value' do + subject.block_until_ready expect(subject.get_treatment(user, feature, matching_attributes)).to eq 'on' end it 'validates the treatment is the default treatment for incorrect attributes hash and nil' do + subject.block_until_ready expect(subject.get_treatment(user, feature, non_matching_value_attributes)).to eq 'default' expect(subject.get_treatment(user, feature, missing_key_attributes)).to eq 'default' expect(subject.get_treatment(user, feature, nil_attributes)).to eq 'default' @@ -60,11 +62,13 @@ end it 'validates the treatment is ON for 0 and -0 attribute values' do + subject.block_until_ready expect(subject.get_treatment(user, feature, matching_zero_attributes)).to eq 'on' expect(subject.get_treatment(user, feature, matching_negative_zero_attributes)).to eq 'on' end it 'validates the treatment is the default treatment for <> 0 and -0 attribute values' do + subject.block_until_ready expect(subject.get_treatment(user, feature, non_matching_value_attributes)).to eq 'default' end end @@ -79,10 +83,12 @@ end it 'validates the treatment is on for negative attribute value' do + subject.block_until_ready expect(subject.get_treatment(user, feature, matching_negative_attributes)).to eq 'on' end it 'validates the treatment is the default treatment for negative attribute value' do + subject.block_until_ready expect(subject.get_treatment(user, feature, non_matching_negative_attributes)).to eq 'default' end end @@ -99,11 +105,13 @@ end it 'validates the treatment is ON for correct number attribute value' do + subject.block_until_ready expect(subject.get_treatment(user, feature, matching_attributes_1)).to eq 'on' expect(subject.get_treatment(user, feature, matching_attributes_2)).to eq 'on' end it 'validates the treatment is the default treatment for incorrect number attributes hash and nil' do + subject.block_until_ready expect(subject.get_treatment(user, feature, non_matching_low_value_attributes)).to eq 'default' expect(subject.get_treatment(user, feature, non_matching_high_value_attributes)).to eq 'default' expect(subject.get_treatment(user, feature, missing_key_attributes)).to eq 'default' diff --git a/spec/engine/matchers/greater_than_or_equal_to_matcher_spec.rb b/spec/engine/matchers/greater_than_or_equal_to_matcher_spec.rb index 1d415d67c..4a0b5b015 100644 --- a/spec/engine/matchers/greater_than_or_equal_to_matcher_spec.rb +++ b/spec/engine/matchers/greater_than_or_equal_to_matcher_spec.rb @@ -38,10 +38,12 @@ end it 'validates the treatment is ON for correct attribute value' do + subject.block_until_ready expect(subject.get_treatment(user, feature, matching_attributes)).to eq 'on' end it 'validates the treatment is the default treatment for incorrect attributes hash and nil' do + subject.block_until_ready expect(subject.get_treatment(user, feature, non_matching_value_attributes)).to eq 'default' expect(subject.get_treatment(user, feature, missing_key_attributes)).to eq 'default' expect(subject.get_treatment(user, feature, nil_attributes)).to eq 'default' @@ -58,14 +60,17 @@ end it 'validates the treatment is ON for correct negative attribute value' do + subject.block_until_ready expect(subject.get_treatment(user, feature, matching_negative_attributes)).to eq 'on' end it 'validates the treatment is the default treatment for incorrect negative attribute value' do + subject.block_until_ready expect(subject.get_treatment(user, feature, non_matching_negative_attributes)).to eq 'default' end it 'validates wrong formatted attribute does not match and returns default treatment' do + subject.block_until_ready expect(subject.get_treatment(user, feature, age: 'asdasd')).to eq 'default' end end @@ -82,11 +87,13 @@ end it 'validates the treatment is ON for correct attribute value' do + subject.block_until_ready expect(subject.get_treatment(user, feature, matching_attributes_1)).to eq 'on' expect(subject.get_treatment(user, feature, matching_attributes_2)).to eq 'on' end it 'validates the treatment is the default treatment for incorrect attributes hash and nil' do + subject.block_until_ready expect(subject.get_treatment(user, feature, non_matching_attributes_1)).to eq 'default' expect(subject.get_treatment(user, feature, non_matching_attributes_2)).to eq 'default' expect(subject.get_treatment(user, feature, missing_key_attributes)).to eq 'default' diff --git a/spec/engine/matchers/less_than_or_equal_to_matcher_spec.rb b/spec/engine/matchers/less_than_or_equal_to_matcher_spec.rb index 24063f0a9..858034caf 100644 --- a/spec/engine/matchers/less_than_or_equal_to_matcher_spec.rb +++ b/spec/engine/matchers/less_than_or_equal_to_matcher_spec.rb @@ -40,10 +40,12 @@ end it 'validates the treatment is ON for correct attribute value' do + subject.block_until_ready expect(subject.get_treatment(user, feature, matching_attributes)).to eq 'on' end it 'validates the treatment is the default treatment for incorrect attributes hash and nil' do + subject.block_until_ready expect(subject.get_treatment(user, feature, non_matching_value_attributes)).to eq 'default' expect(subject.get_treatment(user, feature, missing_key_attributes)).to eq 'default' expect(subject.get_treatment(user, feature, nil_attributes)).to eq 'default' @@ -60,10 +62,12 @@ end it 'validates the treatment is ON for correct negative attribute value' do + subject.block_until_ready expect(subject.get_treatment(user, feature, matching_negative_attributes)).to eq 'on' end it 'validates the treatment is the default treatment for incorrect negative attributes hash and nil' do + subject.block_until_ready expect(subject.get_treatment(user, feature, non_matching_negative_attributes)).to eq 'default' expect(subject.get_treatment(user, feature, missing_key_attributes)).to eq 'default' expect(subject.get_treatment(user, feature, nil_attributes)).to eq 'default' @@ -82,11 +86,13 @@ end it 'validates the treatment is ON for correct attribute value' do + subject.block_until_ready expect(subject.get_treatment(user, feature, matching_attributes_1)).to eq 'on' expect(subject.get_treatment(user, feature, matching_attributes_2)).to eq 'on' end it 'validates the treatment is the default treatment for incorrect attributes hash and nil' do + subject.block_until_ready expect(subject.get_treatment(user, feature, non_matching_attributes_1)).to eq 'default' expect(subject.get_treatment(user, feature, non_matching_attributes_2)).to eq 'default' expect(subject.get_treatment(user, feature, missing_key_attributes)).to eq 'default' @@ -101,6 +107,7 @@ end it 'validates the treatment is the default for wrongly formed date attribute' do + subject.block_until_ready expect(subject.get_treatment(user, 'RUBY_isOnOrBeforeDateTimeWithAttributeValueThatDoesNotMatch', join: 'fer')) .to eq 'V1' end diff --git a/spec/engine/matchers/whitelist_matcher_spec.rb b/spec/engine/matchers/whitelist_matcher_spec.rb index 3c72d6b30..7937dcee3 100644 --- a/spec/engine/matchers/whitelist_matcher_spec.rb +++ b/spec/engine/matchers/whitelist_matcher_spec.rb @@ -25,10 +25,12 @@ end it 'validates the treatment is ON for correct attribute value' do + subject.block_until_ready expect(subject.get_treatment(user, feature, matching_attributes)).to eq 'on' end it 'validates the treatment is the default treatment for incorrect attributes hash and nil' do + subject.block_until_ready expect(subject.get_treatment(user, feature, non_matching_value_attributes)).to eq 'default' expect(subject.get_treatment(user, feature, missing_key_attributes)).to eq 'default' expect(subject.get_treatment(user, feature, nil_attributes)).to eq 'default' diff --git a/spec/engine/push_manager_spec.rb b/spec/engine/push_manager_spec.rb index eb1278711..77b45e311 100644 --- a/spec/engine/push_manager_spec.rb +++ b/spec/engine/push_manager_spec.rb @@ -15,13 +15,12 @@ let(:config) { SplitIoClient::SplitConfig.new(logger: Logger.new(log)) } let(:splits_repository) { SplitIoClient::Cache::Repositories::SplitsRepository.new(config) } let(:segments_repository) { SplitIoClient::Cache::Repositories::SegmentsRepository.new(config) } - let(:sdk_blocker) { SplitIoClient::Cache::Stores::SDKBlocker.new(splits_repository, segments_repository, config) } let(:runtime_producer) { SplitIoClient::Telemetry::RuntimeProducer.new(config) } let(:split_fetcher) do - SplitIoClient::Cache::Fetchers::SplitFetcher.new(splits_repository, api_key, config, sdk_blocker, runtime_producer) + SplitIoClient::Cache::Fetchers::SplitFetcher.new(splits_repository, api_key, config, runtime_producer) end let(:segment_fetcher) do - SplitIoClient::Cache::Fetchers::SegmentFetcher.new(segments_repository, api_key, config, sdk_blocker, runtime_producer) + SplitIoClient::Cache::Fetchers::SegmentFetcher.new(segments_repository, api_key, config, runtime_producer) end let(:splits_worker) { SplitIoClient::SSE::Workers::SplitsWorker.new(split_fetcher, config, splits_repository) } let(:segments_worker) { SplitIoClient::SSE::Workers::SegmentsWorker.new(segment_fetcher, config, segments_repository) } @@ -36,7 +35,7 @@ telemetry_runtime_producer: runtime_producer } end - let(:synchronizer) { SplitIoClient::Engine::Synchronizer.new(repositories, api_key, config, sdk_blocker, params) } + let(:synchronizer) { SplitIoClient::Engine::Synchronizer.new(repositories, api_key, config, params) } context 'start_sse' do it 'must connect to server' do diff --git a/spec/engine/status_manager_spec.rb b/spec/engine/status_manager_spec.rb index 2c993d2f0..3805a21a7 100644 --- a/spec/engine/status_manager_spec.rb +++ b/spec/engine/status_manager_spec.rb @@ -25,9 +25,9 @@ it 'wait until ready - should return false' do status_manager = subject.new(config) - expect(status_manager.wait_until_ready(0.5)).to eq(false) + expect { status_manager.wait_until_ready(0.5) }.to raise_error(SplitIoClient::SplitIoError, 'SDK start up timeout expired') status_manager.ready! - expect(status_manager.wait_until_ready(0)).to eq(true) + expect { status_manager.wait_until_ready(0) }.not_to raise_error end end diff --git a/spec/engine/sync_manager_spec.rb b/spec/engine/sync_manager_spec.rb index 6134c3e90..7084eec05 100644 --- a/spec/engine/sync_manager_spec.rb +++ b/spec/engine/sync_manager_spec.rb @@ -12,10 +12,7 @@ let(:segment1) { File.read(File.join(SplitIoClient.root, 'spec/test_data/integrations/segment1.json')) } let(:segment2) { File.read(File.join(SplitIoClient.root, 'spec/test_data/integrations/segment2.json')) } let(:segment3) { File.read(File.join(SplitIoClient.root, 'spec/test_data/integrations/segment3.json')) } - let(:body_response) do - File.read(File.join(SplitIoClient.root, 'spec/test_data/integrations/auth_body_response.json')) - end - + let(:body_response) { File.read(File.join(SplitIoClient.root, 'spec/test_data/integrations/auth_body_response.json')) } let(:api_key) { 'api-key-test' } let(:log) { StringIO.new } let(:config) { SplitIoClient::SplitConfig.new(logger: Logger.new(log), streaming_enabled: true) } @@ -24,7 +21,6 @@ let(:impressions_repository) { SplitIoClient::Cache::Repositories::ImpressionsRepository.new(config) } let(:telemetry_runtime_producer) { SplitIoClient::Telemetry::RuntimeProducer.new(config) } let(:events_repository) { SplitIoClient::Cache::Repositories::EventsRepository.new(config, api_key, telemetry_runtime_producer) } - let(:sdk_blocker) { SplitIoClient::Cache::Stores::SDKBlocker.new(splits_repository, segments_repository, config) } let(:impression_counter) { SplitIoClient::Engine::Common::ImpressionCounter.new } let(:repositories) do { @@ -36,14 +32,13 @@ end let(:sync_params) do { - split_fetcher: SplitIoClient::Cache::Fetchers::SplitFetcher.new(splits_repository, api_key, config, sdk_blocker, telemetry_runtime_producer), - segment_fetcher: SplitIoClient::Cache::Fetchers::SegmentFetcher.new(segments_repository, api_key, config, sdk_blocker, telemetry_runtime_producer), + split_fetcher: SplitIoClient::Cache::Fetchers::SplitFetcher.new(splits_repository, api_key, config, telemetry_runtime_producer), + segment_fetcher: SplitIoClient::Cache::Fetchers::SegmentFetcher.new(segments_repository, api_key, config, telemetry_runtime_producer), imp_counter: impression_counter, telemetry_runtime_producer: telemetry_runtime_producer } end - let(:synchronizer) { SplitIoClient::Engine::Synchronizer.new(repositories, api_key, config, sdk_blocker, sync_params) } - + let(:synchronizer) { SplitIoClient::Engine::Synchronizer.new(repositories, api_key, config, sync_params) } let(:init_producer) { SplitIoClient::Telemetry::InitProducer.new(config) } let(:init_consumer) { SplitIoClient::Telemetry::InitConsumer.new(config) } let(:runtime_consumer) { SplitIoClient::Telemetry::RuntimeConsumer.new(config) } @@ -51,6 +46,7 @@ let(:telemetry_consumers) { { init: init_consumer, runtime: runtime_consumer, evaluation: evaluation_consumer } } let(:telemetry_api) { SplitIoClient::Api::TelemetryApi.new(config, api_key, telemetry_runtime_producer) } let(:telemetry_synchronizer) { SplitIoClient::Telemetry::Synchronizer.new(config, telemetry_consumers, init_producer, repositories, telemetry_api) } + let(:status_manager) { SplitIoClient::Engine::StatusManager.new(config) } before do mock_split_changes_with_since(splits, '-1') @@ -61,6 +57,7 @@ mock_segment_changes('segment2', segment2, '1470947453878') mock_segment_changes('segment3', segment3, '-1') stub_request(:get, config.auth_service_url).to_return(status: 200, body: body_response) + status_manager.ready! end it 'start sync manager with success sse connection.' do @@ -71,13 +68,13 @@ config.streaming_service_url = server.base_uri - sync_manager = subject.new(repositories, api_key, config, synchronizer, telemetry_runtime_producer, sdk_blocker, telemetry_synchronizer) + sync_manager = subject.new(repositories, api_key, config, synchronizer, telemetry_runtime_producer, telemetry_synchronizer, status_manager) sync_manager.start sleep(2) expect(a_request(:get, 'https://sdk.split.io/api/splitChanges?since=-1')).to have_been_made.once expect(a_request(:get, 'https://sdk.split.io/api/splitChanges?since=1506703262916')).to have_been_made.once - expect(config.threads.size).to eq(11) + expect(config.threads.size).to eq(10) end end @@ -90,13 +87,13 @@ config.streaming_service_url = 'https://fake-sse.io' config.connection_timeout = 1 - sync_manager = subject.new(repositories, api_key, config, synchronizer, telemetry_runtime_producer, sdk_blocker, telemetry_synchronizer) + sync_manager = subject.new(repositories, api_key, config, synchronizer, telemetry_runtime_producer, telemetry_synchronizer, status_manager) sync_manager.start sleep(2) expect(a_request(:get, 'https://sdk.split.io/api/splitChanges?since=-1')).to have_been_made.once expect(a_request(:get, 'https://sdk.split.io/api/splitChanges?since=1506703262916')).to have_been_made.at_least_times(1) - expect(config.threads.size).to eq(8) + expect(config.threads.size).to eq(6) end end @@ -108,7 +105,7 @@ config.streaming_service_url = server.base_uri - sync_manager = subject.new(repositories, api_key, config, synchronizer, telemetry_runtime_producer, sdk_blocker, telemetry_synchronizer) + sync_manager = subject.new(repositories, api_key, config, synchronizer, telemetry_runtime_producer, telemetry_synchronizer, status_manager) sync_manager.start sleep(2) diff --git a/spec/engine/synchronizer_spec.rb b/spec/engine/synchronizer_spec.rb index 87d4311ae..011c16161 100644 --- a/spec/engine/synchronizer_spec.rb +++ b/spec/engine/synchronizer_spec.rb @@ -17,12 +17,11 @@ let(:impressions_repository) { SplitIoClient::Cache::Repositories::ImpressionsRepository.new(config) } let(:runtime_producer) { SplitIoClient::Telemetry::RuntimeProducer.new(config) } let(:events_repository) { SplitIoClient::Cache::Repositories::EventsRepository.new(config, api_key, runtime_producer) } - let(:sdk_blocker) { SplitIoClient::Cache::Stores::SDKBlocker.new(splits_repository, segments_repository, config) } let(:split_fetcher) do - SplitIoClient::Cache::Fetchers::SplitFetcher.new(splits_repository, api_key, config, sdk_blocker, runtime_producer) + SplitIoClient::Cache::Fetchers::SplitFetcher.new(splits_repository, api_key, config, runtime_producer) end let(:segment_fetcher) do - SplitIoClient::Cache::Fetchers::SegmentFetcher.new(segments_repository, api_key, config, sdk_blocker, runtime_producer) + SplitIoClient::Cache::Fetchers::SegmentFetcher.new(segments_repository, api_key, config, runtime_producer) end let(:repositories) do repos = {} @@ -40,7 +39,7 @@ params end - let(:synchronizer) { subject.new(repositories, api_key, config, sdk_blocker, parameters) } + let(:synchronizer) { subject.new(repositories, api_key, config, parameters) } context 'tests with mock data' do before do @@ -53,8 +52,8 @@ mock_segment_changes('segment3', segment3, '1470947453879') end - it 'sync_all' do - synchronizer.sync_all + it 'sync_all asynchronous - should return true' do + result = synchronizer.sync_all sleep(2) @@ -64,6 +63,21 @@ expect(a_request(:get, 'https://sdk.split.io/api/segmentChanges/segment2?since=-1')).to have_been_made.once expect(a_request(:get, 'https://sdk.split.io/api/segmentChanges/segment2?since=1470947453878')).to have_been_made.once expect(a_request(:get, 'https://sdk.split.io/api/segmentChanges/segment3?since=-1')).to have_been_made.once + expect(result).to eq(true) + end + + it 'sync_all synchronous - should return true' do + result = synchronizer.sync_all + + sleep(2) + + expect(a_request(:get, 'https://sdk.split.io/api/splitChanges?since=-1')).to have_been_made.once + expect(a_request(:get, 'https://sdk.split.io/api/segmentChanges/segment1?since=-1')).to have_been_made.once + expect(a_request(:get, 'https://sdk.split.io/api/segmentChanges/segment1?since=1470947453877')).to have_been_made.once + expect(a_request(:get, 'https://sdk.split.io/api/segmentChanges/segment2?since=-1')).to have_been_made.once + expect(a_request(:get, 'https://sdk.split.io/api/segmentChanges/segment2?since=1470947453878')).to have_been_made.once + expect(a_request(:get, 'https://sdk.split.io/api/segmentChanges/segment3?since=-1')).to have_been_made.once + expect(result).to eq(true) end it 'start_periodic_data_recording' do @@ -84,6 +98,16 @@ end end + it 'sync_all synchronous - should return false' do + stub_request(:get, 'https://sdk.split.io/api/splitChanges?since=-1').to_return(status: 500) + + result = synchronizer.sync_all(false) + + sleep(2) + + expect(result).to eq(false) + end + it 'fetch_splits' do mock_split_changes(splits) mock_segment_changes('segment2', segment2, '-1') diff --git a/spec/engine_spec.rb b/spec/engine_spec.rb index 1e3e975af..4f07827b1 100644 --- a/spec/engine_spec.rb +++ b/spec/engine_spec.rb @@ -84,6 +84,7 @@ context '#equal_to_set_matcher and get_treatment validation attributes' do before do load_splits(equal_to_set_matcher_json) + subject.block_until_ready end it 'get_treatment_with_config returns off' do @@ -123,6 +124,7 @@ context '#get_treatment' do before do load_splits(all_keys_matcher_json) + subject.block_until_ready end it 'returns CONTROL for random id' do @@ -296,6 +298,7 @@ context '#get_treatment_with_config' do before do load_splits(configurations_json) + subject.block_until_ready end it 'returns the config' do @@ -384,6 +387,7 @@ context '#get_treatments_with_config' do before do load_splits(configurations_json) + subject.block_until_ready end split_names = %w[test_feature no_configs_feature killed_feature] @@ -401,6 +405,7 @@ context 'all keys matcher' do before do load_splits(all_keys_matcher_json) + subject.block_until_ready end it 'validates the feature is on for all ids' do @@ -419,6 +424,7 @@ load_segments(segments_json) load_splits(segment_matcher_json) + subject.block_until_ready end it 'validates the feature is on for all ids' do @@ -496,6 +502,7 @@ load_segments(segments_json) load_splits(segment_matcher2_json) + subject.block_until_ready end it 'validates the feature is on for all ids' do @@ -538,6 +545,7 @@ context 'whitelist matcher' do before do load_splits(whitelist_matcher_json) + subject.block_until_ready end it 'validates the feature is on for all ids' do @@ -552,6 +560,7 @@ context 'dependency matcher' do before do load_splits(dependency_matcher_json) + subject.block_until_ready end it 'returns on treatment' do @@ -572,6 +581,7 @@ end it 'returns default treatment for killed splits' do + subject.block_until_ready expect(subject.get_treatment('fake_user_id_1', 'test_killed')).to eq 'def_test' expect(subject.get_treatment('fake_user_id_2', 'test_killed')).to eq 'def_test' expect(subject.get_treatment('fake_user_id_3', 'test_killed')).to eq 'def_test' @@ -662,6 +672,7 @@ context 'traffic allocations' do before do load_splits(traffic_allocation_json) + subject.block_until_ready end it 'returns expected treatment' do @@ -697,6 +708,7 @@ it 'returns expected treatment' do allow_any_instance_of(SplitIoClient::Splitter).to receive(:bucket).and_return(1) + subject.block_until_ready expect(subject.get_treatment('test', 'Traffic_Allocation_One_Percent')).to eq('on') end end @@ -710,6 +722,7 @@ stub_request(:post, 'https://events.split.io/api/testImpressions/bulk') .to_return(status: 200, body: '', headers: {}) + subject.block_until_ready expect(subject.get_treatment('fake_user_id_1', 'test_feature')).to eq 'on' subject.destroy expect(subject.get_treatment('fake_user_id_1', 'test_feature')).to eq 'control' @@ -952,6 +965,10 @@ end it 'fetch splits' do + stub_request(:post, 'https://telemetry.split.io/api/v1/metrics/config') + .to_return(status: 200, body: 'ok') + + subject.block_until_ready expect(subject.instance_variable_get(:@splits_repository).splits.size).to eq(1) end end diff --git a/spec/integrations/dedupe_impression_spec.rb b/spec/integrations/dedupe_impression_spec.rb index 0c0e7e703..439867fe0 100644 --- a/spec/integrations/dedupe_impression_spec.rb +++ b/spec/integrations/dedupe_impression_spec.rb @@ -96,6 +96,7 @@ factory = SplitIoClient::SplitFactory.new('test_api_key-2', streaming_enabled: false, impressions_mode: :optimized) client = factory.client + client.block_until_ready client.get_treatments('nico_test', %w[FACUNDO_TEST MAURO_TEST Test_Save_1]) client.get_treatments('admin', %w[FACUNDO_TEST MAURO_TEST Test_Save_1]) client.get_treatments('maldo', %w[FACUNDO_TEST Test_Save_1]) diff --git a/spec/splitclient/split_manager_spec.rb b/spec/splitclient/split_manager_spec.rb index 29be3bf1c..40c35dd62 100644 --- a/spec/splitclient/split_manager_spec.rb +++ b/spec/splitclient/split_manager_spec.rb @@ -3,16 +3,11 @@ require 'spec_helper' describe SplitIoClient do - let(:factory) do - SplitIoClient::SplitFactory.new('test_api_key', logger: Logger.new(log), streaming_enabled: false) - end - + let(:factory) { SplitIoClient::SplitFactory.new('test_api_key', logger: Logger.new(log), streaming_enabled: false) } let(:log) { StringIO.new } subject { factory.manager } let(:splits) { File.read(File.expand_path(File.join(File.dirname(__FILE__), '../test_data/splits/splits.json'))) } - let(:segments) do - File.read(File.expand_path(File.join(File.dirname(__FILE__), '../test_data/segments/engine_segments.json'))) - end + let(:segments) { File.read(File.expand_path(File.join(File.dirname(__FILE__), '../test_data/segments/engine_segments.json'))) } before do stub_request(:get, 'https://sdk.split.io/api/splitChanges?since=-1') @@ -49,13 +44,14 @@ end it 'returns on invalid split_name' do + subject.block_until_ready split_name = ' test_1_ruby ' expect(subject.split(split_name)).not_to be_nil - expect(log.string) - .to include "split: split_name #{split_name} has extra whitespace, trimming" + expect(log.string).to include "split: split_name #{split_name} has extra whitespace, trimming" end it 'returns and logs warning when ready and split does not exist' do + subject.block_until_ready expect(subject.split('non_existing_feature')).to be_nil expect(log.string).to include 'split: you passed non_existing_feature ' \ 'that does not exist in this environment, please double check what Splits exist ' \ @@ -72,6 +68,7 @@ context '#split_names' do it 'returns split names' do + subject.block_until_ready expect(subject.split_names).to match_array(%w[test_1_ruby sample_feature]) end @@ -101,13 +98,14 @@ end it 'returns configurations' do - expect(subject.send(:build_split_view, - 'test_1_ruby', - subject.instance_variable_get(:@splits_repository).get_split('test_1_ruby'))[:configs]) - .to eq(on: '{"size":15,"test":20}') + subject.block_until_ready + split = subject.instance_variable_get(:@splits_repository).get_split('test_1_ruby') + result = subject.send(:build_split_view, 'test_1_ruby', split)[:configs] + expect(result).to eq(on: '{"size":15,"test":20}') end it 'returns empty hash when no configurations' do + subject.block_until_ready expect(subject.send(:build_split_view, 'sample_feature', subject.instance_variable_get(:@splits_repository).get_split('sample_feature'))[:configs]) @@ -124,6 +122,7 @@ end it 'returns expected treatments' do + subject.block_until_ready expect(subject.send(:build_split_view, 'uber_feature', subject.instance_variable_get(:@splits_repository).get_split('uber_feature'))[:treatments]) diff --git a/spec/splitclient_rb_corner_cases_spec.rb b/spec/splitclient_rb_corner_cases_spec.rb index 745f54063..938ff4d9e 100644 --- a/spec/splitclient_rb_corner_cases_spec.rb +++ b/spec/splitclient_rb_corner_cases_spec.rb @@ -30,7 +30,7 @@ end it 'validates the feature is "default" for id when segment used does not exist' do - allow_any_instance_of(SplitIoClient::Cache::Stores::SDKBlocker).to receive(:ready?).and_return(true) + subject.block_until_ready expect(subject.get_treatment(user, feature)).to eq 'default' end end diff --git a/spec/sse/sse_handler_spec.rb b/spec/sse/sse_handler_spec.rb index f5943138d..760dde92b 100644 --- a/spec/sse/sse_handler_spec.rb +++ b/spec/sse/sse_handler_spec.rb @@ -27,12 +27,11 @@ let(:impressions_repository) { SplitIoClient::Cache::Repositories::ImpressionsRepository.new(config) } let(:telemetry_runtime_producer) { SplitIoClient::Telemetry::RuntimeProducer.new(config) } let(:events_repository) { SplitIoClient::Cache::Repositories::EventsRepository.new(config, api_key, telemetry_runtime_producer) } - let(:sdk_blocker) { SplitIoClient::Cache::Stores::SDKBlocker.new(splits_repository, segments_repository, config) } let(:split_fetcher) do - SplitIoClient::Cache::Fetchers::SplitFetcher.new(splits_repository, api_key, config, sdk_blocker, telemetry_runtime_producer) + SplitIoClient::Cache::Fetchers::SplitFetcher.new(splits_repository, api_key, config, telemetry_runtime_producer) end let(:segment_fetcher) do - SplitIoClient::Cache::Fetchers::SegmentFetcher.new(segments_repository, api_key, config, sdk_blocker, telemetry_runtime_producer) + SplitIoClient::Cache::Fetchers::SegmentFetcher.new(segments_repository, api_key, config, telemetry_runtime_producer) end let(:notification_manager_keeper) { SplitIoClient::SSE::NotificationManagerKeeper.new(config, telemetry_runtime_producer) } let(:repositories) do @@ -52,7 +51,7 @@ telemetry_runtime_producer: telemetry_runtime_producer } end - let(:synchronizer) { SplitIoClient::Engine::Synchronizer.new(repositories, api_key, config, sdk_blocker, parameters) } + let(:synchronizer) { SplitIoClient::Engine::Synchronizer.new(repositories, api_key, config, parameters) } before do mock_split_changes(splits) diff --git a/spec/sse/workers/segments_worker_spec.rb b/spec/sse/workers/segments_worker_spec.rb index e0b11ba6c..963095978 100644 --- a/spec/sse/workers/segments_worker_spec.rb +++ b/spec/sse/workers/segments_worker_spec.rb @@ -18,13 +18,12 @@ let(:impressions_repository) { SplitIoClient::Cache::Repositories::ImpressionsRepository.new(config) } let(:telemetry_runtime_producer) { SplitIoClient::Telemetry::RuntimeProducer.new(config) } let(:events_repository) { SplitIoClient::Cache::Repositories::EventsRepository.new(config, api_key, telemetry_runtime_producer) } - let(:sdk_blocker) { SplitIoClient::Cache::Stores::SDKBlocker.new(splits_repository, segments_repository, config) } - let(:split_fetcher) { SplitIoClient::Cache::Fetchers::SplitFetcher.new(splits_repository, api_key, config, sdk_blocker, telemetry_runtime_producer) } - let(:segment_fetcher) { SplitIoClient::Cache::Fetchers::SegmentFetcher.new(segments_repository, api_key, config, sdk_blocker, telemetry_runtime_producer) } + let(:split_fetcher) { SplitIoClient::Cache::Fetchers::SplitFetcher.new(splits_repository, api_key, config, telemetry_runtime_producer) } + let(:segment_fetcher) { SplitIoClient::Cache::Fetchers::SegmentFetcher.new(segments_repository, api_key, config, telemetry_runtime_producer) } let(:repositories) { { splits: splits_repository, segments: segments_repository } } let(:impression_counter) { SplitIoClient::Engine::Common::ImpressionCounter.new } let(:params) { { split_fetcher: split_fetcher, segment_fetcher: segment_fetcher, imp_counter: impression_counter, telemetry_runtime_producer: telemetry_runtime_producer } } - let(:synchronizer) { SplitIoClient::Engine::Synchronizer.new(repositories, api_key, config, sdk_blocker, params) } + let(:synchronizer) { SplitIoClient::Engine::Synchronizer.new(repositories, api_key, config, params) } before do mock_split_changes(splits) diff --git a/spec/sse/workers/splits_worker_spec.rb b/spec/sse/workers/splits_worker_spec.rb index bd4991661..5b8de3e49 100644 --- a/spec/sse/workers/splits_worker_spec.rb +++ b/spec/sse/workers/splits_worker_spec.rb @@ -18,13 +18,12 @@ let(:impressions_repository) { SplitIoClient::Cache::Repositories::ImpressionsRepository.new(config) } let(:telemetry_runtime_producer) { SplitIoClient::Telemetry::RuntimeProducer.new(config) } let(:events_repository) { SplitIoClient::Cache::Repositories::EventsRepository.new(config, api_key, telemetry_runtime_producer) } - let(:sdk_blocker) { SplitIoClient::Cache::Stores::SDKBlocker.new(splits_repository, segments_repository, config) } - let(:split_fetcher) { SplitIoClient::Cache::Fetchers::SplitFetcher.new(splits_repository, api_key, config, sdk_blocker, telemetry_runtime_producer) } - let(:segment_fetcher) { SplitIoClient::Cache::Fetchers::SegmentFetcher.new(segments_repository, api_key, config, sdk_blocker, telemetry_runtime_producer) } + let(:split_fetcher) { SplitIoClient::Cache::Fetchers::SplitFetcher.new(splits_repository, api_key, config, telemetry_runtime_producer) } + let(:segment_fetcher) { SplitIoClient::Cache::Fetchers::SegmentFetcher.new(segments_repository, api_key, config, telemetry_runtime_producer) } let(:repositories) { { splits: splits_repository, segments: segments_repository } } let(:impression_counter) { SplitIoClient::Engine::Common::ImpressionCounter.new } let(:params) { { split_fetcher: split_fetcher, segment_fetcher: segment_fetcher, imp_counter: impression_counter, telemetry_runtime_producer: telemetry_runtime_producer } } - let(:synchronizer) { SplitIoClient::Engine::Synchronizer.new(repositories, api_key, config, sdk_blocker, params) } + let(:synchronizer) { SplitIoClient::Engine::Synchronizer.new(repositories, api_key, config, params) } it 'add change number - must tigger fetcch - with retries' do stub_request(:get, 'https://sdk.split.io/api/splitChanges?since=-1') From 17efeaecdd55b782d9f57ad2d899e7b4520cc811 Mon Sep 17 00:00:00 2001 From: Mauro Sanz Date: Wed, 24 Nov 2021 15:09:31 -0300 Subject: [PATCH 2/3] fixed tests --- .../engine/common/impressions_manager.rb | 8 ++-- lib/splitclient-rb/engine/sync_manager.rb | 16 +++---- lib/splitclient-rb/engine/synchronizer.rb | 4 +- lib/splitclient-rb/sse/event_source/client.rb | 6 +-- .../engine/matchers/combining_matcher_spec.rb | 2 + spec/engine/sync_manager_spec.rb | 46 +++++++++--------- spec/integrations/dedupe_impression_spec.rb | 31 +++++++++--- spec/integrations/in_memory_client_spec.rb | 32 +++++++++++++ spec/integrations/push_client_spec.rb | 47 +++++++++++-------- spec/splitclient/split_factory_spec.rb | 14 +++++- spec/splitclient_rb_corner_cases_spec.rb | 16 +++---- spec/sse/sse_handler_spec.rb | 38 +++++++-------- splitclient-rb.gemspec | 2 +- 13 files changed, 166 insertions(+), 96 deletions(-) diff --git a/lib/splitclient-rb/engine/common/impressions_manager.rb b/lib/splitclient-rb/engine/common/impressions_manager.rb index 44700857d..9bc541ee8 100644 --- a/lib/splitclient-rb/engine/common/impressions_manager.rb +++ b/lib/splitclient-rb/engine/common/impressions_manager.rb @@ -21,8 +21,8 @@ def build_impression(matching_key, bucketing_key, split_name, treatment, params @impression_counter.inc(split_name, impression_data[:m]) if optimized? && !redis? impression(impression_data, params[:attributes]) - rescue StandardError => error - @config.log_found_exception(__method__.to_s, error) + rescue StandardError => e + @config.log_found_exception(__method__.to_s, e) end def track(impressions) @@ -48,8 +48,8 @@ def track(impressions) end record_stats(queued, dropped, dedupe) - rescue StandardError => error - @config.log_found_exception(__method__.to_s, error) + rescue StandardError => e + @config.log_found_exception(__method__.to_s, e) end private diff --git a/lib/splitclient-rb/engine/sync_manager.rb b/lib/splitclient-rb/engine/sync_manager.rb index eab05728b..2768208ac 100644 --- a/lib/splitclient-rb/engine/sync_manager.rb +++ b/lib/splitclient-rb/engine/sync_manager.rb @@ -44,24 +44,22 @@ def start @status_manager.ready! @telemetry_synchronizer.synchronize_config @synchronizer.start_periodic_data_recording + connected = false if @config.streaming_enabled @config.logger.debug('Starting Straming mode ...') - connected = @push_manager.start_sse if defined?(PhusionPassenger) - PhusionPassenger.on_event(:starting_worker_process) do |forked| - sse_thread_forked if forked - end + PhusionPassenger.on_event(:starting_worker_process) { |forked| sse_thread_forked if forked } end - - return if connected end - @config.logger.debug('Starting polling mode ...') - @synchronizer.start_periodic_fetch - record_telemetry(Telemetry::Domain::Constants::SYNC_MODE, SYNC_MODE_POLLING) + unless connected + @config.logger.debug('Starting polling mode ...') + @synchronizer.start_periodic_fetch + record_telemetry(Telemetry::Domain::Constants::SYNC_MODE, SYNC_MODE_POLLING) + end end end diff --git a/lib/splitclient-rb/engine/synchronizer.rb b/lib/splitclient-rb/engine/synchronizer.rb index 56e107ab8..77fef4037 100644 --- a/lib/splitclient-rb/engine/synchronizer.rb +++ b/lib/splitclient-rb/engine/synchronizer.rb @@ -30,7 +30,9 @@ def initialize( end def sync_all(asynchronous = true) - return sync_splits_and_segments unless asynchronous + unless asynchronous + return sync_splits_and_segments + end @config.threads[:sync_all_thread] = Thread.new do sync_splits_and_segments diff --git a/lib/splitclient-rb/sse/event_source/client.rb b/lib/splitclient-rb/sse/event_source/client.rb index d7677bc88..8f291fa98 100644 --- a/lib/splitclient-rb/sse/event_source/client.rb +++ b/lib/splitclient-rb/sse/event_source/client.rb @@ -38,7 +38,6 @@ def on_action(&action) def close(action = Constants::PUSH_NONRETRYABLE_ERROR) dispatch_action(action) @connected.make_false - SplitIoClient::Helpers::ThreadHelper.stop(:connect_stream, @config) @socket&.close rescue StandardError => e @config.logger.error("SSEClient close Error: #{e.inspect}") @@ -77,7 +76,7 @@ def connect_thread(latch) end def connect_stream(latch) - socket_write + socket_write(latch) while connected? || @first_event.value begin @@ -96,13 +95,14 @@ def connect_stream(latch) end end - def socket_write + def socket_write(latch) @first_event.make_true @socket = socket_connect @socket.write(build_request(@uri)) rescue StandardError => e @config.logger.error("Error during connecting to #{@uri.host}. Error: #{e.inspect}") close(Constants::PUSH_NONRETRYABLE_ERROR) + latch.count_down end def read_first_event(data, latch) diff --git a/spec/engine/matchers/combining_matcher_spec.rb b/spec/engine/matchers/combining_matcher_spec.rb index 4b142764d..c5277eb57 100644 --- a/spec/engine/matchers/combining_matcher_spec.rb +++ b/spec/engine/matchers/combining_matcher_spec.rb @@ -25,6 +25,8 @@ describe 'anding' do it 'matches' do + subject.block_until_ready + expect(subject.get_treatment( 'user_for_testing_do_no_erase', 'PASSENGER_anding', diff --git a/spec/engine/sync_manager_spec.rb b/spec/engine/sync_manager_spec.rb index 7084eec05..11613f1da 100644 --- a/spec/engine/sync_manager_spec.rb +++ b/spec/engine/sync_manager_spec.rb @@ -57,10 +57,10 @@ mock_segment_changes('segment2', segment2, '1470947453878') mock_segment_changes('segment3', segment3, '-1') stub_request(:get, config.auth_service_url).to_return(status: 200, body: body_response) - status_manager.ready! + stub_request(:post, 'https://telemetry.split.io/api/v1/metrics/config').to_return(status: 200, body: '') end - it 'start sync manager with success sse connection.' do + it 'start sync manager with success sse connection.' do mock_server do |server| server.setup_response('/') do |_, res| send_content(res, 'content') @@ -73,7 +73,7 @@ sleep(2) expect(a_request(:get, 'https://sdk.split.io/api/splitChanges?since=-1')).to have_been_made.once - expect(a_request(:get, 'https://sdk.split.io/api/splitChanges?since=1506703262916')).to have_been_made.once + expect(config.threads.size).to eq(10) end end @@ -92,7 +92,7 @@ sleep(2) expect(a_request(:get, 'https://sdk.split.io/api/splitChanges?since=-1')).to have_been_made.once - expect(a_request(:get, 'https://sdk.split.io/api/splitChanges?since=1506703262916')).to have_been_made.at_least_times(1) + expect(config.threads.size).to eq(6) end end @@ -117,27 +117,27 @@ expect(sse_handler.connected?).to eq(false) end end -end -private + private -def mock_split_changes_with_since(splits_json, since) - stub_request(:get, "https://sdk.split.io/api/splitChanges?since=#{since}") - .to_return(status: 200, body: splits_json) -end + def mock_split_changes_with_since(splits_json, since) + stub_request(:get, "https://sdk.split.io/api/splitChanges?since=#{since}") + .to_return(status: 200, body: splits_json) + end -def mock_segment_changes(segment_name, segment_json, since) - stub_request(:get, "https://sdk.split.io/api/segmentChanges/#{segment_name}?since=#{since}") - .to_return(status: 200, body: segment_json) -end + def mock_segment_changes(segment_name, segment_json, since) + stub_request(:get, "https://sdk.split.io/api/segmentChanges/#{segment_name}?since=#{since}") + .to_return(status: 200, body: segment_json) + end -def send_content(res, content) - res.content_type = 'text/event-stream' - res.status = 200 - res.chunked = true - rd, wr = IO.pipe - wr.write(content) - res.body = rd - wr.close - wr + def send_content(res, content) + res.content_type = 'text/event-stream' + res.status = 200 + res.chunked = true + rd, wr = IO.pipe + wr.write(content) + res.body = rd + wr.close + wr + end end diff --git a/spec/integrations/dedupe_impression_spec.rb b/spec/integrations/dedupe_impression_spec.rb index 439867fe0..8d917e538 100644 --- a/spec/integrations/dedupe_impression_spec.rb +++ b/spec/integrations/dedupe_impression_spec.rb @@ -32,9 +32,15 @@ context 'checking logic impressions' do it 'get_treament should post 5 impressions - debug mode' do + stub_request(:post, 'https://telemetry.split.io/api/v1/metrics/config').to_return(status: 200, body: '') + stub_request(:get, 'https://sdk.split.io/api/splitChanges?since=1506703262916').to_return(status: 200, body: '') + factory = SplitIoClient::SplitFactory.new('test_api_key_debug-1', streaming_enabled: false, impressions_mode: :debug) debug_client = factory.client + debug_client.block_until_ready + sleep 1 + expect(debug_client.get_treatment('nico_test', 'FACUNDO_TEST')).to eq 'on' expect(debug_client.get_treatment('nico_test', 'FACUNDO_TEST')).to eq 'on' expect(debug_client.get_treatment('admin', 'FACUNDO_TEST')).to eq 'off' @@ -49,6 +55,9 @@ end it 'get_treaments should post 11 impressions - debug mode' do + stub_request(:post, 'https://telemetry.split.io/api/v1/metrics/config').to_return(status: 200, body: '') + stub_request(:get, 'https://sdk.split.io/api/splitChanges?since=1506703262916').to_return(status: 200, body: '') + factory = SplitIoClient::SplitFactory.new('test_api_key_debug-2', streaming_enabled: false, impressions_mode: :debug) debug_client = factory.client @@ -65,8 +74,14 @@ end it 'get_treament should post 3 impressions - optimized mode' do - factory = SplitIoClient::SplitFactory.new('test_api_key-1', streaming_enabled: false, impressions_mode: :optimized) + stub_request(:post, 'https://telemetry.split.io/api/v1/metrics/config').to_return(status: 200, body: '') + stub_request(:get, 'https://sdk.split.io/api/splitChanges?since=1506703262916').to_return(status: 200, body: '') + stub_request(:post, 'https://telemetry.split.io/api/v1/metrics/usage').to_return(status: 200, body: '') + + factory = SplitIoClient::SplitFactory.new('test_api_key-1', streaming_enabled: false, impressions_mode: :optimized, impressions_refresh_rate: 60) client = factory.client + client.block_until_ready + sleep 1 expect(client.get_treatment('nico_test', 'FACUNDO_TEST')).to eq 'on' expect(client.get_treatment('nico_test', 'FACUNDO_TEST')).to eq 'on' @@ -76,11 +91,9 @@ time_frame = SplitIoClient::Engine::Common::ImpressionCounter.truncate_time_frame((Time.now.to_f * 1000.0).to_i) - impressions = client.instance_variable_get(:@impressions_repository).batch - + client.destroy sleep 0.5 - expect(impressions.size).to eq 3 expect(a_request(:post, 'https://events.split.io/api/testImpressions/count') .with( body: { @@ -93,10 +106,16 @@ end it 'get_treaments should post 8 impressions - optimized mode' do + stub_request(:post, 'https://telemetry.split.io/api/v1/metrics/config').to_return(status: 200, body: '') + stub_request(:post, 'https://telemetry.split.io/api/v1/metrics/usage').to_return(status: 200, body: '') + stub_request(:get, 'https://sdk.split.io/api/splitChanges?since=1506703262916').to_return(status: 200, body: '') + factory = SplitIoClient::SplitFactory.new('test_api_key-2', streaming_enabled: false, impressions_mode: :optimized) client = factory.client client.block_until_ready + sleep 1 + client.get_treatments('nico_test', %w[FACUNDO_TEST MAURO_TEST Test_Save_1]) client.get_treatments('admin', %w[FACUNDO_TEST MAURO_TEST Test_Save_1]) client.get_treatments('maldo', %w[FACUNDO_TEST Test_Save_1]) @@ -104,11 +123,9 @@ time_frame = SplitIoClient::Engine::Common::ImpressionCounter.truncate_time_frame((Time.now.to_f * 1000.0).to_i) - impressions = client.instance_variable_get(:@impressions_repository).batch - + client.destroy sleep 0.5 - expect(impressions.size).to eq 8 expect(a_request(:post, 'https://events.split.io/api/testImpressions/count') .with( body: { diff --git a/spec/integrations/in_memory_client_spec.rb b/spec/integrations/in_memory_client_spec.rb index 6992cfeae..f8a6f4d66 100644 --- a/spec/integrations/in_memory_client_spec.rb +++ b/spec/integrations/in_memory_client_spec.rb @@ -42,10 +42,12 @@ mock_segment_changes('segment3', segment3, '-1') stub_request(:post, 'https://events.split.io/api/testImpressions/bulk').to_return(status: 200, body: 'ok') stub_request(:post, 'https://events.split.io/api/testImpressions/count').to_return(status: 200, body: 'ok') + stub_request(:post, 'https://telemetry.split.io/api/v1/metrics/config').to_return(status: 200, body: 'ok') end context '#get_treatment' do it 'returns treatments with FACUNDO_TEST feature and check impressions' do + client.block_until_ready expect(client.get_treatment('nico_test', 'FACUNDO_TEST')).to eq 'on' expect(client.get_treatment('mauro_test', 'FACUNDO_TEST')).to eq 'off' @@ -68,6 +70,7 @@ end it 'returns treatments with Test_Save_1 feature and check impressions' do + client.block_until_ready expect(client.get_treatment('1', 'Test_Save_1')).to eq 'on' expect(client.get_treatment('24', 'Test_Save_1')).to eq 'off' @@ -90,6 +93,7 @@ end it 'returns treatments with input validations' do + client.block_until_ready expect(client.get_treatment('nico_test', 'FACUNDO_TEST')).to eq 'on' expect(client.get_treatment('', 'FACUNDO_TEST')).to eq 'control' expect(client.get_treatment(nil, 'FACUNDO_TEST')).to eq 'control' @@ -176,6 +180,11 @@ client3 = factory3.client client4 = factory4.client + client1.block_until_ready + client2.block_until_ready + client3.block_until_ready + client4.block_until_ready + expect(client1.get_treatment('nico_test', 'FACUNDO_TEST')).to eq 'on' expect(client2.get_treatment('nico_test', 'FACUNDO_TEST')).to eq 'on' expect(client3.get_treatment('nico_test', 'FACUNDO_TEST')).to eq 'on' @@ -196,6 +205,7 @@ context '#get_treatment_with_config' do it 'returns treatments and configs with FACUNDO_TEST treatment and check impressions' do + client.block_until_ready expect(client.get_treatment_with_config('nico_test', 'FACUNDO_TEST')).to eq( treatment: 'on', config: '{"color":"green"}' @@ -224,6 +234,8 @@ end it 'returns treatments and configs with MAURO_TEST treatment and check impressions' do + client.block_until_ready + expect(client.get_treatment_with_config('mauro', 'MAURO_TEST')).to eq( treatment: 'on', config: '{"version":"v2"}' @@ -252,6 +264,8 @@ end it 'returns treatments with input validations' do + client.block_until_ready + expect(client.get_treatment_with_config('nico_test', 'FACUNDO_TEST')).to eq( treatment: 'on', config: '{"color":"green"}' @@ -296,6 +310,8 @@ end it 'returns CONTROL with treatment doesnt exist' do + client.block_until_ready + expect(client.get_treatment_with_config('nico_test', 'random_treatment')).to eq( treatment: 'control', config: nil @@ -328,6 +344,7 @@ context '#get_treatments' do it 'returns treatments and check impressions' do + client.block_until_ready result = client.get_treatments('nico_test', %w[FACUNDO_TEST MAURO_TEST Test_Save_1]) expect(result[:FACUNDO_TEST]).to eq 'on' @@ -359,6 +376,7 @@ end it 'returns treatments with input validation' do + client.block_until_ready result1 = client.get_treatments('nico_test', ['FACUNDO_TEST', '', nil]) result2 = client.get_treatments('', ['', 'MAURO_TEST', 'Test_Save_1']) result3 = client.get_treatments(nil, ['', 'MAURO_TEST', 'Test_Save_1']) @@ -381,6 +399,7 @@ end it 'returns CONTROL with treatment doesnt exist' do + client.block_until_ready result = client.get_treatments('nico_test', %w[FACUNDO_TEST random_treatment]) expect(result[:FACUNDO_TEST]).to eq 'on' @@ -425,6 +444,7 @@ context '#get_treatments_with_config' do it 'returns treatments and check impressions' do + client.block_until_ready result = client.get_treatments_with_config('nico_test', %w[FACUNDO_TEST MAURO_TEST Test_Save_1]) expect(result[:FACUNDO_TEST]).to eq( treatment: 'on', @@ -463,6 +483,7 @@ end it 'returns treatments with input validation' do + client.block_until_ready result1 = client.get_treatments_with_config('nico_test', %w[FACUNDO_TEST "" nil]) result2 = client.get_treatments_with_config('', %w["" MAURO_TEST Test_Save_1]) result3 = client.get_treatments_with_config(nil, %w["" MAURO_TEST Test_Save_1]) @@ -500,6 +521,7 @@ end it 'returns CONTROL with treatment doesnt exist' do + client.block_until_ready result = client.get_treatments_with_config('nico_test', %w[FACUNDO_TEST random_treatment]) expect(result[:FACUNDO_TEST]).to eq( @@ -594,6 +616,10 @@ client2 = factory2.client client3 = factory3.client + client1.block_until_ready + client2.block_until_ready + client3.block_until_ready + result1 = client1.get_treatments_with_config('nico_test', %w[MAURO_TEST]) result2 = client2.get_treatments_with_config('nico_test', %w[MAURO_TEST]) result3 = client3.get_treatments_with_config('nico_test', %w[FACUNDO_TEST]) @@ -624,11 +650,17 @@ context '#track' do it 'returns true' do + stub_request(:post, 'https://events.split.io/api/events/bulk').to_return(status: 200, body: '') + stub_request(:get, 'https://sdk.split.io/api/splitChanges?since=1506703262916').to_return(status: 200, body: '') + properties = { property_1: 1, property_2: 2 } + client.block_until_ready + sleep 1 + expect(client.track('key_1', 'traffic_type_1', 'event_type_1', 123, properties)).to be_truthy expect(client.track('key_2', 'traffic_type_2', 'event_type_2', 125)).to be_truthy diff --git a/spec/integrations/push_client_spec.rb b/spec/integrations/push_client_spec.rb index 97a6a1b16..b6cdaed95 100644 --- a/spec/integrations/push_client_spec.rb +++ b/spec/integrations/push_client_spec.rb @@ -36,6 +36,13 @@ File.read(File.join(SplitIoClient.root, 'spec/test_data/integrations/auth_body_response.json')) end + before do + stub_request(:post, 'https://telemetry.split.io/api/v1/metrics/config').to_return(status: 200, body: '') + stub_request(:get, 'https://sdk.split.io/api/splitChanges?since=1585948850111').to_return(status: 200, body: '') + stub_request(:get, 'https://sdk.split.io/api/splitChanges?since=1585948850110').to_return(status: 200, body: '') + stub_request(:get, 'https://sdk.split.io/api/segmentChanges/segment3?since=-1&till=1470947453879').to_return(status: 200, body: '') + end + context 'SPLIT_UPDATE' do it 'processing split update event' do mock_splits_request(splits, -1) @@ -363,27 +370,27 @@ end end end -end -private - -def send_content(res, content) - res.content_type = 'text/event-stream' - res.status = 200 - res.chunked = true - rd, wr = IO.pipe - wr.write(content) - res.body = rd - wr.close - wr -end + private + + def send_content(res, content) + res.content_type = 'text/event-stream' + res.status = 200 + res.chunked = true + rd, wr = IO.pipe + wr.write(content) + res.body = rd + wr.close + wr + end -def mock_splits_request(splits_json, since) - stub_request(:get, "https://sdk.split.io/api/splitChanges?since=#{since}") - .to_return(status: 200, body: splits_json) -end + def mock_splits_request(splits_json, since) + stub_request(:get, "https://sdk.split.io/api/splitChanges?since=#{since}") + .to_return(status: 200, body: splits_json) + end -def mock_segment_changes(segment_name, segment_json, since) - stub_request(:get, "https://sdk.split.io/api/segmentChanges/#{segment_name}?since=#{since}") - .to_return(status: 200, body: segment_json) + def mock_segment_changes(segment_name, segment_json, since) + stub_request(:get, "https://sdk.split.io/api/segmentChanges/#{segment_name}?since=#{since}") + .to_return(status: 200, body: segment_json) + end end diff --git a/spec/splitclient/split_factory_spec.rb b/spec/splitclient/split_factory_spec.rb index b62b4105e..138050828 100644 --- a/spec/splitclient/split_factory_spec.rb +++ b/spec/splitclient/split_factory_spec.rb @@ -71,6 +71,8 @@ it 'log an error stating Api Key is invalid' do stub_request(:get, 'https://sdk.split.io/api/splitChanges?since=-1') .to_return(status: 200, body: []) + stub_request(:post, 'https://telemetry.split.io/api/v1/metrics/config') + .to_return(status: 200, body: '') factory = described_class.new(nil, options) @@ -89,6 +91,8 @@ it 'log an error stating Api Key is invalid' do stub_request(:get, 'https://sdk.split.io/api/splitChanges?since=-1') .to_return(status: 200, body: []) + stub_request(:post, 'https://telemetry.split.io/api/v1/metrics/config') + .to_return(status: 200, body: '') factory = described_class.new('', options) @@ -114,15 +118,20 @@ .to_return(status: 200, body: []) stub_request(:get, 'https://sdk.split.io/api/segmentChanges/employees?since=-1') .to_return(status: 403, body: []) + stub_request(:post, 'https://telemetry.split.io/api/v1/metrics/config') + .to_return(status: 200, body: '') factory = described_class.new('browser_key', options) - factory.start! + sleep 1 expect(log.string).to include 'Factory Instantiation: You passed a browser type api_key,' \ ' please grab an api key from the Split console that is of type sdk' expect(factory.instance_variable_get(:@config).valid_mode).to be false expect(factory.manager.split('test_split')) .to be nil + + puts '###### log' + puts log.string end end @@ -197,6 +206,9 @@ stub_request(:get, 'https://sdk.split.io/api/splitChanges?since=-1') .to_return(status: 200, body: []) + stub_request(:post, 'https://telemetry.split.io/api/v1/metrics/config') + .to_return(status: 200, body: '') + described_class.new('API_KEY', options) described_class.new('ANOTHER_API_KEY', options) diff --git a/spec/splitclient_rb_corner_cases_spec.rb b/spec/splitclient_rb_corner_cases_spec.rb index 938ff4d9e..bbed04681 100644 --- a/spec/splitclient_rb_corner_cases_spec.rb +++ b/spec/splitclient_rb_corner_cases_spec.rb @@ -17,16 +17,16 @@ let(:non_matching_value_attributes) { { list: 'random' } } let(:missing_key_attributes) { {} } let(:nil_attributes) { nil } + let(:segment_res) { '{"name":"mauro_1","added":[],"removed":[],"since":-1,"till":-1 }' } before do - stub_request(:get, 'https://sdk.split.io/api/splitChanges?since=-1') - .to_return(status: 200, body: splits_json) - - stub_request(:get, 'https://sdk.split.io/api/segmentChanges/demo?since=-1') - .to_return(status: 200, body: []) - - stub_request(:get, 'https://sdk.split.io/api/segmentChanges/employees?since=-1') - .to_return(status: 200, body: []) + stub_request(:post, 'https://events.split.io/api/testImpressions/bulk').to_return(status: 200, body: '') + stub_request(:get, 'https://sdk.split.io/api/splitChanges?since=-1').to_return(status: 200, body: splits_json) + stub_request(:get, 'https://sdk.split.io/api/segmentChanges/demo?since=-1').to_return(status: 200, body: segment_res) + stub_request(:get, 'https://sdk.split.io/api/segmentChanges/employees?since=-1').to_return(status: 200, body: segment_res) + stub_request(:get, 'https://sdk.split.io/api/splitChanges?since=1473413807667').to_return(status: 200, body: segment_res) + stub_request(:post, 'https://telemetry.split.io/api/v1/metrics/config').to_return(status: 200, body: segment_res) + stub_request(:post, 'https://events.split.io/api/testImpressions/count').to_return(status: 200, body: '') end it 'validates the feature is "default" for id when segment used does not exist' do diff --git a/spec/sse/sse_handler_spec.rb b/spec/sse/sse_handler_spec.rb index 760dde92b..958592530 100644 --- a/spec/sse/sse_handler_spec.rb +++ b/spec/sse/sse_handler_spec.rb @@ -276,27 +276,27 @@ end end end -end -private + private -def mock_split_changes(splits_json) - stub_request(:get, 'https://sdk.split.io/api/splitChanges?since=-1') - .to_return(status: 200, body: splits_json) -end + def mock_split_changes(splits_json) + stub_request(:get, 'https://sdk.split.io/api/splitChanges?since=-1') + .to_return(status: 200, body: splits_json) + end -def mock_segment_changes(segment_name, segment_json, since) - stub_request(:get, "https://sdk.split.io/api/segmentChanges/#{segment_name}?since=#{since}") - .to_return(status: 200, body: segment_json) -end + def mock_segment_changes(segment_name, segment_json, since) + stub_request(:get, "https://sdk.split.io/api/segmentChanges/#{segment_name}?since=#{since}") + .to_return(status: 200, body: segment_json) + end -def send_content(res, content) - res.content_type = 'text/event-stream' - res.status = 200 - res.chunked = true - rd, wr = IO.pipe - wr.write(content) - res.body = rd - wr.close - wr + def send_content(res, content) + res.content_type = 'text/event-stream' + res.status = 200 + res.chunked = true + rd, wr = IO.pipe + wr.write(content) + res.body = rd + wr.close + wr + end end diff --git a/splitclient-rb.gemspec b/splitclient-rb.gemspec index 3c7aeb73c..1e10682ab 100644 --- a/splitclient-rb.gemspec +++ b/splitclient-rb.gemspec @@ -38,7 +38,7 @@ Gem::Specification.new do |spec| spec.add_development_dependency 'allocation_stats' spec.add_development_dependency 'appraisal' - spec.add_development_dependency 'bundler' + spec.add_development_dependency 'bundler', '~> 2.2' spec.add_development_dependency 'pry' spec.add_development_dependency 'pry-nav' spec.add_development_dependency 'rake', '12.3.3' From 107c85eb783aa71b9149603e604e52984cee038f Mon Sep 17 00:00:00 2001 From: Mauro Sanz Date: Thu, 25 Nov 2021 15:31:09 -0300 Subject: [PATCH 3/3] polishing --- lib/splitclient-rb/engine/sync_manager.rb | 9 +++++++-- lib/splitclient-rb/split_factory.rb | 14 ++++++++++---- lib/splitclient-rb/sse/event_source/client.rb | 1 + lib/splitclient-rb/version.rb | 2 +- spec/integrations/push_client_spec.rb | 4 ++-- 5 files changed, 21 insertions(+), 9 deletions(-) diff --git a/lib/splitclient-rb/engine/sync_manager.rb b/lib/splitclient-rb/engine/sync_manager.rb index 2768208ac..e796f389d 100644 --- a/lib/splitclient-rb/engine/sync_manager.rb +++ b/lib/splitclient-rb/engine/sync_manager.rb @@ -38,6 +38,13 @@ def initialize( end def start + start_thread + PhusionPassenger.on_event(:starting_worker_process) { |forked| start_thread if forked } if defined?(PhusionPassenger) + end + + private + + def start_thread @config.threads[:start_sdk] = Thread.new do sleep(0.5) until @synchronizer.sync_all(false) @@ -63,8 +70,6 @@ def start end end - private - def process_action(action) case action when Constants::PUSH_CONNECTED diff --git a/lib/splitclient-rb/split_factory.rb b/lib/splitclient-rb/split_factory.rb index 72771950b..5af14fd6c 100644 --- a/lib/splitclient-rb/split_factory.rb +++ b/lib/splitclient-rb/split_factory.rb @@ -28,6 +28,10 @@ def initialize(api_key, config_hash = {}) raise 'Invalid SDK mode' unless valid_mode + validate_api_key + + register_factory + build_telemetry_components @splits_repository = SplitsRepository.new(@config) @@ -44,14 +48,16 @@ def initialize(api_key, config_hash = {}) @client = SplitClient.new(@api_key, repositories, @status_manager, @config, @impressions_manager, @evaluation_producer) @manager = SplitManager.new(@splits_repository, @status_manager, @config) - - validate_api_key - - register_factory end def start! return start_localhost_components if @config.localhost_mode + + if @config.consumer? + @status_manager.ready! + @telemetry_synchronizer.synchronize_config + return + end split_fetcher = SplitFetcher.new(@splits_repository, @api_key, config, @runtime_producer) segment_fetcher = SegmentFetcher.new(@segments_repository, @api_key, config, @runtime_producer) diff --git a/lib/splitclient-rb/sse/event_source/client.rb b/lib/splitclient-rb/sse/event_source/client.rb index 8f291fa98..d441768ed 100644 --- a/lib/splitclient-rb/sse/event_source/client.rb +++ b/lib/splitclient-rb/sse/event_source/client.rb @@ -38,6 +38,7 @@ def on_action(&action) def close(action = Constants::PUSH_NONRETRYABLE_ERROR) dispatch_action(action) @connected.make_false + SplitIoClient::Helpers::ThreadHelper.stop(:connect_stream, @config) @socket&.close rescue StandardError => e @config.logger.error("SSEClient close Error: #{e.inspect}") diff --git a/lib/splitclient-rb/version.rb b/lib/splitclient-rb/version.rb index 0e1cd8aef..32478a89b 100644 --- a/lib/splitclient-rb/version.rb +++ b/lib/splitclient-rb/version.rb @@ -1,3 +1,3 @@ module SplitIoClient - VERSION = '7.3.1' + VERSION = '7.3.2.pre.rc4' end diff --git a/spec/integrations/push_client_spec.rb b/spec/integrations/push_client_spec.rb index b6cdaed95..92b91c486 100644 --- a/spec/integrations/push_client_spec.rb +++ b/spec/integrations/push_client_spec.rb @@ -66,10 +66,10 @@ ) client = factory.client - client.block_until_ready(1) + client.block_until_ready sleep(2) expect(client.get_treatment('admin', 'push_test')).to eq('after_fetch') - expect(a_request(:get, 'https://sdk.split.io/api/splitChanges?since=-1')).to have_been_made.times(1) + expect(a_request(:get, 'https://sdk.split.io/api/splitChanges?since=-1')).to have_been_made.at_least_times(1) expect(a_request(:get, 'https://sdk.split.io/api/splitChanges?since=1585948850109')).to have_been_made.times(1) expect(a_request(:get, 'https://sdk.split.io/api/splitChanges?since=1585948850110')).to have_been_made.times(1) end