-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
new no identity available event #184
Changes from 17 commits
a34660a
8460640
bacfbf6
59856db
f8b39dd
d9b53b0
d549c07
d8cc05d
75b4952
ea6f0e4
1879ddc
f0d0bed
b991a98
7aa9c23
dc38026
2a7f821
50d0cea
3c0a13a
ccd68ae
6fcb0d3
6bea2a5
c16234b
f1d89c5
12ae483
1c4167c
4053cb7
c4061fe
86501e8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ export enum EventType { | |
IdentityUpdated = 'IdentityUpdated', | ||
SdkLoaded = 'SdkLoaded', | ||
OptoutReceived = 'OptoutReceived', | ||
NoIdentityAvailable = 'NoIdentityAvailable', | ||
} | ||
|
||
export type CallbackPayload = SdkLoadedPayload | PayloadWithIdentity; | ||
|
@@ -52,12 +53,17 @@ export class CallbackManager { | |
public runCallbacks(event: EventType, payload: CallbackPayload) { | ||
if (event === EventType.InitCompleted) this._sentInit = true; | ||
if (event === EventType.SdkLoaded) CallbackManager._sentSdkLoaded[this._productName] = true; | ||
if (!this._sentInit && event !== EventType.SdkLoaded) return; | ||
// if SDK has not been loaded yet, callbacks should not run | ||
if (!CallbackManager._sentSdkLoaded[this._productName]) return; | ||
|
||
const enrichedPayload = { | ||
...payload, | ||
identity: this._getIdentity() ?? null, | ||
}; | ||
if (event === EventType.NoIdentityAvailable && enrichedPayload.identity) { | ||
enrichedPayload.identity = null; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added this because it was sending the expired or opted out identity back, but I thought this may be confusing if we are sending a "no identity available" event, but then attaching an identity. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure about this. It seems possible that someone could be expecting an expired identity and then acting on it in some way. this could be a breaking change? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Took this check out. Will add info to the docs to denote the two types of identities that can be returned with this event (null or expired) |
||
|
||
for (const callback of this._sdk.callbacks) { | ||
this.safeRunCallback(callback, event, enrichedPayload); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -84,7 +84,7 @@ export abstract class SdkBase { | |
this._callbackManager = new CallbackManager( | ||
this, | ||
this._product.name, | ||
() => this.getIdentity(), | ||
() => this.getIdentityForCallback(), | ||
this._logger | ||
); | ||
} | ||
|
@@ -159,9 +159,22 @@ export abstract class SdkBase { | |
} | ||
this._callbackManager.runCallbacks(EventType.IdentityUpdated, {}); | ||
} | ||
this.isIdentityAvailable(); | ||
} | ||
|
||
public getIdentity(): Identity | null { | ||
const identity = this._identity ?? this.getIdentityNoInit(); | ||
const isValid = | ||
identity && !this.temporarilyUnavailable(identity) && !isOptoutIdentity(identity); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. an Optout Identity if valid so I don't think we want that in this condition. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This code was here before - and when I remove OptoutIdentity, it does change the return value of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. seems the code always checks for optout separately so it assumes a valid identity will have a particular shape. So this is fine. |
||
if (!isValid) { | ||
this._callbackManager.runCallbacks(EventType.NoIdentityAvailable, {}); | ||
return null; | ||
} else { | ||
return identity; | ||
} | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Callback uses |
||
private getIdentityForCallback(): Identity | null { | ||
const identity = this._identity ?? this.getIdentityNoInit(); | ||
return identity && !this.temporarilyUnavailable(identity) && !isOptoutIdentity(identity) | ||
? identity | ||
|
@@ -181,11 +194,27 @@ export abstract class SdkBase { | |
} | ||
|
||
public isLoginRequired() { | ||
const identity = this._identity ?? this.getIdentityNoInit(); | ||
// if identity temporarily unavailable, login is not required | ||
if (this.temporarilyUnavailable(identity)) { | ||
return false; | ||
} | ||
return !this.isIdentityAvailable(); | ||
} | ||
|
||
public isIdentityAvailable() { | ||
return this.isIdentityValid() || this._apiClient?.hasActiveRequests(); | ||
const identity = this._identity ?? this.getIdentityNoInit(); | ||
const identityAvailable = | ||
(this.isIdentityValid(identity) && !this.temporarilyUnavailable(identity)) || | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added this extra check because identity can be valid, but unavailable |
||
this._apiClient?.hasActiveRequests(); | ||
|
||
if (!identityAvailable) { | ||
if (this._callbackManager) { | ||
this._callbackManager.runCallbacks(EventType.NoIdentityAvailable, {}); | ||
} | ||
return false; | ||
} | ||
return true; | ||
} | ||
|
||
public hasOptedOut() { | ||
|
@@ -230,10 +259,14 @@ export abstract class SdkBase { | |
this._initCallbackManager?.addInitCallback(opts.callback); | ||
} | ||
|
||
const useNewIdentity = | ||
opts.identity && | ||
(!previousOpts.identity || | ||
opts.identity.identity_expires > previousOpts.identity.identity_expires); | ||
let useNewIdentity; | ||
if (!opts.identity) useNewIdentity = true; | ||
else { | ||
useNewIdentity = | ||
!previousOpts.identity || | ||
opts.identity.identity_expires > previousOpts.identity.identity_expires; | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. found in testing that we didnt have the possibility that opts.identity just doesn't exist, which would automatically make |
||
if (useNewIdentity || opts.callback) { | ||
let identity = useNewIdentity ? opts.identity : previousOpts.identity ?? null; | ||
if (identity) { | ||
|
@@ -277,11 +310,11 @@ export abstract class SdkBase { | |
|
||
this.setInitComplete(true); | ||
this._callbackManager?.runCallbacks(EventType.InitCompleted, {}); | ||
this.isIdentityAvailable(); | ||
if (this.hasOptedOut()) this._callbackManager.runCallbacks(EventType.OptoutReceived, {}); | ||
} | ||
|
||
private isIdentityValid() { | ||
const identity = this._identity ?? this.getIdentityNoInit(); | ||
private isIdentityValid(identity: Identity | OptoutIdentity | null | undefined) { | ||
return identity && !hasExpired(identity.refresh_expires); | ||
} | ||
|
||
|
@@ -499,6 +532,7 @@ export abstract class SdkBase { | |
} else { | ||
const errorText = 'Unexpected status received from CSTG endpoint.'; | ||
this._logger.warn(errorText); | ||
this.isIdentityAvailable(); | ||
throw new Error(errorText); | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With everything that can be done without init, I dont think it makes sense anymore to not run callbacks if init is not complete