Skip to content
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

Resolves offline sound playback by caching assets locally #50130

Merged
merged 14 commits into from
Oct 21, 2024
2 changes: 0 additions & 2 deletions config/webpack/webpack.common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,8 +227,6 @@ const getCommonConfiguration = ({file = '.env', platform = 'web'}: Environment):
'react-native-config': 'react-web-config',
// eslint-disable-next-line @typescript-eslint/naming-convention
'react-native$': 'react-native-web',
// eslint-disable-next-line @typescript-eslint/naming-convention
'react-native-sound': 'react-native-web-sound',
// Module alias for web & desktop
// https://webpack.js.org/configuration/resolve/#resolvealias
// eslint-disable-next-line @typescript-eslint/naming-convention
Expand Down
22 changes: 10 additions & 12 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@
"expo-image-manipulator": "12.0.5",
"fast-equals": "^4.0.3",
"focus-trap-react": "^10.2.3",
"howler": "^2.2.4",
"htmlparser2": "^7.2.0",
"idb-keyval": "^6.2.1",
"lodash-es": "4.17.21",
Expand Down Expand Up @@ -170,7 +171,6 @@
"react-native-view-shot": "3.8.0",
"react-native-vision-camera": "4.0.0-beta.13",
"react-native-web": "^0.19.12",
"react-native-web-sound": "^0.1.3",
"react-native-webview": "13.8.6",
"react-plaid-link": "3.3.2",
"react-web-config": "^1.0.0",
Expand Down Expand Up @@ -228,6 +228,7 @@
"@types/base-64": "^1.0.2",
"@types/canvas-size": "^1.2.2",
"@types/concurrently": "^7.0.0",
"@types/howler": "^2.2.12",
"@types/jest": "^29.5.2",
"@types/jest-when": "^3.5.2",
"@types/js-yaml": "^4.0.5",
Expand Down
59 changes: 59 additions & 0 deletions src/libs/Sound/BaseSound.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import Onyx from 'react-native-onyx';
import ONYXKEYS from '@src/ONYXKEYS';

let isMuted = false;

Onyx.connect({
key: ONYXKEYS.USER,
callback: (val) => (isMuted = !!val?.isMutedAllSounds),
});

const SOUNDS = {
DONE: 'done',
SUCCESS: 'success',
ATTENTION: 'attention',
RECEIVE: 'receive',
} as const;

const getIsMuted = () => isMuted;

/**
* Creates a version of the given function that, when called, queues the execution and ensures that
* calls are spaced out by at least the specified `minExecutionTime`, even if called more frequently. This allows
* for throttling frequent calls to a function, ensuring each is executed with a minimum `minExecutionTime` between calls.
* Each call returns a promise that resolves when the function call is executed, allowing for asynchronous handling.
*/
function withMinimalExecutionTime<F extends (...args: Parameters<F>) => ReturnType<F>>(func: F, minExecutionTime: number) {
const queue: Array<[() => ReturnType<F>, (value?: unknown) => void]> = [];
let timerId: NodeJS.Timeout | null = null;

function processQueue() {
if (queue.length > 0) {
const next = queue.shift();

if (!next) {
return;
}

const [nextFunc, resolve] = next;
nextFunc();
resolve();
timerId = setTimeout(processQueue, minExecutionTime);
} else {
timerId = null;
}
}

return function (...args: Parameters<F>) {
return new Promise((resolve) => {
queue.push([() => func(...args), resolve]);

if (!timerId) {
// If the timer isn't running, start processing the queue
processQueue();
}
});
};
}

export {SOUNDS, withMinimalExecutionTime, getIsMuted};
19 changes: 19 additions & 0 deletions src/libs/Sound/index.native.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import Sound from 'react-native-sound';
import type {ValueOf} from 'type-fest';
import {getIsMuted, SOUNDS, withMinimalExecutionTime} from './BaseSound';
import config from './config';

const playSound = (soundFile: ValueOf<typeof SOUNDS>) => {
const sound = new Sound(`${config.prefix}${soundFile}.mp3`, Sound.MAIN_BUNDLE, (error) => {
if (error || getIsMuted()) {
return;
}

sound.play();
});
};

function clearSoundAssetsCache() {}

export {SOUNDS, clearSoundAssetsCache};
export default withMinimalExecutionTime(playSound, 300);
127 changes: 75 additions & 52 deletions src/libs/Sound/index.ts
Original file line number Diff line number Diff line change
@@ -1,71 +1,94 @@
import Onyx from 'react-native-onyx';
import Sound from 'react-native-sound';
import {Howl} from 'howler';
import type {ValueOf} from 'type-fest';
import ONYXKEYS from '@src/ONYXKEYS';
import Log from '@libs/Log';
import {getIsMuted, SOUNDS, withMinimalExecutionTime} from './BaseSound';
import config from './config';

let isMuted = false;
function cacheSoundAssets() {
// Exit early if the Cache API is not available in the current browser.
if (!('caches' in window)) {
return;
}

Onyx.connect({
key: ONYXKEYS.USER,
callback: (val) => (isMuted = !!val?.isMutedAllSounds),
});
caches.open('sound-assets').then((cache) => {
const soundFiles = Object.values(SOUNDS).map((sound) => `${config.prefix}${sound}.mp3`);

const SOUNDS = {
DONE: 'done',
SUCCESS: 'success',
ATTENTION: 'attention',
RECEIVE: 'receive',
} as const;
// Cache each sound file if it's not already cached.
const cachePromises = soundFiles.map((soundFile) => {
return cache.match(soundFile).then((response) => {
if (response) {
return;
}
return cache.add(soundFile);
});
});

Comment on lines +16 to +24
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential Issue: Stale Cache Entries
Clients may continue using outdated cached sound files if the server updates them. To ensure users always have the latest sounds, we should refresh the cache on every app load.

Copy link
Contributor Author

@wildan-m wildan-m Oct 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rayane-djouah Not sure about this suggestion, but is it better to use in-memory cache if we want to update the sound assets on each page load?

Replacing the cache during the page load would result in fetching assets from the remote URL every time we load.

I think server will rarely update sound assets. To optimize, how about clearing sound assets during logout and resetting the app on the troubleshooting page?

src/libs/Sound/index.ts

function clearSoundAssetsCache() {
    // Exit early if the Cache API is not available in the current browser.
    if (!('caches' in window)) {
        return;
    }

    caches.delete('sound-assets').then((success) => {
        if (success) {
            console.log('Sound assets cache cleared.');
        } else {
            console.log('Failed to clear sound assets cache.');
        }
    }).catch((error) => {
        console.error('Error clearing sound assets cache:', error);
    });
}

And execute that function in:

src/libs/actions/App.ts

function clearOnyxAndResetApp(shouldNavigateToHomepage?: boolean) {
......
    clearSoundAssetsCache();
.....

And for clear action in Troubleshoot page:

src/libs/actions/App.ts

function clearOnyxAndResetApp(shouldNavigateToHomepage?: boolean) {
.........
    clearSoundAssetsCache();
}
Kapture.2024-10-04.at.08.01.29.mp4

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wildan-m, I agree that clearing the cache upon logout and during troubleshooting is a smart approach to handle updates to sound assets.

To complement this, we could consider a versioned cache strategy. This involves appending a version number to the cache name. We can update the cache version whenever we change or add a sound.

// Update this version string when new sound files are added or existing ones are changed to invalidate old caches and prompt caching of the new assets.
const CURRENT_CACHE_VERSION = 'sound-assets-v1';

caches.open(CURRENT_CACHE_VERSION).then(...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rayane-djouah how would we determine version? usually server will send the version to invalidate the cache. Is there any API for that purpose?

Copy link
Contributor Author

@wildan-m wildan-m Oct 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another common approach is version the file name sound-v1.mp3, since the name already stored as key to put the cache I think we don't need any other change to version the cache in FE

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The updated main has been merged and clearSoundAssetsCache has been implemented

/**
* Creates a version of the given function that, when called, queues the execution and ensures that
* calls are spaced out by at least the specified `minExecutionTime`, even if called more frequently. This allows
* for throttling frequent calls to a function, ensuring each is executed with a minimum `minExecutionTime` between calls.
* Each call returns a promise that resolves when the function call is executed, allowing for asynchronous handling.
*/
function withMinimalExecutionTime<F extends (...args: Parameters<F>) => ReturnType<F>>(func: F, minExecutionTime: number) {
const queue: Array<[() => ReturnType<F>, (value?: unknown) => void]> = [];
let timerId: NodeJS.Timeout | null = null;
return Promise.all(cachePromises);
});
}

function processQueue() {
if (queue.length > 0) {
const next = queue.shift();
const initializeAndPlaySound = (src: string) => {
const sound = new Howl({
src: [src],
format: ['mp3'],
onloaderror: (_id: number, error: unknown) => {
Log.alert('[sound] Load error:', {message: (error as Error).message});
},
onplayerror: (_id: number, error: unknown) => {
Log.alert('[sound] Play error:', {message: (error as Error).message});
},
});
sound.play();
};

if (!next) {
const playSound = (soundFile: ValueOf<typeof SOUNDS>) => {
if (getIsMuted()) {
return;
}

const soundSrc = `${config.prefix}${soundFile}.mp3`;

if (!('caches' in window)) {
// Fallback to fetching from network if not in cache
initializeAndPlaySound(soundSrc);
return;
}

caches.open('sound-assets').then((cache) => {
cache.match(soundSrc).then((response) => {
if (response) {
response.blob().then((soundBlob) => {
const soundUrl = URL.createObjectURL(soundBlob);
initializeAndPlaySound(soundUrl);
});
return;
}
initializeAndPlaySound(soundSrc);
});
});
};

const [nextFunc, resolve] = next;
nextFunc();
resolve();
timerId = setTimeout(processQueue, minExecutionTime);
} else {
timerId = null;
}
function clearSoundAssetsCache() {
// Exit early if the Cache API is not available in the current browser.
if (!('caches' in window)) {
return;
}

return function (...args: Parameters<F>) {
return new Promise((resolve) => {
queue.push([() => func(...args), resolve]);

if (!timerId) {
// If the timer isn't running, start processing the queue
processQueue();
caches
.delete('sound-assets')
.then((success) => {
if (success) {
return;
}
Log.alert('[sound] Failed to clear sound assets cache.');
})
.catch((error) => {
Log.alert('[sound] Error clearing sound assets cache:', {message: (error as Error).message});
});
};
}

const playSound = (soundFile: ValueOf<typeof SOUNDS>) => {
const sound = new Sound(`${config.prefix}${soundFile}.mp3`, Sound.MAIN_BUNDLE, (error) => {
if (error || isMuted) {
return;
}

sound.play();
});
};
// Cache sound assets on load
cacheSoundAssets();

export {SOUNDS};
export {SOUNDS, clearSoundAssetsCache};
export default withMinimalExecutionTime(playSound, 300);
2 changes: 2 additions & 0 deletions src/libs/actions/App.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import Navigation from '@libs/Navigation/Navigation';
import Performance from '@libs/Performance';
import * as ReportActionsUtils from '@libs/ReportActionsUtils';
import * as SessionUtils from '@libs/SessionUtils';
import {clearSoundAssetsCache} from '@libs/Sound';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type {OnyxKey} from '@src/ONYXKEYS';
Expand Down Expand Up @@ -559,6 +560,7 @@ function clearOnyxAndResetApp(shouldNavigateToHomepage?: boolean) {
});
});
});
clearSoundAssetsCache();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will not it crash mobile app? It looks like clearSoundAssetsCache is implemented only on web and such function doesn't exist on mobile? Correct me if I'm wrong 🙏

Also, why do we need to clear a cache on logout? In case if we want to replace sound we can just change name and we'll simply download new version of the file? Don't you think that clearSoundAssetsCache is kind of over engineering?

Copy link
Contributor Author

@wildan-m wildan-m Oct 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kirillzyusko

Will not it crash mobile app?

I think it won't crash, there is check if (!('caches' in window)) { and also try catch, but let me know if we need to extract the function and create platform specific code

function clearSoundAssetsCache() {
    // Exit early if the Cache API is not available in the current browser.
    if (!('caches' in window)) {
        return;
    }

    caches
        .delete('sound-assets')
        .then((success) => {
            if (success) {
                return;
            }
            Log.alert('[sound] Failed to clear sound assets cache.');
        })
        .catch((error) => {
            Log.alert('[sound] Error clearing sound assets cache:', {message: (error as Error).message});
        });

Also, why do we need to clear a cache on logout?

The assets might be replaced without changing the name.

we can just change name and we'll simply download new version of the file?

replacing the name might need code change to the assets references and we might need to rebuild the app each time we change the name

Don't you think that clearSoundAssetsCache is kind of over engineering?

I don't think so, the code is quiet simple and it's also make sense to make it as part of app reset

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it won't crash, there is check if (!('caches' in window)) {

I think clearSoundAssetsCache will be undefined and you will receive undefined is not a function. Did you test native code in this PR?

The assets might be replaced without changing the name.

At the moment only developer can update asset. So if they update it without changing a name it'll be kind of developer fault...

replacing the name might need code change to the assets references and we might need to rebuild the app each time we change the name

We can replace only name of SOUNDS, i. e.:

const SOUNDS = {
    DONE: 'done-v1',
    SUCCESS: 'success',
    ATTENTION: 'attention',
    RECEIVE: 'receive',
} as const;

And other code will be untouched 🤷‍♂️

I don't think so, the code is quiet simple and it's also make sense to make it as part of app reset

Oki doki, just wanted to be sure, that alternative solution also has been considered 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think clearSoundAssetsCache will be undefined and you will receive undefined is not a function.

@kirillzyusko you are right, I've updated the native implementation to empty function. thank you!

}

export {
Expand Down
2 changes: 2 additions & 0 deletions src/libs/actions/Session/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import NetworkConnection from '@libs/NetworkConnection';
import * as Pusher from '@libs/Pusher/pusher';
import * as ReportUtils from '@libs/ReportUtils';
import * as SessionUtils from '@libs/SessionUtils';
import {clearSoundAssetsCache} from '@libs/Sound';
import Timers from '@libs/Timers';
import {hideContextMenu} from '@pages/home/report/ContextMenu/ReportActionContextMenu';
import {KEYS_TO_PRESERVE, openApp} from '@userActions/App';
Expand Down Expand Up @@ -761,6 +762,7 @@ function cleanupSession() {
clearCache().then(() => {
Log.info('Cleared all cache data', true, {}, true);
});
clearSoundAssetsCache();
Timing.clearData();
}

Expand Down
Loading