Skip to content

Commit 8405e8a

Browse files
parfeonparfeon
committed
refactor(shared-worker): react only on actual 'auth' and 'userId' change
Adjust the code that is called on identity (`userId`) and auth token change to modify the state only if values actually have been changed. refactor(shared-worker): change condition for stalled `ping` timer Change the condition that is used to identify whether the `offline` detection timer has been suspended by the browser or not before trying to evict "offline" PubNub clients.
1 parent d47da8a commit 8405e8a

File tree

8 files changed

+146
-47
lines changed

8 files changed

+146
-47
lines changed

‎dist/web/pubnub.worker.js

Lines changed: 62 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1767,11 +1767,22 @@
17671767
/**
17681768
* Check whether two access token objects represent the same permissions or not.
17691769
*
1770-
* @param other - Other access token which should be used in comparison.
1771-
* @returns `true` if received and another access token object represents the same permissions.
1770+
* @param other - Other access token that should be used in comparison.
1771+
* @param checkExpiration - Whether the token expiration date also should be compared or not.
1772+
* @returns `true` if received and another access token object represents the same permissions (and `expiration` if
1773+
* has been requested).
17721774
*/
1773-
equalTo(other) {
1774-
return this.asIdentifier === other.asIdentifier;
1775+
equalTo(other, checkExpiration = false) {
1776+
return this.asIdentifier === other.asIdentifier && (checkExpiration ? this.expiration === other.expiration : true);
1777+
}
1778+
/**
1779+
* Check whether the receiver is a newer auth token than another.
1780+
*
1781+
* @param other - Other access token that should be used in comparison.
1782+
* @returns `true` if received has a more distant expiration date than another token.
1783+
*/
1784+
isNewerThan(other) {
1785+
return this.simplifiedToken ? this.expiration > other.expiration : false;
17751786
}
17761787
/**
17771788
* Stringify object to actual access token / key value.
@@ -2196,6 +2207,15 @@
21962207
// --------------------- Aggregation ----------------------
21972208
// --------------------------------------------------------
21982209
// region Aggregation
2210+
/**
2211+
* Update access token for the client which should be used with next subscribe request.
2212+
*
2213+
* @param accessToken - Access token for next subscribe REST API call.
2214+
*/
2215+
updateClientAccessToken(accessToken) {
2216+
if (!this.accessToken || accessToken.isNewerThan(this.accessToken))
2217+
this.accessToken = accessToken;
2218+
}
21992219
/**
22002220
* Mark specific client as suitable for state invalidation when it will be appropriate.
22012221
*
@@ -2286,8 +2306,6 @@
22862306
const newInitialServiceRequests = [];
22872307
const cancelledServiceRequests = [];
22882308
let serviceLeaveRequest;
2289-
[...continuationRequests];
2290-
[...initialRequests];
22912309
// Identify token override for initial requests.
22922310
let timetokenOverrideRefreshTimestamp;
22932311
let decidedTimetokenRegionOverride;
@@ -2460,7 +2478,7 @@
24602478
// region Event handlers
24612479
addListenersForRequestEvents(request) {
24622480
const abortController = (this.requestListenersAbort[request.identifier] = new AbortController());
2463-
const cleanUpCallback = (evt) => {
2481+
const cleanUpCallback = () => {
24642482
this.removeListenersFromRequestEvents(request);
24652483
if (!request.isServiceRequest) {
24662484
if (this.requests[request.client.identifier]) {
@@ -2872,7 +2890,6 @@
28722890
}
28732891
if (!identifier)
28742892
return;
2875-
//
28762893
if (request) {
28772894
// Unset `service`-provided request because we can't receive a response with new `userId`.
28782895
request.serviceRequest = undefined;
@@ -3039,10 +3056,28 @@
30393056
// Keep track of the client's listener abort controller.
30403057
const abortController = new AbortController();
30413058
this.clientAbortControllers[client.identifier] = abortController;
3042-
client.addEventListener(PubNubClientEvent.IdentityChange, () => this.moveClient(client), {
3059+
client.addEventListener(PubNubClientEvent.IdentityChange, (event) => {
3060+
if (!(event instanceof PubNubClientIdentityChangeEvent))
3061+
return;
3062+
// Make changes into state only if `userId` actually changed.
3063+
if (!!event.oldUserId !== !!event.newUserId ||
3064+
(event.oldUserId && event.newUserId && event.newUserId !== event.oldUserId))
3065+
this.moveClient(client);
3066+
}, {
30433067
signal: abortController.signal,
30443068
});
3045-
client.addEventListener(PubNubClientEvent.AuthChange, () => this.moveClient(client), {
3069+
client.addEventListener(PubNubClientEvent.AuthChange, (event) => {
3070+
var _a;
3071+
if (!(event instanceof PubNubClientAuthChangeEvent))
3072+
return;
3073+
// Check whether the client should be moved to another state because of a permissions change or whether the
3074+
// same token with the same permissions should be used for the next requests.
3075+
if (!!event.oldAuth !== !!event.newAuth ||
3076+
(event.oldAuth && event.newAuth && !event.oldAuth.equalTo(event.newAuth)))
3077+
this.moveClient(client);
3078+
else if (event.oldAuth && event.newAuth && event.oldAuth.equalTo(event.newAuth))
3079+
(_a = this.subscriptionStateForClient(client)) === null || _a === void 0 ? void 0 : _a.updateClientAccessToken(event.newAuth);
3080+
}, {
30463081
signal: abortController.signal,
30473082
});
30483083
client.addEventListener(PubNubClientEvent.SendSubscribeRequest, (event) => {
@@ -3853,11 +3888,15 @@
38533888
client.addEventListener(PubNubClientEvent.IdentityChange, (event) => {
38543889
if (!(event instanceof PubNubClientIdentityChangeEvent))
38553890
return;
3856-
const state = this.heartbeatStateForClient(client);
3857-
const request = state ? state.requestForClient(client) : undefined;
3858-
if (request)
3859-
request.userId = event.newUserId;
3860-
this.moveClient(client);
3891+
// Make changes into state only if `userId` actually changed.
3892+
if (!!event.oldUserId !== !!event.newUserId ||
3893+
(event.oldUserId && event.newUserId && event.newUserId !== event.oldUserId)) {
3894+
const state = this.heartbeatStateForClient(client);
3895+
const request = state ? state.requestForClient(client) : undefined;
3896+
if (request)
3897+
request.userId = event.newUserId;
3898+
this.moveClient(client);
3899+
}
38613900
}, {
38623901
signal: abortController.signal,
38633902
});
@@ -3868,7 +3907,11 @@
38683907
const request = state ? state.requestForClient(client) : undefined;
38693908
if (request)
38703909
request.accessToken = event.newAuth;
3871-
this.moveClient(client);
3910+
// Check whether the client should be moved to another state because of a permissions change or whether the
3911+
// same token with the same permissions should be used for the next requests.
3912+
if (!!event.oldAuth !== !!event.newAuth ||
3913+
(event.oldAuth && event.newAuth && !event.newAuth.equalTo(event.oldAuth)))
3914+
this.moveClient(client);
38723915
}, {
38733916
signal: abortController.signal,
38743917
});
@@ -4252,7 +4295,7 @@
42524295
const accessToken = authKey ? new AccessToken(authKey, (token !== null && token !== void 0 ? token : {}).token, (token !== null && token !== void 0 ? token : {}).expiration) : undefined;
42534296
// Check whether the access token really changed or not.
42544297
if (!!accessToken !== !!this.accessToken ||
4255-
(!!accessToken && this.accessToken && !accessToken.equalTo(this.accessToken))) {
4298+
(!!accessToken && this.accessToken && !accessToken.equalTo(this.accessToken, true))) {
42564299
const oldValue = this._accessToken;
42574300
this._accessToken = accessToken;
42584301
// Make sure that all ongoing subscribe (usually should be only one at a time) requests use proper
@@ -4590,12 +4633,12 @@
45904633
[...this.clientBySubscribeKey[subKey]].forEach((client) => {
45914634
// Handle potential SharedWorker timers throttling and early eviction of the PubNub core client.
45924635
// If timer fired later than specified interval - it has been throttled and shouldn't unregister client.
4593-
if (client.lastPingRequest && Date.now() / 1000 - client.lastPingRequest > interval * 0.5) {
4636+
if (client.lastPingRequest && Date.now() / 1000 - client.lastPingRequest - 0.2 > interval * 0.5) {
45944637
client.logger.warn('PubNub clients timeout timer fired after throttling past due time.');
45954638
client.lastPingRequest = undefined;
45964639
}
45974640
if (client.lastPingRequest &&
4598-
(!client.lastPongEvent || Math.abs(client.lastPongEvent - client.lastPingRequest) > interval * 0.5)) {
4641+
(!client.lastPongEvent || Math.abs(client.lastPongEvent - client.lastPingRequest) > interval)) {
45994642
this.unregisterClient(client, this.timeouts[subKey].unsubscribeOffline);
46004643
// Notify other clients with same subscription key that one of them became inactive.
46014644
this.forEachClient(subKey, (subKeyClient) => {

‎dist/web/pubnub.worker.min.js

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/transport/subscription-worker/components/access-token.ts

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,23 @@ export class AccessToken {
4848
/**
4949
* Check whether two access token objects represent the same permissions or not.
5050
*
51-
* @param other - Other access token which should be used in comparison.
52-
* @returns `true` if received and another access token object represents the same permissions.
51+
* @param other - Other access token that should be used in comparison.
52+
* @param checkExpiration - Whether the token expiration date also should be compared or not.
53+
* @returns `true` if received and another access token object represents the same permissions (and `expiration` if
54+
* has been requested).
5355
*/
54-
equalTo(other: AccessToken): boolean {
55-
return this.asIdentifier === other.asIdentifier;
56+
equalTo(other: AccessToken, checkExpiration: boolean = false): boolean {
57+
return this.asIdentifier === other.asIdentifier && (checkExpiration ? this.expiration === other.expiration : true);
58+
}
59+
60+
/**
61+
* Check whether the receiver is a newer auth token than another.
62+
*
63+
* @param other - Other access token that should be used in comparison.
64+
* @returns `true` if received has a more distant expiration date than another token.
65+
*/
66+
isNewerThan(other: AccessToken) {
67+
return this.simplifiedToken ? this.expiration! > other.expiration! : false;
5668
}
5769

5870
/**

‎src/transport/subscription-worker/components/heartbeat-requests-manager.ts

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ import {
33
PubNubClientSendLeaveEvent,
44
PubNubClientAuthChangeEvent,
55
PubNubClientSendHeartbeatEvent,
6-
PubNubClientHeartbeatIntervalChangeEvent,
76
PubNubClientIdentityChangeEvent,
7+
PubNubClientHeartbeatIntervalChangeEvent,
88
} from './custom-events/client-event';
99
import {
1010
PubNubClientsManagerEvent,
@@ -166,11 +166,17 @@ export class HeartbeatRequestsManager extends RequestsManager {
166166
PubNubClientEvent.IdentityChange,
167167
(event) => {
168168
if (!(event instanceof PubNubClientIdentityChangeEvent)) return;
169-
const state = this.heartbeatStateForClient(client);
170-
const request = state ? state.requestForClient(client) : undefined;
171-
if (request) request.userId = event.newUserId;
169+
// Make changes into state only if `userId` actually changed.
170+
if (
171+
!!event.oldUserId !== !!event.newUserId ||
172+
(event.oldUserId && event.newUserId && event.newUserId !== event.oldUserId)
173+
) {
174+
const state = this.heartbeatStateForClient(client);
175+
const request = state ? state.requestForClient(client) : undefined;
176+
if (request) request.userId = event.newUserId;
172177

173-
this.moveClient(client);
178+
this.moveClient(client);
179+
}
174180
},
175181
{
176182
signal: abortController.signal,
@@ -184,7 +190,13 @@ export class HeartbeatRequestsManager extends RequestsManager {
184190
const request = state ? state.requestForClient(client) : undefined;
185191
if (request) request.accessToken = event.newAuth;
186192

187-
this.moveClient(client);
193+
// Check whether the client should be moved to another state because of a permissions change or whether the
194+
// same token with the same permissions should be used for the next requests.
195+
if (
196+
!!event.oldAuth !== !!event.newAuth ||
197+
(event.oldAuth && event.newAuth && !event.newAuth.equalTo(event.oldAuth))
198+
)
199+
this.moveClient(client);
188200
},
189201
{
190202
signal: abortController.signal,

‎src/transport/subscription-worker/components/pubnub-client.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ export class PubNubClient extends EventTarget {
293293
// Check whether the access token really changed or not.
294294
if (
295295
!!accessToken !== !!this.accessToken ||
296-
(!!accessToken && this.accessToken && !accessToken.equalTo(this.accessToken))
296+
(!!accessToken && this.accessToken && !accessToken.equalTo(this.accessToken, true))
297297
) {
298298
const oldValue = this._accessToken;
299299
this._accessToken = accessToken;

‎src/transport/subscription-worker/components/pubnub-clients-manager.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -208,14 +208,14 @@ export class PubNubClientsManager extends EventTarget {
208208
[...this.clientBySubscribeKey[subKey]].forEach((client) => {
209209
// Handle potential SharedWorker timers throttling and early eviction of the PubNub core client.
210210
// If timer fired later than specified interval - it has been throttled and shouldn't unregister client.
211-
if (client.lastPingRequest && Date.now() / 1000 - client.lastPingRequest > interval * 0.5) {
211+
if (client.lastPingRequest && Date.now() / 1000 - client.lastPingRequest - 0.2 > interval * 0.5) {
212212
client.logger.warn('PubNub clients timeout timer fired after throttling past due time.');
213213
client.lastPingRequest = undefined;
214214
}
215215

216216
if (
217217
client.lastPingRequest &&
218-
(!client.lastPongEvent || Math.abs(client.lastPongEvent - client.lastPingRequest) > interval * 0.5)
218+
(!client.lastPongEvent || Math.abs(client.lastPongEvent - client.lastPingRequest) > interval)
219219
) {
220220
this.unregisterClient(client, this.timeouts[subKey].unsubscribeOffline);
221221

‎src/transport/subscription-worker/components/subscribe-requests-manager.ts

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import {
22
PubNubClientEvent,
33
PubNubClientSendLeaveEvent,
4+
PubNubClientAuthChangeEvent,
45
PubNubClientSendSubscribeEvent,
5-
PubNubClientCancelSubscribeEvent,
66
PubNubClientIdentityChangeEvent,
7+
PubNubClientCancelSubscribeEvent,
78
} from './custom-events/client-event';
89
import {
910
PubNubClientsManagerEvent,
@@ -18,7 +19,6 @@ import { RequestsManager } from './requests-manager';
1819
import { PubNubClient } from './pubnub-client';
1920
import { LeaveRequest } from './leave-request';
2021
import { leaveRequest } from './helpers';
21-
import { requests } from 'sinon';
2222

2323
/**
2424
* Aggregation timer timeout.
@@ -147,7 +147,6 @@ export class SubscribeRequestsManager extends RequestsManager {
147147

148148
if (!identifier) return;
149149

150-
//
151150
if (request) {
152151
// Unset `service`-provided request because we can't receive a response with new `userId`.
153152
request.serviceRequest = undefined;
@@ -346,12 +345,39 @@ export class SubscribeRequestsManager extends RequestsManager {
346345
const abortController = new AbortController();
347346
this.clientAbortControllers[client.identifier] = abortController;
348347

349-
client.addEventListener(PubNubClientEvent.IdentityChange, () => this.moveClient(client), {
350-
signal: abortController.signal,
351-
});
352-
client.addEventListener(PubNubClientEvent.AuthChange, () => this.moveClient(client), {
353-
signal: abortController.signal,
354-
});
348+
client.addEventListener(
349+
PubNubClientEvent.IdentityChange,
350+
(event) => {
351+
if (!(event instanceof PubNubClientIdentityChangeEvent)) return;
352+
// Make changes into state only if `userId` actually changed.
353+
if (
354+
!!event.oldUserId !== !!event.newUserId ||
355+
(event.oldUserId && event.newUserId && event.newUserId !== event.oldUserId)
356+
)
357+
this.moveClient(client);
358+
},
359+
{
360+
signal: abortController.signal,
361+
},
362+
);
363+
client.addEventListener(
364+
PubNubClientEvent.AuthChange,
365+
(event) => {
366+
if (!(event instanceof PubNubClientAuthChangeEvent)) return;
367+
// Check whether the client should be moved to another state because of a permissions change or whether the
368+
// same token with the same permissions should be used for the next requests.
369+
if (
370+
!!event.oldAuth !== !!event.newAuth ||
371+
(event.oldAuth && event.newAuth && !event.oldAuth.equalTo(event.newAuth))
372+
)
373+
this.moveClient(client);
374+
else if (event.oldAuth && event.newAuth && event.oldAuth.equalTo(event.newAuth))
375+
this.subscriptionStateForClient(client)?.updateClientAccessToken(event.newAuth);
376+
},
377+
{
378+
signal: abortController.signal,
379+
},
380+
);
355381
client.addEventListener(
356382
PubNubClientEvent.SendSubscribeRequest,
357383
(event) => {

‎src/transport/subscription-worker/components/subscription-state.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,15 @@ export class SubscriptionState extends EventTarget {
315315
// --------------------------------------------------------
316316
// region Aggregation
317317

318+
/**
319+
* Update access token for the client which should be used with next subscribe request.
320+
*
321+
* @param accessToken - Access token for next subscribe REST API call.
322+
*/
323+
updateClientAccessToken(accessToken: AccessToken) {
324+
if (!this.accessToken || accessToken.isNewerThan(this.accessToken)) this.accessToken = accessToken;
325+
}
326+
318327
/**
319328
* Mark specific client as suitable for state invalidation when it will be appropriate.
320329
*
@@ -429,9 +438,6 @@ export class SubscriptionState extends EventTarget {
429438
const cancelledServiceRequests: SubscribeRequest[] = [];
430439
let serviceLeaveRequest: LeaveRequest | undefined;
431440

432-
const originalContinuationRequests = [...continuationRequests];
433-
const originalInitialRequests = [...initialRequests];
434-
435441
// Identify token override for initial requests.
436442
let timetokenOverrideRefreshTimestamp: number | undefined;
437443
let decidedTimetokenRegionOverride: string | undefined;
@@ -645,7 +651,7 @@ export class SubscriptionState extends EventTarget {
645651
private addListenersForRequestEvents(request: SubscribeRequest) {
646652
const abortController = (this.requestListenersAbort[request.identifier] = new AbortController());
647653

648-
const cleanUpCallback = (evt: Event) => {
654+
const cleanUpCallback = () => {
649655
this.removeListenersFromRequestEvents(request);
650656
if (!request.isServiceRequest) {
651657
if (this.requests[request.client.identifier]) {

0 commit comments

Comments
 (0)