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.

Sign up for GitHub

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

[Audit][Implementation] getOrderedReportIDs - cache removal #3

Closed
wants to merge 1 commit into from
Closed

[Audit][Implementation] getOrderedReportIDs - cache removal #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 0 additions & 45 deletions src/libs/SidebarUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
/* eslint-disable rulesdir/prefer-underscore-method */
import Str from 'expensify-common/lib/str';
import type {OnyxCollection} from 'react-native-onyx';
import Onyx from 'react-native-onyx';
import type {ValueOf} from 'type-fest';
import CONST from '@src/CONST';
Expand Down Expand Up @@ -94,22 +93,6 @@ function setIsSidebarLoadedReady() {
resolveSidebarIsReadyPromise();
}

// Define a cache object to store the memoized results
const reportIDsCache = new Map<string, string[]>();

// Function to set a key-value pair while maintaining the maximum key limit
function setWithLimit<TKey, TValue>(map: Map<TKey, TValue>, key: TKey, value: TValue) {
if (map.size >= 5) {
// If the map has reached its limit, remove the first (oldest) key-value pair
const firstKey = map.keys().next().value;
map.delete(firstKey);
}
map.set(key, value);
}

// Variable to verify if ONYX actions are loaded
let hasInitialReportActions = false;

/**
* @returns An array of reportIDs sorted in the proper order
*/
Expand All @@ -119,34 +102,7 @@ function getOrderedReportIDs(
betas: Beta[],
policies: Record<string, Policy>,
priorityMode: ValueOf<typeof CONST.PRIORITY_MODE>,
allReportActions: OnyxCollection<ReportAction[]>,
): string[] {
// Generate a unique cache key based on the function arguments
const cachedReportsKey = JSON.stringify(
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
[currentReportId, allReports, betas, policies, priorityMode, allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${currentReportId}`]?.length || 1],
(key, value: unknown) => {
/**
* Exclude some properties not to overwhelm a cached key value with huge data,
* which we don't need to store in a cacheKey
*/
if (key === 'participantAccountIDs' || key === 'participants' || key === 'lastMessageText' || key === 'visibleChatMemberAccountIDs') {
return undefined;
}

return value;
},
);

// Check if the result is already in the cache
const cachedIDs = reportIDsCache.get(cachedReportsKey);
if (cachedIDs && hasInitialReportActions) {
return cachedIDs;
}

// This is needed to prevent caching when Onyx is empty for a second render
hasInitialReportActions = Object.values(lastReportActions).length > 0;

const isInGSDMode = priorityMode === CONST.PRIORITY_MODE.GSD;
const isInDefaultMode = !isInGSDMode;
const allReportsDictValues = Object.values(allReports);
Expand Down Expand Up @@ -219,7 +175,6 @@ function getOrderedReportIDs(
// Now that we have all the reports grouped and sorted, they must be flattened into an array and only return the reportID.
// The order the arrays are concatenated in matters and will determine the order that the groups are displayed in the sidebar.
const LHNReports = [...pinnedAndGBRReports, ...draftReports, ...nonArchivedReports, ...archivedReports].map((report) => report.reportID);
setWithLimit(reportIDsCache, cachedReportsKey, LHNReports);
return LHNReports;
}

Expand Down
48 changes: 5 additions & 43 deletions src/pages/home/sidebar/SidebarLinksData.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {deepEqual} from 'fast-equals';
import lodashGet from 'lodash/get';
import PropTypes from 'prop-types';
import React, {useCallback, useMemo, useRef} from 'react';
import {View} from 'react-native';
Expand All @@ -25,22 +24,6 @@ const propTypes = {
/** List of reports */
chatReports: PropTypes.objectOf(reportPropTypes),

/** All report actions for all reports */
allReportActions: PropTypes.objectOf(
PropTypes.arrayOf(
PropTypes.shape({
error: PropTypes.string,
message: PropTypes.arrayOf(
PropTypes.shape({
moderationDecision: PropTypes.shape({
decision: PropTypes.string,
}),
}),
),
}),
),
),

/** Whether the reports are loading. When false it means they are ready to be used. */
isLoadingApp: PropTypes.bool,

Expand All @@ -59,21 +42,20 @@ const propTypes = {

const defaultProps = {
chatReports: {},
allReportActions: {},
isLoadingApp: true,
priorityMode: CONST.PRIORITY_MODE.DEFAULT,
betas: [],
policies: {},
};

function SidebarLinksData({isFocused, allReportActions, betas, chatReports, currentReportID, insets, isLoadingApp, onLinkClick, policies, priorityMode, network}) {
function SidebarLinksData({isFocused, betas, chatReports, currentReportID, insets, isLoadingApp, onLinkClick, policies, priorityMode, network}) {
const styles = useThemeStyles();
const {translate} = useLocalize();

const reportIDsRef = useRef(null);
const isLoading = isLoadingApp;
const optionListItems = useMemo(() => {
const reportIDs = SidebarUtils.getOrderedReportIDs(null, chatReports, betas, policies, priorityMode, allReportActions);
const reportIDs = SidebarUtils.getOrderedReportIDs(null, chatReports, betas, policies, priorityMode);

if (deepEqual(reportIDsRef.current, reportIDs)) {
return reportIDsRef.current;
Expand All @@ -85,7 +67,7 @@ function SidebarLinksData({isFocused, allReportActions, betas, chatReports, curr
reportIDsRef.current = reportIDs;
}
return reportIDsRef.current || [];
}, [allReportActions, betas, chatReports, policies, priorityMode, isLoading, network.isOffline]);
}, [betas, chatReports, policies, priorityMode, isLoading, network.isOffline]);

// We need to make sure the current report is in the list of reports, but we do not want
// to have to re-generate the list every time the currentReportID changes. To do that
Expand All @@ -94,10 +76,10 @@ function SidebarLinksData({isFocused, allReportActions, betas, chatReports, curr
// case we re-generate the list a 2nd time with the current report included.
const optionListItemsWithCurrentReport = useMemo(() => {
if (currentReportID && !_.contains(optionListItems, currentReportID)) {
return SidebarUtils.getOrderedReportIDs(currentReportID, chatReports, betas, policies, priorityMode, allReportActions);
return SidebarUtils.getOrderedReportIDs(currentReportID, chatReports, betas, policies, priorityMode);
}
return optionListItems;
}, [currentReportID, optionListItems, chatReports, betas, policies, priorityMode, allReportActions]);
}, [currentReportID, optionListItems, chatReports, betas, policies, priorityMode]);

const currentReportIDRef = useRef(currentReportID);
currentReportIDRef.current = currentReportID;
Expand Down Expand Up @@ -171,21 +153,6 @@ const chatReportSelector = (report) =>
isDeletedParentAction: report.isDeletedParentAction,
};

/**
* @param {Object} [reportActions]
* @returns {Object|undefined}
*/
const reportActionsSelector = (reportActions) =>
reportActions &&
_.map(reportActions, (reportAction) => ({
errors: lodashGet(reportAction, 'errors', []),
message: [
{
moderationDecision: {decision: lodashGet(reportAction, 'message[0].moderationDecision.decision')},
},
],
}));

/**
* @param {Object} [policy]
* @returns {Object|undefined}
Expand Down Expand Up @@ -218,11 +185,6 @@ export default compose(
key: ONYXKEYS.BETAS,
initialValue: [],
},
allReportActions: {
key: ONYXKEYS.COLLECTION.REPORT_ACTIONS,
selector: reportActionsSelector,
initialValue: {},
},
policies: {
key: ONYXKEYS.COLLECTION.POLICY,
selector: policySelector,
Expand Down
Loading