refactor(audience): make identityType mandatory on Identify and Alias (SDK-222)#696
Merged
Merged
Conversation
…erload Reverts 9b00cce, which added a traits-only Identify overload that mirrored the @imtbl/audience Web SDK's identify(traits) shape. The overload emits identify events with no identityType and cannot carry one meaningfully (no userId to attach one to). CDP requires identityType on every identify event to match records during data deletion, so records produced by this path would be unreachable for erasure. Mechanical prerequisite for SDK-222: making identityType mandatory on MessageBuilder.Identify is a compile error while this overload exists. Web SDK parity will be reopened under a separate ticket once the Web SDK drops its equivalent for the same CDP reason. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CDP requires identityType on every identify / alias event so it can match records to the correct identity namespace during data deletion. Enforce the invariant through the type system end to end: - `Identify(userId, identityType, traits?)` — identityType is non-nullable. The null/empty warn-and-drop block is gone; the type signature no longer lies about optionality. - `Alias(fromId, fromType, toId, toType)` — fromType and toType are non-nullable. Same block removed. - `MessageBuilder.Identify` — identityType parameter is non-nullable and always emitted; the conditional that omitted an empty field is gone so the wire shape cannot drift. - `IdentityType.ToLowercaseString()` — casting to an out-of-range enum value now throws `ArgumentOutOfRangeException` rather than returning null. A silent null used to leak through to a dropped event; now the programmer error fails loud at the call site. Tests repointed: invalid-enum-cast cases assert the throw, and MessageBuilder tests that passed `null` for identityType now pass a valid value since the parameter is no longer nullable. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
nattb8
reviewed
Apr 22, 2026
nattb8
reviewed
Apr 22, 2026
ImmutableJeffrey
added a commit
that referenced
this pull request
Apr 22, 2026
Rewrites the comments on IdentityType.ToLowercaseString, the Identify(string) and Alias(string) overloads in ImmutableAudience, and the matching test comments and assertion messages so they explain why identityType is mandatory — downstream data-deletion needs it to match a user's events — without naming the specific internal pipeline. Same invariant, just no internal name in the source. The test comment on ToLowercaseString_UnknownValue_Throws is also rewritten to read cold: leads with the rule and a concrete bad-cast example, names what ToLowercaseString emits and how the backend uses it, then explains why a default return would hide the bug. Addresses PR #696 review: #696 (comment) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Rewrites the comments on IdentityType.ToLowercaseString, the Identify(string) and Alias(string) overloads in ImmutableAudience, and the matching test comments and assertion messages so they explain why identityType is mandatory — downstream data-deletion needs it to match a user's events — without naming the specific internal pipeline. Same invariant, just no internal name in the source. The test comment on ToLowercaseString_UnknownValue_Throws is also rewritten to read cold: leads with the rule and a concrete bad-cast example, names what ToLowercaseString emits and how the backend uses it, then explains why a default return would hide the bug. Addresses PR #696 review thread (discussion_r3122316478). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ImmutableJeffrey
force-pushed
the
feat/sdk-222-identitytype-mandatory
branch
from
6606636 to
d8c7f48
Compare
nattb8
approved these changes
Apr 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
identityType/fromType/toTypeto non-nullable on theIdentifyandAliasstring overloads, removes the runtime null/empty warn-and-drop blocks so the type signature stops lying about optionality.MessageBuilder.Identify'sidentityTypeparameter non-nullable and always emits the field — the conditional that omitted an empty field is gone so the wire shape cannot drift.IdentityType.ToLowercaseString()to throwArgumentOutOfRangeExceptionon out-of-range enum casts rather than returning null. A silent null used to leak through to a dropped event; now the programmer error fails loud at the call site.Identify(Dictionary<string, object> traits)overload added in feat(audience): ImmutableAudience singleton + GDPR (SDK-147) #694 — it emits identify events with no identityType and cannot carry one meaningfully. Required precondition: the revert and the mandate are a single compile unit.MessageBuildertests that passednullfor identityType now pass a valid value. Test count 161 → 155 (the sixIdentifyTraits_*tests are gone with the overload).Rationale
CDP requires identityType on every identify / alias event to match records to the correct identity namespace when processing data-deletion requests. The signatures merged in #694 accepted
string? identityType/string? fromType/string? toTypeand silently dropped null-or-empty inputs, leaving the contract implicit and letting events ship without an identity namespace CDP can use.Raised in PR #694 review thread
Linear: SDK-222
Parent: SDK-99 Unity SDK v1