From a0a18a78d5af5e0fbef2fbebf90bf8b03ea17a47 Mon Sep 17 00:00:00 2001 From: Thomas Belin Date: Wed, 20 Dec 2023 15:55:54 +0100 Subject: [PATCH 1/5] refactor: Do not rely on amplify for sound notifications --- src/script/audio/AudioRepository.ts | 23 +++++--------- src/script/audio/AudioState.ts | 31 ------------------- src/script/conversation/MessageRepository.ts | 4 ++- src/script/main/app.ts | 7 ++++- .../notification/NotificationRepository.ts | 6 ++-- test/helper/TestFactory.js | 2 ++ 6 files changed, 22 insertions(+), 51 deletions(-) delete mode 100644 src/script/audio/AudioState.ts diff --git a/src/script/audio/AudioRepository.ts b/src/script/audio/AudioRepository.ts index bd14907e8d6..430fa0d64fd 100644 --- a/src/script/audio/AudioRepository.ts +++ b/src/script/audio/AudioRepository.ts @@ -19,14 +19,13 @@ import {AudioPreference, WebappProperties} from '@wireapp/api-client/lib/user/data/'; import {amplify} from 'amplify'; -import {container} from 'tsyringe'; +import ko from 'knockout'; import {WebAppEvents} from '@wireapp/webapp-events'; import {Logger, getLogger} from 'Util/Logger'; import {AudioPlayingType} from './AudioPlayingType'; -import {AudioState} from './AudioState'; import {AudioType} from './AudioType'; import {NOTIFICATION_HANDLING_STATE} from '../event/NotificationHandlingState'; @@ -42,14 +41,12 @@ enum AUDIO_PLAY_PERMISSION { export class AudioRepository { private readonly logger: Logger; private readonly audioElements: Record; + private readonly audioPreference = ko.observable(AudioPreference.ALL); private muted: boolean; - constructor( - private readonly devicesHandler: MediaDevicesHandler, - private readonly audioState = container.resolve(AudioState), - ) { + constructor(private readonly devicesHandler: MediaDevicesHandler) { this.logger = getLogger('AudioRepository'); - this.audioState.audioPreference.subscribe(audioPreference => { + this.audioPreference.subscribe(audioPreference => { if (audioPreference === AudioPreference.NONE) { this.stopAll(); } @@ -64,12 +61,12 @@ export class AudioRepository { return AUDIO_PLAY_PERMISSION.DISALLOWED_BY_MUTE_STATE; } - const preferenceIsNone = this.audioState.audioPreference() === AudioPreference.NONE; + const preferenceIsNone = this.audioPreference() === AudioPreference.NONE; if (preferenceIsNone && !AudioPlayingType.NONE.includes(audioId)) { return AUDIO_PLAY_PERMISSION.DISALLOWED_BY_PREFERENCES; } - const preferenceIsSome = this.audioState.audioPreference() === AudioPreference.SOME; + const preferenceIsSome = this.audioPreference() === AudioPreference.SOME; if (preferenceIsSome && !AudioPlayingType.SOME.includes(audioId)) { return AUDIO_PLAY_PERMISSION.DISALLOWED_BY_PREFERENCES; } @@ -113,11 +110,6 @@ export class AudioRepository { Object.keys(this.audioElements).forEach((audioId: AudioType) => this.stop(audioId)); } - private subscribeToAudioEvents(): void { - amplify.subscribe(WebAppEvents.AUDIO.PLAY, this.play); - amplify.subscribe(WebAppEvents.AUDIO.STOP, this.stop); - } - private subscribeToEvents(): void { amplify.subscribe(WebAppEvents.EVENT.NOTIFICATION_HANDLING_STATE, this.setMutedState); amplify.subscribe(WebAppEvents.PROPERTIES.UPDATED, this.updatedProperties); @@ -133,7 +125,6 @@ export class AudioRepository { init(preload: boolean = false): void { this.initSounds(preload); this.updateSinkIds(); - this.subscribeToAudioEvents(); } loop(audioId: AudioType): Promise { @@ -184,7 +175,7 @@ export class AudioRepository { }; readonly setAudioPreference = (audioPreference: AudioPreference): void => { - this.audioState.audioPreference(audioPreference); + this.audioPreference(audioPreference); }; readonly setMutedState = (handlingNotifications: NOTIFICATION_HANDLING_STATE): void => { diff --git a/src/script/audio/AudioState.ts b/src/script/audio/AudioState.ts deleted file mode 100644 index f7be72a60d3..00000000000 --- a/src/script/audio/AudioState.ts +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Wire - * Copyright (C) 2021 Wire Swiss GmbH - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see http://www.gnu.org/licenses/. - * - */ - -import {AudioPreference} from '@wireapp/api-client/lib/user/data/'; -import ko from 'knockout'; -import {singleton} from 'tsyringe'; - -@singleton() -export class AudioState { - public readonly audioPreference: ko.Observable; - - constructor() { - this.audioPreference = ko.observable(AudioPreference.ALL); - } -} diff --git a/src/script/conversation/MessageRepository.ts b/src/script/conversation/MessageRepository.ts index 4d3d9761efd..27a951752cb 100644 --- a/src/script/conversation/MessageRepository.ts +++ b/src/script/conversation/MessageRepository.ts @@ -71,6 +71,7 @@ import {getLinkPreviewFromString} from './linkPreviews'; import {buildMetadata, ImageMetadata, isAudio, isImage, isVideo} from '../assets/AssetMetaDataBuilder'; import {AssetRepository} from '../assets/AssetRepository'; import {AssetTransferState} from '../assets/AssetTransferState'; +import {AudioRepository} from '../audio/AudioRepository'; import {AudioType} from '../audio/AudioType'; import {ClientState} from '../client/ClientState'; import {PrimaryModal} from '../components/Modals/PrimaryModal'; @@ -165,6 +166,7 @@ export class MessageRepository { private readonly serverTimeHandler: ServerTimeHandler, private readonly userRepository: UserRepository, private readonly assetRepository: AssetRepository, + private readonly audioRepository: AudioRepository, private readonly userState = container.resolve(UserState), private readonly clientState = container.resolve(ClientState), private readonly conversationState = container.resolve(ConversationState), @@ -240,7 +242,7 @@ export class MessageRepository { legalHoldStatus: conversation.legalHoldStatus(), }); - amplify.publish(WebAppEvents.AUDIO.PLAY, AudioType.OUTGOING_PING); + void this.audioRepository.play(AudioType.OUTGOING_PING); return this.sendAndInjectMessage(ping, conversation, {enableEphemeral: true}); } diff --git a/src/script/main/app.ts b/src/script/main/app.ts index 7989dcc29d1..376f2f42a7f 100644 --- a/src/script/main/app.ts +++ b/src/script/main/app.ts @@ -244,6 +244,7 @@ export class App { serverTimeHandler, repositories.user, repositories.asset, + repositories.audio, ); repositories.calling = new CallingRepository( @@ -280,7 +281,11 @@ export class App { repositories.team, ); repositories.permission = new PermissionRepository(); - repositories.notification = new NotificationRepository(repositories.conversation, repositories.permission); + repositories.notification = new NotificationRepository( + repositories.conversation, + repositories.permission, + repositories.audio, + ); repositories.preferenceNotification = new PreferenceNotificationRepository(repositories.user['userState'].self); return repositories; diff --git a/src/script/notification/NotificationRepository.ts b/src/script/notification/NotificationRepository.ts index 3a458127636..d49ff8b0214 100644 --- a/src/script/notification/NotificationRepository.ts +++ b/src/script/notification/NotificationRepository.ts @@ -37,6 +37,7 @@ import {ValidationUtilError} from 'Util/ValidationUtil'; import {PermissionState} from './PermissionState'; import {AssetRepository} from '../assets/AssetRepository'; +import {AudioRepository} from '../audio/AudioRepository'; import {AudioType} from '../audio/AudioType'; import {CallState} from '../calling/CallState'; import {TERMINATION_REASON} from '../calling/enum/TerminationReason'; @@ -126,6 +127,7 @@ export class NotificationRepository { constructor( conversationRepository: ConversationRepository, permissionRepository: PermissionRepository, + private readonly audioRepository: AudioRepository, private readonly userState = container.resolve(UserState), private readonly conversationState = container.resolve(ConversationState), private readonly callState = container.resolve(CallState), @@ -766,12 +768,12 @@ export class NotificationRepository { if (shouldPlaySound) { switch (messageEntity.super_type) { case SuperType.CONTENT: { - amplify.publish(WebAppEvents.AUDIO.PLAY, AudioType.NEW_MESSAGE); + void this.audioRepository.play(AudioType.NEW_MESSAGE); break; } case SuperType.PING: { - amplify.publish(WebAppEvents.AUDIO.PLAY, AudioType.INCOMING_PING); + void this.audioRepository.play(AudioType.INCOMING_PING); break; } } diff --git a/test/helper/TestFactory.js b/test/helper/TestFactory.js index cb4eb47877d..7658d896d1d 100644 --- a/test/helper/TestFactory.js +++ b/test/helper/TestFactory.js @@ -68,6 +68,7 @@ import {UserState} from 'src/script/user/UserState'; import {entities} from '../api/payloads'; import {SelfRepository} from 'src/script/self/SelfRepository'; +import {AudioRepository} from 'src/script/audio/AudioRepository'; export class TestFactory { constructor() { @@ -269,6 +270,7 @@ export class TestFactory { serverTimeHandler, this.user_repository, this.assetRepository, + new AudioRepository({}), this.user_repository['userState'], clientState, ); From 36ba77d2fefb9797c960a69f34701fbde570cd6a Mon Sep 17 00:00:00 2001 From: Thomas Belin Date: Wed, 20 Dec 2023 16:08:04 +0100 Subject: [PATCH 2/5] migrate tests --- .../script/audio/AudioRepository.test.ts | 54 ++++++++----------- src/script/audio/AudioRepository.ts | 19 ++++--- src/script/main/app.ts | 2 +- test/helper/TestFactory.js | 2 +- 4 files changed, 34 insertions(+), 43 deletions(-) rename test/unit_tests/audio/AudioRepositorySpec.js => src/script/audio/AudioRepository.test.ts (71%) diff --git a/test/unit_tests/audio/AudioRepositorySpec.js b/src/script/audio/AudioRepository.test.ts similarity index 71% rename from test/unit_tests/audio/AudioRepositorySpec.js rename to src/script/audio/AudioRepository.test.ts index 05f5fcdae96..e84147750be 100644 --- a/test/unit_tests/audio/AudioRepositorySpec.js +++ b/src/script/audio/AudioRepository.test.ts @@ -29,29 +29,19 @@ describe('AudioRepository', () => { const audioRepository = new AudioRepository(); describe('init', () => { - it('inits all the sounds without preload', () => { - spyOn(window, 'Audio').and.callFake(function () { - this.load = () => {}; - spyOn(this, 'load'); - }); - audioRepository.init(); - - expect(window.Audio).toHaveBeenCalledTimes(Object.keys(AudioType).length); - Object.values(audioRepository.audioElements).forEach(audioElement => { - expect(audioElement.load).not.toHaveBeenCalled(); - }); - }); - it('inits all the sounds with preload', () => { - spyOn(window, 'Audio').and.callFake(function () { - this.load = () => {}; - spyOn(this, 'load'); - }); - audioRepository.init(true); + jest.spyOn(window, 'Audio').mockImplementation( + () => + ({ + load: jest.fn(), + }) as unknown as HTMLAudioElement, + ); + + audioRepository.init(); expect(window.Audio).toHaveBeenCalledTimes(Object.keys(AudioType).length); - Object.values(audioRepository.audioElements).forEach(audioElement => { + Object.values(audioRepository['audioElements']).forEach(audioElement => { expect(audioElement.load).toHaveBeenCalledTimes(1); }); }); @@ -59,24 +49,26 @@ describe('AudioRepository', () => { describe('play', () => { beforeEach(() => { - spyOn(window, 'Audio').and.callFake(function () { - this.load = () => {}; - this.play = () => {}; - this.paused = true; - spyOn(this, 'play'); - }); + jest.spyOn(window, 'Audio').mockImplementation( + () => + ({ + load: jest.fn(), + play: jest.fn(), + paused: true, + }) as unknown as HTMLAudioElement, + ); audioRepository.init(); audioRepository.setMutedState(NOTIFICATION_HANDLING_STATE.WEB_SOCKET); }); it('only plays muted allowed sounds when in muted state', () => { audioRepository.setAudioPreference(AudioPreference.NONE); - audioRepository.setMutedState('whatever'); + audioRepository.setMutedState(NOTIFICATION_HANDLING_STATE.RECOVERY); const forcedSounds = AudioPlayingType.MUTED; const forcedPromises = forcedSounds.map(audioId => { return audioRepository.play(audioId).then(() => { - expect(audioRepository.audioElements[audioId].play).toHaveBeenCalledTimes(1); + expect(audioRepository['audioElements'][audioId].play).toHaveBeenCalledTimes(1); }); }); @@ -84,7 +76,7 @@ describe('AudioRepository', () => { const ignoredPromises = ignoredSounds.map(audioId => { return audioRepository.play(audioId).then(() => { - expect(audioRepository.audioElements[audioId].play).not.toHaveBeenCalledTimes(1); + expect(audioRepository['audioElements'][audioId].play).not.toHaveBeenCalledTimes(1); }); }); @@ -97,7 +89,7 @@ describe('AudioRepository', () => { const allowedPromises = allowedSounds.map(audioId => { return audioRepository.play(audioId).then(() => { - expect(audioRepository.audioElements[audioId].play).toHaveBeenCalledTimes(1); + expect(audioRepository['audioElements'][audioId].play).toHaveBeenCalledTimes(1); }); }); @@ -105,7 +97,7 @@ describe('AudioRepository', () => { const ignoredPromises = ignoredSounds.map(audioId => { return audioRepository.play(audioId).then(() => { - expect(audioRepository.audioElements[audioId].play).not.toHaveBeenCalledTimes(1); + expect(audioRepository['audioElements'][audioId].play).not.toHaveBeenCalledTimes(1); }); }); @@ -118,7 +110,7 @@ describe('AudioRepository', () => { const testPromises = sounds.map(audioId => { return audioRepository.play(audioId).then(() => { - expect(audioRepository.audioElements[audioId].play).not.toHaveBeenCalled(); + expect(audioRepository['audioElements'][audioId].play).not.toHaveBeenCalled(); }); }); diff --git a/src/script/audio/AudioRepository.ts b/src/script/audio/AudioRepository.ts index 430fa0d64fd..039ba0fc4d7 100644 --- a/src/script/audio/AudioRepository.ts +++ b/src/script/audio/AudioRepository.ts @@ -44,7 +44,7 @@ export class AudioRepository { private readonly audioPreference = ko.observable(AudioPreference.ALL); private muted: boolean; - constructor(private readonly devicesHandler: MediaDevicesHandler) { + constructor(private readonly devicesHandler?: MediaDevicesHandler) { this.logger = getLogger('AudioRepository'); this.audioPreference.subscribe(audioPreference => { if (audioPreference === AudioPreference.NONE) { @@ -74,12 +74,11 @@ export class AudioRepository { return AUDIO_PLAY_PERMISSION.ALLOWED; } - private createAudioElement(sourcePath: string, preload: boolean): HTMLAudioElement { + private createAudioElement(sourcePath: string): HTMLAudioElement { const audioElement = new Audio(); - audioElement.preload = preload ? 'auto' : 'none'; - if (preload) { - audioElement.load(); - } + audioElement.preload = 'auto'; + // preload element + audioElement.load(); audioElement.src = sourcePath; return audioElement; } @@ -100,9 +99,9 @@ export class AudioRepository { return this.audioElements[audioId]; } - private initSounds(preload: boolean): void { + private initSounds(): void { Object.values(AudioType).forEach(audioId => { - this.audioElements[audioId] = this.createAudioElement(`./audio/${audioId}.mp3`, preload); + this.audioElements[audioId] = this.createAudioElement(`./audio/${audioId}.mp3`); }); } @@ -122,8 +121,8 @@ export class AudioRepository { } } - init(preload: boolean = false): void { - this.initSounds(preload); + init(): void { + this.initSounds(); this.updateSinkIds(); } diff --git a/src/script/main/app.ts b/src/script/main/app.ts index 376f2f42a7f..1f9452dd9c7 100644 --- a/src/script/main/app.ts +++ b/src/script/main/app.ts @@ -508,7 +508,7 @@ export class App { // start regularly polling the server to check if there is a new version of Wire startNewVersionPolling(Environment.version(false), this.update); } - audioRepository.init(true); + audioRepository.init(); await conversationRepository.cleanupEphemeralMessages(); callingRepository.setReady(); telemetry.timeStep(AppInitTimingsStep.APP_LOADED); diff --git a/test/helper/TestFactory.js b/test/helper/TestFactory.js index 7658d896d1d..9c7eff37a42 100644 --- a/test/helper/TestFactory.js +++ b/test/helper/TestFactory.js @@ -270,7 +270,7 @@ export class TestFactory { serverTimeHandler, this.user_repository, this.assetRepository, - new AudioRepository({}), + new AudioRepository(), this.user_repository['userState'], clientState, ); From be328506ab806499c87dfebcebe4e1582a6f0c58 Mon Sep 17 00:00:00 2001 From: Thomas Belin Date: Wed, 20 Dec 2023 16:17:08 +0100 Subject: [PATCH 3/5] remove arrow functions --- src/script/audio/AudioRepository.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/script/audio/AudioRepository.ts b/src/script/audio/AudioRepository.ts index 039ba0fc4d7..d748a744e42 100644 --- a/src/script/audio/AudioRepository.ts +++ b/src/script/audio/AudioRepository.ts @@ -145,7 +145,7 @@ export class AudioRepository { return audioElement.play(); } - play = async (audioId: AudioType, playInLoop: boolean = false): Promise => { + async play(audioId: AudioType, playInLoop: boolean = false): Promise { this.updateSinkIds(); const audioElement = this.getSoundById(audioId); if (!audioElement) { @@ -171,7 +171,7 @@ export class AudioRepository { this.logger.debug(`Playing '${audioId}' was disallowed because of user's preferences`); break; } - }; + } readonly setAudioPreference = (audioPreference: AudioPreference): void => { this.audioPreference(audioPreference); @@ -187,7 +187,7 @@ export class AudioRepository { } }; - readonly stop = (audioId: AudioType): void => { + stop(audioId: AudioType): void { const audioElement = this.getSoundById(audioId); if (!audioElement?.paused) { // This log is used by QA @@ -195,7 +195,7 @@ export class AudioRepository { audioElement.pause(); audioElement.load(); } - }; + } readonly updatedProperties = (properties: WebappProperties): void => { this.setAudioPreference(properties.settings.sound.alerts); From 6fb62e25ee1129c16990360407b8f95bb6ce7cfb Mon Sep 17 00:00:00 2001 From: Thomas Belin Date: Wed, 20 Dec 2023 16:32:30 +0100 Subject: [PATCH 4/5] fix types --- src/script/notification/NotificationRepository.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/script/notification/NotificationRepository.test.ts b/src/script/notification/NotificationRepository.test.ts index 8cdada68c67..931e18a6306 100644 --- a/src/script/notification/NotificationRepository.test.ts +++ b/src/script/notification/NotificationRepository.test.ts @@ -59,6 +59,7 @@ import {createUuid} from 'Util/uuid'; import {NotificationRepository} from './NotificationRepository'; +import {AudioRepository} from '../audio/AudioRepository'; import {CallState} from '../calling/CallState'; import {ConnectionEntity} from '../connection/ConnectionEntity'; import {ConversationState} from '../conversation/ConversationState'; @@ -72,6 +73,7 @@ function buildNotificationRepository() { const notificationRepository = new NotificationRepository( {} as any, new PermissionRepository(), + new AudioRepository(), userState, container.resolve(ConversationState), container.resolve(CallState), From 81bfdb02a9401db0d3398eb2c214a3f80d0c6929 Mon Sep 17 00:00:00 2001 From: Thomas Belin Date: Thu, 21 Dec 2023 08:42:06 +0100 Subject: [PATCH 5/5] fix tests --- src/script/conversation/MessageRepository.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/script/conversation/MessageRepository.test.ts b/src/script/conversation/MessageRepository.test.ts index a6df5cdee32..8499df657fc 100644 --- a/src/script/conversation/MessageRepository.test.ts +++ b/src/script/conversation/MessageRepository.test.ts @@ -39,6 +39,7 @@ import {ConversationRepository} from './ConversationRepository'; import {ConversationState} from './ConversationState'; import {AssetRepository} from '../assets/AssetRepository'; +import {AudioRepository} from '../audio/AudioRepository'; import {ClientEntity} from '../client/ClientEntity'; import {ClientState} from '../client/ClientState'; import {CryptographyRepository} from '../cryptography/CryptographyRepository'; @@ -99,6 +100,7 @@ async function buildMessageRepository(): Promise<[MessageRepository, MessageRepo assignAllClients: jest.fn().mockResolvedValue(true), } as unknown as UserRepository, assetRepository: {} as AssetRepository, + audioRepository: new AudioRepository(), userState, clientState, conversationState,