From 3f6b0acc1876f83e22e2fb1a0856495e13398d88 Mon Sep 17 00:00:00 2001 From: Jae Kim Date: Thu, 25 Jun 2026 08:54:49 -0700 Subject: [PATCH 1/5] [FSSDK-12813] Normalize decision event campaign_id, variation_id, and entity_id --- pkg/event/event_id_normalizer.go | 82 +++++ pkg/event/event_id_normalizer_test.go | 415 ++++++++++++++++++++++++++ pkg/event/events.go | 9 +- pkg/event/events_test.go | 5 +- pkg/event/factory.go | 31 +- pkg/event/factory_test.go | 15 +- 6 files changed, 546 insertions(+), 11 deletions(-) create mode 100644 pkg/event/event_id_normalizer.go create mode 100644 pkg/event/event_id_normalizer_test.go diff --git a/pkg/event/event_id_normalizer.go b/pkg/event/event_id_normalizer.go new file mode 100644 index 000000000..13359840a --- /dev/null +++ b/pkg/event/event_id_normalizer.go @@ -0,0 +1,82 @@ +/**************************************************************************** + * Copyright 2026, Optimizely, Inc. 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. * + ***************************************************************************/ + +// Package event // +package event + +// Decision event ID normalization (FSSDK-12813). +// +// These helpers normalize the campaign_id, variation_id, and entity_id fields +// of outgoing decision events so that every decision type (experiment, feature +// test, rollout, holdout) produces uniform, valid wire output regardless of +// what upstream code supplies. +// +// Numeric string definition: a non-empty string consisting entirely of decimal +// digits [0-9]. Leading zeros are allowed. Whitespace, signs, decimals, and +// exponents are NOT numeric strings. +// +// Normalization rules: +// - campaign_id: must be a numeric string. If empty / non-numeric, substitute +// experiment_id (which already comes from datafile and is expected numeric). +// - variation_id: must be a numeric string OR JSON null. If empty / +// non-numeric, substitute nil so it serializes as JSON null. +// - entity_id (impression events only): same rule as campaign_id, and must +// equal the normalized campaign_id byte-for-byte for a given impression. +// +// This normalization MUST NOT log, warn, drop, defer, or fail event dispatch. + +// IsNumericIDString reports whether value is a non-empty string consisting +// entirely of decimal digits [0-9]. Leading zeros are permitted. Whitespace, +// signs (+/-), decimals, and exponents are rejected. +func IsNumericIDString(value string) bool { + if value == "" { + return false + } + for i := 0; i < len(value); i++ { + c := value[i] + if c < '0' || c > '9' { + return false + } + } + return true +} + +// NormalizeCampaignID returns campaignID if it is a numeric string; otherwise +// it returns experimentID. The caller is responsible for ensuring experimentID +// is itself a valid numeric ID (it comes from the datafile and is expected to +// be numeric). If experimentID is also non-numeric, it is returned as-is so +// downstream behavior is unchanged for malformed datafiles. +// +// This function is used for both decisions[].campaign_id and (for impression +// events) events[].entity_id so both fields are normalized identically and +// remain byte-equivalent. +func NormalizeCampaignID(campaignID, experimentID string) string { + if IsNumericIDString(campaignID) { + return campaignID + } + return experimentID +} + +// NormalizeVariationID returns a pointer to variationID if it is a numeric +// string; otherwise it returns nil. A nil return value causes the field to +// serialize as JSON null when marshaled via the wire format. +func NormalizeVariationID(variationID string) *string { + if IsNumericIDString(variationID) { + v := variationID + return &v + } + return nil +} diff --git a/pkg/event/event_id_normalizer_test.go b/pkg/event/event_id_normalizer_test.go new file mode 100644 index 000000000..f6aaa8007 --- /dev/null +++ b/pkg/event/event_id_normalizer_test.go @@ -0,0 +1,415 @@ +/**************************************************************************** + * Copyright 2026, Optimizely, Inc. 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. * + ***************************************************************************/ + +// Package event // +package event + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/optimizely/go-sdk/v2/pkg/config" + decisionPkg "github.com/optimizely/go-sdk/v2/pkg/decision" + "github.com/optimizely/go-sdk/v2/pkg/entities" +) + +// FSSDK-12813: Decision-event ID normalization tests. +// +// These tests verify the cross-SDK contract for outgoing decision events: +// - campaign_id is a non-empty numeric string (substitute experiment_id otherwise) +// - variation_id is a non-empty numeric string OR JSON null +// - entity_id (impression events) mirrors campaign_id byte-for-byte +// - rules apply uniformly to every decision type +// - no logging, no warning, no dropping of events on the normalization path + +// --- IsNumericIDString ------------------------------------------------------- + +func TestIsNumericIDString(t *testing.T) { + cases := []struct { + name string + value string + want bool + }{ + {"plain digits", "12345", true}, + {"single digit", "0", true}, + {"all zeros", "0000", true}, + {"leading zero numeric", "0123456789", true}, + {"all nines", "9999999999", true}, + {"large numeric id", "15402980349", true}, + + {"empty string is invalid", "", false}, + {"whitespace only", " ", false}, + {"leading space", " 123", false}, + {"trailing space", "123 ", false}, + {"interior space", "12 34", false}, + {"alpha placeholder", "holdout_var_123", false}, + {"alpha only", "abc", false}, + {"mixed digits and letters", "12a34", false}, + {"negative sign", "-123", false}, + {"positive sign", "+123", false}, + {"decimal point", "12.34", false}, + {"exponent notation", "1e5", false}, + {"tab character", "12\t34", false}, + {"newline character", "12\n34", false}, + {"unicode digits", "१२३", false}, // devanagari digits are not [0-9] + {"hex prefix", "0x123", false}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, IsNumericIDString(tc.value)) + }) + } +} + +// --- NormalizeCampaignID ----------------------------------------------------- + +func TestNormalizeCampaignID_ReturnsCampaignIDWhenNumeric(t *testing.T) { + got := NormalizeCampaignID("15399420423", "15402980349") + assert.Equal(t, "15399420423", got) +} + +func TestNormalizeCampaignID_SubstitutesExperimentIDWhenCampaignEmpty(t *testing.T) { + // FR-001/FR-002: empty campaign_id is replaced with experiment_id. + got := NormalizeCampaignID("", "15402980349") + assert.Equal(t, "15402980349", got) +} + +func TestNormalizeCampaignID_SubstitutesExperimentIDWhenCampaignNonNumeric(t *testing.T) { + got := NormalizeCampaignID("layer_abc", "15402980349") + assert.Equal(t, "15402980349", got) +} + +func TestNormalizeCampaignID_SubstitutesExperimentIDForWhitespaceCampaign(t *testing.T) { + got := NormalizeCampaignID(" ", "15402980349") + assert.Equal(t, "15402980349", got) +} + +func TestNormalizeCampaignID_AllowsLeadingZeros(t *testing.T) { + got := NormalizeCampaignID("0123", "9999") + assert.Equal(t, "0123", got) +} + +func TestNormalizeCampaignID_PreservesMalformedExperimentIDFallback(t *testing.T) { + // If both inputs are non-numeric, return experimentID as-is so we don't + // silently invent data. Downstream behavior for malformed datafiles is + // unchanged. + got := NormalizeCampaignID("", "exp_42") + assert.Equal(t, "exp_42", got) +} + +// --- NormalizeVariationID ---------------------------------------------------- + +func TestNormalizeVariationID_ReturnsPointerWhenNumeric(t *testing.T) { + got := NormalizeVariationID("15410990633") + if assert.NotNil(t, got) { + assert.Equal(t, "15410990633", *got) + } +} + +func TestNormalizeVariationID_ReturnsNilWhenEmpty(t *testing.T) { + // FR-003/FR-004: empty variation_id becomes JSON null. + got := NormalizeVariationID("") + assert.Nil(t, got) +} + +func TestNormalizeVariationID_ReturnsNilWhenNonNumeric(t *testing.T) { + got := NormalizeVariationID("variation_a") + assert.Nil(t, got) +} + +func TestNormalizeVariationID_ReturnsNilForWhitespace(t *testing.T) { + got := NormalizeVariationID(" ") + assert.Nil(t, got) +} + +func TestNormalizeVariationID_AllowsLeadingZeros(t *testing.T) { + got := NormalizeVariationID("0042") + if assert.NotNil(t, got) { + assert.Equal(t, "0042", *got) + } +} + +func TestNormalizeVariationID_NilSerializesToJSONNull(t *testing.T) { + // FR-003/FR-004: variation_id with nil pointer must marshal to JSON null, + // matching the cross-SDK wire contract. + decision := Decision{ + CampaignID: "15402980349", + ExperimentID: "15402980349", + VariationID: NormalizeVariationID(""), // nil + } + + b, err := json.Marshal(decision) + assert.NoError(t, err) + + var raw map[string]interface{} + assert.NoError(t, json.Unmarshal(b, &raw)) + + // json.Unmarshal of null into interface{} produces a nil value, and the + // key must still be present. + got, ok := raw["variation_id"] + assert.True(t, ok, "variation_id key must be present in JSON output") + assert.Nil(t, got, "variation_id must serialize as JSON null when nil") +} + +func TestNormalizeVariationID_NumericSerializesToJSONString(t *testing.T) { + decision := Decision{ + CampaignID: "15402980349", + ExperimentID: "15402980349", + VariationID: NormalizeVariationID("15410990633"), + } + + b, err := json.Marshal(decision) + assert.NoError(t, err) + + var raw map[string]interface{} + assert.NoError(t, json.Unmarshal(b, &raw)) + assert.Equal(t, "15410990633", raw["variation_id"]) +} + +// --- Integration: createImpressionEvent + createImpressionVisitor ------------ + +// testNormConfig is a minimal ProjectConfig stub for normalization integration +// tests. It is intentionally separate from TestConfig in factory_test.go so +// these tests do not depend on shared global state. +type testNormConfig struct { + config.ProjectConfig +} + +func (testNormConfig) GetAttributeByKey(string) (entities.Attribute, error) { + return entities.Attribute{ID: "100000", Key: "sample_attribute"}, nil +} +func (testNormConfig) GetProjectID() string { return "15389410617" } +func (testNormConfig) GetRevision() string { return "7" } +func (testNormConfig) GetAccountID() string { return "8362480420" } +func (testNormConfig) GetAnonymizeIP() bool { return true } +func (testNormConfig) GetAttributeID(string) string { return "" } +func (testNormConfig) GetBotFiltering() bool { return false } +func (testNormConfig) GetClientName() string { return "go-sdk" } +func (testNormConfig) GetClientVersion() string { return "1.0.0" } +func (testNormConfig) SendFlagDecisions() bool { return true } +func (testNormConfig) GetRegion() string { return "US" } + +func newNormUserContext() entities.UserContext { + return entities.UserContext{ID: "user-1", Attributes: map[string]interface{}{}} +} + +func numericExperiment() entities.Experiment { + return entities.Experiment{Key: "exp_key", LayerID: "15399420423", ID: "15402980349"} +} + +func numericVariation() entities.Variation { + return entities.Variation{Key: "variation_a", ID: "15410990633"} +} + +func holdoutExperiment() entities.Experiment { + // FSSDK-12813: Holdouts ship with no LayerID. Normalizer must fall back + // to ExperimentID for both campaign_id and entity_id. + return entities.Experiment{Key: "holdout_key", LayerID: "", ID: "9876543210"} +} + +// TestImpressionEvent_NormalizesCampaignAndEntityIDsForHoldout verifies +// FR-001/FR-002 and FR-009 in the impression event flow for holdouts. +func TestImpressionEvent_NormalizesCampaignAndEntityIDsForHoldout(t *testing.T) { + tc := testNormConfig{} + exp := holdoutExperiment() + variation := numericVariation() + + userEvent, ok := CreateImpressionUserEvent(tc, exp, &variation, newNormUserContext(), + "flag_key", exp.Key, decisionPkg.Holdout, false, nil) + assert.True(t, ok) + + // FR-001/FR-002: empty LayerID is substituted with ExperimentID. + assert.Equal(t, exp.ID, userEvent.Impression.CampaignID, + "holdout campaign_id must fall back to experiment_id when LayerID is empty (FSSDK-12813)") + // FR-009: entity_id mirrors campaign_id byte-for-byte. + assert.Equal(t, exp.ID, userEvent.Impression.EntityID, + "holdout entity_id must equal campaign_id byte-for-byte (FSSDK-12813)") + + // Same invariant must hold in the wire visitor / decision payload. + visitor := createVisitorFromUserEvent(userEvent) + assert.Len(t, visitor.Snapshots, 1) + assert.Len(t, visitor.Snapshots[0].Decisions, 1) + assert.Equal(t, exp.ID, visitor.Snapshots[0].Decisions[0].CampaignID) + assert.Equal(t, exp.ID, visitor.Snapshots[0].Events[0].EntityID) + // Byte-equivalent (FR-009). + assert.Equal(t, visitor.Snapshots[0].Decisions[0].CampaignID, + visitor.Snapshots[0].Events[0].EntityID, + "decisions[].campaign_id and events[].entity_id must be byte-equivalent") +} + +// TestImpressionEvent_PassesThroughNumericCampaignID verifies happy path for +// non-holdout decisions where LayerID is already a numeric ID. +func TestImpressionEvent_PassesThroughNumericCampaignID(t *testing.T) { + tc := testNormConfig{} + exp := numericExperiment() + variation := numericVariation() + + userEvent, ok := CreateImpressionUserEvent(tc, exp, &variation, newNormUserContext(), + "flag_key", exp.Key, decisionPkg.FeatureTest, true, nil) + assert.True(t, ok) + + assert.Equal(t, exp.LayerID, userEvent.Impression.CampaignID) + assert.Equal(t, exp.LayerID, userEvent.Impression.EntityID) + + visitor := createVisitorFromUserEvent(userEvent) + assert.Equal(t, exp.LayerID, visitor.Snapshots[0].Decisions[0].CampaignID) + assert.Equal(t, exp.LayerID, visitor.Snapshots[0].Events[0].EntityID) +} + +// TestImpressionEvent_NormalizesVariationIDToJSONNull verifies FR-003/FR-004: +// a non-numeric variation ID must become JSON null on the wire. +func TestImpressionEvent_NormalizesVariationIDToJSONNull(t *testing.T) { + tc := testNormConfig{} + exp := numericExperiment() + badVariation := entities.Variation{Key: "variation_a", ID: "variation_a"} // non-numeric ID + + userEvent, ok := CreateImpressionUserEvent(tc, exp, &badVariation, newNormUserContext(), + "flag_key", exp.Key, decisionPkg.FeatureTest, true, nil) + assert.True(t, ok) + + visitor := createVisitorFromUserEvent(userEvent) + assert.Nil(t, visitor.Snapshots[0].Decisions[0].VariationID, + "non-numeric variation_id must be normalized to nil so it serializes as JSON null (FSSDK-12813)") + + // Verify on-the-wire JSON shape. + b, err := json.Marshal(visitor.Snapshots[0].Decisions[0]) + assert.NoError(t, err) + var raw map[string]interface{} + assert.NoError(t, json.Unmarshal(b, &raw)) + got, ok := raw["variation_id"] + assert.True(t, ok) + assert.Nil(t, got, "variation_id must marshal as JSON null when normalized") +} + +// TestImpressionEvent_KeepsNumericVariationID verifies happy path for +// variation IDs that are already valid numeric strings. +func TestImpressionEvent_KeepsNumericVariationID(t *testing.T) { + tc := testNormConfig{} + exp := numericExperiment() + variation := numericVariation() + + userEvent, ok := CreateImpressionUserEvent(tc, exp, &variation, newNormUserContext(), + "flag_key", exp.Key, decisionPkg.FeatureTest, true, nil) + assert.True(t, ok) + + visitor := createVisitorFromUserEvent(userEvent) + if assert.NotNil(t, visitor.Snapshots[0].Decisions[0].VariationID) { + assert.Equal(t, variation.ID, *visitor.Snapshots[0].Decisions[0].VariationID) + } +} + +// TestImpressionEvent_NormalizationAppliesUniformlyAcrossRuleTypes verifies +// FR-005: identical normalization is applied for every decision type. No +// per-type branching is permitted in the normalization path. +func TestImpressionEvent_NormalizationAppliesUniformlyAcrossRuleTypes(t *testing.T) { + tc := testNormConfig{} + exp := holdoutExperiment() // empty LayerID forces normalization + variation := numericVariation() + + ruleTypes := []string{ + decisionPkg.FeatureTest, + decisionPkg.Holdout, + decisionPkg.Rollout, + "experiment", + "anything-else", + } + + for _, rt := range ruleTypes { + t.Run(rt, func(t *testing.T) { + userEvent, ok := CreateImpressionUserEvent(tc, exp, &variation, newNormUserContext(), + "flag_key", exp.Key, rt, true, nil) + if !ok { + t.Skipf("rule type %q does not emit impressions in this config", rt) + return + } + + visitor := createVisitorFromUserEvent(userEvent) + d := visitor.Snapshots[0].Decisions[0] + e := visitor.Snapshots[0].Events[0] + + // Same normalization for every rule type. + assert.Equal(t, exp.ID, d.CampaignID, "rule type %q: campaign_id", rt) + assert.Equal(t, exp.ID, e.EntityID, "rule type %q: entity_id", rt) + assert.Equal(t, d.CampaignID, e.EntityID, "rule type %q: byte-equivalent", rt) + if assert.NotNil(t, d.VariationID, "rule type %q: variation_id pointer", rt) { + assert.Equal(t, variation.ID, *d.VariationID, "rule type %q: variation_id value", rt) + } + }) + } +} + +// TestImpressionEvent_NormalizationIsByteEquivalentOnTheWire is a full +// wire-output regression. It marshals a complete Batch and asserts the +// normalized campaign_id and entity_id are identical strings in the JSON. +func TestImpressionEvent_NormalizationIsByteEquivalentOnTheWire(t *testing.T) { + tc := testNormConfig{} + exp := holdoutExperiment() + variation := numericVariation() + + userEvent, ok := CreateImpressionUserEvent(tc, exp, &variation, newNormUserContext(), + "flag_key", exp.Key, decisionPkg.Holdout, false, nil) + assert.True(t, ok) + + batch := createBatchEvent(userEvent, createVisitorFromUserEvent(userEvent)) + b, err := json.Marshal(batch) + assert.NoError(t, err) + + // Decode opaquely and inspect the relevant fields by path. Avoids + // coupling the test to internal Go struct layout. + var raw map[string]interface{} + assert.NoError(t, json.Unmarshal(b, &raw)) + + visitors, _ := raw["visitors"].([]interface{}) + if !assert.Len(t, visitors, 1) { + return + } + snapshots, _ := visitors[0].(map[string]interface{})["snapshots"].([]interface{}) + if !assert.Len(t, snapshots, 1) { + return + } + decisions, _ := snapshots[0].(map[string]interface{})["decisions"].([]interface{}) + events, _ := snapshots[0].(map[string]interface{})["events"].([]interface{}) + if !assert.Len(t, decisions, 1) || !assert.Len(t, events, 1) { + return + } + + campaignID, _ := decisions[0].(map[string]interface{})["campaign_id"].(string) + entityID, _ := events[0].(map[string]interface{})["entity_id"].(string) + + assert.Equal(t, exp.ID, campaignID, "wire campaign_id must equal normalized experiment id") + assert.Equal(t, exp.ID, entityID, "wire entity_id must equal normalized campaign id") + assert.Equal(t, campaignID, entityID, "wire campaign_id and entity_id must be byte-equivalent (FR-009)") +} + +// TestConversionEvent_EntityIDIsUnchanged verifies FR-010: conversion events +// derive entity_id from the event definition (event.ID), not from a +// decision. Normalization must NOT touch the conversion path. +func TestConversionEvent_EntityIDIsUnchanged(t *testing.T) { + tc := testNormConfig{} + conv := entities.Event{ID: "15368860886", Key: "sample_conversion"} + + userEvent := CreateConversionUserEvent(tc, conv, newNormUserContext(), map[string]interface{}{}) + visitor := createVisitorFromUserEvent(userEvent) + + assert.Equal(t, conv.ID, visitor.Snapshots[0].Events[0].EntityID, + "conversion entity_id must be the event ID untouched (FR-010)") + // And conversion visitors do not emit a decisions array, so no + // campaign_id / variation_id normalization happens here. + assert.Empty(t, visitor.Snapshots[0].Decisions) +} diff --git a/pkg/event/events.go b/pkg/event/events.go index 5950fe404..819766393 100644 --- a/pkg/event/events.go +++ b/pkg/event/events.go @@ -112,9 +112,14 @@ type Snapshot struct { Events []SnapshotEvent `json:"events"` } -// Decision represents a decision of a snapshot +// Decision represents a decision of a snapshot. +// +// VariationID is a pointer so that an empty / non-numeric upstream value +// can be serialized as JSON null per the cross-SDK decision-event +// normalization spec (FSSDK-12813). A nil VariationID marshals to +// "variation_id": null on the wire. type Decision struct { - VariationID string `json:"variation_id"` + VariationID *string `json:"variation_id"` CampaignID string `json:"campaign_id"` ExperimentID string `json:"experiment_id"` Metadata DecisionMetadata `json:"metadata"` diff --git a/pkg/event/events_test.go b/pkg/event/events_test.go index 1a4eabaa4..69cdd07f1 100644 --- a/pkg/event/events_test.go +++ b/pkg/event/events_test.go @@ -24,10 +24,13 @@ import ( ) func TestSnapshotHasOptionalDecisions(t *testing.T) { + // FSSDK-12813: Decision.VariationID is *string so that an empty / non-numeric + // upstream value can serialize as JSON null. Pass &v with a numeric ID. + v := "1" snapshot := Snapshot{ Decisions: []Decision{ Decision{ - VariationID: "1", + VariationID: &v, }, }, } diff --git a/pkg/event/factory.go b/pkg/event/factory.go index 13f412ddd..2c0bf70fb 100644 --- a/pkg/event/factory.go +++ b/pkg/event/factory.go @@ -103,10 +103,20 @@ func createImpressionEvent( variationID = variation.ID } + // FSSDK-12813: Normalize campaign_id, variation_id, and entity_id uniformly + // for every decision type (experiment, feature test, rollout, holdout). + // - campaign_id falls back to experiment.ID when LayerID is empty / non-numeric + // (e.g. holdouts have no layer). + // - entity_id mirrors the normalized campaign_id so both fields are + // byte-equivalent on the wire (FR-009). + // - variation_id is normalized later when the Decision is built, since the + // Decision struct uses *string to allow JSON null output (FR-003/FR-004). + normalizedCampaignID := NormalizeCampaignID(experiment.LayerID, experiment.ID) + event := ImpressionEvent{ Attributes: getEventAttributes(projectConfig, attributes), - CampaignID: experiment.LayerID, - EntityID: experiment.LayerID, + CampaignID: normalizedCampaignID, + EntityID: normalizedCampaignID, ExperimentID: experiment.ID, Key: impressionKey, Metadata: metadata, @@ -174,16 +184,27 @@ func CreateImpressionUserEvent( // create an impression visitor func createImpressionVisitor(userEvent UserEvent) Visitor { + // FSSDK-12813: Apply decision-event ID normalization at the visitor / + // wire-payload boundary as a defense-in-depth pass. The ImpressionEvent + // is already normalized when constructed (see createImpressionEvent), but + // normalizing again here guarantees that any callers who mutate the + // in-memory ImpressionEvent before dispatch still produce a valid wire + // payload. This call is idempotent for already-normalized values. + normalizedCampaignID := NormalizeCampaignID(userEvent.Impression.CampaignID, userEvent.Impression.ExperimentID) + decision := Decision{} - decision.CampaignID = userEvent.Impression.CampaignID + decision.CampaignID = normalizedCampaignID decision.ExperimentID = userEvent.Impression.ExperimentID - decision.VariationID = userEvent.Impression.VariationID + decision.VariationID = NormalizeVariationID(userEvent.Impression.VariationID) decision.Metadata = userEvent.Impression.Metadata dispatchEvent := SnapshotEvent{} dispatchEvent.Timestamp = userEvent.Timestamp dispatchEvent.Key = userEvent.Impression.Key - dispatchEvent.EntityID = userEvent.Impression.EntityID + // FR-009: entity_id for impression events must equal decisions[].campaign_id + // byte-for-byte. Reuse the normalized value rather than re-reading + // userEvent.Impression.EntityID so the two fields cannot drift apart. + dispatchEvent.EntityID = normalizedCampaignID dispatchEvent.UUID = guuid.New().String() dispatchEvent.Tags = make(map[string]interface{}) diff --git a/pkg/event/factory_test.go b/pkg/event/factory_test.go index 1a488b435..84dd6ee4d 100644 --- a/pkg/event/factory_test.go +++ b/pkg/event/factory_test.go @@ -339,15 +339,20 @@ func TestCreateImpressionUserEventWithCmabUUID(t *testing.T) { func TestCreateImpressionUserEventForHoldout(t *testing.T) { tc := TestConfig{} + // FSSDK-12813: Decision-event ID fields (campaign_id, variation_id, + // experiment_id, entity_id) must be numeric strings on the wire. Use + // numeric IDs in fixtures so the assertions below exercise the + // happy-path normalization (campaign_id falls back to experiment_id + // when LayerID is empty). testHoldout := entities.Experiment{ Key: "test_holdout", LayerID: "", - ID: "holdout_123", + ID: "9876543210", } testHoldoutVariation := entities.Variation{ Key: "holdout_variation", - ID: "holdout_var_123", + ID: "1234567890", } // Test 1: Holdout with variation should ALWAYS send impression event @@ -440,6 +445,10 @@ func TestCreateImpressionUserEventForHoldout(t *testing.T) { // Verify IDs are set correctly assert.Equal(t, testHoldoutVariation.ID, impression.VariationID) assert.Equal(t, testHoldout.ID, impression.ExperimentID) - assert.Equal(t, "", impression.CampaignID) // Empty for holdouts (no layer) + // FSSDK-12813: Holdouts have no LayerID, so the impression's CampaignID + // is normalized to ExperimentID (FR-001/FR-002). The same normalized + // value is also reused as the impression's EntityID (FR-009). + assert.Equal(t, testHoldout.ID, impression.CampaignID) + assert.Equal(t, testHoldout.ID, impression.EntityID) }) } From 91abb36407f551bed078a214c7f9593cad4adfeb Mon Sep 17 00:00:00 2001 From: Jae Kim Date: Thu, 25 Jun 2026 11:08:42 -0700 Subject: [PATCH 2/5] [FSSDK-12813] Relax campaign_id/entity_id validation to non-empty string per updated spec --- pkg/event/event_id_normalizer.go | 46 ++++++++---- pkg/event/event_id_normalizer_test.go | 101 ++++++++++++++++++++++---- pkg/event/factory.go | 6 +- pkg/event/factory_test.go | 12 +-- 4 files changed, 127 insertions(+), 38 deletions(-) diff --git a/pkg/event/event_id_normalizer.go b/pkg/event/event_id_normalizer.go index 13359840a..11f9e3425 100644 --- a/pkg/event/event_id_normalizer.go +++ b/pkg/event/event_id_normalizer.go @@ -24,23 +24,28 @@ package event // test, rollout, holdout) produces uniform, valid wire output regardless of // what upstream code supplies. // -// Numeric string definition: a non-empty string consisting entirely of decimal -// digits [0-9]. Leading zeros are allowed. Whitespace, signs, decimals, and -// exponents are NOT numeric strings. -// -// Normalization rules: -// - campaign_id: must be a numeric string. If empty / non-numeric, substitute -// experiment_id (which already comes from datafile and is expected numeric). -// - variation_id: must be a numeric string OR JSON null. If empty / -// non-numeric, substitute nil so it serializes as JSON null. +// Contract (per relaxed FSSDK-12813 spec): +// - campaign_id / entity_id: must be a non-empty string. Any character +// content is allowed (IDs may be opaque, e.g. "default-12345", "layer_abc"). +// When the value is empty (""), fall back to experiment_id. +// - variation_id: STRICT numeric-string-only. Must be a non-empty string +// consisting entirely of decimal digits [0-9]. Anything else (empty, +// whitespace, non-numeric) becomes nil so it serializes as JSON null. // - entity_id (impression events only): same rule as campaign_id, and must // equal the normalized campaign_id byte-for-byte for a given impression. // +// Numeric string definition (for variation_id only): a non-empty string +// consisting entirely of decimal digits [0-9]. Leading zeros are allowed. +// Whitespace, signs, decimals, and exponents are NOT numeric strings. +// // This normalization MUST NOT log, warn, drop, defer, or fail event dispatch. // IsNumericIDString reports whether value is a non-empty string consisting // entirely of decimal digits [0-9]. Leading zeros are permitted. Whitespace, // signs (+/-), decimals, and exponents are rejected. +// +// This predicate is used for the strict variation_id contract only. The +// campaign_id / entity_id contract has been relaxed to IsNonEmptyString. func IsNumericIDString(value string) bool { if value == "" { return false @@ -54,17 +59,25 @@ func IsNumericIDString(value string) bool { return true } -// NormalizeCampaignID returns campaignID if it is a numeric string; otherwise -// it returns experimentID. The caller is responsible for ensuring experimentID -// is itself a valid numeric ID (it comes from the datafile and is expected to -// be numeric). If experimentID is also non-numeric, it is returned as-is so -// downstream behavior is unchanged for malformed datafiles. +// IsNonEmptyString reports whether value has at least one character. It is +// the predicate used for the relaxed campaign_id / entity_id contract: any +// character content is allowed (IDs may be opaque, e.g. "default-12345", +// "layer_abc"). Only the empty string triggers fallback to experiment_id. +func IsNonEmptyString(value string) bool { + return value != "" +} + +// NormalizeCampaignID returns campaignID if it is a non-empty string; +// otherwise it returns experimentID. Per the relaxed FSSDK-12813 spec, any +// non-empty character content is accepted for campaign_id / entity_id — +// IDs may be opaque (e.g. "default-12345", "layer_abc"). Fallback to +// experiment_id fires ONLY when campaignID is the empty string. // // This function is used for both decisions[].campaign_id and (for impression // events) events[].entity_id so both fields are normalized identically and // remain byte-equivalent. func NormalizeCampaignID(campaignID, experimentID string) string { - if IsNumericIDString(campaignID) { + if IsNonEmptyString(campaignID) { return campaignID } return experimentID @@ -72,7 +85,8 @@ func NormalizeCampaignID(campaignID, experimentID string) string { // NormalizeVariationID returns a pointer to variationID if it is a numeric // string; otherwise it returns nil. A nil return value causes the field to -// serialize as JSON null when marshaled via the wire format. +// serialize as JSON null when marshaled via the wire format. The strict +// numeric-string contract is intentional — variation_id remains numeric-only. func NormalizeVariationID(variationID string) *string { if IsNumericIDString(variationID) { v := variationID diff --git a/pkg/event/event_id_normalizer_test.go b/pkg/event/event_id_normalizer_test.go index f6aaa8007..6ff58da79 100644 --- a/pkg/event/event_id_normalizer_test.go +++ b/pkg/event/event_id_normalizer_test.go @@ -30,12 +30,14 @@ import ( // FSSDK-12813: Decision-event ID normalization tests. // -// These tests verify the cross-SDK contract for outgoing decision events: -// - campaign_id is a non-empty numeric string (substitute experiment_id otherwise) -// - variation_id is a non-empty numeric string OR JSON null -// - entity_id (impression events) mirrors campaign_id byte-for-byte -// - rules apply uniformly to every decision type -// - no logging, no warning, no dropping of events on the normalization path +// These tests verify the cross-SDK contract for outgoing decision events +// (per the relaxed spec): +// - campaign_id / entity_id: non-empty string (any character content; +// opaque IDs allowed). Fallback to experiment_id ONLY when empty. +// - variation_id: STRICT non-empty numeric string OR JSON null. +// - entity_id (impression events) mirrors campaign_id byte-for-byte. +// - rules apply uniformly to every decision type. +// - no logging, no warning, no dropping of events on the normalization path. // --- IsNumericIDString ------------------------------------------------------- @@ -77,6 +79,36 @@ func TestIsNumericIDString(t *testing.T) { } } +// --- IsNonEmptyString -------------------------------------------------------- + +func TestIsNonEmptyString(t *testing.T) { + // Relaxed campaign_id / entity_id predicate. Only the empty string is + // rejected — any other character content is accepted, including + // whitespace-only, opaque IDs, and non-numeric content. + cases := []struct { + name string + value string + want bool + }{ + {"plain digits", "12345", true}, + {"opaque layer id", "layer_abc", true}, + {"opaque default id", "default-12345", true}, + {"alpha placeholder", "holdout_123", true}, + {"single character", "a", true}, + {"whitespace only", " ", true}, + {"leading zero numeric", "0123456789", true}, + {"unicode", "१२३", true}, + + {"empty string is invalid", "", false}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, IsNonEmptyString(tc.value)) + }) + } +} + // --- NormalizeCampaignID ----------------------------------------------------- func TestNormalizeCampaignID_ReturnsCampaignIDWhenNumeric(t *testing.T) { @@ -85,19 +117,31 @@ func TestNormalizeCampaignID_ReturnsCampaignIDWhenNumeric(t *testing.T) { } func TestNormalizeCampaignID_SubstitutesExperimentIDWhenCampaignEmpty(t *testing.T) { - // FR-001/FR-002: empty campaign_id is replaced with experiment_id. + // FR-001/FR-002 (relaxed): empty campaign_id is replaced with experiment_id. + // Fallback fires ONLY for the empty string. got := NormalizeCampaignID("", "15402980349") assert.Equal(t, "15402980349", got) } -func TestNormalizeCampaignID_SubstitutesExperimentIDWhenCampaignNonNumeric(t *testing.T) { +func TestNormalizeCampaignID_PassesThroughOpaqueCampaignID(t *testing.T) { + // Relaxed spec: opaque, non-numeric campaign IDs pass through unchanged. + // IDs may be of the form "layer_abc", "default-12345", etc. got := NormalizeCampaignID("layer_abc", "15402980349") - assert.Equal(t, "15402980349", got) + assert.Equal(t, "layer_abc", got) } -func TestNormalizeCampaignID_SubstitutesExperimentIDForWhitespaceCampaign(t *testing.T) { +func TestNormalizeCampaignID_PassesThroughHoldoutPlaceholder(t *testing.T) { + // Relaxed spec: alphanumeric placeholders like "holdout_123" are valid + // campaign IDs and pass through; experiment_id substitution does NOT fire. + got := NormalizeCampaignID("holdout_123", "15402980349") + assert.Equal(t, "holdout_123", got) +} + +func TestNormalizeCampaignID_PassesThroughWhitespaceCampaign(t *testing.T) { + // Relaxed spec: whitespace-only is non-empty, so it passes through. + // This is intentionally permissive — we no longer try to "fix" content. got := NormalizeCampaignID(" ", "15402980349") - assert.Equal(t, "15402980349", got) + assert.Equal(t, " ", got) } func TestNormalizeCampaignID_AllowsLeadingZeros(t *testing.T) { @@ -105,10 +149,10 @@ func TestNormalizeCampaignID_AllowsLeadingZeros(t *testing.T) { assert.Equal(t, "0123", got) } -func TestNormalizeCampaignID_PreservesMalformedExperimentIDFallback(t *testing.T) { - // If both inputs are non-numeric, return experimentID as-is so we don't - // silently invent data. Downstream behavior for malformed datafiles is - // unchanged. +func TestNormalizeCampaignID_PreservesExperimentIDFallback(t *testing.T) { + // When campaign_id is empty, fall back to experiment_id verbatim — even + // when experiment_id itself is non-numeric. Downstream behavior for + // malformed datafiles is unchanged. got := NormalizeCampaignID("", "exp_42") assert.Equal(t, "exp_42", got) } @@ -253,6 +297,33 @@ func TestImpressionEvent_NormalizesCampaignAndEntityIDsForHoldout(t *testing.T) "decisions[].campaign_id and events[].entity_id must be byte-equivalent") } +// TestImpressionEvent_PassesThroughOpaqueLayerID verifies the relaxed spec: +// a non-numeric but non-empty LayerID (e.g. "layer_abc", "default-12345") +// passes through to campaign_id and entity_id unchanged. Fallback to +// experiment_id fires ONLY when LayerID is the empty string. +func TestImpressionEvent_PassesThroughOpaqueLayerID(t *testing.T) { + tc := testNormConfig{} + exp := entities.Experiment{Key: "exp_key", LayerID: "default-12345", ID: "15402980349"} + variation := numericVariation() + + userEvent, ok := CreateImpressionUserEvent(tc, exp, &variation, newNormUserContext(), + "flag_key", exp.Key, decisionPkg.FeatureTest, true, nil) + assert.True(t, ok) + + // Opaque LayerID passes through under the relaxed spec — NOT substituted + // with experiment_id. + assert.Equal(t, "default-12345", userEvent.Impression.CampaignID, + "opaque LayerID must pass through (FSSDK-12813 relaxed)") + assert.Equal(t, "default-12345", userEvent.Impression.EntityID, + "entity_id must mirror campaign_id (FR-009)") + + visitor := createVisitorFromUserEvent(userEvent) + assert.Equal(t, "default-12345", visitor.Snapshots[0].Decisions[0].CampaignID) + assert.Equal(t, "default-12345", visitor.Snapshots[0].Events[0].EntityID) + assert.Equal(t, visitor.Snapshots[0].Decisions[0].CampaignID, + visitor.Snapshots[0].Events[0].EntityID) +} + // TestImpressionEvent_PassesThroughNumericCampaignID verifies happy path for // non-holdout decisions where LayerID is already a numeric ID. func TestImpressionEvent_PassesThroughNumericCampaignID(t *testing.T) { diff --git a/pkg/event/factory.go b/pkg/event/factory.go index 2c0bf70fb..3f6c0fda5 100644 --- a/pkg/event/factory.go +++ b/pkg/event/factory.go @@ -105,12 +105,14 @@ func createImpressionEvent( // FSSDK-12813: Normalize campaign_id, variation_id, and entity_id uniformly // for every decision type (experiment, feature test, rollout, holdout). - // - campaign_id falls back to experiment.ID when LayerID is empty / non-numeric - // (e.g. holdouts have no layer). + // - campaign_id falls back to experiment.ID when LayerID is the empty + // string (e.g. holdouts have no layer). Any non-empty value — numeric + // or opaque (e.g. "layer_abc") — passes through per the relaxed spec. // - entity_id mirrors the normalized campaign_id so both fields are // byte-equivalent on the wire (FR-009). // - variation_id is normalized later when the Decision is built, since the // Decision struct uses *string to allow JSON null output (FR-003/FR-004). + // variation_id retains the STRICT numeric-string contract. normalizedCampaignID := NormalizeCampaignID(experiment.LayerID, experiment.ID) event := ImpressionEvent{ diff --git a/pkg/event/factory_test.go b/pkg/event/factory_test.go index 84dd6ee4d..bb7248ab0 100644 --- a/pkg/event/factory_test.go +++ b/pkg/event/factory_test.go @@ -339,11 +339,13 @@ func TestCreateImpressionUserEventWithCmabUUID(t *testing.T) { func TestCreateImpressionUserEventForHoldout(t *testing.T) { tc := TestConfig{} - // FSSDK-12813: Decision-event ID fields (campaign_id, variation_id, - // experiment_id, entity_id) must be numeric strings on the wire. Use - // numeric IDs in fixtures so the assertions below exercise the - // happy-path normalization (campaign_id falls back to experiment_id - // when LayerID is empty). + // FSSDK-12813: variation_id retains a strict numeric-string contract on + // the wire (non-numeric values serialize as JSON null). campaign_id / + // entity_id are relaxed to non-empty strings (opaque IDs allowed; the + // only fallback to experiment_id is when LayerID is the empty string). + // Use numeric IDs in fixtures so the happy-path assertions below exercise + // both the variation_id pass-through and the empty-LayerID fallback for + // campaign_id without entanglement. testHoldout := entities.Experiment{ Key: "test_holdout", LayerID: "", From 07f1ca6efe09c7bf9bce4c6d7735aa5475bf2843 Mon Sep 17 00:00:00 2001 From: Jae Kim Date: Thu, 25 Jun 2026 14:23:56 -0700 Subject: [PATCH 3/5] [FSSDK-12813] Remove ticket references from code comments per cross-sdk guideline --- pkg/event/event_id_normalizer.go | 12 ++++++------ pkg/event/event_id_normalizer_test.go | 17 ++++++++--------- pkg/event/events.go | 4 ++-- pkg/event/events_test.go | 2 +- pkg/event/factory.go | 21 +++++---------------- pkg/event/factory_test.go | 13 ++++--------- 6 files changed, 26 insertions(+), 43 deletions(-) diff --git a/pkg/event/event_id_normalizer.go b/pkg/event/event_id_normalizer.go index 11f9e3425..a6ddf04a9 100644 --- a/pkg/event/event_id_normalizer.go +++ b/pkg/event/event_id_normalizer.go @@ -17,14 +17,14 @@ // Package event // package event -// Decision event ID normalization (FSSDK-12813). +// Decision event ID normalization. // // These helpers normalize the campaign_id, variation_id, and entity_id fields // of outgoing decision events so that every decision type (experiment, feature // test, rollout, holdout) produces uniform, valid wire output regardless of // what upstream code supplies. // -// Contract (per relaxed FSSDK-12813 spec): +// Contract: // - campaign_id / entity_id: must be a non-empty string. Any character // content is allowed (IDs may be opaque, e.g. "default-12345", "layer_abc"). // When the value is empty (""), fall back to experiment_id. @@ -68,10 +68,10 @@ func IsNonEmptyString(value string) bool { } // NormalizeCampaignID returns campaignID if it is a non-empty string; -// otherwise it returns experimentID. Per the relaxed FSSDK-12813 spec, any -// non-empty character content is accepted for campaign_id / entity_id — -// IDs may be opaque (e.g. "default-12345", "layer_abc"). Fallback to -// experiment_id fires ONLY when campaignID is the empty string. +// otherwise it returns experimentID. Any non-empty character content is +// accepted for campaign_id / entity_id — IDs may be opaque (e.g. +// "default-12345", "layer_abc"). Fallback to experiment_id fires ONLY when +// campaignID is the empty string. // // This function is used for both decisions[].campaign_id and (for impression // events) events[].entity_id so both fields are normalized identically and diff --git a/pkg/event/event_id_normalizer_test.go b/pkg/event/event_id_normalizer_test.go index 6ff58da79..b22b04a3f 100644 --- a/pkg/event/event_id_normalizer_test.go +++ b/pkg/event/event_id_normalizer_test.go @@ -28,10 +28,9 @@ import ( "github.com/optimizely/go-sdk/v2/pkg/entities" ) -// FSSDK-12813: Decision-event ID normalization tests. +// Decision-event ID normalization tests. // -// These tests verify the cross-SDK contract for outgoing decision events -// (per the relaxed spec): +// These tests verify the cross-SDK contract for outgoing decision events: // - campaign_id / entity_id: non-empty string (any character content; // opaque IDs allowed). Fallback to experiment_id ONLY when empty. // - variation_id: STRICT non-empty numeric string OR JSON null. @@ -262,8 +261,8 @@ func numericVariation() entities.Variation { } func holdoutExperiment() entities.Experiment { - // FSSDK-12813: Holdouts ship with no LayerID. Normalizer must fall back - // to ExperimentID for both campaign_id and entity_id. + // Holdouts ship with no LayerID. Normalizer must fall back to + // ExperimentID for both campaign_id and entity_id. return entities.Experiment{Key: "holdout_key", LayerID: "", ID: "9876543210"} } @@ -280,10 +279,10 @@ func TestImpressionEvent_NormalizesCampaignAndEntityIDsForHoldout(t *testing.T) // FR-001/FR-002: empty LayerID is substituted with ExperimentID. assert.Equal(t, exp.ID, userEvent.Impression.CampaignID, - "holdout campaign_id must fall back to experiment_id when LayerID is empty (FSSDK-12813)") + "holdout campaign_id must fall back to experiment_id when LayerID is empty") // FR-009: entity_id mirrors campaign_id byte-for-byte. assert.Equal(t, exp.ID, userEvent.Impression.EntityID, - "holdout entity_id must equal campaign_id byte-for-byte (FSSDK-12813)") + "holdout entity_id must equal campaign_id byte-for-byte") // Same invariant must hold in the wire visitor / decision payload. visitor := createVisitorFromUserEvent(userEvent) @@ -313,7 +312,7 @@ func TestImpressionEvent_PassesThroughOpaqueLayerID(t *testing.T) { // Opaque LayerID passes through under the relaxed spec — NOT substituted // with experiment_id. assert.Equal(t, "default-12345", userEvent.Impression.CampaignID, - "opaque LayerID must pass through (FSSDK-12813 relaxed)") + "opaque LayerID must pass through (relaxed contract)") assert.Equal(t, "default-12345", userEvent.Impression.EntityID, "entity_id must mirror campaign_id (FR-009)") @@ -356,7 +355,7 @@ func TestImpressionEvent_NormalizesVariationIDToJSONNull(t *testing.T) { visitor := createVisitorFromUserEvent(userEvent) assert.Nil(t, visitor.Snapshots[0].Decisions[0].VariationID, - "non-numeric variation_id must be normalized to nil so it serializes as JSON null (FSSDK-12813)") + "non-numeric variation_id must be normalized to nil so it serializes as JSON null") // Verify on-the-wire JSON shape. b, err := json.Marshal(visitor.Snapshots[0].Decisions[0]) diff --git a/pkg/event/events.go b/pkg/event/events.go index 819766393..f8d6e1e0a 100644 --- a/pkg/event/events.go +++ b/pkg/event/events.go @@ -116,8 +116,8 @@ type Snapshot struct { // // VariationID is a pointer so that an empty / non-numeric upstream value // can be serialized as JSON null per the cross-SDK decision-event -// normalization spec (FSSDK-12813). A nil VariationID marshals to -// "variation_id": null on the wire. +// normalization spec. A nil VariationID marshals to "variation_id": null +// on the wire. type Decision struct { VariationID *string `json:"variation_id"` CampaignID string `json:"campaign_id"` diff --git a/pkg/event/events_test.go b/pkg/event/events_test.go index 69cdd07f1..06e6be86e 100644 --- a/pkg/event/events_test.go +++ b/pkg/event/events_test.go @@ -24,7 +24,7 @@ import ( ) func TestSnapshotHasOptionalDecisions(t *testing.T) { - // FSSDK-12813: Decision.VariationID is *string so that an empty / non-numeric + // Decision.VariationID is *string so that an empty / non-numeric // upstream value can serialize as JSON null. Pass &v with a numeric ID. v := "1" snapshot := Snapshot{ diff --git a/pkg/event/factory.go b/pkg/event/factory.go index 3f6c0fda5..98b69f670 100644 --- a/pkg/event/factory.go +++ b/pkg/event/factory.go @@ -103,16 +103,8 @@ func createImpressionEvent( variationID = variation.ID } - // FSSDK-12813: Normalize campaign_id, variation_id, and entity_id uniformly - // for every decision type (experiment, feature test, rollout, holdout). - // - campaign_id falls back to experiment.ID when LayerID is the empty - // string (e.g. holdouts have no layer). Any non-empty value — numeric - // or opaque (e.g. "layer_abc") — passes through per the relaxed spec. - // - entity_id mirrors the normalized campaign_id so both fields are - // byte-equivalent on the wire (FR-009). - // - variation_id is normalized later when the Decision is built, since the - // Decision struct uses *string to allow JSON null output (FR-003/FR-004). - // variation_id retains the STRICT numeric-string contract. + // entity_id mirrors the normalized campaign_id so both fields are + // byte-equivalent on the wire. normalizedCampaignID := NormalizeCampaignID(experiment.LayerID, experiment.ID) event := ImpressionEvent{ @@ -186,12 +178,9 @@ func CreateImpressionUserEvent( // create an impression visitor func createImpressionVisitor(userEvent UserEvent) Visitor { - // FSSDK-12813: Apply decision-event ID normalization at the visitor / - // wire-payload boundary as a defense-in-depth pass. The ImpressionEvent - // is already normalized when constructed (see createImpressionEvent), but - // normalizing again here guarantees that any callers who mutate the - // in-memory ImpressionEvent before dispatch still produce a valid wire - // payload. This call is idempotent for already-normalized values. + // Re-normalize at the wire-payload boundary as defense-in-depth so + // callers that mutate the ImpressionEvent before dispatch still produce + // a valid payload. Idempotent for already-normalized values. normalizedCampaignID := NormalizeCampaignID(userEvent.Impression.CampaignID, userEvent.Impression.ExperimentID) decision := Decision{} diff --git a/pkg/event/factory_test.go b/pkg/event/factory_test.go index bb7248ab0..6859dcc12 100644 --- a/pkg/event/factory_test.go +++ b/pkg/event/factory_test.go @@ -339,12 +339,8 @@ func TestCreateImpressionUserEventWithCmabUUID(t *testing.T) { func TestCreateImpressionUserEventForHoldout(t *testing.T) { tc := TestConfig{} - // FSSDK-12813: variation_id retains a strict numeric-string contract on - // the wire (non-numeric values serialize as JSON null). campaign_id / - // entity_id are relaxed to non-empty strings (opaque IDs allowed; the - // only fallback to experiment_id is when LayerID is the empty string). - // Use numeric IDs in fixtures so the happy-path assertions below exercise - // both the variation_id pass-through and the empty-LayerID fallback for + // Use numeric IDs in fixtures so the assertions below exercise the + // variation_id pass-through and the empty-LayerID fallback for // campaign_id without entanglement. testHoldout := entities.Experiment{ Key: "test_holdout", @@ -447,9 +443,8 @@ func TestCreateImpressionUserEventForHoldout(t *testing.T) { // Verify IDs are set correctly assert.Equal(t, testHoldoutVariation.ID, impression.VariationID) assert.Equal(t, testHoldout.ID, impression.ExperimentID) - // FSSDK-12813: Holdouts have no LayerID, so the impression's CampaignID - // is normalized to ExperimentID (FR-001/FR-002). The same normalized - // value is also reused as the impression's EntityID (FR-009). + // Holdouts have no LayerID, so the impression's CampaignID is + // normalized to ExperimentID. EntityID mirrors CampaignID. assert.Equal(t, testHoldout.ID, impression.CampaignID) assert.Equal(t, testHoldout.ID, impression.EntityID) }) From 38ed708d6c2f7b5932ff66d09181de39c0b879a4 Mon Sep 17 00:00:00 2001 From: Jae Kim Date: Thu, 25 Jun 2026 20:30:16 -0700 Subject: [PATCH 4/5] [FSSDK-12813] Add 2026 to copyright year in changed files --- pkg/event/events.go | 2 +- pkg/event/events_test.go | 2 +- pkg/event/factory.go | 2 +- pkg/event/factory_test.go | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/event/events.go b/pkg/event/events.go index f8d6e1e0a..a24cd8d8a 100644 --- a/pkg/event/events.go +++ b/pkg/event/events.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2020, Optimizely, Inc. and contributors * + * Copyright 2020, 2026, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * diff --git a/pkg/event/events_test.go b/pkg/event/events_test.go index 06e6be86e..b520e9bb0 100644 --- a/pkg/event/events_test.go +++ b/pkg/event/events_test.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2020, Optimizely, Inc. and contributors * + * Copyright 2020, 2026, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * diff --git a/pkg/event/factory.go b/pkg/event/factory.go index 98b69f670..21d4db70a 100644 --- a/pkg/event/factory.go +++ b/pkg/event/factory.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019-2020,2022 Optimizely, Inc. and contributors * + * Copyright 2019-2020,2022,2026 Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * diff --git a/pkg/event/factory_test.go b/pkg/event/factory_test.go index 6859dcc12..0be45adfb 100644 --- a/pkg/event/factory_test.go +++ b/pkg/event/factory_test.go @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2019,2022 Optimizely, Inc. and contributors * + * Copyright 2019,2022,2026 Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * From 24c3e59d61e293cf6fca2fb04185bf4eaf67f392 Mon Sep 17 00:00:00 2001 From: Jae Kim Date: Fri, 26 Jun 2026 09:27:45 -0700 Subject: [PATCH 5/5] [FSSDK-12813] Trigger CI