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..2df6e70a1 100644 --- a/src/Packages/Audience/Runtime/IdentityType.cs +++ b/src/Packages/Audience/Runtime/IdentityType.cs @@ -17,10 +17,11 @@ 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. 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", IdentityType.Steam => "steam", @@ -30,7 +31,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 ae29c3f28..48ff65f88 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: 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; @@ -181,11 +185,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()); // Link two user ids for the same player. String overload for // providers not in IdentityType. - public static void Alias(string fromId, string? fromType, string toId, string? toType) + // + // 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; @@ -248,11 +220,6 @@ public static void Alias(string fromId, string? fromType, string toId, string? t Log.Warn("Alias called with null or empty fromId/toId — dropping."); return; } - if (string.IsNullOrEmpty(fromType) || string.IsNullOrEmpty(toType)) - { - Log.Warn("Alias called with null or empty fromType/toType — dropping."); - return; - } if (_consent != ConsentLevel.Full) { Log.Warn($"Alias discarded — requires Full consent, current is {_consent}"); diff --git a/src/Packages/Audience/Tests/Runtime/Events/MessageBuilderTests.cs b/src/Packages/Audience/Tests/Runtime/Events/MessageBuilderTests.cs index bd3aad3cc..2a68d8825 100644 --- a/src/Packages/Audience/Tests/Runtime/Events/MessageBuilderTests.cs +++ b/src/Packages/Audience/Tests/Runtime/Events/MessageBuilderTests.cs @@ -75,7 +75,7 @@ public void Alias_AllFourFieldsPresent() public void AllMessages_ContextContainsLibraryAndLibraryVersion() { var track = MessageBuilder.Track("evt", null, null, PackageVersion); - var identify = MessageBuilder.Identify(null, "u1", null, PackageVersion); + var identify = MessageBuilder.Identify(null, "u1", "steam", PackageVersion); var alias = MessageBuilder.Alias("f", "t1", "t", "t2", PackageVersion); foreach (var msg in new[] { track, identify, alias }) @@ -90,7 +90,7 @@ public void AllMessages_ContextContainsLibraryAndLibraryVersion() public void AllMessages_SurfaceIsUnity() { var track = MessageBuilder.Track("evt", null, null, PackageVersion); - var identify = MessageBuilder.Identify(null, "u1", null, PackageVersion); + var identify = MessageBuilder.Identify(null, "u1", "steam", PackageVersion); var alias = MessageBuilder.Alias("f", "t1", "t", "t2", PackageVersion); Assert.AreEqual("unity", track["surface"]); diff --git a/src/Packages/Audience/Tests/Runtime/IdentityTypeTests.cs b/src/Packages/Audience/Tests/Runtime/IdentityTypeTests.cs index 7b2ed71c7..746b7e3f7 100644 --- a/src/Packages/Audience/Tests/Runtime/IdentityTypeTests.cs +++ b/src/Packages/Audience/Tests/Runtime/IdentityTypeTests.cs @@ -1,3 +1,4 @@ +using System; using NUnit.Framework; namespace Immutable.Audience.Tests @@ -19,14 +20,20 @@ public void ToLowercaseString_MapsEachEnumValueToLowercaseBackendString(Identity } [Test] - public void ToLowercaseString_UnknownValue_ReturnsNull() + public void ToLowercaseString_UnknownValue_Throws() { - // Never-throw contract: an out-of-range cast should not surface an - // exception on the game thread. Callers drop the event via their - // null/empty check 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.IsNull(invalid.ToLowercaseString()); + Assert.Throws(() => invalid.ToLowercaseString()); } } } diff --git a/src/Packages/Audience/Tests/Runtime/ImmutableAudienceTests.cs b/src/Packages/Audience/Tests/Runtime/ImmutableAudienceTests.cs index 49250a8fc..b4aec1cb5 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 that cannot be matched 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 that cannot be matched for deletion"); } [Test] @@ -396,105 +389,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() {