[FSSDK-12813] Relax campaign_id/entity_id validation to non-empty string per updated spec

Author: jaeopt

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,25 +59,34 @@ 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
 }
 
 // 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

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,30 +117,42 @@ 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) {
 	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.
+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) {

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{

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: "",