diff --git a/examples/audience/Assets/SampleApp/Scripts/AudienceSample.Events.cs b/examples/audience/Assets/SampleApp/Scripts/AudienceSample.Events.cs index f9c625fb..fa0f0d41 100644 --- a/examples/audience/Assets/SampleApp/Scripts/AudienceSample.Events.cs +++ b/examples/audience/Assets/SampleApp/Scripts/AudienceSample.Events.cs @@ -53,7 +53,7 @@ internal readonly struct EventSpec new EventSpec("wishlist_remove", new[] { EventField.Text("gameId") }), new EventSpec("purchase", new[] { EventField.Text("currency"), - EventField.Number("value"), + EventField.Text("value"), EventField.Text("itemId", optional: true), EventField.Text("itemName", optional: true), EventField.Number("quantity", optional: true), @@ -129,7 +129,7 @@ internal readonly struct EventSpec return new Purchase { Currency = OptionalString(props, "currency") ?? "", - Value = OptionalDecimal(props, "value") ?? 0m, + Value = OptionalString(props, "value") ?? "0", ItemId = OptionalString(props, "itemId"), ItemName = OptionalString(props, "itemName"), Quantity = OptionalInt(props, "quantity"), diff --git a/src/Packages/Audience/README.md b/src/Packages/Audience/README.md index 25e1877d..bf6a7f77 100644 --- a/src/Packages/Audience/README.md +++ b/src/Packages/Audience/README.md @@ -35,7 +35,7 @@ public static class Analytics Debug = true, }); - ImmutableAudience.Track(new Purchase { Currency = "USD", Value = 9.99m }); + ImmutableAudience.Track(new Purchase { Currency = "USD", Value = "9.99" }); } } ``` diff --git a/src/Packages/Audience/Runtime/Events/IEvent.cs b/src/Packages/Audience/Runtime/Events/IEvent.cs index c4fbfdea..241a6af0 100644 --- a/src/Packages/Audience/Runtime/Events/IEvent.cs +++ b/src/Packages/Audience/Runtime/Events/IEvent.cs @@ -10,10 +10,10 @@ namespace Immutable.Audience /// . /// /// - /// Implementations validate required fields inside - /// ; - /// catches the throw and - /// drops the event with a warning. + /// Implementations validate required fields inside . + /// catches the throw and warns + /// for a consumer-authored ; see + /// for the exception to that. /// public interface IEvent { @@ -32,4 +32,17 @@ public interface IEvent /// Dictionary ToProperties(); } + + /// + /// Marks an implementation as one of Immutable's own + /// built-in event types (, , + /// , , + /// ). Not implementable outside this + /// assembly: it exists only so + /// can tell a validation failure in one of these apart from an exception + /// thrown by a consumer's own custom . + /// + internal interface IBuiltInEvent : IEvent + { + } } diff --git a/src/Packages/Audience/Runtime/Events/ReservedEvents.cs b/src/Packages/Audience/Runtime/Events/ReservedEvents.cs new file mode 100644 index 00000000..91bf8583 --- /dev/null +++ b/src/Packages/Audience/Runtime/Events/ReservedEvents.cs @@ -0,0 +1,27 @@ +#nullable enable + +using System.Collections.Generic; + +namespace Immutable.Audience +{ + /// + /// Required property names per reserved event, enforced at runtime by + /// . + /// The typed classes enforce the same rules + /// at compile time (constructor/property level) and again in + /// ToProperties; this closes the gap for callers who use the + /// string overload instead and so bypass all of that. + /// + internal static class ReservedEvents + { + internal static readonly IReadOnlyDictionary RequiredProperties = + new Dictionary + { + ["purchase"] = new[] { "currency", "value" }, + ["progression"] = new[] { "status" }, + ["resource"] = new[] { "flow", "currency", "amount" }, + ["achievement_unlocked"] = new[] { "achievement_id", "achievement_name" }, + ["milestone_reached"] = new[] { "name" }, + }; + } +} diff --git a/src/Packages/Audience/Runtime/Events/ReservedEvents.cs.meta b/src/Packages/Audience/Runtime/Events/ReservedEvents.cs.meta new file mode 100644 index 00000000..1dc4e560 --- /dev/null +++ b/src/Packages/Audience/Runtime/Events/ReservedEvents.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 11f4ec3e32e164c5fb21fdf8fbdbdce1 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/src/Packages/Audience/Runtime/Events/TypedEvents.cs b/src/Packages/Audience/Runtime/Events/TypedEvents.cs index 52474e01..b422499d 100644 --- a/src/Packages/Audience/Runtime/Events/TypedEvents.cs +++ b/src/Packages/Audience/Runtime/Events/TypedEvents.cs @@ -44,7 +44,7 @@ internal static class ProgressionStatusExtensions /// Player progressing through a world / level / stage. Track via /// . /// - public class Progression : IEvent + public class Progression : IBuiltInEvent { /// /// Required. Where the player is in the progression flow. @@ -134,7 +134,7 @@ internal static class ResourceFlowExtensions /// In-game currency earned or spent. Track via /// . /// - public class Resource : IEvent + public class Resource : IBuiltInEvent { /// /// Required. Whether this is a gain or a spend. @@ -193,7 +193,7 @@ public Dictionary ToProperties() /// Real-money transaction. Track via /// . /// - public class Purchase : IEvent + public class Purchase : IBuiltInEvent { /// /// Required. ISO 4217 three-letter uppercase currency code (for @@ -202,9 +202,11 @@ public class Purchase : IEvent public string? Currency { get; set; } /// - /// Required. The transaction amount in . + /// Required. The transaction amount in , as a + /// numeric-looking string (e.g. "9.99"), not a number, to + /// avoid floating-point precision loss. /// - public decimal? Value { get; set; } + public string? Value { get; set; } /// /// Optional. Stable identifier of the item purchased. @@ -247,13 +249,13 @@ public Dictionary ToProperties() if (Currency == null || !IsIso4217(Currency)) throw new ArgumentException( $"Purchase.Currency '{Currency}' must be a three-letter uppercase ISO 4217 code"); - if (Value is null) + if (string.IsNullOrEmpty(Value)) throw new ArgumentException("Purchase.Value is required. Set it before calling Track(IEvent)."); var props = new Dictionary { ["currency"] = Currency, - ["value"] = Value.Value + ["value"] = Value }; if (ItemId != null) props["item_id"] = ItemId; @@ -316,7 +318,7 @@ internal static class AchievementTypeExtensions /// Player unlocked an achievement. Track via /// . /// - public class AchievementUnlocked : IEvent + public class AchievementUnlocked : IBuiltInEvent { /// /// Required. Stable identifier for the achievement (for example, @@ -362,7 +364,7 @@ public Dictionary ToProperties() /// Named milestone or achievement reached by the player. Track via /// . /// - public class MilestoneReached : IEvent + public class MilestoneReached : IBuiltInEvent { /// /// Required. The milestone identifier (for example, diff --git a/src/Packages/Audience/Runtime/ImmutableAudience.cs b/src/Packages/Audience/Runtime/ImmutableAudience.cs index 7826c21c..5420610e 100644 --- a/src/Packages/Audience/Runtime/ImmutableAudience.cs +++ b/src/Packages/Audience/Runtime/ImmutableAudience.cs @@ -328,6 +328,10 @@ internal static void OnResume() /// /// The event to send. Must not be null, and /// must not be empty (both throw). + /// + /// One of Immutable's own built-in events (, + /// , etc.) is missing a required field. + /// public static void Track(IEvent evt) { if (!_initialized) return; @@ -339,7 +343,6 @@ public static void Track(IEvent evt) var config = _config; if (config == null) return; - // Consumer-supplied impl; catch so a buggy IEvent cannot crash the game. string eventName; Dictionary properties; try @@ -349,6 +352,8 @@ public static void Track(IEvent evt) } catch (Exception ex) { + // See IBuiltInEvent: built-in events throw straight through. + if (evt is IBuiltInEvent) throw; Log.Warn(AudienceLogs.TrackIEventThrew(evt.GetType().Name, ex)); return; } @@ -367,11 +372,17 @@ public static void Track(IEvent evt) /// /// The wire-format event name. Must not be empty (throws otherwise). /// Optional event properties. + /// + /// is empty, or is a reserved event + /// (purchase, progression, resource, + /// achievement_unlocked) missing one of its required properties. + /// public static void Track(string eventName, Dictionary? properties = null) { if (!_initialized) return; if (string.IsNullOrEmpty(eventName)) throw new ArgumentException(AudienceLogs.TrackStringEmptyName, nameof(eventName)); + ValidateReservedEventProperties(eventName, properties); var state = _state; if (!state.Level.CanTrack()) return; @@ -382,6 +393,20 @@ public static void Track(string eventName, Dictionary? propertie EnqueueTrackedEvent(eventName, SnapshotCallerDict(properties), _session?.SessionId, state, config); } + // See ReservedEvents. Unrecognised event names are never checked here. + private static void ValidateReservedEventProperties(string eventName, Dictionary? properties) + { + if (!ReservedEvents.RequiredProperties.TryGetValue(eventName, out var required)) return; + + var missing = new List(); + foreach (var key in required) + { + if (properties == null || !properties.ContainsKey(key)) missing.Add(key); + } + if (missing.Count > 0) + throw new ArgumentException(AudienceLogs.TrackStringMissingRequiredProps(eventName, missing), nameof(properties)); + } + // Shared tail for both Track overloads and TrackFromSession. private static void EnqueueTrackedEvent( string eventName, diff --git a/src/Packages/Audience/Runtime/Utility/Log.cs b/src/Packages/Audience/Runtime/Utility/Log.cs index eef021aa..90b3f0f8 100644 --- a/src/Packages/Audience/Runtime/Utility/Log.cs +++ b/src/Packages/Audience/Runtime/Utility/Log.cs @@ -1,6 +1,7 @@ #nullable enable using System; +using System.Collections.Generic; namespace Immutable.Audience { @@ -77,6 +78,10 @@ internal static string TrackIEventThrew(string evtTypeName, Exception ex) => internal static string TrackIEventEmptyName(string evtTypeName) => $"Track(IEvent): {evtTypeName}.EventName returned null or empty."; + internal static string TrackStringMissingRequiredProps(string eventName, IReadOnlyList missing) => + $"Track(\"{eventName}\", ...) is missing required propert{(missing.Count > 1 ? "ies" : "y")}: " + + $"{string.Join(", ", missing)}."; + // ---- Identify / Alias ---- // Empty/malformed ids are caller bugs: thrown as ArgumentException, not // logged (see ImmutableAudience.Identify/Alias). IdentifyDiscarded/ diff --git a/src/Packages/Audience/Tests/Runtime/Events/TypedEventTests.cs b/src/Packages/Audience/Tests/Runtime/Events/TypedEventTests.cs index c5952714..f5df6fcc 100644 --- a/src/Packages/Audience/Tests/Runtime/Events/TypedEventTests.cs +++ b/src/Packages/Audience/Tests/Runtime/Events/TypedEventTests.cs @@ -1,4 +1,5 @@ using System; +using System.Linq; using NUnit.Framework; namespace Immutable.Audience.Tests @@ -6,6 +7,24 @@ namespace Immutable.Audience.Tests [TestFixture] internal class TypedEventTests { + // Guards against a new built-in typed event being added without a + // matching ReservedEvents entry, which would leave it silently + // unvalidated on the Track(string, Dictionary) path. + [Test] + public void EveryBuiltInEventHasAReservedEventsEntry() + { + var builtInTypes = typeof(IBuiltInEvent).Assembly.GetTypes() + .Where(t => typeof(IBuiltInEvent).IsAssignableFrom(t) && !t.IsInterface && !t.IsAbstract); + + foreach (var type in builtInTypes) + { + var evt = (IEvent)Activator.CreateInstance(type)!; + Assert.That(ReservedEvents.RequiredProperties.ContainsKey(evt.EventName), Is.True, + $"{type.Name} implements IBuiltInEvent but ReservedEvents.RequiredProperties has no entry " + + $"for \"{evt.EventName}\" (add one, even [])."); + } + } + [Test] public void Progression_EventName_IsProgression() { @@ -115,7 +134,7 @@ public void Purchase_ProducesCorrectProperties() var evt = new Purchase { Currency = "USD", - Value = 9.99m, + Value = "9.99", ItemId = "gem_pack_01", ItemName = "Starter Gem Pack", Quantity = 1, @@ -125,7 +144,7 @@ public void Purchase_ProducesCorrectProperties() var props = evt.ToProperties(); Assert.AreEqual("USD", props["currency"]); - Assert.AreEqual(9.99m, props["value"]); + Assert.AreEqual("9.99", props["value"]); Assert.AreEqual("gem_pack_01", props["item_id"]); Assert.AreEqual("Starter Gem Pack", props["item_name"]); Assert.AreEqual(1, props["quantity"]); @@ -135,7 +154,7 @@ public void Purchase_ProducesCorrectProperties() [Test] public void Purchase_OptionalFieldsOmitted_WhenNull() { - var props = new Purchase { Currency = "EUR", Value = 5.00m }.ToProperties(); + var props = new Purchase { Currency = "EUR", Value = "5.00" }.ToProperties(); Assert.IsTrue(props.ContainsKey("currency")); Assert.IsTrue(props.ContainsKey("value")); @@ -154,7 +173,7 @@ public void Purchase_EventName_IsPurchase() [Test] public void Purchase_WithoutCurrency_ThrowsOnToProperties() { - var evt = new Purchase { Value = 9.99m }; + var evt = new Purchase { Value = "9.99" }; var ex = Assert.Throws(() => evt.ToProperties()); Assert.That(ex!.Message, Does.Contain("Currency")); diff --git a/src/Packages/Audience/Tests/Runtime/ImmutableAudienceTests.cs b/src/Packages/Audience/Tests/Runtime/ImmutableAudienceTests.cs index f32634db..14797c93 100644 --- a/src/Packages/Audience/Tests/Runtime/ImmutableAudienceTests.cs +++ b/src/Packages/Audience/Tests/Runtime/ImmutableAudienceTests.cs @@ -405,7 +405,32 @@ public void Track_NullEvent_ThrowsEvenAtNoneConsent() } [Test] - public void Track_IEventMissingRequiredField_DropsWithWarn() + public void Track_BuiltInEventMissingRequiredField_Throws() + { + ImmutableAudience.Init(MakeConfig()); + + // Purchase with no Value set: ToProperties throws. Purchase is one of + // Immutable's own built-in events, so that's a caller bug and must + // throw straight through, the same as Identify()/Alias() do for an + // equivalent mistake, rather than ship an incomplete event silently. + Assert.Throws(() => ImmutableAudience.Track(new Purchase { Currency = "USD" })); + + ImmutableAudience.Shutdown(); + var queueDir = AudiencePaths.QueueDir(_testDir); + var contents = Directory.GetFiles(queueDir, "*.json") + .Select(File.ReadAllText).ToList(); + Assert.IsFalse(contents.Any(c => c.Contains("\"purchase\"")), + "purchase event with missing required Value must never reach the queue"); + } + + private class ThrowingCustomEvent : IEvent + { + public string EventName => "custom_event"; + public Dictionary ToProperties() => throw new InvalidOperationException("boom"); + } + + [Test] + public void Track_CustomEventThrows_DropsWithWarn() { ImmutableAudience.Init(MakeConfig()); @@ -413,12 +438,12 @@ public void Track_IEventMissingRequiredField_DropsWithWarn() Log.Writer = lines.Add; try { - // Purchase with no Value set: ToProperties throws; Track must - // catch, warn, and drop rather than ship an incomplete event. - Assert.DoesNotThrow(() => ImmutableAudience.Track(new Purchase { Currency = "USD" })); - // Assert the stable parts (event-type name and trailing "Dropping") - // so the test survives any change to the exception type or message. - Assert.That(lines, Has.Some.Contains(nameof(Purchase))); + // A consumer's own custom IEvent (not one of Immutable's built-in + // types) is not held to the same "throw straight through" standard: + // we can't predict what a third-party implementation might throw, + // so it's caught, warned, and dropped instead of crashing the game. + Assert.DoesNotThrow(() => ImmutableAudience.Track(new ThrowingCustomEvent())); + Assert.That(lines, Has.Some.Contains(nameof(ThrowingCustomEvent))); Assert.That(lines, Has.Some.Contains("Dropping")); } finally { Log.Writer = null; } @@ -427,8 +452,8 @@ public void Track_IEventMissingRequiredField_DropsWithWarn() var queueDir = AudiencePaths.QueueDir(_testDir); var contents = Directory.GetFiles(queueDir, "*.json") .Select(File.ReadAllText).ToList(); - Assert.IsFalse(contents.Any(c => c.Contains("\"purchase\"")), - "purchase event with missing required Value must be dropped, not enqueued"); + Assert.IsFalse(contents.Any(c => c.Contains("custom_event")), + "custom event whose ToProperties() throws must be dropped, not enqueued"); } [Test] @@ -450,6 +475,51 @@ public void Track_NullOrEmptyEventName_ThrowsEvenAtNoneConsent() Assert.Throws(() => ImmutableAudience.Track("")); } + [Test] + public void Track_StringOverload_ReservedEventMissingRequiredProps_Throws() + { + // The typed IEvent classes can't stop a caller from bypassing them + // entirely and calling the string overload directly for a reserved + // event name; this closes that gap. + ImmutableAudience.Init(MakeConfig()); + + Assert.Throws( + () => ImmutableAudience.Track("purchase", new Dictionary { ["currency"] = "USD" }), + "purchase is missing required 'value'"); + Assert.Throws( + () => ImmutableAudience.Track("progression"), + "progression is missing required 'status'"); + Assert.Throws( + () => ImmutableAudience.Track("resource", new Dictionary { ["flow"] = "source" }), + "resource is missing required 'currency' and 'amount'"); + Assert.Throws( + () => ImmutableAudience.Track("achievement_unlocked", new Dictionary { ["achievement_id"] = "a1" }), + "achievement_unlocked is missing required 'achievement_name'"); + } + + [Test] + public void Track_StringOverload_ReservedEventWithAllRequiredProps_DoesNotThrow() + { + ImmutableAudience.Init(MakeConfig()); + + Assert.DoesNotThrow(() => ImmutableAudience.Track("purchase", + new Dictionary { ["currency"] = "USD", ["value"] = 9.99m })); + Assert.DoesNotThrow(() => ImmutableAudience.Track("progression", + new Dictionary { ["status"] = "start" })); + Assert.DoesNotThrow(() => ImmutableAudience.Track("resource", + new Dictionary { ["flow"] = "source", ["currency"] = "gold", ["amount"] = 10f })); + Assert.DoesNotThrow(() => ImmutableAudience.Track("achievement_unlocked", + new Dictionary { ["achievement_id"] = "a1", ["achievement_name"] = "First Steps" })); + } + + [Test] + public void Track_StringOverload_UnrecognisedEventName_DoesNotValidate() + { + ImmutableAudience.Init(MakeConfig()); + + Assert.DoesNotThrow(() => ImmutableAudience.Track("some_custom_event")); + } + [Test] public void Identify_NullOrEmptyUserId_Throws() { @@ -674,7 +744,7 @@ public void Track_TypedPurchase_WritesCorrectEventName() ImmutableAudience.Track(new Purchase { Currency = "USD", - Value = 9.99m + Value = "9.99" }); ImmutableAudience.Shutdown(); @@ -958,7 +1028,7 @@ public void Track_IEvent_ActiveSession_EnvelopeIncludesSessionId() var sessionId = ImmutableAudience.SessionId; Assert.IsFalse(string.IsNullOrEmpty(sessionId), "sanity: a session should be active after Init"); - ImmutableAudience.Track(new Purchase { Currency = "USD", Value = 9.99m }); + ImmutableAudience.Track(new Purchase { Currency = "USD", Value = "9.99" }); ImmutableAudience.FlushQueueToDiskForTesting(); var queueDir = AudiencePaths.QueueDir(_testDir);