Skip to content

Commit d142092

Browse files
feat(audience): enforce required properties on Track(IEvent) predefined events
Attribution and conversion pipelines depend on predefined events carrying their required fields. The C# compiler enforces field types but does not enforce that required fields are set to a non-default value, so a caller like `Track(new Purchase { Currency = "USD" })` was silently shipping a zero-value purchase and breaking downstream reporting. Make each required value-typed field nullable on the typed IEvent implementations so an unset caller is distinguishable from a zero / default value, and throw a clear `ArgumentException` from `ToProperties()` when a required field is null. The existing try/catch inside `ImmutableAudience.Track(IEvent)` catches the throw, logs a warning naming the event and the missing field, and drops the event — consistent with the never-crash convention on the game thread. - Progression.Status: ProgressionStatus -> ProgressionStatus? - Resource.Flow: ResourceFlow -> ResourceFlow? - Resource.Amount: float -> float? - Purchase.Value: decimal -> decimal? Track(string) is explicitly out of scope for runtime validation — callers using the string overload opt out of the typed path. Tighten the existing doc comment with a concrete example (purchase without currency / value) so the gap is unambiguous. Tests: each predefined event gets a missing-required-field case asserting the throw; an integration test on Track(IEvent) confirms the throw is caught, logged, and the event dropped rather than shipping a malformed payload. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent d08fe12 commit d142092

4 files changed

Lines changed: 114 additions & 17 deletions

File tree

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

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,9 @@ internal static class ProgressionStatusExtensions
3030
// Player progressing through a world / level / stage.
3131
public class Progression : IEvent
3232
{
33-
// Required.
34-
public ProgressionStatus Status { get; set; }
33+
// Required. Nullable so an unset caller produces a clear validation
34+
// error at send time instead of silently shipping the enum default.
35+
public ProgressionStatus? Status { get; set; }
3536
// Optional.
3637
public string? World { get; set; }
3738
public string? Level { get; set; }
@@ -43,9 +44,12 @@ public class Progression : IEvent
4344

4445
public Dictionary<string, object> ToProperties()
4546
{
47+
if (Status is null)
48+
throw new ArgumentException("Progression.Status is required — set it before calling Track(IEvent)");
49+
4650
var props = new Dictionary<string, object>
4751
{
48-
["status"] = Status.ToLowercaseString()
52+
["status"] = Status.Value.ToLowercaseString()
4953
};
5054

5155
if (World != null) props["world"] = World;
@@ -81,10 +85,12 @@ internal static class ResourceFlowExtensions
8185
// In-game currency earned or spent.
8286
public class Resource : IEvent
8387
{
84-
// Required.
85-
public ResourceFlow Flow { get; set; }
88+
// Required. Nullable so an unset caller produces a clear validation
89+
// error at send time instead of silently shipping the enum / zero
90+
// default.
91+
public ResourceFlow? Flow { get; set; }
8692
public string? Currency { get; set; }
87-
public float Amount { get; set; }
93+
public float? Amount { get; set; }
8894
// Optional.
8995
public string? ItemType { get; set; }
9096
public string? ItemId { get; set; }
@@ -93,14 +99,18 @@ public class Resource : IEvent
9399

94100
public Dictionary<string, object> ToProperties()
95101
{
102+
if (Flow is null)
103+
throw new ArgumentException("Resource.Flow is required — set it before calling Track(IEvent)");
96104
if (string.IsNullOrEmpty(Currency))
97-
throw new ArgumentException("Resource.Currency must not be null or empty");
105+
throw new ArgumentException("Resource.Currency is required — set a non-empty string before calling Track(IEvent)");
106+
if (Amount is null)
107+
throw new ArgumentException("Resource.Amount is required — set it before calling Track(IEvent)");
98108

99109
var props = new Dictionary<string, object>
100110
{
101-
["flow"] = Flow.ToLowercaseString(),
111+
["flow"] = Flow.Value.ToLowercaseString(),
102112
["currency"] = Currency,
103-
["amount"] = Amount
113+
["amount"] = Amount.Value
104114
};
105115

106116
if (ItemType != null) props["itemType"] = ItemType;
@@ -115,8 +125,10 @@ public class Purchase : IEvent
115125
{
116126
// Required. ISO 4217 three-letter uppercase currency code.
117127
public string? Currency { get; set; }
118-
// Required.
119-
public decimal Value { get; set; }
128+
// Required. Nullable so an unset caller produces a clear validation
129+
// error at send time instead of silently shipping a zero-value
130+
// purchase that breaks attribution and conversion reporting.
131+
public decimal? Value { get; set; }
120132
// Optional.
121133
public string? ItemId { get; set; }
122134
public string? ItemName { get; set; }
@@ -142,11 +154,13 @@ public Dictionary<string, object> ToProperties()
142154
if (Currency == null || !IsIso4217(Currency))
143155
throw new ArgumentException(
144156
$"Purchase.Currency '{Currency}' must be a three-letter uppercase ISO 4217 code");
157+
if (Value is null)
158+
throw new ArgumentException("Purchase.Value is required — set it before calling Track(IEvent)");
145159

146160
var props = new Dictionary<string, object>
147161
{
148162
["currency"] = Currency,
149-
["value"] = Value
163+
["value"] = Value.Value
150164
};
151165

152166
if (ItemId != null) props["itemId"] = ItemId;

src/Packages/Audience/Runtime/ImmutableAudience.cs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -142,11 +142,14 @@ public static void Track(IEvent evt)
142142

143143
// Send a custom event.
144144
//
145-
// For predefined event names (e.g. purchase), prefer the typed
146-
// overload Track(new Purchase { ... }) — it enforces required fields
147-
// and value types at compile time. This overload does not validate
148-
// property shapes, so missing or mistyped fields can break
149-
// attribution/conversion reporting.
145+
// For predefined event names (e.g. purchase, progression, resource,
146+
// milestone_reached), prefer the typed overload —
147+
// Track(new Purchase { Currency = "USD", Value = 9.99m }) — which
148+
// validates required fields at send time. This overload accepts any
149+
// property shape and does not: Track("purchase", new Dictionary...)
150+
// that omits currency or value still enqueues and ships, but breaks
151+
// attribution and conversion reporting downstream because the
152+
// payload is missing the fields CDP needs to reconstruct the event.
150153
public static void Track(string eventName, Dictionary<string, object>? properties = null)
151154
{
152155
if (!CanTrack()) return;

src/Packages/Audience/Tests/Runtime/Events/TypedEventTests.cs

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System;
12
using NUnit.Framework;
23

34
namespace Immutable.Audience.Tests
@@ -11,6 +12,15 @@ public void Progression_EventName_IsProgression()
1112
Assert.AreEqual("progression", new Progression().EventName);
1213
}
1314

15+
[Test]
16+
public void Progression_WithoutStatus_ThrowsOnToProperties()
17+
{
18+
var evt = new Progression { World = "tutorial" };
19+
20+
var ex = Assert.Throws<ArgumentException>(() => evt.ToProperties());
21+
Assert.That(ex!.Message, Does.Contain("Status"));
22+
}
23+
1424
[Test]
1525
public void Progression_Complete_ProducesCorrectProperties()
1626
{
@@ -72,6 +82,33 @@ public void Resource_EventName_IsResource()
7282
Assert.AreEqual("resource", new Resource().EventName);
7383
}
7484

85+
[Test]
86+
public void Resource_WithoutFlow_ThrowsOnToProperties()
87+
{
88+
var evt = new Resource { Currency = "gold", Amount = 100 };
89+
90+
var ex = Assert.Throws<ArgumentException>(() => evt.ToProperties());
91+
Assert.That(ex!.Message, Does.Contain("Flow"));
92+
}
93+
94+
[Test]
95+
public void Resource_WithoutCurrency_ThrowsOnToProperties()
96+
{
97+
var evt = new Resource { Flow = ResourceFlow.Source, Amount = 100 };
98+
99+
var ex = Assert.Throws<ArgumentException>(() => evt.ToProperties());
100+
Assert.That(ex!.Message, Does.Contain("Currency"));
101+
}
102+
103+
[Test]
104+
public void Resource_WithoutAmount_ThrowsOnToProperties()
105+
{
106+
var evt = new Resource { Flow = ResourceFlow.Source, Currency = "gold" };
107+
108+
var ex = Assert.Throws<ArgumentException>(() => evt.ToProperties());
109+
Assert.That(ex!.Message, Does.Contain("Amount"));
110+
}
111+
75112
[Test]
76113
public void Purchase_ProducesCorrectProperties()
77114
{
@@ -114,6 +151,24 @@ public void Purchase_EventName_IsPurchase()
114151
Assert.AreEqual("purchase", new Purchase().EventName);
115152
}
116153

154+
[Test]
155+
public void Purchase_WithoutCurrency_ThrowsOnToProperties()
156+
{
157+
var evt = new Purchase { Value = 9.99m };
158+
159+
var ex = Assert.Throws<ArgumentException>(() => evt.ToProperties());
160+
Assert.That(ex!.Message, Does.Contain("Currency"));
161+
}
162+
163+
[Test]
164+
public void Purchase_WithoutValue_ThrowsOnToProperties()
165+
{
166+
var evt = new Purchase { Currency = "USD" };
167+
168+
var ex = Assert.Throws<ArgumentException>(() => evt.ToProperties());
169+
Assert.That(ex!.Message, Does.Contain("Value"));
170+
}
171+
117172
[Test]
118173
public void MilestoneReached_ProducesCorrectProperties()
119174
{

src/Packages/Audience/Tests/Runtime/ImmutableAudienceTests.cs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,31 @@ public void Track_NullEvent_DoesNotThrow_AndLogsWarning()
106106
finally { Log.Writer = null; }
107107
}
108108

109+
[Test]
110+
public void Track_IEventMissingRequiredField_DropsWithWarn()
111+
{
112+
ImmutableAudience.Init(MakeConfig());
113+
114+
var lines = new List<string>();
115+
Log.Writer = lines.Add;
116+
try
117+
{
118+
// Purchase with no Value set — ToProperties throws; Track must
119+
// catch, warn, and drop rather than ship an incomplete event.
120+
Assert.DoesNotThrow(() => ImmutableAudience.Track(new Purchase { Currency = "USD" }));
121+
Assert.That(lines, Has.Some.Contains("Purchase"));
122+
Assert.That(lines, Has.Some.Contains("Dropping"));
123+
}
124+
finally { Log.Writer = null; }
125+
126+
ImmutableAudience.Shutdown();
127+
var queueDir = AudiencePaths.QueueDir(_testDir);
128+
var contents = Directory.GetFiles(queueDir, "*.json")
129+
.Select(File.ReadAllText).ToList();
130+
Assert.IsFalse(contents.Any(c => c.Contains("\"purchase\"")),
131+
"purchase event with missing required Value must be dropped, not enqueued");
132+
}
133+
109134
[Test]
110135
public void Track_NullOrEmptyEventName_DoesNotEnqueue()
111136
{

0 commit comments

Comments
 (0)