Resolver Specific Parameters (ID5 Mobile In-App)#58
Resolver Specific Parameters (ID5 Mobile In-App)#58OlenaPostindustria wants to merge 14 commits into
Conversation
eugenedorfman-optable
left a comment
eugenedorfman-optable
left a comment
There was a problem hiding this comment.
Review of the ID5 Mobile In-App changes. Overall a clean, well-tested feature, and the hid= wire encoding is correct: each hint is emitted as its own repeated hid= query item (never comma-joined), which is what the edge expects since it does not split hid on commas.
Inline comments below, tagged major / minor / nit. Two cross-cutting points that are not line-specific:
- README not updated. There is no
targetingcode sample in the README, so the newhidsparameter and the whole ID5 Mobile In-App feature are undocumented for integrators. A short README/demo snippet showing how to pass hints would help. - ObjC + Swift docs. See the inline note on the doc comment: it should actually explain what a hint ID is and which identifier types are valid.
Most important items to resolve before merge are the three major comments (ObjC API break, IDFA casing, and the implicit opt-in for all existing callers).
| var hids = hids ?? [] | ||
|
|
||
| enrichIfNeeded(ids: &ids) | ||
| enrichIfNeeded(ids: &hids) |
There was a problem hiding this comment.
Major - all existing callers are silently opted into hint/ID5 resolution. enrichIfNeeded(ids: &hids) runs even when the caller passed hids: nil, and in EdgeAPI.targeting the bundle/ver/ua/id5_signature params are appended unconditionally. So every current targeting() call will start sending hid=a:<idfa> plus the resolver params, with no opt-out other than skipAdvertisingIdDetection (which also disables the id= enrichment).
If that implicit opt-in is intended edge behavior, fine - otherwise gate hid emission (and arguably the resolver params) on the caller actually supplying hids.
There was a problem hiding this comment.
hid=a:<idfa> contains the same IDFA we already send as id=a:<idfa>, and it's still subject to the same ATT and skipAdvertisingIdDetection checks. id5_signature is only sent if the DCN has already returned one. ID5 usage is controlled entirely server-side. if it's enabled, the hid and the related parameters are used, otherwise, it should be ignored. this should let ID5 be enabled through the configuration without requiring an app update
3 participants
Closes #57