diff --git a/packages/mgt-chat/src/statefulClient/Caching/SubscriptionCache.ts b/packages/mgt-chat/src/statefulClient/Caching/SubscriptionCache.ts index 7065f8a77..3bbd8206f 100644 --- a/packages/mgt-chat/src/statefulClient/Caching/SubscriptionCache.ts +++ b/packages/mgt-chat/src/statefulClient/Caching/SubscriptionCache.ts @@ -12,28 +12,24 @@ import { IDBPObjectStore } from 'idb'; type CachedSubscriptionData = CacheItem & { chatId: string; - sessionId: string; subscriptions: Subscription[]; lastAccessDateTime: string; }; -const buildCacheKey = (chatId: string, sessionId: string): string => `${chatId}:${sessionId}`; - export class SubscriptionsCache { private get cache(): CacheStore { const conversation: CacheSchema = schemas.conversation; return CacheService.getCache(conversation, conversation.stores.subscriptions); } - public async loadSubscriptions(chatId: string, sessionId: string): Promise { + public async loadSubscriptions(chatId: string): Promise { if (isConversationCacheEnabled()) { - const cacheKey = buildCacheKey(chatId, sessionId); let data; await this.cache.transaction(async (store: IDBPObjectStore) => { - data = (await store.get(cacheKey)) as CachedSubscriptionData | undefined; + data = (await store.get(chatId)) as CachedSubscriptionData | undefined; if (data) { data.lastAccessDateTime = new Date().toISOString(); - await store.put(data, cacheKey); + await store.put(data, chatId); } }); return data || undefined; @@ -41,12 +37,11 @@ export class SubscriptionsCache { return undefined; } - public async cacheSubscription(chatId: string, sessionId: string, subscriptionRecord: Subscription): Promise { + public async cacheSubscription(chatId: string, subscriptionRecord: Subscription): Promise { await this.cache.transaction(async (store: IDBPObjectStore) => { log('cacheSubscription', subscriptionRecord); - const cacheKey = buildCacheKey(chatId, sessionId); - let cacheEntry = (await store.get(cacheKey)) as CachedSubscriptionData | undefined; + let cacheEntry = (await store.get(chatId)) as CachedSubscriptionData | undefined; if (cacheEntry && cacheEntry.chatId === chatId) { const subIndex = cacheEntry.subscriptions.findIndex(s => s.resource === subscriptionRecord.resource); if (subIndex !== -1) { @@ -57,7 +52,6 @@ export class SubscriptionsCache { } else { cacheEntry = { chatId, - sessionId, subscriptions: [subscriptionRecord], // we're cheating a bit here to ensure that we have a defined lastAccessDateTime // but we're updating the value for all cases before storing it. @@ -66,12 +60,12 @@ export class SubscriptionsCache { } cacheEntry.lastAccessDateTime = new Date().toISOString(); - await store.put(cacheEntry, cacheKey); + await store.put(cacheEntry, chatId); }); } - public deleteCachedSubscriptions(chatId: string, sessionId: string): Promise { - return this.cache.delete(buildCacheKey(chatId, sessionId)); + public deleteCachedSubscriptions(chatId: string): Promise { + return this.cache.delete(chatId); } public loadInactiveSubscriptions(inactivityThreshold: string): Promise { diff --git a/packages/mgt-chat/src/statefulClient/GraphConfig.ts b/packages/mgt-chat/src/statefulClient/GraphConfig.ts index 5c278ff23..b96dc3633 100644 --- a/packages/mgt-chat/src/statefulClient/GraphConfig.ts +++ b/packages/mgt-chat/src/statefulClient/GraphConfig.ts @@ -6,6 +6,7 @@ */ import { GraphEndpoint } from '@microsoft/mgt-element'; +import { replaceOrAppendSessionId } from './replaceOrAppendSessionId'; export class GraphConfig { public static ackAsString = false; @@ -30,11 +31,14 @@ export class GraphConfig { return GraphConfig.useCanary ? `${GraphConfig.baseCanaryUrl}/subscriptions` : '/subscriptions'; } - public static adjustNotificationUrl(url: string): string { + public static adjustNotificationUrl(url: string, sessionId = 'default'): string { if (GraphConfig.useCanary && url) { url = url.replace('https://graph.microsoft.com/1.0', GraphConfig.baseCanaryUrl); url = url.replace('https://graph.microsoft.com/beta', GraphConfig.baseCanaryUrl); } - return url.replace(GraphConfig.webSocketsPrefix, ''); + url = url.replace(GraphConfig.webSocketsPrefix, ''); + // update or append sessionid + url = replaceOrAppendSessionId(url, sessionId); + return url; } } diff --git a/packages/mgt-chat/src/statefulClient/GraphNotificationClient.ts b/packages/mgt-chat/src/statefulClient/GraphNotificationClient.ts index 190b79ee5..953c790e6 100644 --- a/packages/mgt-chat/src/statefulClient/GraphNotificationClient.ts +++ b/packages/mgt-chat/src/statefulClient/GraphNotificationClient.ts @@ -18,6 +18,7 @@ import type { import { GraphConfig } from './GraphConfig'; import { SubscriptionsCache } from './Caching/SubscriptionCache'; import { Timer } from '../utils/Timer'; +import { getOrGenerateGroupId } from './getOrGenerateGroupId'; export const appSettings = { defaultSubscriptionLifetimeInMinutes: 10, @@ -84,14 +85,12 @@ export class GraphNotificationClient { /** * Removes any active timers that may exist to prevent memory leaks and perf issues. * Call this method when the component that depends an instance of this class is being removed from the DOM - * i.e */ - public async tearDown() { + public tearDown() { log('cleaning up graph notification resources'); if (this.cleanupTimeout) this.timer.clearTimeout(this.cleanupTimeout); if (this.renewalTimeout) this.timer.clearTimeout(this.renewalTimeout); this.timer.close(); - await this.unsubscribeFromChatNotifications(this.chatId, this.sessionId); } private readonly getToken = async () => { @@ -177,7 +176,7 @@ export class GraphNotificationClient { private readonly cacheSubscription = async (subscriptionRecord: Subscription): Promise => { log(subscriptionRecord); - await this.subscriptionCache.cacheSubscription(this.chatId, this.sessionId, subscriptionRecord); + await this.subscriptionCache.cacheSubscription(this.chatId, subscriptionRecord); // only start timer once. undefined for renewalInterval is semaphore it has stopped. if (this.renewalTimeout === undefined) this.startRenewalTimer(); @@ -190,7 +189,7 @@ export class GraphNotificationClient { ).toISOString(); const subscriptionDefinition: Subscription = { changeType: changeTypes.join(','), - notificationUrl: `${GraphConfig.webSocketsPrefix}?groupId=${this.chatId}&sessionId=${this.sessionId}`, + notificationUrl: `${GraphConfig.webSocketsPrefix}?groupId=${getOrGenerateGroupId(this.chatId)}`, resource: resourcePath, expirationDateTime, includeResourceData: true, @@ -227,8 +226,7 @@ export class GraphNotificationClient { private readonly renewalTimer = async () => { log(`running subscription renewal timer for chatId: ${this.chatId} sessionId: ${this.sessionId}`); - const subscriptions = - (await this.subscriptionCache.loadSubscriptions(this.chatId, this.sessionId))?.subscriptions || []; + const subscriptions = (await this.subscriptionCache.loadSubscriptions(this.chatId))?.subscriptions || []; if (subscriptions.length === 0) { log(`No subscriptions found in session state. Stop renewal timer ${this.renewalTimeout}.`); if (this.renewalTimeout) this.timer.clearTimeout(this.renewalTimeout); @@ -260,7 +258,7 @@ export class GraphNotificationClient { new Date().getTime() + appSettings.defaultSubscriptionLifetimeInMinutes * 60 * 1000 ); - const subscriptionCache = await this.subscriptionCache.loadSubscriptions(this.chatId, this.sessionId); + const subscriptionCache = await this.subscriptionCache.loadSubscriptions(this.chatId); const awaits: Promise[] = []; for (const subscription of subscriptionCache?.subscriptions || []) { if (!subscription.id) continue; @@ -291,7 +289,7 @@ export class GraphNotificationClient { withCredentials: false }; const connection = new HubConnectionBuilder() - .withUrl(GraphConfig.adjustNotificationUrl(notificationUrl), connectionOptions) + .withUrl(GraphConfig.adjustNotificationUrl(notificationUrl, this.sessionId), connectionOptions) .withAutomaticReconnect() .configureLogging(LogLevel.Information) .build(); @@ -352,7 +350,7 @@ export class GraphNotificationClient { await Promise.all(tasks); tasks = []; for (const inactive of inactiveSubs) { - tasks.push(this.subscriptionCache.deleteCachedSubscriptions(inactive.chatId, inactive.sessionId)); + tasks.push(this.subscriptionCache.deleteCachedSubscriptions(inactive.chatId)); } this.startCleanupTimer(); }; @@ -363,26 +361,22 @@ export class GraphNotificationClient { this.connection = undefined; } - private async unsubscribeFromChatNotifications(chatId: string, sessionId: string) { + private async unsubscribeFromChatNotifications(chatId: string) { await this.closeSignalRConnection(); - const cacheData = await this.subscriptionCache.loadSubscriptions(chatId, sessionId); + const cacheData = await this.subscriptionCache.loadSubscriptions(chatId); if (cacheData) { await Promise.all([ this.removeSubscriptions(cacheData.subscriptions), - this.subscriptionCache.deleteCachedSubscriptions(chatId, sessionId) + this.subscriptionCache.deleteCachedSubscriptions(chatId) ]); } } public async subscribeToChatNotifications(chatId: string, sessionId: string) { - // if we have a "previous" chat state at present, unsubscribe for the previous chatId - if (this.chatId && this.sessionId && chatId !== this.chatId) { - await this.unsubscribeFromChatNotifications(this.chatId, this.sessionId); - } this.chatId = chatId; this.sessionId = sessionId; // MGT uses a per-user cache, so no concerns of loading the cached data for another user. - const cacheData = await this.subscriptionCache.loadSubscriptions(chatId, sessionId); + const cacheData = await this.subscriptionCache.loadSubscriptions(chatId); if (cacheData) { // check subscription validity & renew if all still valid otherwise recreate const someExpired = cacheData.subscriptions.some( @@ -390,14 +384,16 @@ export class GraphNotificationClient { ); // for a given user + app + chatId + sessionId they only get one websocket and receive all notifications via that websocket. const webSocketUrl = cacheData.subscriptions.find(s => s.notificationUrl)?.notificationUrl; - if (someExpired) { - await this.removeSubscriptions(cacheData.subscriptions); - } else if (webSocketUrl) { + if (!someExpired && webSocketUrl) { + // if we have a websocket url and all the subscriptions are valid, we can reuse the websocket and return before recreating subscriptions. await this.createSignalRConnection(webSocketUrl); await this.renewChatSubscriptions(); return; + } else if (someExpired) { + // if some are expired, remove them and continue to recreate the subscription + await this.removeSubscriptions(cacheData.subscriptions); } - await this.subscriptionCache.deleteCachedSubscriptions(chatId, sessionId); + await this.subscriptionCache.deleteCachedSubscriptions(chatId); } const promises: Promise[] = []; promises.push(this.subscribeToResource(`/chats/${chatId}/messages`, ['created', 'updated', 'deleted'])); diff --git a/packages/mgt-chat/src/statefulClient/StatefulGraphChatClient.ts b/packages/mgt-chat/src/statefulClient/StatefulGraphChatClient.ts index 7dfacfc8d..3ea946eae 100644 --- a/packages/mgt-chat/src/statefulClient/StatefulGraphChatClient.ts +++ b/packages/mgt-chat/src/statefulClient/StatefulGraphChatClient.ts @@ -221,8 +221,8 @@ class StatefulGraphChatClient implements StatefulClient { /** * Provides a method to clean up any resources being used internally when a consuming component is being removed from the DOM */ - public async tearDown() { - await this._notificationClient?.tearDown(); + public tearDown() { + this._notificationClient?.tearDown(); } /** diff --git a/packages/mgt-chat/src/statefulClient/chatOperationScopes.tests.ts b/packages/mgt-chat/src/statefulClient/chatOperationScopes.tests.ts index b4b87302d..9b49e05ca 100644 --- a/packages/mgt-chat/src/statefulClient/chatOperationScopes.tests.ts +++ b/packages/mgt-chat/src/statefulClient/chatOperationScopes.tests.ts @@ -1,3 +1,10 @@ +/** + * ------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All Rights Reserved. Licensed under the MIT License. + * See License in the project root for license information. + * ------------------------------------------------------------------------------------------- + */ + import { expect } from '@open-wc/testing'; import { allChatScopes } from './chatOperationScopes'; diff --git a/packages/mgt-chat/src/statefulClient/getOrGenerateGroupId.ts b/packages/mgt-chat/src/statefulClient/getOrGenerateGroupId.ts new file mode 100644 index 000000000..889773a77 --- /dev/null +++ b/packages/mgt-chat/src/statefulClient/getOrGenerateGroupId.ts @@ -0,0 +1,26 @@ +/** + * ------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All Rights Reserved. Licensed under the MIT License. + * See License in the project root for license information. + * ------------------------------------------------------------------------------------------- + */ + +import { v4 as uuid } from 'uuid'; + +const keyPrefix = 'mgt-chat-group-id'; + +/** + * reads a string from session storage, or if there is no string for the keyName, generate a new uuid and place in storage + */ +export const getOrGenerateGroupId = (chatId: string) => { + const key = `${keyPrefix}::${chatId}`; + const value = localStorage.getItem(key); + + if (value) { + return value; + } else { + const newValue = uuid(); + localStorage.setItem(key, newValue); + return newValue; + } +}; diff --git a/packages/mgt-chat/src/statefulClient/replaceOrAppendSessionId.tests.ts b/packages/mgt-chat/src/statefulClient/replaceOrAppendSessionId.tests.ts new file mode 100644 index 000000000..1275427c5 --- /dev/null +++ b/packages/mgt-chat/src/statefulClient/replaceOrAppendSessionId.tests.ts @@ -0,0 +1,43 @@ +/** + * ------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All Rights Reserved. Licensed under the MIT License. + * See License in the project root for license information. + * ------------------------------------------------------------------------------------------- + */ + +import { replaceOrAppendSessionId } from './replaceOrAppendSessionId'; +import { expect } from '@open-wc/testing'; + +describe('replaceOrAppendSessionId tests', () => { + it('should replace existing sessionid', async () => { + const result = replaceOrAppendSessionId('https://graph.microsoft.com/1.0/me?sessionid=default', '123'); + await expect(result).to.equal('https://graph.microsoft.com/1.0/me?sessionid=123'); + }); + it('should replace existing sessionid when there are query string params after sessionid', async () => { + const result = replaceOrAppendSessionId('https://graph.microsoft.com/1.0/me?sessionid=default&foo=bar', '123'); + await expect(result).to.equal('https://graph.microsoft.com/1.0/me?sessionid=123&foo=bar'); + }); + it('should replace existing sessionid when there are query string params before sessionid', async () => { + const result = replaceOrAppendSessionId('https://graph.microsoft.com/1.0/me?foo=bar&sessionid=default', '123'); + await expect(result).to.equal('https://graph.microsoft.com/1.0/me?foo=bar&sessionid=123'); + }); + it('should append sessionid there is a query string params with sessionid as a substring', async () => { + const result = replaceOrAppendSessionId('https://graph.microsoft.com/1.0/me?foosessionid=bar', '123'); + await expect(result).to.equal('https://graph.microsoft.com/1.0/me?foosessionid=bar&sessionid=123'); + }); + it('should replace sessionid there is also a query string params with sessionid as a substring', async () => { + const result = replaceOrAppendSessionId( + 'https://graph.microsoft.com/1.0/me?foosessionid=bar&sessionid=default&bar=too', + '123' + ); + await expect(result).to.equal('https://graph.microsoft.com/1.0/me?foosessionid=bar&sessionid=123&bar=too'); + }); + it('should append sessionid with ? when no query params present', async () => { + const result = replaceOrAppendSessionId('https://graph.microsoft.com/1.0/me', '123'); + await expect(result).to.equal('https://graph.microsoft.com/1.0/me?sessionid=123'); + }); + it('should append sessionid with & when query params are present', async () => { + const result = replaceOrAppendSessionId('https://graph.microsoft.com/1.0/me?foo=bar', '123'); + await expect(result).to.equal('https://graph.microsoft.com/1.0/me?foo=bar&sessionid=123'); + }); +}); diff --git a/packages/mgt-chat/src/statefulClient/replaceOrAppendSessionId.ts b/packages/mgt-chat/src/statefulClient/replaceOrAppendSessionId.ts new file mode 100644 index 000000000..0e8ba91d4 --- /dev/null +++ b/packages/mgt-chat/src/statefulClient/replaceOrAppendSessionId.ts @@ -0,0 +1,18 @@ +/** + * ------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All Rights Reserved. Licensed under the MIT License. + * See License in the project root for license information. + * ------------------------------------------------------------------------------------------- + */ + +export const replaceOrAppendSessionId = (url: string, sessionId: string) => { + // match on any whole query parameter of sessionid and include the value + const sessionIdRegex = /([?&]{1})sessionid=[^&]*/; + if (url.match(sessionIdRegex)) { + url = url.replace(sessionIdRegex, `$1sessionid=${sessionId}`); + } else { + const paramSeparator = url.indexOf('?') > -1 ? '&' : '?'; + url = `${url}${paramSeparator}sessionid=${sessionId}`; + } + return url; +}; diff --git a/packages/mgt-chat/src/statefulClient/useGraphChatClient.ts b/packages/mgt-chat/src/statefulClient/useGraphChatClient.ts index fa0cc45ae..e7bb49688 100644 --- a/packages/mgt-chat/src/statefulClient/useGraphChatClient.ts +++ b/packages/mgt-chat/src/statefulClient/useGraphChatClient.ts @@ -10,33 +10,12 @@ import { v4 as uuid } from 'uuid'; import { StatefulGraphChatClient } from './StatefulGraphChatClient'; import { log } from '@microsoft/mgt-element'; -/** - * The key name to use for storing the sessionId in session storage - */ -const keyName = 'mgt-chat-session-id'; - -/** - * reads a string from session storage, or if there is no string for the keyName, generate a new uuid and place in storage - */ -const getOrGenerateSessionId = () => { - const value = sessionStorage.getItem(keyName); - - if (value) { - return value; - } else { - const newValue = uuid(); - sessionStorage.setItem(keyName, newValue); - return newValue; - } -}; - /** * Provides a stable sessionId for the lifetime of the browser tab. * @returns a string that is either read from session storage or generated and placed in session storage */ const useSessionId = (): string => { - // when a function is passed to useState, it is only invoked on the first render - const [sessionId] = useState(getOrGenerateSessionId); + const [sessionId] = useState(() => uuid()); return sessionId; }; @@ -63,7 +42,7 @@ export const useGraphChatClient = (chatId: string): StatefulGraphChatClient => { useEffect(() => { return () => { log('invoked clean up effect'); - void chatClient.tearDown(); + chatClient.tearDown(); }; }, [chatClient]); diff --git a/packages/mgt-chat/src/utils/Timer.ts b/packages/mgt-chat/src/utils/Timer.ts index 64e525293..bb3c9b86a 100644 --- a/packages/mgt-chat/src/utils/Timer.ts +++ b/packages/mgt-chat/src/utils/Timer.ts @@ -1,3 +1,10 @@ +/** + * ------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All Rights Reserved. Licensed under the MIT License. + * See License in the project root for license information. + * ------------------------------------------------------------------------------------------- + */ + import { v4 as uuid } from 'uuid'; import { TimerWork } from './timerWorker'; diff --git a/packages/mgt-chat/src/utils/timerWorker.ts b/packages/mgt-chat/src/utils/timerWorker.ts index 9b0f0b77c..d3aa66be1 100644 --- a/packages/mgt-chat/src/utils/timerWorker.ts +++ b/packages/mgt-chat/src/utils/timerWorker.ts @@ -1,3 +1,10 @@ +/** + * ------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All Rights Reserved. Licensed under the MIT License. + * See License in the project root for license information. + * ------------------------------------------------------------------------------------------- + */ + export interface TimerWork { id: string; type: 'clearInterval' | 'setInterval' | 'runCallback' | 'setTimeout' | 'clearTimeout';