fix(blocking-api): fixed issue related to default blocking-timeout and blocking API methods. (#202)

Summary:

- Fixed bug when `blocking_timeout` is set to default.
- Block all APIs for the `blocking_timeout` period or datafile is not loaded successfully.
- Updated doc string.

Author: rashidsp

README.md

@@ -44,7 +44,7 @@ You can initialize the Optimizely instance in two ways: directly with a datafile
 
    When the `datafile` is given then it will be used initially before any update.
 
- 2. Initialize Optimizely by providing a Config Manager that implements a 'get_config' method. You can customize our `HTTPConfigManager` as needed.
+ 2. Initialize Optimizely by providing a Config Manager that implements a `config` method. You can customize our `HTTPConfigManager` as needed.
 
     ```
     custom_config_manager = CustomConfigManager.new
@@ -124,7 +124,7 @@ The following properties can be set to override the default configurations for `
 | datafile | nil | Initial datafile, typically sourced from a local cached source
 | auto_update | true | Boolean flag to specify if callback needs to execute infinitely or only once
 | start_by_default | true | Boolean flag to specify if datafile polling should start right away as soon as `HTTPConfigManager` initializes
-| blocking_timeout | 15 seconds | Maximum time in seconds to block the `get_config` call until config has been initialized
+| blocking_timeout | 15 seconds | Maximum time in seconds to block the `config` call until config has been initialized
 
 A notification signal will be triggered whenever a _new_ datafile is fetched and Project Config is updated. To subscribe to these notifications, use the `notification_center.add_notification_listener(Optimizely::NotificationCenter::NOTIFICATION_TYPES[:OPTIMIZELY_CONFIG_UPDATE], @callback)`
 

lib/optimizely.rb

@@ -52,7 +52,7 @@ class Project
     # @param skip_json_validation - Optional boolean param to skip JSON schema validation of the provided datafile.
     # @params sdk_key - Optional string uniquely identifying the datafile corresponding to project and environment combination.
     #                   Must provide at least one of datafile or sdk_key.
-    # @param config_manager - Optional Responds to get_config.
+    # @param config_manager - Optional Responds to 'config' method.
     # @param notification_center - Optional Instance of NotificationCenter.
     # @param event_processor - Optional Responds to process.
 
@@ -82,7 +82,7 @@ def initialize(
 
       @notification_center = notification_center.is_a?(Optimizely::NotificationCenter) ? notification_center : NotificationCenter.new(@logger, @error_handler)
 
-      @config_manager = if config_manager.respond_to?(:get_config)
+      @config_manager = if config_manager.respond_to?(:config)
                           config_manager
                         elsif sdk_key
                           HTTPProjectConfigManager.new(

lib/optimizely/config_manager/http_project_config_manager.rb

@@ -30,17 +30,17 @@ module Optimizely
   class HTTPProjectConfigManager < ProjectConfigManager
     # Config manager that polls for the datafile and updated ProjectConfig based on an update interval.
 
-    attr_reader :config, :closed
+    attr_reader :stopped
 
     # Initialize config manager. One of sdk_key or url has to be set to be able to use.
     #
     # sdk_key - Optional string uniquely identifying the datafile. It's required unless a URL is passed in.
     # datafile: Optional JSON string representing the project.
     # polling_interval - Optional floating point number representing time interval in seconds
     #                  at which to request datafile and set ProjectConfig.
-    # blocking_timeout -
-    # auto_update -
-    # start_by_default -
+    # blocking_timeout - Optional Time in seconds to block the config call until config object has been initialized.
+    # auto_update - Boolean indicates to run infinitely or only once.
+    # start_by_default - Boolean indicates to start by default AsyncScheduler.
     # url - Optional string representing URL from where to fetch the datafile. If set it supersedes the sdk_key.
     # url_template - Optional string template which in conjunction with sdk_key
     #               determines URL from where to fetch the datafile.
@@ -72,7 +72,7 @@ def initialize(
       @last_modified = nil
       @async_scheduler = AsyncScheduler.new(method(:fetch_datafile_config), @polling_interval, auto_update, @logger)
       @async_scheduler.start! if start_by_default == true
-      @closed = false
+      @stopped = false
       @skip_json_validation = skip_json_validation
       @notification_center = notification_center.is_a?(Optimizely::NotificationCenter) ? notification_center : NotificationCenter.new(@logger, @error_handler)
       @config = datafile.nil? ? nil : DatafileProjectConfig.create(datafile, @logger, @error_handler, @skip_json_validation)
@@ -85,33 +85,36 @@ def ready?
     end
 
     def start!
-      if @closed
-        @logger.log(Logger::WARN, 'Not starting. Already closed.')
+      if @stopped
+        @logger.log(Logger::WARN, 'Not starting. Already stopped.')
         return
       end
 
       @async_scheduler.start!
-      @closed = false
+      @stopped = false
     end
 
     def stop!
-      if @closed
+      if @stopped
         @logger.log(Logger::WARN, 'Not pausing. Manager has not been started.')
         return
       end
 
       @async_scheduler.stop!
       @config = nil
-      @closed = true
+      @stopped = true
     end
 
-    def get_config
+    def config
       # Get Project Config.
 
+      # if stopped is true, then simply return @config.
       # If the background datafile polling thread is running. and config has been initalized,
-      # we simply return config.
+      # we simply return @config.
       # If it is not, we wait and block maximum for @blocking_timeout.
       # If thread is not running, we fetch the datafile and update config.
+      return @config if @stopped
+
       if @async_scheduler.running
         return @config if ready?
 
@@ -232,9 +235,9 @@ def polling_interval(polling_interval)
     end
 
     def blocking_timeout(blocking_timeout)
-      # Sets time in seconds to block the get_config call until config has been initialized.
+      # Sets time in seconds to block the config call until config has been initialized.
       #
-      # blocking_timeout - Time in seconds after which to update datafile.
+      # blocking_timeout - Time in seconds to block the config call.
 
       # If valid set given timeout, default blocking_timeout otherwise.
 
@@ -243,7 +246,7 @@ def blocking_timeout(blocking_timeout)
           Logger::DEBUG,
           "Blocking timeout is not provided. Defaulting to #{Helpers::Constants::CONFIG_MANAGER['DEFAULT_BLOCKING_TIMEOUT']} seconds."
         )
-        @polling_interval = Helpers::Constants::CONFIG_MANAGER['DEFAULT_BLOCKING_TIMEOUT']
+        @blocking_timeout = Helpers::Constants::CONFIG_MANAGER['DEFAULT_BLOCKING_TIMEOUT']
         return
       end
 
@@ -252,7 +255,7 @@ def blocking_timeout(blocking_timeout)
           Logger::ERROR,
           "Blocking timeout '#{blocking_timeout}' has invalid type. Defaulting to #{Helpers::Constants::CONFIG_MANAGER['DEFAULT_BLOCKING_TIMEOUT']} seconds."
         )
-        @polling_interval = Helpers::Constants::CONFIG_MANAGER['DEFAULT_BLOCKING_TIMEOUT']
+        @blocking_timeout = Helpers::Constants::CONFIG_MANAGER['DEFAULT_BLOCKING_TIMEOUT']
         return
       end
 
@@ -261,7 +264,7 @@ def blocking_timeout(blocking_timeout)
           Logger::DEBUG,
           "Blocking timeout '#{blocking_timeout}' has invalid range. Defaulting to #{Helpers::Constants::CONFIG_MANAGER['DEFAULT_BLOCKING_TIMEOUT']} seconds."
         )
-        @polling_interval = Helpers::Constants::CONFIG_MANAGER['DEFAULT_BLOCKING_TIMEOUT']
+        @blocking_timeout = Helpers::Constants::CONFIG_MANAGER['DEFAULT_BLOCKING_TIMEOUT']
         return
       end
 

lib/optimizely/helpers/constants.rb

@@ -362,7 +362,7 @@ module Constants
 
       CONFIG_MANAGER = {
         'DATAFILE_URL_TEMPLATE' => 'https://cdn.optimizely.com/datafiles/%s.json',
-        # Default time in seconds to block the get_config call until config has been initialized.
+        # Default time in seconds to block the 'config' method call until 'config' instance has been initialized.
         'DEFAULT_BLOCKING_TIMEOUT' => 15,
         # Default config update interval of 5 minutes
         'DEFAULT_UPDATE_INTERVAL' => 5 * 60,

lib/optimizely/optimizely_factory.rb

@@ -77,7 +77,7 @@ def self.default_instance(sdk_key, datafile = nil)
 
     # Returns a new optimizely instance.
     #
-    # @param config_manager - Required ConfigManagerInterface Responds to get_config.
+    # @param config_manager - Required ConfigManagerInterface Responds to 'config' method.
     def self.default_instance_with_config_manager(config_manager)
       Optimizely::Project.new(nil, nil, nil, nil, nil, nil, nil, config_manager)
     end
@@ -92,7 +92,7 @@ def self.default_instance_with_config_manager(config_manager)
     #                 By default all exceptions will be suppressed.
     # @param skip_json_validation - Optional Boolean param to skip JSON schema validation of the provided datafile.
     # @param user_profile_service - Optional UserProfileServiceInterface Provides methods to store and retreive user profiles.
-    # @param config_manager - Optional ConfigManagerInterface Responds to get_config.
+    # @param config_manager - Optional ConfigManagerInterface Responds to 'config' method.
     # @param notification_center - Optional Instance of NotificationCenter.
     #
     # if @max_event_batch_size and @max_event_flush_interval are nil then default batchsize and flush_interval

spec/config_manager/http_project_config_manager_spec.rb

@@ -36,7 +36,7 @@
       )
 
       until http_project_config_manager.ready?; end
-      expect(http_project_config_manager.get_config).not_to eq(nil)
+      expect(http_project_config_manager.config).not_to eq(nil)
     end
 
     it 'should get project config when sdk_key is given' do
@@ -45,7 +45,7 @@
       )
 
       until http_project_config_manager.ready?; end
-      expect(http_project_config_manager.get_config).not_to eq(nil)
+      expect(http_project_config_manager.config).not_to eq(nil)
     end
 
     it 'should get project config when sdk_key and valid url_template is given' do
@@ -55,7 +55,7 @@
       )
       until http_project_config_manager.ready?; end
 
-      expect(http_project_config_manager.get_config).not_to eq(nil)
+      expect(http_project_config_manager.config).not_to eq(nil)
     end
 
     it 'should get instance ready immediately when datafile is provided' do
@@ -80,7 +80,7 @@
       until http_project_config_manager.ready?; end
       finish = Time.now
       expect(finish - start).to be > 0
-      expect(http_project_config_manager.get_config).not_to eq(nil)
+      expect(http_project_config_manager.config).not_to eq(nil)
     end
 
     it 'should send config update notification when project config is updated' do
@@ -96,7 +96,7 @@
       )
 
       until http_project_config_manager.ready?; end
-      expect(http_project_config_manager.get_config).not_to eq(nil)
+      expect(http_project_config_manager.config).not_to eq(nil)
     end
 
     it 'should not send config update notification when datafile is provided' do
@@ -129,8 +129,8 @@
       http_project_config_manager.start!
 
       # All instance variables values of http_project_config_manager
-      http_project_config_manager_arr = http_project_config_manager.get_config.instance_variables.map do |attr|
-        http_project_config_manager.get_config.instance_variable_get attr
+      http_project_config_manager_arr = http_project_config_manager.config.instance_variables.map do |attr|
+        http_project_config_manager.config.instance_variable_get attr
       end
 
       # All instance variables values of datafile_project_config
@@ -149,7 +149,7 @@
         auto_update: false,
         start_by_default: false
       )
-      expect(http_project_config_manager.get_config).not_to eq(nil)
+      expect(http_project_config_manager.config).not_to eq(nil)
       finish = Time.now
       expect(finish - start).to be < 1
     end
@@ -181,7 +181,7 @@
         'Old revision number: 42. New revision number: 81.').once
 
       # Asserts that config has updated from URL.
-      expect(http_project_config_manager.get_config.account_id).not_to eql(datafile_project_config.account_id)
+      expect(http_project_config_manager.config.account_id).not_to eql(datafile_project_config.account_id)
     end
   end
 
@@ -195,7 +195,7 @@
         error_handler: error_handler
       )
 
-      expect(http_project_config_manager.get_config).not_to eq(nil)
+      expect(http_project_config_manager.config).not_to eq(nil)
     end
 
     it 'should get instance ready immediately' do
@@ -236,7 +236,7 @@
       expect(spy_logger).to have_received(:log).with(Logger::DEBUG, 'Received new datafile and updated config. ' \
         'Old revision number: 42. New revision number: 81.').once
 
-      expect(http_project_config_manager.get_config.account_id).not_to eql(datafile_project_config.account_id)
+      expect(http_project_config_manager.config.account_id).not_to eql(datafile_project_config.account_id)
     end
   end
 
@@ -246,7 +246,7 @@
         sdk_key: 'QBw9gFM8oTn7ogY9ANCC1z'
       )
 
-      expect(http_project_config_manager.get_config).not_to eq(nil)
+      expect(http_project_config_manager.config).not_to eq(nil)
     end
 
     it 'should fetch datafile url and get instance ready' do
@@ -275,7 +275,7 @@
         blocking_timeout: 5
       )
       http_project_config_manager.start!
-      expect(http_project_config_manager.get_config).to eq(nil)
+      expect(http_project_config_manager.config).to eq(nil)
     end
   end
 
@@ -294,77 +294,89 @@
   end
 
   describe '.polling_interval' do
-    it 'should log an error when polling_interval is nil' do
-      Optimizely::HTTPProjectConfigManager.new(
+    it 'should set default and log an error when polling_interval is nil' do
+      http_project_config_manager = Optimizely::HTTPProjectConfigManager.new(
         sdk_key: 'sdk_key',
         url: nil,
         polling_interval: nil,
         blocking_timeout: 5,
         error_handler: error_handler,
         logger: spy_logger
       )
+
+      expect(http_project_config_manager.instance_variable_get(:@polling_interval)).to eq(300)
       expect(spy_logger).to have_received(:log).once.with(Logger::DEBUG, 'Polling interval is not provided. Defaulting to 300 seconds.')
     end
 
-    it 'should log an error when polling_interval has invalid type' do
-      Optimizely::HTTPProjectConfigManager.new(
+    it 'should set default and log an error when polling_interval has invalid type' do
+      http_project_config_manager = Optimizely::HTTPProjectConfigManager.new(
         sdk_key: 'sdk_key',
         url: nil,
         polling_interval: true,
         blocking_timeout: 5,
         error_handler: error_handler,
         logger: spy_logger
       )
+
+      expect(http_project_config_manager.instance_variable_get(:@polling_interval)).to eq(300)
       expect(spy_logger).to have_received(:log).once.with(Logger::ERROR, "Polling interval 'true' has invalid type. Defaulting to 300 seconds.")
     end
 
-    it 'should log an error when polling_interval has invalid range' do
-      Optimizely::HTTPProjectConfigManager.new(
+    it 'should set default and log an error when polling_interval has invalid range' do
+      http_project_config_manager = Optimizely::HTTPProjectConfigManager.new(
         sdk_key: 'sdk_key',
         url: nil,
         polling_interval: 999_999_999_999_999_999,
         blocking_timeout: 5,
         error_handler: error_handler,
         logger: spy_logger
       )
+
+      expect(http_project_config_manager.instance_variable_get(:@polling_interval)).to eq(300)
       expect(spy_logger).to have_received(:log).once.with(Logger::DEBUG, "Polling interval '999999999999999999' has invalid range. Defaulting to 300 seconds.")
     end
   end
 
   describe '.blocking_timeout' do
-    it 'should log an error when blocking_timeout is nil' do
-      Optimizely::HTTPProjectConfigManager.new(
+    it 'should set default and log an error when blocking_timeout is nil' do
+      http_project_config_manager = Optimizely::HTTPProjectConfigManager.new(
         sdk_key: 'sdk_key',
         url: nil,
         polling_interval: 5,
         blocking_timeout: nil,
         error_handler: error_handler,
         logger: spy_logger
       )
+
+      expect(http_project_config_manager.instance_variable_get(:@blocking_timeout)).to eq(15)
       expect(spy_logger).to have_received(:log).once.with(Logger::DEBUG, 'Blocking timeout is not provided. Defaulting to 15 seconds.')
     end
 
-    it 'should log an error when blocking_timeout has invalid type' do
-      Optimizely::HTTPProjectConfigManager.new(
+    it 'should set default and log an error when blocking_timeout has invalid type' do
+      http_project_config_manager = Optimizely::HTTPProjectConfigManager.new(
         sdk_key: 'sdk_key',
         url: nil,
         polling_interval: 5,
         blocking_timeout: true,
         error_handler: error_handler,
         logger: spy_logger
       )
+
+      expect(http_project_config_manager.instance_variable_get(:@blocking_timeout)).to eq(15)
       expect(spy_logger).to have_received(:log).once.with(Logger::ERROR, "Blocking timeout 'true' has invalid type. Defaulting to 15 seconds.")
     end
 
-    it 'should log an error when blocking_timeout has invalid range' do
-      Optimizely::HTTPProjectConfigManager.new(
+    it 'should set default and log an error when blocking_timeout has invalid range' do
+      http_project_config_manager = Optimizely::HTTPProjectConfigManager.new(
         sdk_key: 'sdk_key',
         url: nil,
         polling_interval: 5,
         blocking_timeout: 999_999_999_999_999_999,
         error_handler: error_handler,
         logger: spy_logger
       )
+
+      expect(http_project_config_manager.instance_variable_get(:@blocking_timeout)).to eq(15)
       expect(spy_logger).to have_received(:log).once.with(Logger::DEBUG, "Blocking timeout '999999999999999999' has invalid range. Defaulting to 15 seconds.")
     end
   end

spec/optimizely_factory_spec.rb

@@ -52,7 +52,7 @@
   describe '.default_instance_with_manager' do
     it 'should take provided custom config manager' do
       class CustomConfigManager
-        def get_config; end
+        attr_reader :config
       end
 
       custom_config_manager = CustomConfigManager.new