Allow the getIdentity command to read ECID from the kndctr cookie #1230
Conversation
| namespaces[ecidNamespace] = ecidFromCookie; | ||
| return undefined; | ||
| } | ||
| return getIdentity(options); | ||
| }) | ||
| .then(() => { | ||
| return { |
There was a problem hiding this comment.
If we get the ecid from cookie, edge is going to be null. Will this be a problem for some customers? If I run getIdentity in the sandbox, I get: {identity: {ECID: "66399393517766757241886600785428160922"}, edge: {regionId: 9}}. Can we pull out the region from the cookie too?
There was a problem hiding this comment.
Did you find the answer for Jon's question?
There was a problem hiding this comment.
We didn't identify any use cases. We will proceed without it and make some changes if something comes up.
carterworks
force-pushed
the
pdcl-12708-read-ecid-from-cookie
branch
from
a4f7d37 to
3f0e28c
Compare
|
I have written some tests to make sure that the failover to network request is solid—if there is no cookie, or the cookie isn't valid base64, or if the cookie isn't a valid protobuf, then we will log a console warning and fall back to network request. |
| @@ -82,7 +94,18 @@ export default ({ | |||
| return consent | |||
| .withConsent() | |||
| .then(() => { | |||
| return namespaces ? undefined : getIdentity(options); | |||
| if (namespaces) { | |||
| return undefined; | |||
There was a problem hiding this comment.
This section seems identical with the section from line 67. Can this be extracted as a method?
@jonsnyder There are functional tests: https://github.com/adobe/alloy/pull/1230/files#diff-0f6f78ea4f3f2771d460497bf4f4b48278e2ea0c597d729de741fcdab4dc6748 for
Do you think we should have any other tests? |
|
Those tests are good I just didn't see them. |
Description
Add a step to the
getIdentity(andappendIdentityToUrl) command that will read thekndctrcookie.For review, the kndctr cookie is a url-safe base64 encoded protobuf. So decoding it involves reading the protobuf bytes. #1171 uses protobuf.js but to significantly increased bundle size.
The current
mainbuilds at 41.024 KB minzipped and with protobuf.js it builds to 56.493 KB minzipped (a 37.7% increase in size). The protobufjs library is not tree shakable, so this is as small as it could be.With that in mind, this PR (unlike #1171) hand-implements the decoding of the protobuf. Since we only care about the ECID, we can skip decoding all the other fields, which simplifies the process significantly. Since the Konductor API also returns the region id and the information is store in the cookie, we could, in the future or in this PR, also decode that value for a little extra effort. Full details for decoding it can be found in the code and I tried to make the process as clear as possible, with many comments.
The hand-implemented version included in this PR weighs in at 41.521 KB minzipped (a 1.2% increase in size).
Now the flow is
getIdentityorappendIdentityToUrlECIDnamespace was requested (which is it, by default)Related Issue
PDCL-12708: Check for ECID stored in cookie prior to sending request to Edge to obtain ECID
Motivation and Context
Screenshots (if appropriate):
Types of changes
Checklist: