Merge pull request #823 from immutable/feat/sdk-687-reserved-event-validation

feat(audience): validate required properties for reserved events at runtime

Author: nattb8

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

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" });
     }
 }
 ```

src/Packages/Audience/Runtime/Events/IEvent.cs

@@ -10,10 +10,10 @@ namespace Immutable.Audience
     /// <see cref="ImmutableAudience.Track(IEvent)"/>.
     /// </summary>
     /// <remarks>
-    /// Implementations validate required fields inside
-    /// <see cref="ToProperties"/>;
-    /// <see cref="ImmutableAudience.Track(IEvent)"/> catches the throw and
-    /// drops the event with a warning.
+    /// Implementations validate required fields inside <see cref="ToProperties"/>.
+    /// <see cref="ImmutableAudience.Track(IEvent)"/> catches the throw and warns
+    /// for a consumer-authored <see cref="IEvent"/>; see <see cref="IBuiltInEvent"/>
+    /// for the exception to that.
     /// </remarks>
     public interface IEvent
     {
@@ -32,4 +32,17 @@ public interface IEvent
         /// </exception>
         Dictionary<string, object> ToProperties();
     }
+
+    /// <summary>
+    /// Marks an <see cref="IEvent"/> implementation as one of Immutable's own
+    /// built-in event types (<see cref="Purchase"/>, <see cref="Progression"/>,
+    /// <see cref="Resource"/>, <see cref="AchievementUnlocked"/>,
+    /// <see cref="MilestoneReached"/>). Not implementable outside this
+    /// assembly: it exists only so <see cref="ImmutableAudience.Track(IEvent)"/>
+    /// can tell a validation failure in one of these apart from an exception
+    /// thrown by a consumer's own custom <see cref="IEvent"/>.
+    /// </summary>
+    internal interface IBuiltInEvent : IEvent
+    {
+    }
 }

src/Packages/Audience/Runtime/Events/ReservedEvents.cs

@@ -0,0 +1,27 @@
+#nullable enable
+
+using System.Collections.Generic;
+
+namespace Immutable.Audience
+{
+    /// <summary>
+    /// Required property names per reserved event, enforced at runtime by
+    /// <see cref="ImmutableAudience.Track(string, Dictionary{string, object})"/>.
+    /// The typed <see cref="IBuiltInEvent"/> classes enforce the same rules
+    /// at compile time (constructor/property level) and again in
+    /// <c>ToProperties</c>; this closes the gap for callers who use the
+    /// string overload instead and so bypass all of that.
+    /// </summary>
+    internal static class ReservedEvents
+    {
+        internal static readonly IReadOnlyDictionary<string, string[]> RequiredProperties =
+            new Dictionary<string, string[]>
+            {
+                ["purchase"] = new[] { "currency", "value" },
+                ["progression"] = new[] { "status" },
+                ["resource"] = new[] { "flow", "currency", "amount" },
+                ["achievement_unlocked"] = new[] { "achievement_id", "achievement_name" },
+                ["milestone_reached"] = new[] { "name" },
+            };
+    }
+}

src/Packages/Audience/Runtime/Events/ReservedEvents.cs.meta

@@ -44,7 +44,7 @@ internal static class ProgressionStatusExtensions
     /// Player progressing through a world / level / stage. Track via
     /// <see cref="ImmutableAudience.Track(IEvent)"/>.
     /// </summary>
-    public class Progression : IEvent
+    public class Progression : IBuiltInEvent
     {
         /// <summary>
         /// 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
     /// <see cref="ImmutableAudience.Track(IEvent)"/>.
     /// </summary>
-    public class Resource : IEvent
+    public class Resource : IBuiltInEvent
     {
         /// <summary>
         /// Required. Whether this is a gain or a spend.
@@ -193,7 +193,7 @@ public Dictionary<string, object> ToProperties()
     /// Real-money transaction. Track via
     /// <see cref="ImmutableAudience.Track(IEvent)"/>.
     /// </summary>
-    public class Purchase : IEvent
+    public class Purchase : IBuiltInEvent
     {
         /// <summary>
         /// Required. ISO 4217 three-letter uppercase currency code (for
@@ -202,9 +202,11 @@ public class Purchase : IEvent
         public string? Currency { get; set; }
 
         /// <summary>
-        /// Required. The transaction amount in <see cref="Currency"/>.
+        /// Required. The transaction amount in <see cref="Currency"/>, as a
+        /// numeric-looking string (e.g. <c>"9.99"</c>), not a number, to
+        /// avoid floating-point precision loss.
         /// </summary>
-        public decimal? Value { get; set; }
+        public string? Value { get; set; }
 
         /// <summary>
         /// Optional. Stable identifier of the item purchased.
@@ -247,13 +249,13 @@ public Dictionary<string, object> 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<string, object>
             {
                 ["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
     /// <see cref="ImmutableAudience.Track(IEvent)"/>.
     /// </summary>
-    public class AchievementUnlocked : IEvent
+    public class AchievementUnlocked : IBuiltInEvent
     {
         /// <summary>
         /// Required. Stable identifier for the achievement (for example,
@@ -362,7 +364,7 @@ public Dictionary<string, object> ToProperties()
     /// Named milestone or achievement reached by the player. Track via
     /// <see cref="ImmutableAudience.Track(IEvent)"/>.
     /// </summary>
-    public class MilestoneReached : IEvent
+    public class MilestoneReached : IBuiltInEvent
     {
         /// <summary>
         /// Required. The milestone identifier (for example,

src/Packages/Audience/Runtime/Events/TypedEvents.cs

@@ -328,6 +328,10 @@ internal static void OnResume()
         /// </summary>
         /// <param name="evt">The event to send. Must not be null, and <see cref="IEvent.EventName"/>
         /// must not be empty (both throw).</param>
+        /// <exception cref="ArgumentException">
+        /// One of Immutable's own built-in events (<see cref="Purchase"/>,
+        /// <see cref="Progression"/>, etc.) is missing a required field.
+        /// </exception>
         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<string, object> 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)
         /// </summary>
         /// <param name="eventName">The wire-format event name. Must not be empty (throws otherwise).</param>
         /// <param name="properties">Optional event properties.</param>
+        /// <exception cref="ArgumentException">
+        /// <paramref name="eventName"/> is empty, or is a reserved event
+        /// (<c>purchase</c>, <c>progression</c>, <c>resource</c>,
+        /// <c>achievement_unlocked</c>) missing one of its required properties.
+        /// </exception>
         public static void Track(string eventName, Dictionary<string, object>? 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<string, object>? 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<string, object>? properties)
+        {
+            if (!ReservedEvents.RequiredProperties.TryGetValue(eventName, out var required)) return;
+
+            var missing = new List<string>();
+            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,

src/Packages/Audience/Runtime/ImmutableAudience.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<string> 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/

src/Packages/Audience/Runtime/Utility/Log.cs

@@ -1,11 +1,30 @@
 using System;
+using System.Linq;
 using NUnit.Framework;
 
 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<ArgumentException>(() => evt.ToProperties());
             Assert.That(ex!.Message, Does.Contain("Currency"));