mirror of
https://github.com/Unleash/unleash.git
synced 2025-09-24 17:51:14 +02:00
fix: cache eviction bug and the eternal loading screen (#10689)
Fixes an issue where the project feature list (and potentially other places in the app that use the `useClearSWRCache` hook) would end up in an infinite loading screen because the latest entry that we want to show was overwritten. The primary reason this happened is that we used `keysToDelete = array.slice(SWR_CACHE_SIZE - 1)`. Because the map keys are returned in insertion order, this would make us never delete the oldest keys, but always anything after the cache reached it's maximum size. The fix was to instead do `slice(0, -(SWR_CACHE_SIZE - 1))`, unless that is `0, 0`. If so, then just delete the entire filtered keys set. As a bonus: this PR also deduplicates cache entries that have the same query params but in different order for the feature search. This further reduces the cache space needed.
This commit is contained in:
parent
3bb317ad6d
commit
e3fc88b11f
@ -2,7 +2,10 @@ import type { FC } from 'react';
|
||||
import { render, screen } from '@testing-library/react';
|
||||
import '@testing-library/jest-dom';
|
||||
import { testServerRoute, testServerSetup } from 'utils/testServer';
|
||||
import { useFeatureSearch } from './useFeatureSearch.ts';
|
||||
import {
|
||||
getFeatureSearchFetcher,
|
||||
useFeatureSearch,
|
||||
} from './useFeatureSearch.ts';
|
||||
import { useSWRConfig } from 'swr';
|
||||
|
||||
const server = testServerSetup();
|
||||
@ -93,4 +96,18 @@ describe('useFeatureSearch', () => {
|
||||
rerender(<TestComponent params={{ project }} />);
|
||||
await screen.findByText(/Total: 0/);
|
||||
});
|
||||
|
||||
test('should give the same cache key, regardless of parameter order', async () => {
|
||||
const params1 = {
|
||||
query: 'query-string',
|
||||
project: 'IS:project',
|
||||
};
|
||||
const params2 = {
|
||||
project: 'IS:project',
|
||||
query: 'query-string',
|
||||
};
|
||||
const { KEY: key1 } = getFeatureSearchFetcher(params1);
|
||||
const { KEY: key2 } = getFeatureSearchFetcher(params2);
|
||||
expect(key1).toEqual(key2);
|
||||
});
|
||||
});
|
||||
|
@ -100,12 +100,13 @@ const createFeatureSearch = () => {
|
||||
|
||||
export const DEFAULT_PAGE_LIMIT = 25;
|
||||
|
||||
const getFeatureSearchFetcher = (params: SearchFeaturesParams) => {
|
||||
export const getFeatureSearchFetcher = (params: SearchFeaturesParams) => {
|
||||
const urlSearchParams = new URLSearchParams(
|
||||
Array.from(
|
||||
Object.entries(params)
|
||||
.filter(([_, value]) => !!value)
|
||||
.map(([key, value]) => [key, value.toString()]), // TODO: parsing non-string parameters
|
||||
.map(([key, value]) => [key, value.toString()]) // TODO: parsing non-string parameters
|
||||
.toSorted(),
|
||||
),
|
||||
).toString();
|
||||
const KEY = `${PATH}${urlSearchParams}`;
|
||||
|
@ -14,7 +14,7 @@ describe('manageCacheEntries', () => {
|
||||
expect(cacheMock.has('prefix-3')).toBe(true);
|
||||
});
|
||||
|
||||
it('should keep the SWR_CACHE_SIZE entries and delete the rest', () => {
|
||||
it('should keep the SWR_CACHE_SIZE entries and delete the rest, keeping the most recent entries', () => {
|
||||
const cacheMock = new Map();
|
||||
cacheMock.set('prefix-1', {});
|
||||
cacheMock.set('prefix-2', {});
|
||||
@ -23,8 +23,19 @@ describe('manageCacheEntries', () => {
|
||||
|
||||
clearCacheEntries(cacheMock, 'prefix-4', 'prefix-', 2);
|
||||
|
||||
expect(cacheMock.has('prefix-4')).toBe(true);
|
||||
expect([...cacheMock.keys()].length).toBe(2);
|
||||
expect([...cacheMock.keys()]).toStrictEqual(['prefix-3', 'prefix-4']);
|
||||
});
|
||||
|
||||
it('should not delete the current key, even if it would be cleared as part of the cache size limit', () => {
|
||||
const cacheMock = new Map();
|
||||
cacheMock.set('prefix-1', {});
|
||||
cacheMock.set('prefix-2', {});
|
||||
cacheMock.set('prefix-3', {});
|
||||
cacheMock.set('prefix-4', {});
|
||||
|
||||
clearCacheEntries(cacheMock, 'prefix-2', 'prefix-', 2);
|
||||
|
||||
expect([...cacheMock.keys()]).toStrictEqual(['prefix-2', 'prefix-4']);
|
||||
});
|
||||
|
||||
it('should handle case when SWR_CACHE_SIZE is larger than number of entries', () => {
|
||||
@ -50,4 +61,16 @@ describe('manageCacheEntries', () => {
|
||||
expect(cacheMock.has('other-2')).toBe(true);
|
||||
expect(cacheMock.has('prefix-3')).toBe(true);
|
||||
});
|
||||
|
||||
it('treats a negative cache size as zero', () => {
|
||||
const cacheMock = new Map();
|
||||
cacheMock.set('prefix-1', {});
|
||||
cacheMock.set('prefix-2', {});
|
||||
cacheMock.set('prefix-3', {});
|
||||
cacheMock.set('prefix-4', {});
|
||||
|
||||
clearCacheEntries(cacheMock, 'prefix-3', 'prefix-', -1);
|
||||
|
||||
expect([...cacheMock.keys()]).toStrictEqual(['prefix-3']);
|
||||
});
|
||||
});
|
||||
|
@ -13,7 +13,12 @@ export const clearCacheEntries = (
|
||||
const filteredKeys = keys.filter(
|
||||
(key) => key.startsWith(clearPrefix) && key !== currentKey,
|
||||
);
|
||||
const keysToDelete = filteredKeys.slice(SWR_CACHE_SIZE - 1);
|
||||
|
||||
const entriesToLeave = SWR_CACHE_SIZE - 1;
|
||||
const keysToDelete =
|
||||
entriesToLeave <= 0
|
||||
? filteredKeys
|
||||
: filteredKeys.slice(0, -entriesToLeave);
|
||||
|
||||
keysToDelete.forEach((key) => cache.delete(key));
|
||||
};
|
||||
|
Loading…
Reference in New Issue
Block a user