From 460b7117a9c9704a08003d60a8cf3532ad7c6b76 Mon Sep 17 00:00:00 2001 From: ImmutableJeffrey Date: Wed, 22 Apr 2026 17:16:59 +1000 Subject: [PATCH 1/3] revert(audience): drop Identify(Dictionary traits) overload Reverts 9b00cce7, which added a traits-only Identify overload that mirrored the @imtbl/audience Web SDK's identify(traits) shape. The overload emits identify events with no identityType and cannot carry one meaningfully (no userId to attach one to). CDP requires identityType on every identify event to match records during data deletion, so records produced by this path would be unreachable for erasure. Mechanical prerequisite for SDK-222: making identityType mandatory on MessageBuilder.Identify is a compile error while this overload exists. Web SDK parity will be reopened under a separate ticket once the Web SDK drops its equivalent for the same CDP reason. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Audience/Runtime/ImmutableAudience.cs | 30 ------ .../Tests/Runtime/ImmutableAudienceTests.cs | 99 ------------------- 2 files changed, 129 deletions(-) diff --git a/src/Packages/Audience/Runtime/ImmutableAudience.cs b/src/Packages/Audience/Runtime/ImmutableAudience.cs index ae29c3f28..662f9ce21 100644 --- a/src/Packages/Audience/Runtime/ImmutableAudience.cs +++ b/src/Packages/Audience/Runtime/ImmutableAudience.cs @@ -203,36 +203,6 @@ public static void Identify(string userId, string? identityType, Dictionary traits) - { - if (!_initialized) return; - - if (traits == null) - { - Log.Warn("Identify(traits) called with null traits — dropping."); - return; - } - if (_consent != ConsentLevel.Full) - { - Log.Warn($"Identify discarded — requires Full consent, current is {_consent}"); - return; - } - - var config = _config; - if (config == null) return; - - var anonymousId = Identity.GetOrCreate(config.PersistentDataPath!, _consent); - var msg = MessageBuilder.Identify(anonymousId, userId: null, identityType: null, - config.PackageVersion, SnapshotCallerDict(traits)); - Enqueue(msg); - } - // Link two user ids for the same player. public static void Alias(string fromId, IdentityType fromType, string toId, IdentityType toType) => Alias(fromId, fromType.ToLowercaseString(), toId, toType.ToLowercaseString()); diff --git a/src/Packages/Audience/Tests/Runtime/ImmutableAudienceTests.cs b/src/Packages/Audience/Tests/Runtime/ImmutableAudienceTests.cs index 49250a8fc..60826378f 100644 --- a/src/Packages/Audience/Tests/Runtime/ImmutableAudienceTests.cs +++ b/src/Packages/Audience/Tests/Runtime/ImmutableAudienceTests.cs @@ -396,105 +396,6 @@ public void Identify_AnonymousConsent_IsIgnored() "identify should be discarded at Anonymous consent"); } - [Test] - public void IdentifyTraits_FullConsent_WritesIdentifyWithTraitsAndNoUserIdField() - { - ImmutableAudience.Init(MakeConfig(ConsentLevel.Full)); - - ImmutableAudience.Identify(new Dictionary { ["plan"] = "pro" }); - ImmutableAudience.Shutdown(); - - var queueDir = AudiencePaths.QueueDir(_testDir); - var contents = Directory.GetFiles(queueDir, "*.json") - .Select(File.ReadAllText).ToList(); - var identifyMsg = contents.FirstOrDefault(c => c.Contains("\"identify\"")); - Assert.IsNotNull(identifyMsg, "traits-only identify should enqueue an identify event"); - Assert.IsTrue(identifyMsg.Contains("\"plan\"") && identifyMsg.Contains("\"pro\""), - "traits payload should be present"); - Assert.IsFalse(identifyMsg.Contains("\"userId\""), - "traits-only identify must not attach a userId field"); - Assert.IsFalse(identifyMsg.Contains("\"identityType\""), - "traits-only identify must not attach an identityType field"); - } - - [Test] - public void IdentifyTraits_AnonymousConsent_IsIgnored() - { - ImmutableAudience.Init(MakeConfig(ConsentLevel.Anonymous)); - - ImmutableAudience.Identify(new Dictionary { ["plan"] = "pro" }); - ImmutableAudience.Shutdown(); - - var queueDir = AudiencePaths.QueueDir(_testDir); - var contents = Directory.GetFiles(queueDir, "*.json") - .Select(File.ReadAllText).ToList(); - Assert.IsFalse(contents.Any(c => c.Contains("\"identify\"")), - "Identify(traits) should be discarded at Anonymous consent"); - } - - [Test] - public void IdentifyTraits_NoneConsent_IsIgnored() - { - ImmutableAudience.Init(MakeConfig(ConsentLevel.None)); - - ImmutableAudience.Identify(new Dictionary { ["plan"] = "pro" }); - ImmutableAudience.Shutdown(); - - var queueDir = AudiencePaths.QueueDir(_testDir); - var files = Directory.Exists(queueDir) - ? Directory.GetFiles(queueDir, "*.json") - : Array.Empty(); - Assert.IsFalse(files.Select(File.ReadAllText).Any(c => c.Contains("\"identify\"")), - "Identify(traits) should be discarded at None consent"); - } - - [Test] - public void IdentifyTraits_NotInitialised_IsIgnored() - { - Assert.DoesNotThrow(() => ImmutableAudience.Identify(new Dictionary())); - } - - [Test] - public void IdentifyTraits_NullTraits_DropsAndWarns() - { - ImmutableAudience.Init(MakeConfig(ConsentLevel.Full)); - - var lines = new List(); - Log.Writer = lines.Add; - try - { - Assert.DoesNotThrow(() => ImmutableAudience.Identify((Dictionary)null)); - Assert.That(lines, Has.Some.Contains("null traits")); - } - finally { Log.Writer = null; } - - ImmutableAudience.Shutdown(); - var queueDir = AudiencePaths.QueueDir(_testDir); - var contents = Directory.GetFiles(queueDir, "*.json") - .Select(File.ReadAllText).ToList(); - Assert.IsFalse(contents.Any(c => c.Contains("\"identify\"")), - "null traits must not produce an identify event"); - } - - [Test] - public void IdentifyTraits_DoesNotOverwritePriorUserId() - { - ImmutableAudience.Init(MakeConfig(ConsentLevel.Full)); - - ImmutableAudience.Identify("user-123", IdentityType.Passport); - ImmutableAudience.Identify(new Dictionary { ["plan"] = "pro" }); - ImmutableAudience.Track("after_traits_identify"); - ImmutableAudience.Shutdown(); - - var queueDir = AudiencePaths.QueueDir(_testDir); - var trackMsg = Directory.GetFiles(queueDir, "*.json") - .Select(File.ReadAllText) - .FirstOrDefault(c => c.Contains("\"after_traits_identify\"")); - Assert.IsNotNull(trackMsg, "track event should be enqueued"); - Assert.IsTrue(trackMsg.Contains("\"user-123\""), - "Track after traits-only Identify must still carry the prior userId"); - } - [Test] public void Alias_FullConsent_WritesAliasEvent() { From 1a2fa8abb900df14394b4db335a6255be49da286 Mon Sep 17 00:00:00 2001 From: ImmutableJeffrey Date: Wed, 22 Apr 2026 17:20:10 +1000 Subject: [PATCH 2/3] refactor(audience): make identityType mandatory on Identify and Alias MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CDP requires identityType on every identify / alias event so it can match records to the correct identity namespace during data deletion. Enforce the invariant through the type system end to end: - `Identify(userId, identityType, traits?)` — identityType is non-nullable. The null/empty warn-and-drop block is gone; the type signature no longer lies about optionality. - `Alias(fromId, fromType, toId, toType)` — fromType and toType are non-nullable. Same block removed. - `MessageBuilder.Identify` — identityType parameter is non-nullable and always emitted; the conditional that omitted an empty field is gone so the wire shape cannot drift. - `IdentityType.ToLowercaseString()` — casting to an out-of-range enum value now throws `ArgumentOutOfRangeException` rather than returning null. A silent null used to leak through to a dropped event; now the programmer error fails loud at the call site. Tests repointed: invalid-enum-cast cases assert the throw, and MessageBuilder tests that passed `null` for identityType now pass a valid value since the parameter is no longer nullable. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Audience/Runtime/Events/MessageBuilder.cs | 5 ++-- src/Packages/Audience/Runtime/IdentityType.cs | 13 +++++---- .../Audience/Runtime/ImmutableAudience.cs | 22 +++++++-------- .../Runtime/Events/MessageBuilderTests.cs | 4 +-- .../Tests/Runtime/IdentityTypeTests.cs | 12 +++++---- .../Tests/Runtime/ImmutableAudienceTests.cs | 27 +++++++------------ 6 files changed, 39 insertions(+), 44 deletions(-) diff --git a/src/Packages/Audience/Runtime/Events/MessageBuilder.cs b/src/Packages/Audience/Runtime/Events/MessageBuilder.cs index 1347e4665..43f815248 100644 --- a/src/Packages/Audience/Runtime/Events/MessageBuilder.cs +++ b/src/Packages/Audience/Runtime/Events/MessageBuilder.cs @@ -35,7 +35,7 @@ internal static Dictionary Track( internal static Dictionary Identify( string? anonymousId, string? userId, - string? identityType, + string identityType, string packageVersion, Dictionary? traits = null) { @@ -47,8 +47,7 @@ internal static Dictionary Identify( if (!string.IsNullOrEmpty(userId)) msg[MessageFields.UserId] = Truncate(userId, Constants.MaxFieldLength); - if (!string.IsNullOrEmpty(identityType)) - msg["identityType"] = Truncate(identityType, Constants.MaxFieldLength); + msg["identityType"] = Truncate(identityType, Constants.MaxFieldLength); if (traits != null && traits.Count > 0) { diff --git a/src/Packages/Audience/Runtime/IdentityType.cs b/src/Packages/Audience/Runtime/IdentityType.cs index eed0312ea..0d80c2799 100644 --- a/src/Packages/Audience/Runtime/IdentityType.cs +++ b/src/Packages/Audience/Runtime/IdentityType.cs @@ -17,10 +17,12 @@ public enum IdentityType internal static class IdentityTypeExtensions { - // Returns null on unknown casts. The string overloads of Identify / - // Alias check for null/empty and drop + warn, so an out-of-range - // cast surfaces as a dropped event, not a corrupt wire payload. - internal static string? ToLowercaseString(this IdentityType type) => type switch + // Throws on unknown casts. The CDP pipeline requires identityType on + // every identify / alias event to match records to the correct + // identity namespace during data deletion, so an out-of-range cast + // must fail loudly rather than ship an event with a missing or + // empty namespace. + internal static string ToLowercaseString(this IdentityType type) => type switch { IdentityType.Passport => "passport", IdentityType.Steam => "steam", @@ -30,7 +32,8 @@ internal static class IdentityTypeExtensions IdentityType.Discord => "discord", IdentityType.Email => "email", IdentityType.Custom => "custom", - _ => null, + _ => throw new System.ArgumentOutOfRangeException(nameof(type), type, + "Unknown IdentityType value; cast an out-of-range value."), }; } } diff --git a/src/Packages/Audience/Runtime/ImmutableAudience.cs b/src/Packages/Audience/Runtime/ImmutableAudience.cs index 662f9ce21..ac8953e06 100644 --- a/src/Packages/Audience/Runtime/ImmutableAudience.cs +++ b/src/Packages/Audience/Runtime/ImmutableAudience.cs @@ -171,7 +171,11 @@ public static void Identify(string userId, IdentityType identityType, Dictionary // Attach a known user id to subsequent events. String overload for // providers not in IdentityType. - public static void Identify(string userId, string? identityType, Dictionary? traits = null) + // + // identityType is required: CDP uses it to match identify events to + // the correct identity namespace when processing data-deletion + // requests, so an event without one cannot be cleaned up. + public static void Identify(string userId, string identityType, Dictionary? traits = null) { if (!_initialized) return; @@ -181,11 +185,6 @@ public static void Identify(string userId, string? identityType, Dictionary(() => invalid.ToLowercaseString()); } } } diff --git a/src/Packages/Audience/Tests/Runtime/ImmutableAudienceTests.cs b/src/Packages/Audience/Tests/Runtime/ImmutableAudienceTests.cs index 60826378f..82a3ada3c 100644 --- a/src/Packages/Audience/Tests/Runtime/ImmutableAudienceTests.cs +++ b/src/Packages/Audience/Tests/Runtime/ImmutableAudienceTests.cs @@ -132,36 +132,29 @@ public void Identify_NullUserId_DoesNotEnqueue() } [Test] - public void Identify_InvalidIdentityTypeCast_DoesNotThrow_AndDropsEvent() + public void Identify_InvalidIdentityTypeCast_Throws() { ImmutableAudience.Init(MakeConfig(ConsentLevel.Full)); var invalid = (IdentityType)999; - Assert.DoesNotThrow(() => ImmutableAudience.Identify("user1", invalid)); - - ImmutableAudience.Shutdown(); - var queueDir = AudiencePaths.QueueDir(_testDir); - var contents = Directory.GetFiles(queueDir, "*.json").Select(File.ReadAllText); - Assert.IsFalse(contents.Any(c => c.Contains("\"identify\"")), - "invalid enum cast must drop the identify event, not enqueue it"); + Assert.Throws( + () => ImmutableAudience.Identify("user1", invalid), + "invalid enum cast must throw so a broken call fails loud rather than " + + "shipping an identify event CDP cannot match for deletion"); } [Test] - public void Alias_InvalidIdentityTypeCast_DoesNotThrow_AndDropsEvent() + public void Alias_InvalidIdentityTypeCast_Throws() { ImmutableAudience.Init(MakeConfig(ConsentLevel.Full)); var invalid = (IdentityType)999; - Assert.DoesNotThrow(() => - ImmutableAudience.Alias("fromId", invalid, "toId", IdentityType.Steam)); - - ImmutableAudience.Shutdown(); - var queueDir = AudiencePaths.QueueDir(_testDir); - var contents = Directory.GetFiles(queueDir, "*.json").Select(File.ReadAllText); - Assert.IsFalse(contents.Any(c => c.Contains("\"alias\"")), - "invalid enum cast must drop the alias event, not enqueue it"); + Assert.Throws( + () => ImmutableAudience.Alias("fromId", invalid, "toId", IdentityType.Steam), + "invalid enum cast must throw so a broken alias call fails loud rather " + + "than shipping an event CDP cannot match for deletion"); } [Test] From d8c7f4812a46c906f4fa7ad8432898fcedb22a22 Mon Sep 17 00:00:00 2001 From: ImmutableJeffrey Date: Wed, 22 Apr 2026 18:39:23 +1000 Subject: [PATCH 3/3] docs(audience): drop internal pipeline name from comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rewrites the comments on IdentityType.ToLowercaseString, the Identify(string) and Alias(string) overloads in ImmutableAudience, and the matching test comments and assertion messages so they explain why identityType is mandatory — downstream data-deletion needs it to match a user's events — without naming the specific internal pipeline. Same invariant, just no internal name in the source. The test comment on ToLowercaseString_UnknownValue_Throws is also rewritten to read cold: leads with the rule and a concrete bad-cast example, names what ToLowercaseString emits and how the backend uses it, then explains why a default return would hide the bug. Addresses PR #696 review thread (discussion_r3122316478). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/Packages/Audience/Runtime/IdentityType.cs | 9 ++++----- src/Packages/Audience/Runtime/ImmutableAudience.cs | 11 +++++------ .../Audience/Tests/Runtime/IdentityTypeTests.cs | 13 +++++++++---- .../Tests/Runtime/ImmutableAudienceTests.cs | 4 ++-- 4 files changed, 20 insertions(+), 17 deletions(-) diff --git a/src/Packages/Audience/Runtime/IdentityType.cs b/src/Packages/Audience/Runtime/IdentityType.cs index 0d80c2799..2df6e70a1 100644 --- a/src/Packages/Audience/Runtime/IdentityType.cs +++ b/src/Packages/Audience/Runtime/IdentityType.cs @@ -17,11 +17,10 @@ public enum IdentityType internal static class IdentityTypeExtensions { - // Throws on unknown casts. The CDP pipeline requires identityType on - // every identify / alias event to match records to the correct - // identity namespace during data deletion, so an out-of-range cast - // must fail loudly rather than ship an event with a missing or - // empty namespace. + // Throws on unknown casts. Every identify / alias event must carry an + // identityType so downstream data-deletion requests can match records + // to the correct identity namespace; an out-of-range cast must fail + // loudly rather than ship an event with a missing or empty namespace. internal static string ToLowercaseString(this IdentityType type) => type switch { IdentityType.Passport => "passport", diff --git a/src/Packages/Audience/Runtime/ImmutableAudience.cs b/src/Packages/Audience/Runtime/ImmutableAudience.cs index ac8953e06..48ff65f88 100644 --- a/src/Packages/Audience/Runtime/ImmutableAudience.cs +++ b/src/Packages/Audience/Runtime/ImmutableAudience.cs @@ -172,9 +172,9 @@ public static void Identify(string userId, IdentityType identityType, Dictionary // Attach a known user id to subsequent events. String overload for // providers not in IdentityType. // - // identityType is required: CDP uses it to match identify events to - // the correct identity namespace when processing data-deletion - // requests, so an event without one cannot be cleaned up. + // identityType is required: data-deletion processing relies on it to + // match identify events to the correct identity namespace, so an + // event without one cannot be cleaned up. public static void Identify(string userId, string identityType, Dictionary? traits = null) { if (!_initialized) return; @@ -209,9 +209,8 @@ public static void Alias(string fromId, IdentityType fromType, string toId, Iden // Link two user ids for the same player. String overload for // providers not in IdentityType. // - // fromType and toType are required: CDP uses them to match alias - // events to the correct identity namespaces when processing - // data-deletion requests. + // fromType and toType are required: data-deletion processing uses + // them to match alias events to the correct identity namespaces. public static void Alias(string fromId, string fromType, string toId, string toType) { if (!_initialized) return; diff --git a/src/Packages/Audience/Tests/Runtime/IdentityTypeTests.cs b/src/Packages/Audience/Tests/Runtime/IdentityTypeTests.cs index 315c33ab7..746b7e3f7 100644 --- a/src/Packages/Audience/Tests/Runtime/IdentityTypeTests.cs +++ b/src/Packages/Audience/Tests/Runtime/IdentityTypeTests.cs @@ -22,10 +22,15 @@ public void ToLowercaseString_MapsEachEnumValueToLowercaseBackendString(Identity [Test] public void ToLowercaseString_UnknownValue_Throws() { - // CDP matches identify / alias events by identityType during data - // deletion; an out-of-range cast would otherwise ship an event - // with an empty namespace that CDP cannot clean up. Fail loud so - // the programmer error surfaces at the call site instead. + // An out-of-range cast like `(IdentityType)999` must throw. + // ToLowercaseString emits the "identityType" string on every + // identify / alias event (e.g. "passport"), and the backend uses + // that string to find and delete a user's events on request. + // + // An unknown enum value must throw so the bug surfaces at the + // cast site. Returning a default string instead would ship the + // event with a blank identityType — invisible to the deletion + // lookup — and hide the bug. var invalid = (IdentityType)999; Assert.Throws(() => invalid.ToLowercaseString()); diff --git a/src/Packages/Audience/Tests/Runtime/ImmutableAudienceTests.cs b/src/Packages/Audience/Tests/Runtime/ImmutableAudienceTests.cs index 82a3ada3c..b4aec1cb5 100644 --- a/src/Packages/Audience/Tests/Runtime/ImmutableAudienceTests.cs +++ b/src/Packages/Audience/Tests/Runtime/ImmutableAudienceTests.cs @@ -141,7 +141,7 @@ public void Identify_InvalidIdentityTypeCast_Throws() Assert.Throws( () => ImmutableAudience.Identify("user1", invalid), "invalid enum cast must throw so a broken call fails loud rather than " + - "shipping an identify event CDP cannot match for deletion"); + "shipping an identify event that cannot be matched for deletion"); } [Test] @@ -154,7 +154,7 @@ public void Alias_InvalidIdentityTypeCast_Throws() Assert.Throws( () => ImmutableAudience.Alias("fromId", invalid, "toId", IdentityType.Steam), "invalid enum cast must throw so a broken alias call fails loud rather " + - "than shipping an event CDP cannot match for deletion"); + "than shipping an event that cannot be matched for deletion"); } [Test]