Skip to content

Commit 452b473

Browse files
nattb8claude
andcommitted
feat(audience): mirror server-side event validation client-side
Brings client-side validation to parity with the ingest API so bad events are caught and dropped locally instead of round-tripping to the server just to be rejected. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 47b0db1 commit 452b473

8 files changed

Lines changed: 115 additions & 10 deletions

File tree

examples/audience/Assets/SampleApp/Scripts/AudienceSample.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -237,10 +237,10 @@ private void OnAlias() => RunAndLog("alias()", () =>
237237
{
238238
var f = CaptureAliasForm();
239239
ImmutableAudience.Alias(f.FromId, ParseIdentityType(f.FromType), f.ToId, ParseIdentityType(f.ToType));
240-
// SDK drops via Log.Warn when fromId/toId is empty or consent < Full.
241-
// The IsAliasReady gate keeps empty endpoints unreachable from the
242-
// UI; this post-call check is defense-in-depth.
243-
var accepted = !string.IsNullOrEmpty(f.FromId) && !string.IsNullOrEmpty(f.ToId);
240+
// SDK drops via Log.Warn when fromId/toId is empty, identical, or
241+
// consent < Full. The IsAliasReady gate keeps empty endpoints
242+
// unreachable from the UI; this post-call check is defense-in-depth.
243+
var accepted = !string.IsNullOrEmpty(f.FromId) && !string.IsNullOrEmpty(f.ToId) && f.FromId != f.ToId;
244244
if (accepted)
245245
{
246246
_mirrorAliases.Add($"{f.FromType}:{f.FromId}{f.ToType}:{f.ToId}");

examples/audience/Assets/SampleApp/Tests/Runtime/SampleAppLiveFireTests.cs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,22 @@ public IEnumerator Alias_AndFlush_FlushReportsOk()
280280
yield return FlushAndAssertNoErrors();
281281
}
282282

283+
[UnityTest]
284+
public IEnumerator Alias_SameId_WarnsAndDropsCall()
285+
{
286+
// Type-independence (same id, different identityType is still
287+
// dropped) is covered by the fast unit test
288+
// Alias_IdenticalIdsDifferentTypes_StillWarnsAndDropsCall; this
289+
// only needs to prove the UI path wires up to that same check.
290+
yield return LoadAndInit(initialConsent: SampleAppUi.Consent.Full);
291+
292+
var sameId = "email|same-" + DateTime.UtcNow.Ticks;
293+
_root!.Q<TextField>(SampleAppUi.IdentityFields.AliasFromId).value = sameId;
294+
_root.Q<TextField>(SampleAppUi.IdentityFields.AliasToId).value = sameId;
295+
_root.Q<Button>(SampleAppUi.Buttons.Alias).Click();
296+
yield return SampleAppTestHelpers.WaitForLogEntry(_root, SampleAppUi.LogLabels.Sdk, LogLevels.Warn, 5f);
297+
}
298+
283299
[UnityTest]
284300
public IEnumerator SetConsent_None_KeepsQueueAndPersists()
285301
{

src/Packages/Audience/Runtime/Core/Constants.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ internal static class Constants
1414
internal const int DefaultFlushSize = 20;
1515
internal const int MaxBatchSize = 100;
1616
internal const int StaleEventDays = 30;
17+
internal const int MaxClockSkewFutureHours = 24; // Backend rejects eventTimestamp further ahead than this.
1718
internal const int MaxFieldLength = 256; // Backend schema limit.
1819
internal const int ControlPlaneRequestTimeoutSeconds = 30;
1920

src/Packages/Audience/Runtime/ImmutableAudience.cs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,8 @@ private static bool IsValidPassportId(string id)
482482
}
483483

484484
/// <summary>
485-
/// Links two user IDs for the same player. When either side's identity
485+
/// Links two user IDs for the same player. <paramref name="fromId"/> and
486+
/// <paramref name="toId"/> must differ. When either side's identity
486487
/// type is <see cref="IdentityType.Passport"/>, that side's id must look
487488
/// like a real Passport ID (<c>connection|id</c> or a UUID) — otherwise
488489
/// the call is dropped and a warning is logged.
@@ -505,6 +506,12 @@ public static void Alias(string fromId, IdentityType fromType, string toId, Iden
505506
fromId = fromId.Trim();
506507
toId = toId.Trim();
507508

509+
if (fromId == toId)
510+
{
511+
Log.Warn(AudienceLogs.AliasIdenticalIds);
512+
return;
513+
}
514+
508515
if (fromType == IdentityType.Passport && !IsValidPassportId(fromId))
509516
{
510517
Log.Warn(AudienceLogs.AliasPassportIdInvalidFormat("from", fromId));

src/Packages/Audience/Runtime/Transport/DiskStore.cs

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,16 +39,21 @@ internal void Write(string json)
3939
}
4040
}
4141

42-
// Returns up to maxSize file paths, oldest first. Stale files
43-
// (older than Constants.StaleEventDays) are deleted and excluded.
42+
// Returns up to maxSize file paths, oldest first. Files outside the
43+
// backend's accepted eventTimestamp window (more than StaleEventDays
44+
// in the past, or more than MaxClockSkewFutureHours in the future --
45+
// e.g. from a device with a badly-skewed system clock) are deleted
46+
// and excluded: the backend would reject them anyway.
4447
internal IReadOnlyList<string> ReadBatch(int maxSize)
4548
{
4649
if (maxSize <= 0)
4750
return Array.Empty<string>();
4851

4952
maxSize = Math.Min(maxSize, Constants.MaxBatchSize);
5053

51-
var cutoff = DateTime.UtcNow.AddDays(-Constants.StaleEventDays);
54+
var now = DateTime.UtcNow;
55+
var pastCutoff = now.AddDays(-Constants.StaleEventDays);
56+
var futureCutoff = now.AddHours(Constants.MaxClockSkewFutureHours);
5257

5358
var result = new List<string>();
5459

@@ -65,13 +70,13 @@ internal IReadOnlyList<string> ReadBatch(int maxSize)
6570
if (result.Count >= maxSize)
6671
break;
6772

68-
// Stale check: parse ticks from filename prefix
73+
// Window check: parse ticks from filename prefix
6974
var name = Path.GetFileNameWithoutExtension(path);
7075
var underscoreIdx = name.IndexOf('_');
7176
if (underscoreIdx > 0 && long.TryParse(name.Substring(0, underscoreIdx), out var ticks))
7277
{
7378
var fileTime = new DateTime(ticks, DateTimeKind.Utc);
74-
if (fileTime < cutoff)
79+
if (fileTime < pastCutoff || fileTime > futureCutoff)
7580
{
7681
TryDelete(path);
7782
continue;

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,9 @@ internal static string TrackIEventEmptyName(string evtTypeName) =>
7373
internal const string AliasEmptyIds =
7474
"Alias called with null or empty fromId/toId. Dropping.";
7575

76+
internal const string AliasIdenticalIds =
77+
"Alias called with identical fromId and toId. Dropping.";
78+
7679
internal static string IdentifyDiscarded(ConsentLevel current) =>
7780
$"Identify discarded. Requires Full consent, current is {current}.";
7881

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

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -933,6 +933,63 @@ public void Alias_PassportFromWithInvalidIdFormat_WarnsAndDropsCall()
933933
}
934934
}
935935

936+
[Test]
937+
public void Alias_IdenticalIds_WarnsAndDropsCall()
938+
{
939+
var lines = new List<string>();
940+
Log.Writer = lines.Add;
941+
try
942+
{
943+
ImmutableAudience.Init(MakeConfig(ConsentLevel.Full));
944+
945+
ImmutableAudience.Alias("same_id", IdentityType.Steam, "same_id", IdentityType.Steam);
946+
ImmutableAudience.Shutdown();
947+
948+
Assert.That(lines, Has.Some.Contains("identical fromId and toId"),
949+
"identical ids should surface a warning");
950+
951+
var queueDir = AudiencePaths.QueueDir(_testDir);
952+
var contents = Directory.GetFiles(queueDir, "*.json")
953+
.Select(File.ReadAllText).ToList();
954+
Assert.IsFalse(contents.Any(c => c.Contains("\"alias\"")),
955+
"the event should be dropped, not just flagged");
956+
}
957+
finally
958+
{
959+
Log.Writer = null;
960+
}
961+
}
962+
963+
[Test]
964+
public void Alias_IdenticalIdsDifferentTypes_StillWarnsAndDropsCall()
965+
{
966+
// Matches the backend: it rejects on id equality alone, ignoring
967+
// identityType, so the client must reject this case too instead
968+
// of sending something the backend will bounce.
969+
var lines = new List<string>();
970+
Log.Writer = lines.Add;
971+
try
972+
{
973+
ImmutableAudience.Init(MakeConfig(ConsentLevel.Full));
974+
975+
ImmutableAudience.Alias("same_id", IdentityType.Steam, "same_id", IdentityType.Email);
976+
ImmutableAudience.Shutdown();
977+
978+
Assert.That(lines, Has.Some.Contains("identical fromId and toId"),
979+
"identical ids should surface a warning even with different identityTypes");
980+
981+
var queueDir = AudiencePaths.QueueDir(_testDir);
982+
var contents = Directory.GetFiles(queueDir, "*.json")
983+
.Select(File.ReadAllText).ToList();
984+
Assert.IsFalse(contents.Any(c => c.Contains("\"alias\"")),
985+
"the event should be dropped, not just flagged");
986+
}
987+
finally
988+
{
989+
Log.Writer = null;
990+
}
991+
}
992+
936993
[Test]
937994
public void Alias_NonPassportIdentityTypes_DoesNotWarn()
938995
{

src/Packages/Audience/Tests/Runtime/Transport/DiskStoreTests.cs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,22 @@ public void ReadBatch_ExcludesAndDeletesStaleFiles()
106106
Assert.IsFalse(File.Exists(Path.Combine(queueDir, staleName)), "stale file should be deleted");
107107
}
108108

109+
[Test]
110+
public void ReadBatch_ExcludesAndDeletesFutureSkewedFiles()
111+
{
112+
_store.Write("{\"fresh\":true}");
113+
114+
var skewedTime = DateTime.UtcNow.AddHours(Constants.MaxClockSkewFutureHours + 1);
115+
var skewedName = $"{skewedTime.Ticks}_{Guid.NewGuid():N}.json";
116+
var queueDir = AudiencePaths.QueueDir(_testDir);
117+
File.WriteAllText(Path.Combine(queueDir, skewedName), "{\"skewed\":true}");
118+
119+
var batch = _store.ReadBatch(10);
120+
121+
Assert.AreEqual(1, batch.Count, "future-skewed file should be excluded from batch");
122+
Assert.IsFalse(File.Exists(Path.Combine(queueDir, skewedName)), "future-skewed file should be deleted");
123+
}
124+
109125
[Test]
110126
public void Delete_RemovesSpecifiedFiles()
111127
{

0 commit comments

Comments
 (0)