Skip to content

Commit 92bf3be

Browse files
fix: odp issues identified by fsc (#322)
* prevent errant identify calls * select first odp integration in datafile * capture uri scheme error * fix validation when data is nil
1 parent d999d72 commit 92bf3be

File tree

9 files changed

+52
-11
lines changed

9 files changed

+52
-11
lines changed

lib/optimizely.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -530,7 +530,7 @@ def is_feature_enabled(feature_flag_key, user_id, attributes = nil)
530530
return false
531531
end
532532

533-
user_context = create_user_context(user_id, attributes)
533+
user_context = OptimizelyUserContext.new(self, user_id, attributes, identify: false)
534534
decision, = @decision_service.get_variation_for_feature(config, feature_flag, user_context)
535535

536536
feature_enabled = false
@@ -770,7 +770,7 @@ def get_all_feature_variables(feature_flag_key, user_id, attributes = nil)
770770
return nil
771771
end
772772

773-
user_context = create_user_context(user_id, attributes)
773+
user_context = OptimizelyUserContext.new(self, user_id, attributes, identify: false)
774774
decision, = @decision_service.get_variation_for_feature(config, feature_flag, user_context)
775775
variation = decision ? decision['variation'] : nil
776776
feature_enabled = variation ? variation['featureEnabled'] : false
@@ -931,7 +931,7 @@ def get_variation_with_config(experiment_key, user_id, attributes, config)
931931

932932
return nil unless user_inputs_valid?(attributes)
933933

934-
user_context = create_user_context(user_id, attributes)
934+
user_context = OptimizelyUserContext.new(self, user_id, attributes, identify: false)
935935
variation_id, = @decision_service.get_variation(config, experiment_id, user_context)
936936
variation = config.get_variation_from_id(experiment_key, variation_id) unless variation_id.nil?
937937
variation_key = variation['key'] if variation
@@ -998,7 +998,7 @@ def get_feature_variable_for_type(feature_flag_key, variable_key, variable_type,
998998
return nil
999999
end
10001000

1001-
user_context = create_user_context(user_id, attributes)
1001+
user_context = OptimizelyUserContext.new(self, user_id, attributes, identify: false)
10021002
decision, = @decision_service.get_variation_for_feature(config, feature_flag, user_context)
10031003
variation = decision ? decision['variation'] : nil
10041004
feature_enabled = variation ? variation['featureEnabled'] : false

lib/optimizely/config/datafile_project_config.rb

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ def initialize(datafile, logger, error_handler)
9393
@experiment_key_map = generate_key_map(@experiments, 'key')
9494
@experiment_id_map = generate_key_map(@experiments, 'id')
9595
@audience_id_map = generate_key_map(@audiences, 'id')
96-
@integration_key_map = generate_key_map(@integrations, 'key')
96+
@integration_key_map = generate_key_map(@integrations, 'key', first_value: true)
9797
@audience_id_map = @audience_id_map.merge(generate_key_map(@typed_audiences, 'id')) unless @typed_audiences.empty?
9898
@variation_id_map = {}
9999
@variation_key_map = {}
@@ -525,15 +525,19 @@ def generate_feature_variation_map(feature_flags)
525525
flag_variation_map
526526
end
527527

528-
def generate_key_map(array, key)
528+
def generate_key_map(array, key, first_value: false)
529529
# Helper method to generate map from key to hash in array of hashes
530530
#
531531
# array - Array consisting of hash
532532
# key - Key in each hash which will be key in the map
533+
# first_value - Determines which value to save if there are duplicate keys. By default the last instance of the key
534+
# will be saved. Set to true to save the first key/value encountered.
533535
#
534536
# Returns map mapping key to hash
535537

536-
Hash[array.map { |obj| [obj[key], obj] }]
538+
array
539+
.group_by { |obj| obj[key] }
540+
.transform_values { |group| first_value ? group.first : group.last }
537541
end
538542
end
539543
end

lib/optimizely/exceptions.rb

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,13 @@ def initialize(msg = 'HTTP call resulted in a response with an error code.')
2525
end
2626
end
2727

28+
class HTTPUriError < Error
29+
# Raised when a provided URI is invalid.
30+
def initialize(msg = 'Provided URI was invalid.')
31+
super
32+
end
33+
end
34+
2835
class InvalidAudienceError < Error
2936
# Raised when an invalid audience is provided
3037

lib/optimizely/helpers/http_utils.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#
1818

1919
require 'net/http'
20+
require_relative '../exceptions'
2021

2122
module Optimizely
2223
module Helpers
@@ -28,6 +29,8 @@ def make_request(url, http_method, request_body = nil, headers = {}, read_timeou
2829
#
2930
uri = URI.parse(url)
3031

32+
raise HTTPUriError unless uri.respond_to?(:request_uri)
33+
3134
case http_method
3235
when :get
3336
request = Net::HTTP::Get.new(uri.request_uri)

lib/optimizely/helpers/validator.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ def finite_number?(value)
181181

182182
def odp_data_types_valid?(data)
183183
valid_types = [String, Float, Integer, TrueClass, FalseClass, NilClass]
184-
data.values.all? { |e| valid_types.member? e.class }
184+
data&.values&.all? { |e| valid_types.member? e.class }
185185
end
186186

187187
def segments_cache_valid?(segments_cache)

lib/optimizely/odp/odp_segment_api_manager.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#
1818

1919
require 'json'
20+
require_relative '../exceptions'
2021

2122
module Optimizely
2223
class OdpSegmentApiManager
@@ -59,7 +60,7 @@ def fetch_segments(api_key, api_host, user_key, user_value, segments_to_check)
5960
@logger.log(Logger::DEBUG, "GraphQL download failed: #{e}")
6061
log_failure('network error')
6162
return nil
62-
rescue Errno::EINVAL, Net::HTTPBadResponse, Net::HTTPHeaderSyntaxError => e
63+
rescue Errno::EINVAL, Net::HTTPBadResponse, Net::HTTPHeaderSyntaxError, HTTPUriError => e
6364
log_failure(e)
6465
return nil
6566
end

lib/optimizely/optimizely_config.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ def lookup_name_from_id(audience_id, audiences_map)
201201
def stringify_conditions(conditions, audiences_map)
202202
operand = 'OR'
203203
conditions_str = ''
204-
length = conditions.length()
204+
length = conditions.length
205205
return '' if length.zero?
206206
return "\"#{lookup_name_from_id(conditions[0], audiences_map)}\"" if length == 1 && !OPERATORS.include?(conditions[0])
207207

spec/project_spec.rb

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4659,5 +4659,25 @@ class InvalidEventManager; end # rubocop:disable Lint/ConstantDefinitionInBlock
46594659
project.send_odp_event(type: 'wow', action: 'great', identifiers: {}, data: {'wow': {}})
46604660
project.close
46614661
end
4662+
4663+
it 'should not send odp events with legacy apis' do
4664+
experiment_key = 'experiment-segment'
4665+
feature_key = 'flag-segment'
4666+
user_id = 'test_user'
4667+
4668+
project = Optimizely::Project.new(config_body_integrations_JSON, nil, spy_logger)
4669+
allow(project.event_dispatcher).to receive(:dispatch_event).with(instance_of(Optimizely::Event))
4670+
expect(project.odp_manager).not_to receive(:send_event)
4671+
4672+
project.activate(experiment_key, user_id)
4673+
project.track('event1', user_id)
4674+
project.get_variation(experiment_key, user_id)
4675+
project.get_all_feature_variables(feature_key, user_id)
4676+
project.is_feature_enabled(feature_key, user_id)
4677+
4678+
expect(spy_logger).not_to have_received(:log).with(Logger::ERROR, anything)
4679+
4680+
project.close
4681+
end
46624682
end
46634683
end

spec/spec_params.rb

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1321,7 +1321,13 @@ module OptimizelySpec
13211321
}
13221322
],
13231323
'accountId' => '10367498574',
1324-
'events' => [],
1324+
'events' => [
1325+
{
1326+
'experimentIds' => ['10420810910'],
1327+
'id' => '10404198134',
1328+
'key' => 'event1'
1329+
}
1330+
],
13251331
'revision' => '101'
13261332
}.freeze
13271333

0 commit comments

Comments
 (0)