Merge pull request #14328 from nextcloud/fix/12885/deep-compare-capab…

…ilities

fix(capabilities): make a deep comparison

src/components/TopBar/CallButton.vue

@@ -353,8 +353,9 @@ export default {
 	},
 
 	watch: {
-		token() {
+		token(newValue, oldValue) {
 			this.callViewStore.resetCallHasJustEnded()
+			this.talkHashStore.resetTalkProxyHashDirty(oldValue)
 		}
 	},
 

src/services/CapabilitiesManager.ts

@@ -4,17 +4,14 @@
  */
 
 import { getCapabilities as _getCapabilities } from '@nextcloud/capabilities'
-import { showError, TOAST_PERMANENT_TIMEOUT } from '@nextcloud/dialogs'
-import { t } from '@nextcloud/l10n'
 
 import { getRemoteCapabilities } from './federationService.ts'
 import BrowserStorage from '../services/BrowserStorage.js'
 import { useTalkHashStore } from '../stores/talkHash.js'
-import type { Capabilities, Conversation, JoinRoomFullResponse } from '../types/index.ts'
-import { messagePleaseReload } from '../utils/talkDesktopUtils.ts'
+import type { acceptShareResponse, Capabilities, Conversation, JoinRoomFullResponse } from '../types/index.ts'
 
 type Config = Capabilities['spreed']['config']
-type RemoteCapability = Capabilities & Partial<{ hash: string }>
+type RemoteCapability = Capabilities & { hash?: string }
 type RemoteCapabilities = Record<string, RemoteCapability>
 type TokenMap = Record<string, string|undefined|null>
 
@@ -113,33 +110,101 @@ function getRemoteCapability(token: string): RemoteCapability | null {
  * @param joinRoomResponse server response
  */
 export async function setRemoteCapabilities(joinRoomResponse: JoinRoomFullResponse): Promise<void> {
+	const talkHashStore = useTalkHashStore()
+
 	const token = joinRoomResponse.data.ocs.data.token
-	const remoteServer = joinRoomResponse.data.ocs.data.remoteServer as string
+	const remoteServer = joinRoomResponse.data.ocs.data.remoteServer!
 
 	// Check if remote capabilities have not changed since last check
 	if (joinRoomResponse.headers['x-nextcloud-talk-proxy-hash'] === remoteCapabilities[remoteServer]?.hash) {
+		talkHashStore.resetTalkProxyHashDirty(token)
 		return
 	}
 
 	// Mark the hash as dirty to prevent any activity in the conversation
-	const talkHashStore = useTalkHashStore()
 	talkHashStore.setTalkProxyHashDirty(token)
 
 	const response = await getRemoteCapabilities(token)
-	if (!Object.keys(response.data.ocs.data).length) {
+	const newRemoteCapabilities = response.data.ocs.data as Capabilities['spreed']
+	if (!Object.keys(newRemoteCapabilities).length) {
 		// data: {} received from server, nothing to update with
 		return
 	}
 
-	remoteCapabilities[remoteServer] = { spreed: (response.data.ocs.data as Capabilities['spreed']) }
-	remoteCapabilities[remoteServer].hash = joinRoomResponse.headers['x-nextcloud-talk-proxy-hash']
+	const shouldShowWarning = checkRemoteCapabilitiesHasChanged(newRemoteCapabilities, remoteCapabilities[remoteServer]?.spreed)
+	remoteCapabilities[remoteServer] = {
+		spreed: newRemoteCapabilities,
+		hash: joinRoomResponse.headers['x-nextcloud-talk-proxy-hash'],
+	}
 	BrowserStorage.setItem('remoteCapabilities', JSON.stringify(remoteCapabilities))
 	patchTokenMap(joinRoomResponse.data.ocs.data)
 
-	// As normal capabilities update, requires a reload to take effect
-	showError(t('spreed', 'Nextcloud Talk Federation was updated.') + '\n' + messagePleaseReload, {
-		timeout: TOAST_PERMANENT_TIMEOUT,
-	})
+	if (shouldShowWarning) {
+		// As normal capabilities update, requires a reload to take effect
+		talkHashStore.showTalkProxyHashDirtyToast()
+	} else {
+		talkHashStore.resetTalkProxyHashDirty(token)
+	}
+}
+
+/**
+ * Fetch new capabilities if remote server is not yet known
+ * @param acceptShareResponse server response
+ */
+export async function setRemoteCapabilitiesIfEmpty(acceptShareResponse: Awaited<acceptShareResponse>): Promise<void> {
+	const token = acceptShareResponse.data.ocs.data.token
+	const remoteServer = acceptShareResponse.data.ocs.data.remoteServer!
+
+	// Check if remote capabilities already exists
+	if (remoteCapabilities[remoteServer]) {
+		return
+	}
+
+	const response = await getRemoteCapabilities(token)
+	const newRemoteCapabilities = response.data.ocs.data as Capabilities['spreed']
+	if (!Object.keys(newRemoteCapabilities).length) {
+		// data: {} received from server, nothing to update with
+		return
+	}
+
+	remoteCapabilities[remoteServer] = { spreed: newRemoteCapabilities }
+	BrowserStorage.setItem('remoteCapabilities', JSON.stringify(remoteCapabilities))
+	patchTokenMap(acceptShareResponse.data.ocs.data)
+}
+
+/**
+ * Deep comparison of remote capabilities, whether there are actual changes that require reload
+ * @param newObject new remote capabilities
+ * @param oldObject old remote capabilities
+ */
+function checkRemoteCapabilitiesHasChanged(newObject: Capabilities['spreed'], oldObject: Capabilities['spreed']): boolean {
+	if (!newObject || !oldObject) {
+		return true
+	}
+
+	/**
+	 * Returns remote config without local-only properties
+	 * @param object remote capabilities object
+	 */
+	function getStrippedCapabilities(object: Capabilities['spreed']): { config: Partial<Config>, features: string[] } {
+		const config = structuredClone(object.config)
+
+		for (const key1 of Object.keys(object['config-local']) as Array<keyof Config>) {
+			const keys2 = object['config-local'][key1]
+			for (const key2 of keys2 as Array<keyof Config[keyof Config]>) {
+				delete config[key1][key2]
+			}
+			if (!Object.keys(config[key1]).length) {
+				delete config[key1]
+			}
+		}
+
+		const features = object.features.filter(feature => !object['features-local'].includes(feature)).sort()
+
+		return { config, features }
+	}
+
+	return JSON.stringify(getStrippedCapabilities(newObject)) !== JSON.stringify(getStrippedCapabilities(oldObject))
 }
 
 /**

src/services/__tests__/CapabilitiesManager.spec.js

@@ -12,6 +12,7 @@ import {
 	hasTalkFeature,
 	getTalkConfig,
 	setRemoteCapabilities,
+	setRemoteCapabilitiesIfEmpty,
 } from '../CapabilitiesManager.ts'
 import { getRemoteCapabilities } from '../federationService.ts'
 
@@ -146,16 +147,102 @@ describe('CapabilitiesManager', () => {
 			expect(BrowserStorage.setItem).toHaveBeenCalledTimes(0)
 		})
 
+		it('should set capabilities for new server and mark talk proxy hash as dirty', async () => {
+			const token = 'TOKEN7FED3'
+			const remoteServer = 'https://nextcloud3.local'
+			const remoteHash = 'abc123'
+			const joinRoomResponseMock = generateOCSResponse({
+				headers: { 'x-nextcloud-talk-proxy-hash': remoteHash },
+				payload: { token, remoteServer }
+			})
+			const responseMock = generateOCSResponse({ payload: mockedCapabilities.spreed })
+			getRemoteCapabilities.mockReturnValue(responseMock)
+			await setRemoteCapabilities(joinRoomResponseMock)
+			expect(talkHashStore.isNextcloudTalkProxyHashDirty[token]).toBeTruthy()
+			expect(BrowserStorage.setItem).toHaveBeenCalledTimes(1)
+		})
+
 		it('should update capabilities from server response and mark talk proxy hash as dirty', async () => {
 			const joinRoomResponseMock = generateOCSResponse({
 				headers: { 'x-nextcloud-talk-proxy-hash': `${remoteCapabilities.hash}002` },
 				payload: { token, remoteServer }
 			})
-			const responseMock = generateOCSResponse({ payload: mockedCapabilities.spreed })
+			const responseMock = generateOCSResponse({
+				payload: {
+					...mockedCapabilities.spreed,
+					features: [...mockedCapabilities.spreed.features, 'new-feature'],
+				}
+			})
 			getRemoteCapabilities.mockReturnValue(responseMock)
 			await setRemoteCapabilities(joinRoomResponseMock)
 			expect(talkHashStore.isNextcloudTalkProxyHashDirty[token]).toBeTruthy()
 			expect(BrowserStorage.setItem).toHaveBeenCalledTimes(1)
 		})
+
+		it('should reset dirty proxy hash after second fetch and negative check for changes', async () => {
+			const joinRoomResponseMock = generateOCSResponse({
+				headers: { 'x-nextcloud-talk-proxy-hash': `${remoteCapabilities.hash}003` },
+				payload: { token, remoteServer }
+			})
+			const joinRoomResponseMock2 = generateOCSResponse({
+				headers: { 'x-nextcloud-talk-proxy-hash': `${remoteCapabilities.hash}004` },
+				payload: { token, remoteServer }
+			})
+			const responseMock = generateOCSResponse({
+				payload: {
+					...mockedCapabilities.spreed,
+					features: [...mockedCapabilities.spreed.features, 'new-feature', 'new-feature-2'],
+				}
+			})
+			const responseMock2 = generateOCSResponse({
+				payload: {
+					...mockedCapabilities.spreed,
+					features: [...mockedCapabilities.spreed.features, 'new-feature', 'new-feature-2'],
+				}
+			})
+			getRemoteCapabilities.mockReturnValueOnce(responseMock).mockReturnValueOnce(responseMock2)
+			await setRemoteCapabilities(joinRoomResponseMock)
+			expect(talkHashStore.isNextcloudTalkProxyHashDirty[token]).toBeTruthy()
+			expect(BrowserStorage.setItem).toHaveBeenCalledTimes(1)
+
+			await setRemoteCapabilities(joinRoomResponseMock2)
+			expect(talkHashStore.isNextcloudTalkProxyHashDirty[token]).not.toBeDefined()
+			expect(BrowserStorage.setItem).toHaveBeenCalledTimes(2)
+		})
+	})
+
+	describe('setRemoteCapabilitiesIfEmpty', () => {
+		const token = 'TOKEN8FED4'
+		const remoteServer = 'https://nextcloud4.local'
+
+		it('should early return if already has capabilities', async () => {
+			const [remoteServer, remoteCapabilities] = Object.entries(mockedRemotes)[0]
+			const token = remoteCapabilities.tokens[0]
+			const acceptShareResponseMock = generateOCSResponse({
+				payload: { token, remoteServer },
+			})
+			await setRemoteCapabilitiesIfEmpty(acceptShareResponseMock)
+			expect(BrowserStorage.setItem).toHaveBeenCalledTimes(0)
+		})
+
+		it('should early return if no capabilities received from server', async () => {
+			const acceptShareResponseMock = generateOCSResponse({
+				payload: { token, remoteServer },
+			})
+			const responseMock = generateOCSResponse({ payload: {} })
+			getRemoteCapabilities.mockReturnValue(responseMock)
+			await setRemoteCapabilitiesIfEmpty(acceptShareResponseMock)
+			expect(BrowserStorage.setItem).toHaveBeenCalledTimes(0)
+		})
+
+		it('should set capabilities for new server', async () => {
+			const acceptShareResponseMock = generateOCSResponse({
+				payload: { token, remoteServer },
+			})
+			const responseMock = generateOCSResponse({ payload: mockedCapabilities.spreed })
+			getRemoteCapabilities.mockReturnValue(responseMock)
+			await setRemoteCapabilitiesIfEmpty(acceptShareResponseMock)
+			expect(BrowserStorage.setItem).toHaveBeenCalledTimes(1)
+		})
 	})
 })

src/stores/__tests__/federation.spec.js

@@ -4,14 +4,16 @@
  */
 import { setActivePinia, createPinia } from 'pinia'
 
-import { getShares, acceptShare, rejectShare } from '../../services/federationService.ts'
+import { mockedCapabilities } from '../../__mocks__/capabilities.ts'
+import { getShares, acceptShare, rejectShare, getRemoteCapabilities } from '../../services/federationService.ts'
 import { generateOCSErrorResponse, generateOCSResponse } from '../../test-helpers.js'
 import { useFederationStore } from '../federation.ts'
 
 jest.mock('../../services/federationService', () => ({
 	getShares: jest.fn(),
 	acceptShare: jest.fn(),
 	rejectShare: jest.fn(),
+	getRemoteCapabilities: jest.fn(),
 }))
 
 describe('federationStore', () => {
@@ -187,6 +189,9 @@ describe('federationStore', () => {
 		const acceptResponse = generateOCSResponse({ payload: room })
 		acceptShare.mockResolvedValueOnce(acceptResponse)
 
+		const responseMock = generateOCSResponse({ payload: mockedCapabilities.spreed })
+		getRemoteCapabilities.mockReturnValue(responseMock)
+
 		// Act: accept invite
 		const conversation = await federationStore.acceptShare(invites[0].id)
 

src/stores/federation.ts

@@ -11,6 +11,7 @@ import { t } from '@nextcloud/l10n'
 import { getBaseUrl } from '@nextcloud/router'
 
 import { FEDERATION } from '../constants.ts'
+import { setRemoteCapabilitiesIfEmpty } from '../services/CapabilitiesManager.ts'
 import { getShares, acceptShare, rejectShare } from '../services/federationService.ts'
 import type { Conversation, FederationInvite, NotificationInvite } from '../types/index.ts'
 
@@ -109,6 +110,7 @@ export const useFederationStore = defineStore('federation', {
 			try {
 				Vue.set(this.pendingShares[id], 'loading', 'accept')
 				const response = await acceptShare(id)
+				await setRemoteCapabilitiesIfEmpty(response)
 				this.markInvitationAccepted(id, response.data.ocs.data)
 				this.updatePendingSharesCount(Object.keys(this.pendingShares).length)
 				return response.data.ocs.data

src/stores/talkHash.js

@@ -32,6 +32,7 @@ export const useTalkHashStore = defineStore('talkHash', {
 		isNextcloudTalkHashDirty: false,
 		isNextcloudTalkProxyHashDirty: {},
 		maintenanceWarningToast: null,
+		proxyHashDirtyToast: null,
 	}),
 
 	actions: {
@@ -60,6 +61,20 @@ export const useTalkHashStore = defineStore('talkHash', {
 			Vue.set(this.isNextcloudTalkProxyHashDirty, token, true)
 		},
 
+		/**
+		 * Clear the current Talk Federation dirty hash marker (as it is irrelevant after reload or leaving)
+		 *
+		 * @param {string} token federated conversation token
+		 */
+		resetTalkProxyHashDirty(token) {
+			Vue.delete(this.isNextcloudTalkProxyHashDirty, token)
+
+			if (this.proxyHashDirtyToast) {
+				this.proxyHashDirtyToast.hideToast()
+				this.proxyHashDirtyToast = null
+			}
+		},
+
 		/**
 		 * Updates a Talk hash from a response
 		 *
@@ -104,5 +119,14 @@ export const useTalkHashStore = defineStore('talkHash', {
 				this.maintenanceWarningToast = null
 			}
 		},
+
+		/**
+		 * Show a toast message when Talk Federation requires rejoining
+		 */
+		showTalkProxyHashDirtyToast() {
+			this.proxyHashDirtyToast = showError(t('spreed', 'Nextcloud Talk Federation was updated.') + '\n' + messagePleaseReload, {
+				timeout: TOAST_PERMANENT_TIMEOUT,
+			})
+		},
 	}
 })

src/test-setup.js

@@ -115,6 +115,8 @@ global.ResizeObserver = jest.fn(() => ({
 	disconnect: jest.fn(),
 }))
 
+global.structuredClone = jest.fn((val) => JSON.parse(JSON.stringify(val)))
+
 // Work around missing "URL.createObjectURL" (which is used in the code but not
 // relevant for the tests) in jsdom: https://github.com/jsdom/jsdom/issues/1721
 window.URL.createObjectURL = jest.fn()