diff --git a/lib/optimizely/event/event_factory.rb b/lib/optimizely/event/event_factory.rb index b1afa103..a5950ff2 100644 --- a/lib/optimizely/event/event_factory.rb +++ b/lib/optimizely/event/event_factory.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # -# Copyright 2019-2020, 2022-2023, Optimizely and contributors +# Copyright 2019-2020, 2022-2023, 2026, Optimizely and contributors # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -23,6 +23,7 @@ require_relative 'entity/snapshot_event' require_relative 'entity/visitor' require 'optimizely/helpers/validator' +require 'optimizely/helpers/event_id_validator' module Optimizely class EventFactory # EventFactory builds LogEvent objects from a given user_event. @@ -108,15 +109,24 @@ def build_attribute_list(user_attributes, project_config) private def create_impression_event_visitor(impression_event) + normalized_campaign_id = Helpers::EventIdValidator.normalize_campaign_id( + impression_event.experiment_layer_id, impression_event.experiment_id + ) + normalized_variation_id = Helpers::EventIdValidator.normalize_variation_id( + impression_event.variation_id + ) + decision = Decision.new( - campaign_id: impression_event.experiment_layer_id, + campaign_id: normalized_campaign_id, experiment_id: impression_event.experiment_id, - variation_id: impression_event.variation_id, + variation_id: normalized_variation_id, metadata: impression_event.metadata ) + # FR-009: entity_id on impression events must equal decisions[].campaign_id + # byte-for-byte. Reuse the already-normalized campaign_id. snapshot_event = Optimizely::SnapshotEvent.new( - entity_id: impression_event.experiment_layer_id, + entity_id: normalized_campaign_id, timestamp: impression_event.timestamp, uuid: impression_event.uuid, key: ACTIVATE_EVENT_KEY diff --git a/lib/optimizely/helpers/event_id_validator.rb b/lib/optimizely/helpers/event_id_validator.rb new file mode 100644 index 00000000..7db3ffdc --- /dev/null +++ b/lib/optimizely/helpers/event_id_validator.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true + +# +# Copyright 2026, Optimizely and contributors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +module Optimizely + module Helpers + # EventIdValidator normalizes ID fields on outgoing decision events so + # that the wire payload is byte-equivalent across SDKs. + # + # Two contracts apply: + # * campaign_id and entity_id (impression events): valid iff non-empty + # string. Any character content is acceptable — IDs may be opaque, + # e.g. "default-12345", "layer_abc". The fallback to experiment_id + # fires ONLY when the value is empty string, nil, or missing. + # * variation_id: valid iff non-empty string of decimal digits [0-9]. + # Leading zeros are allowed. Whitespace, signs, decimal points, and + # exponents are NOT allowed. Falls back to nil otherwise. + # + # Non-string types (raw number, boolean, object) are out of scope per + # spec — behavior on such inputs is undefined and not asserted. + # + # This module is silent by design: per spec it must NOT log or warn on the + # normalization path (FR-007), and must NOT fail or defer event dispatch + # (FR-006). + module EventIdValidator + module_function + + # Matches a non-empty string of decimal digits only (no sign, no + # whitespace, no decimal point, no exponent). + NUMERIC_STRING_PATTERN = /\A[0-9]+\z/ + + # Returns true when value is a non-empty string consisting entirely of + # decimal digits. Returns false for nil, non-strings, empty strings, or + # strings containing any non-digit character (including whitespace). + # Used for variation_id (which retains the stricter numeric-only + # contract). + def numeric_string?(value) + value.is_a?(String) && NUMERIC_STRING_PATTERN.match?(value) + end + + # Returns true when value is a non-empty string of any character + # content. Returns false for nil, non-strings, or empty strings. + # Used for campaign_id and entity_id (which accept opaque IDs). + def non_empty_string?(value) + value.is_a?(String) && !value.empty? + end + + # Normalize a decision's campaign_id (and impression event's entity_id). + # + # When the provided campaign_id is a non-empty string (numeric or + # opaque), return it unchanged. Otherwise, substitute experiment_id + # when experiment_id is itself a non-empty string. When neither is a + # non-empty string, return whatever experiment_id was (typically '' or + # nil) so the wire payload remains consistent with upstream contract. + def normalize_campaign_id(campaign_id, experiment_id) + return campaign_id if non_empty_string?(campaign_id) + + # Per FR-002, fall back to experiment_id. Per the spec edge case, + # when experiment_id is itself empty/null we still emit the event + # (FR-006) carrying whatever experiment_id value was present — + # represented here as an empty string to preserve string typing on + # the wire payload. + return experiment_id if non_empty_string?(experiment_id) + + '' + end + + # Normalize a decision's variation_id. + # + # When the provided variation_id is a valid numeric string, return it + # unchanged. Otherwise return nil so the wire payload encodes the field + # as JSON null. variation_id retains the stricter numeric-string-only + # contract — opaque/non-numeric placeholders are normalized to nil. + def normalize_variation_id(variation_id) + return variation_id if numeric_string?(variation_id) + + nil + end + end + end +end diff --git a/spec/event/event_factory_spec.rb b/spec/event/event_factory_spec.rb index f9771e4f..ba618532 100644 --- a/spec/event/event_factory_spec.rb +++ b/spec/event/event_factory_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # -# Copyright 2019-2020, Optimizely and contributors +# Copyright 2019-2020, 2026, Optimizely and contributors # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -741,4 +741,241 @@ expect(log_event.url).to eq(@expected_endpoints[:US]) expect(log_event.http_verb).to eq(:post) end + + # ---------------------------------------------------------------------- + # Decision-event ID normalization tests. + # + # These integration tests exercise the EventFactory wire payload to verify + # that campaign_id, variation_id, and entity_id are normalized uniformly + # across every decision type (experiment, feature test, rollout, holdout). + # ---------------------------------------------------------------------- + describe 'decision event ID normalization' do + let(:event_context) do + Optimizely::EventContext.new( + region: 'US', + account_id: '12001', + project_id: '111001', + anonymize_ip: false, + revision: '42', + client_name: Optimizely::CLIENT_ENGINE, + client_version: Optimizely::VERSION + ).as_json + end + let(:metadata) do + { + flag_key: 'flag_a', + rule_key: 'rule_a', + rule_type: 'experiment', + variation_key: 'var_a', + enabled: true + } + end + + def build_impression(experiment_layer_id:, experiment_id:, variation_id:) + Optimizely::ImpressionEvent.new( + event_context: event_context, + user_id: 'test_user', + experiment_layer_id: experiment_layer_id, + experiment_id: experiment_id, + variation_id: variation_id, + metadata: metadata, + visitor_attributes: [], + bot_filtering: nil + ) + end + + def first_decision(log_event) + log_event.params[:visitors][0][:snapshots][0][:decisions][0] + end + + def first_event(log_event) + log_event.params[:visitors][0][:snapshots][0][:events][0] + end + + it 'passes valid numeric campaign_id and variation_id through unchanged' do + impression = build_impression( + experiment_layer_id: '111111', experiment_id: '222222', variation_id: '333333' + ) + log_event = Optimizely::EventFactory.create_log_event(impression, spy_logger) + decision = first_decision(log_event) + event = first_event(log_event) + + expect(decision[:campaign_id]).to eq('111111') + expect(decision[:experiment_id]).to eq('222222') + expect(decision[:variation_id]).to eq('333333') + expect(event[:entity_id]).to eq('111111') + expect(event[:entity_id]).to eq(decision[:campaign_id]) + end + + it 'substitutes experiment_id when campaign_id (layerId) is nil' do + # FR-001/FR-002: nil campaign_id must be replaced with experiment_id. + impression = build_impression( + experiment_layer_id: nil, experiment_id: '222222', variation_id: '333333' + ) + log_event = Optimizely::EventFactory.create_log_event(impression, spy_logger) + decision = first_decision(log_event) + event = first_event(log_event) + + expect(decision[:campaign_id]).to eq('222222') + # FR-009: entity_id must equal decisions[].campaign_id byte-for-byte. + expect(event[:entity_id]).to eq('222222') + end + + it 'substitutes experiment_id when campaign_id is an empty string' do + impression = build_impression( + experiment_layer_id: '', experiment_id: '222222', variation_id: '333333' + ) + log_event = Optimizely::EventFactory.create_log_event(impression, spy_logger) + expect(first_decision(log_event)[:campaign_id]).to eq('222222') + expect(first_event(log_event)[:entity_id]).to eq('222222') + end + + it 'passes whitespace campaign_id through unchanged (relaxed contract: non-empty string)' do + # Per relaxed spec, any non-empty string is valid for campaign_id — + # only empty string / nil / missing trigger the experiment_id fallback. + impression = build_impression( + experiment_layer_id: ' ', experiment_id: '222222', variation_id: '333333' + ) + log_event = Optimizely::EventFactory.create_log_event(impression, spy_logger) + expect(first_decision(log_event)[:campaign_id]).to eq(' ') + expect(first_event(log_event)[:entity_id]).to eq(' ') + end + + it 'passes non-numeric opaque campaign_id through unchanged (relaxed contract)' do + # Per relaxed spec, opaque IDs such as "default-12345" or "layer_abc" + # are valid for campaign_id and entity_id; only empty/null trigger the + # experiment_id fallback. + impression = build_impression( + experiment_layer_id: 'campaign_a', experiment_id: '222222', variation_id: '333333' + ) + log_event = Optimizely::EventFactory.create_log_event(impression, spy_logger) + expect(first_decision(log_event)[:campaign_id]).to eq('campaign_a') + expect(first_event(log_event)[:entity_id]).to eq('campaign_a') + end + + it 'passes prefixed opaque campaign_id (default-12345) through unchanged' do + impression = build_impression( + experiment_layer_id: 'default-12345', experiment_id: '222222', variation_id: '333333' + ) + log_event = Optimizely::EventFactory.create_log_event(impression, spy_logger) + expect(first_decision(log_event)[:campaign_id]).to eq('default-12345') + expect(first_event(log_event)[:entity_id]).to eq('default-12345') + end + + it 'falls back to empty string when both campaign_id and experiment_id are invalid' do + # Mirrors the legacy empty-slot impression case that send_impression emits + # when there is no decision (e.g. send_flag_decisions and no rule matched). + impression = build_impression( + experiment_layer_id: '', experiment_id: '', variation_id: nil + ) + log_event = Optimizely::EventFactory.create_log_event(impression, spy_logger) + decision = first_decision(log_event) + event = first_event(log_event) + + expect(decision[:campaign_id]).to eq('') + expect(decision[:variation_id]).to be_nil + expect(event[:entity_id]).to eq('') + end + + it 'normalizes invalid variation_id to nil (empty string case)' do + # FR-003/FR-004: empty variation_id becomes nil. + impression = build_impression( + experiment_layer_id: '111111', experiment_id: '222222', variation_id: '' + ) + log_event = Optimizely::EventFactory.create_log_event(impression, spy_logger) + expect(first_decision(log_event)[:variation_id]).to be_nil + end + + it 'normalizes invalid variation_id to nil (non-numeric placeholder case)' do + impression = build_impression( + experiment_layer_id: '111111', experiment_id: '222222', variation_id: 'variation_a' + ) + log_event = Optimizely::EventFactory.create_log_event(impression, spy_logger) + expect(first_decision(log_event)[:variation_id]).to be_nil + end + + it 'normalizes invalid variation_id to nil (whitespace case)' do + impression = build_impression( + experiment_layer_id: '111111', experiment_id: '222222', variation_id: ' ' + ) + log_event = Optimizely::EventFactory.create_log_event(impression, spy_logger) + expect(first_decision(log_event)[:variation_id]).to be_nil + end + + it 'normalizes invalid variation_id to nil (non-string case)' do + impression = build_impression( + experiment_layer_id: '111111', experiment_id: '222222', variation_id: 333_333 + ) + log_event = Optimizely::EventFactory.create_log_event(impression, spy_logger) + expect(first_decision(log_event)[:variation_id]).to be_nil + end + + it 'leaves valid nil variation_id as nil (already-normalized)' do + impression = build_impression( + experiment_layer_id: '111111', experiment_id: '222222', variation_id: nil + ) + log_event = Optimizely::EventFactory.create_log_event(impression, spy_logger) + expect(first_decision(log_event)[:variation_id]).to be_nil + end + + it 'applies the same normalization for holdout decision metadata (FR-005)' do + # FR-005: normalization must be uniform across decision types. A holdout + # decision carries rule_type 'holdout' in metadata but still flows through + # the same impression factory path — so the same normalization applies. + holdout_metadata = metadata.merge(rule_type: 'holdout') + impression = Optimizely::ImpressionEvent.new( + event_context: event_context, + user_id: 'test_user', + experiment_layer_id: '', # holdout with no layer id + experiment_id: '999777', # falls back to holdout id + variation_id: 'invalid_placeholder', + metadata: holdout_metadata, + visitor_attributes: [], + bot_filtering: nil + ) + log_event = Optimizely::EventFactory.create_log_event(impression, spy_logger) + decision = first_decision(log_event) + event = first_event(log_event) + + expect(decision[:campaign_id]).to eq('999777') + expect(decision[:variation_id]).to be_nil + expect(event[:entity_id]).to eq('999777') + expect(decision[:metadata][:rule_type]).to eq('holdout') + end + + it 'does not log or warn on the normalization path (FR-007)' do + impression = build_impression( + experiment_layer_id: '', experiment_id: '222222', variation_id: 'bad' + ) + Optimizely::EventFactory.create_log_event(impression, spy_logger) + + # spy_logger.log should not have been invoked for any normalization + # bookkeeping (we still allow other log calls from upstream code paths, + # but in this isolated test there are none). + expect(spy_logger).not_to have_received(:log) + end + + it 'still emits the event payload when IDs are invalid (FR-006: do not drop)' do + impression = build_impression( + experiment_layer_id: nil, experiment_id: nil, variation_id: 'bad' + ) + log_event = Optimizely::EventFactory.create_log_event(impression, spy_logger) + expect(log_event).not_to be_nil + expect(log_event.params[:visitors][0][:snapshots][0][:decisions]).not_to be_empty + expect(log_event.params[:visitors][0][:snapshots][0][:events]).not_to be_empty + end + + it 'does not normalize conversion event entity_id (FR-010)' do + # Conversion events derive entity_id from the event id source and must + # remain unchanged. The conversion fixture's entity_id (111095) is + # already numeric and must be preserved verbatim. + experiment_event = project_config.get_event_from_key('test_event') + allow(project_config).to receive(:bot_filtering).and_return(true) + conversion_event = Optimizely::UserEventFactory.create_conversion_event( + project_config, experiment_event, 'test_user', nil, nil + ) + log_event = Optimizely::EventFactory.create_log_event(conversion_event, spy_logger) + expect(first_event(log_event)[:entity_id]).to eq('111095') + end + end end diff --git a/spec/helpers/event_id_validator_spec.rb b/spec/helpers/event_id_validator_spec.rb new file mode 100644 index 00000000..b9c90756 --- /dev/null +++ b/spec/helpers/event_id_validator_spec.rb @@ -0,0 +1,206 @@ +# frozen_string_literal: true + +# +# Copyright 2026, Optimizely and contributors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +require 'spec_helper' +require 'optimizely/helpers/event_id_validator' + +describe Optimizely::Helpers::EventIdValidator do + describe '.numeric_string?' do + it 'accepts a non-empty string of decimal digits' do + expect(described_class.numeric_string?('12345')).to be true + end + + it 'accepts a single digit' do + expect(described_class.numeric_string?('0')).to be true + end + + it 'accepts leading zeros' do + expect(described_class.numeric_string?('007')).to be true + end + + it 'rejects nil' do + expect(described_class.numeric_string?(nil)).to be false + end + + it 'rejects empty string' do + expect(described_class.numeric_string?('')).to be false + end + + it 'rejects whitespace-only string' do + expect(described_class.numeric_string?(' ')).to be false + end + + it 'rejects string with leading whitespace' do + expect(described_class.numeric_string?(' 12345')).to be false + end + + it 'rejects string with trailing whitespace' do + expect(described_class.numeric_string?('12345 ')).to be false + end + + it 'rejects integer (non-string)' do + expect(described_class.numeric_string?(12_345)).to be false + end + + it 'rejects symbol' do + expect(described_class.numeric_string?(:'12345')).to be false + end + + it 'rejects negative numeric strings' do + expect(described_class.numeric_string?('-1')).to be false + end + + it 'rejects decimal strings' do + expect(described_class.numeric_string?('1.5')).to be false + end + + it 'rejects exponent notation' do + expect(described_class.numeric_string?('1e10')).to be false + end + + it 'rejects hex strings' do + expect(described_class.numeric_string?('0xff')).to be false + end + + it 'rejects alphanumeric strings' do + expect(described_class.numeric_string?('exp_42')).to be false + end + end + + describe '.non_empty_string?' do + it 'accepts a non-empty numeric string' do + expect(described_class.non_empty_string?('12345')).to be true + end + + it 'accepts a non-empty opaque string with prefix' do + expect(described_class.non_empty_string?('default-12345')).to be true + end + + it 'accepts a non-empty opaque alphanumeric string' do + expect(described_class.non_empty_string?('layer_abc')).to be true + end + + it 'accepts a whitespace-only string (non-empty per spec)' do + # FR-001: non-empty string is the only requirement; any character + # content is acceptable for campaign_id/entity_id. + expect(described_class.non_empty_string?(' ')).to be true + end + + it 'rejects nil' do + expect(described_class.non_empty_string?(nil)).to be false + end + + it 'rejects empty string' do + expect(described_class.non_empty_string?('')).to be false + end + + it 'rejects integer (non-string)' do + # Non-string types are out of scope per spec; non_empty_string? rejects + # them defensively to keep the wire payload string-typed. + expect(described_class.non_empty_string?(12_345)).to be false + end + + it 'rejects symbol' do + expect(described_class.non_empty_string?(:'12345')).to be false + end + end + + describe '.normalize_campaign_id' do + it 'returns the campaign_id unchanged when it is a non-empty numeric string' do + expect(described_class.normalize_campaign_id('111122', '999888')).to eq('111122') + end + + it 'returns the campaign_id unchanged when it is a non-empty opaque string (relaxed contract)' do + # FR-001: any non-empty string is valid for campaign_id; opaque IDs + # like "default-12345" or "layer_abc" pass through unchanged. + expect(described_class.normalize_campaign_id('default-12345', '999888')).to eq('default-12345') + expect(described_class.normalize_campaign_id('layer_abc', '999888')).to eq('layer_abc') + expect(described_class.normalize_campaign_id('campaign_a', '999888')).to eq('campaign_a') + end + + it 'returns the campaign_id unchanged when it is a whitespace-only string (non-empty per spec)' do + # Whitespace is non-empty, so it passes through. The upstream datafile + # producer is responsible for content quality; SDK only enforces + # non-emptiness. + expect(described_class.normalize_campaign_id(' ', '999888')).to eq(' ') + end + + it 'returns experiment_id when campaign_id is nil' do + expect(described_class.normalize_campaign_id(nil, '999888')).to eq('999888') + end + + it 'returns experiment_id when campaign_id is empty string' do + expect(described_class.normalize_campaign_id('', '999888')).to eq('999888') + end + + it 'returns experiment_id (opaque string) when campaign_id is empty (relaxed contract)' do + # FR-002 fallback also accepts opaque experiment_id values. + expect(described_class.normalize_campaign_id('', 'exp_42')).to eq('exp_42') + end + + it 'returns experiment_id when campaign_id is an integer (non-string, out of scope)' do + # Non-string types are out of scope per spec; non_empty_string? rejects + # them defensively so the fallback path still produces a string output. + expect(described_class.normalize_campaign_id(111_122, '999888')).to eq('999888') + end + + it 'returns empty string when both campaign_id and experiment_id are nil or empty' do + expect(described_class.normalize_campaign_id(nil, nil)).to eq('') + expect(described_class.normalize_campaign_id('', '')).to eq('') + expect(described_class.normalize_campaign_id(nil, '')).to eq('') + expect(described_class.normalize_campaign_id('', nil)).to eq('') + end + + it 'preserves leading zeros' do + expect(described_class.normalize_campaign_id('007', '999')).to eq('007') + end + end + + describe '.normalize_variation_id' do + it 'returns the variation_id unchanged when it is a valid numeric string' do + expect(described_class.normalize_variation_id('555444')).to eq('555444') + end + + it 'returns nil when variation_id is nil' do + expect(described_class.normalize_variation_id(nil)).to be_nil + end + + it 'returns nil when variation_id is empty string' do + expect(described_class.normalize_variation_id('')).to be_nil + end + + it 'returns nil when variation_id is whitespace' do + # variation_id retains the stricter numeric-only contract — whitespace + # is not numeric, so it normalizes to nil even though it is non-empty. + expect(described_class.normalize_variation_id(' ')).to be_nil + end + + it 'returns nil when variation_id is a non-numeric placeholder string' do + # variation_id stays strict (FR-003/FR-004) — opaque placeholders like + # "variation_a" normalize to nil unlike campaign_id. + expect(described_class.normalize_variation_id('variation_a')).to be_nil + end + + it 'returns nil when variation_id is an integer (non-string, out of scope)' do + expect(described_class.normalize_variation_id(555_444)).to be_nil + end + + it 'preserves leading zeros' do + expect(described_class.normalize_variation_id('042')).to eq('042') + end + end +end diff --git a/spec/project_spec.rb b/spec/project_spec.rb index 99ffac38..cd23575c 100644 --- a/spec/project_spec.rb +++ b/spec/project_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # -# Copyright 2016-2020, 2022-2023, Optimizely and contributors +# Copyright 2016-2020, 2022-2023, 2026, Optimizely and contributors # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -4052,7 +4052,7 @@ def callback(_args); end decisions: [{ campaign_id: '', experiment_id: '', - variation_id: '', + variation_id: nil, metadata: { flag_key: 'multi_variate_feature', rule_key: '',