Skip to content

[FSSDK-9074] fix: odp event validation #334

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion lib/optimizely.rb
Original file line number Diff line number Diff line change
Expand Up @@ -895,7 +895,7 @@ def get_optimizely_config

# Send an event to the ODP server.
#
# @param action - the event action name.
# @param action - the event action name. Cannot be nil or empty string.
# @param identifiers - a hash for identifiers. The caller must provide at least one key-value pair.
# @param type - the event type (default = "fullstack").
# @param data - a hash for associated data. The default event data will be added to this data before sending to the ODP server.
Expand All @@ -911,6 +911,13 @@ def send_odp_event(action:, identifiers:, type: Helpers::Constants::ODP_MANAGER_
return
end

if action.nil? || action.empty?
@logger.log(Logger::ERROR, Helpers::Constants::ODP_LOGS[:ODP_INVALID_ACTION])
return
end

type = Helpers::Constants::ODP_MANAGER_CONFIG[:EVENT_TYPE] if type.nil? || type.empty?

@odp_manager.send_event(type: type, action: action, identifiers: identifiers, data: data)
end

Expand Down
3 changes: 2 additions & 1 deletion lib/optimizely/helpers/constants.rb
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,8 @@ module Constants
ODP_EVENT_FAILED: 'ODP event send failed (%s).',
ODP_NOT_ENABLED: 'ODP is not enabled.',
ODP_NOT_INTEGRATED: 'ODP is not integrated.',
ODP_INVALID_DATA: 'ODP data is not valid.'
ODP_INVALID_DATA: 'ODP data is not valid.',
ODP_INVALID_ACTION: 'ODP action is not valid (cannot be empty).'
}.freeze

DECISION_NOTIFICATION_TYPES = {
Expand Down
22 changes: 21 additions & 1 deletion lib/optimizely/odp/odp_event.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,18 @@
#

require 'json'
require_relative '../helpers/constants'

module Optimizely
class OdpEvent
# Representation of an odp event which can be sent to the Optimizely odp platform.

KEY_FOR_USER_ID = Helpers::Constants::ODP_MANAGER_CONFIG[:KEY_FOR_USER_ID]

def initialize(type:, action:, identifiers:, data:)
@type = type
@action = action
@identifiers = identifiers
@identifiers = convert_identifiers(identifiers)
@data = add_common_event_data(data)
end

Expand All @@ -39,6 +43,22 @@ def add_common_event_data(custom_data)
data
end

def convert_identifiers(identifiers)
# Convert incorrect case/separator of identifier key `fs_user_id`
# (ie. `fs-user-id`, `FS_USER_ID`).

identifiers.clone.each_key do |key|
break if key == KEY_FOR_USER_ID

if ['fs-user-id', KEY_FOR_USER_ID].include?(key.downcase)
identifiers[KEY_FOR_USER_ID] = identifiers.delete(key)
break
end
end

identifiers
end

def to_json(*_args)
{
type: @type,
Expand Down
14 changes: 14 additions & 0 deletions spec/odp/odp_event_manager_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,20 @@
event[:data]['invalid-item'] = {}
expect(Optimizely::Helpers::Validator.odp_data_types_valid?(event[:data])).to be false
end

it 'should convert invalid event identifier' do
event = Optimizely::OdpEvent.new(type: 'type', action: 'action', identifiers: {'fs-user-id' => 'great'}, data: {})
expect(event.instance_variable_get('@identifiers')).to eq({'fs_user_id' => 'great'})

event = Optimizely::OdpEvent.new(type: 'type', action: 'action', identifiers: {'FS-user-ID' => 'great'}, data: {})
expect(event.instance_variable_get('@identifiers')).to eq({'fs_user_id' => 'great'})

event = Optimizely::OdpEvent.new(type: 'type', action: 'action', identifiers: {'FS_USER_ID' => 'great', 'fs.user.id' => 'wow'}, data: {})
expect(event.instance_variable_get('@identifiers')).to eq({'fs_user_id' => 'great', 'fs.user.id' => 'wow'})

event = Optimizely::OdpEvent.new(type: 'type', action: 'action', identifiers: {'fs_user_id' => 'great', 'fsuserid' => 'wow'}, data: {})
expect(event.instance_variable_get('@identifiers')).to eq({'fs_user_id' => 'great', 'fsuserid' => 'wow'})
end
end

describe '#initialize' do
Expand Down
34 changes: 34 additions & 0 deletions spec/project_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4916,5 +4916,39 @@ class InvalidEventManager; end # rubocop:disable Lint/ConstantDefinitionInBlock

project.close
end

it 'should log error with nil action' do
expect(spy_logger).to receive(:log).once.with(Logger::ERROR, 'ODP action is not valid (cannot be empty).')
project = Optimizely::Project.new(config_body_integrations_JSON, nil, spy_logger)
project.send_odp_event(type: 'wow', action: nil, identifiers: {amazing: 'fantastic'}, data: {})
project.close
end

it 'should log error with empty string action' do
expect(spy_logger).to receive(:log).once.with(Logger::ERROR, 'ODP action is not valid (cannot be empty).')
project = Optimizely::Project.new(config_body_integrations_JSON, nil, spy_logger)
project.send_odp_event(type: 'wow', action: '', identifiers: {amazing: 'fantastic'}, data: {})
project.close
end

it 'should use default with nil type' do
project = Optimizely::Project.new(config_body_integrations_JSON, nil, spy_logger)
expect(project.odp_manager).to receive('send_event').with(type: 'fullstack', action: 'great', identifiers: {amazing: 'fantastic'}, data: {})
project.send_odp_event(type: nil, action: 'great', identifiers: {amazing: 'fantastic'}, data: {})

expect(spy_logger).not_to have_received(:log).with(Logger::ERROR, anything)

project.close
end

it 'should use default with empty string type' do
project = Optimizely::Project.new(config_body_integrations_JSON, nil, spy_logger)
expect(project.odp_manager).to receive('send_event').with(type: 'fullstack', action: 'great', identifiers: {amazing: 'fantastic'}, data: {})
project.send_odp_event(type: '', action: 'great', identifiers: {amazing: 'fantastic'}, data: {})

expect(spy_logger).not_to have_received(:log).with(Logger::ERROR, anything)

project.close
end
end
end