paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions 42 Packages Projects Releases Wiki Activity

Merge pull request #30545 from telia-oss/entity-filter-race-condition

Browse Source 
Entity filter race condition
This commit is contained in:
Fredrik Adelöw
2025-09-02 11:25:24 +02:00
committed by GitHub
parent bfd5e68ba8 629900817a
commit 3960b37a59
3 changed files with 134 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/cold-donuts-train.md
+5
View File
@@ -0,0 +1,5 @@
---
'@backstage/plugin-catalog-react': patch
---
Fix a potential race condition in EntityListProvider when selecting filters
plugins/catalog-react/src/hooks/useEntityListProvider.test.tsx
+62 -4
View File
@@ -14,7 +14,7 @@
* limitations under the License.
*/
import { catalogApiMock } from '@backstage/plugin-catalog-react/testUtils';
import type { GetEntitiesResponse } from '@backstage/catalog-client';
import { Entity } from '@backstage/catalog-model';
import {
alertApiRef,
@@ -23,7 +23,10 @@ import {
identityApiRef,
storageApiRef,
} from '@backstage/core-plugin-api';
import { translationApiRef } from '@backstage/core-plugin-api/alpha';
import { catalogApiMock } from '@backstage/plugin-catalog-react/testUtils';
import { mockApis, TestApiProvider } from '@backstage/test-utils';
import { useMountEffect } from '@react-hookz/web';
import { act, renderHook, waitFor } from '@testing-library/react';
import qs from 'qs';
import { PropsWithChildren } from 'react';
@@ -37,10 +40,9 @@ import {
EntityTypeFilter,
EntityUserFilter,
} from '../filters';
import { EntityListProvider, useEntityList } from './useEntityListProvider';
import { useMountEffect } from '@react-hookz/web';
import { translationApiRef } from '@backstage/core-plugin-api/alpha';
import { createDeferred } from '@backstage/types';
import { EntityListPagination } from '../types';
import { EntityListProvider, useEntityList } from './useEntityListProvider';
const entities: Entity[] = [
{
@@ -341,6 +343,62 @@ describe('<EntityListProvider />', () => {
filter: { kind: 'group' },
});
});
it('uses the last applied filter even if an earlier request finishes later', async () => {
const { result } = renderHook(() => useEntityList(), {
wrapper: createWrapper({ pagination }),
});
const firstResult = createDeferred<GetEntitiesResponse>();
const secondResult = createDeferred<GetEntitiesResponse>();
await waitFor(() => {
expect(result.current.backendEntities.length).toBeGreaterThan(0);
});
expect(result.current.totalItems).toBe(2);
expect(result.current.backendEntities.length).toBe(2);
expect(mockCatalogApi.getEntities).toHaveBeenCalledTimes(1);
mockCatalogApi.getEntities!.mockReturnValueOnce(firstResult);
await act(async () => {
result.current.updateFilters({
kind: new EntityKindFilter('api', 'API'),
});
});
await waitFor(() => {
expect(mockCatalogApi.getEntities).toHaveBeenNthCalledWith(2, {
filter: { kind: 'api' },
});
});
mockCatalogApi.getEntities!.mockReturnValueOnce(secondResult);
await act(async () => {
result.current.updateFilters({
kind: new EntityKindFilter('system', 'System'),
});
});
await waitFor(() => {
expect(mockCatalogApi.getEntities).toHaveBeenNthCalledWith(3, {
filter: { kind: 'system' },
});
});
await act(async () => {
secondResult.resolve({
items: [],
});
firstResult.resolve({
items: entities,
});
});
expect(result.current.filters.kind!.value).toBe('system');
expect(result.current.backendEntities.length).toBe(0);
});
});
describe('<EntityListProvider pagination />', () => {
plugins/catalog-react/src/hooks/useEntityListProvider.tsx
+67 -51
View File
@@ -14,7 +14,9 @@
* limitations under the License.
*/
import { QueryEntitiesResponse } from '@backstage/catalog-client';
import { Entity } from '@backstage/catalog-model';
import { useApi } from '@backstage/core-plugin-api';
import { compact, isEqual } from 'lodash';
import qs from 'qs';
import {
@@ -22,6 +24,7 @@ import {
PropsWithChildren,
useCallback,
useContext,
useEffect,
useMemo,
useState,
} from 'react';
@@ -50,8 +53,6 @@ import {
reduceCatalogFilters,
reduceEntityFilters,
} from '../utils/filters';
import { useApi } from '@backstage/core-plugin-api';
import { QueryEntitiesResponse } from '@backstage/catalog-client';
/** @public */
export type DefaultEntityFilters = {
@@ -239,7 +240,7 @@ export const EntityListProvider = <EntityFilters extends DefaultEntityFilters>(
// The main async filter worker. Note that while it has a lot of dependencies
// in terms of its implementation, the triggering only happens (debounced)
// based on the requested filters changing.
const [{ loading, error }, refresh] = useAsyncFn(
const [{ value: resolvedValue, loading, error }, refresh] = useAsyncFn(
async () => {
const kindValue =
requestedFilters.kind?.value?.toLocaleLowerCase('en-US');
@@ -249,19 +250,6 @@ export const EntityListProvider = <EntityFilters extends DefaultEntityFilters>(
: requestedFilters;
const compacted = compact(Object.values(adjustedFilters));
const queryParams = Object.keys(requestedFilters).reduce(
(params, key) => {
const filter = requestedFilters[key as keyof EntityFilters] as
| EntityFilter
| undefined;
if (filter?.toQueryValue) {
params[key] = filter.toQueryValue();
}
return params;
},
{} as Record<string, string | string[]>,
);
if (paginationMode !== 'none') {
if (cursor) {
if (cursor !== outputState.appliedCursor) {
@@ -270,14 +258,14 @@ export const EntityListProvider = <EntityFilters extends DefaultEntityFilters>(
cursor,
limit,
});
setOutputState({
return {
appliedFilters: requestedFilters,
appliedCursor: cursor,
backendEntities: response.items,
entities: response.items.filter(entityFilter),
pageInfo: response.pageInfo,
totalItems: response.totalItems,
});
};
}
} else {
const entityFilter = reduceEntityFilters(compacted);
@@ -296,7 +284,7 @@ export const EntityListProvider = <EntityFilters extends DefaultEntityFilters>(
limit,
offset,
});
setOutputState({
return {
appliedFilters: requestedFilters,
backendEntities: response.items,
entities: response.items.filter(entityFilter),
@@ -304,7 +292,7 @@ export const EntityListProvider = <EntityFilters extends DefaultEntityFilters>(
totalItems: response.totalItems,
limit,
offset,
});
};
}
}
} else {
@@ -324,43 +312,22 @@ export const EntityListProvider = <EntityFilters extends DefaultEntityFilters>(
filter: backendFilter,
});
const entities = response.items.filter(entityFilter);
setOutputState({
return {
appliedFilters: requestedFilters,
backendEntities: response.items,
entities,
totalItems: entities.length,
});
} else {
const entities = outputState.backendEntities.filter(entityFilter);
setOutputState({
appliedFilters: requestedFilters,
backendEntities: outputState.backendEntities,
entities,
totalItems: entities.length,
});
};
}
const entities = outputState.backendEntities.filter(entityFilter);
return {
appliedFilters: requestedFilters,
backendEntities: outputState.backendEntities,
entities,
totalItems: entities.length,
};
}
if (isMounted()) {
const oldParams = qs.parse(location.search, {
ignoreQueryPrefix: true,
});
const newParams = qs.stringify(
{
...oldParams,
filters: queryParams,
...(paginationMode === 'none' ? {} : { cursor, limit, offset }),
},
{ addQueryPrefix: true, arrayFormat: 'repeat' },
);
const newUrl = `${window.location.pathname}${newParams}`;
// We use direct history manipulation since useSearchParams and
// useNavigate in react-router-dom cause unnecessary extra rerenders.
// Also make sure to replace the state rather than pushing, since we
// don't want there to be back/forward slots for every single filter
// change.
window.history?.replaceState(null, document.title, newUrl);
}
return undefined;
},
[
catalogApi,
@@ -379,6 +346,55 @@ export const EntityListProvider = <EntityFilters extends DefaultEntityFilters>(
// filters will be calling this in rapid succession.
useDebounce(refresh, 10, [requestedFilters, cursor, limit, offset]);
useEffect(() => {
if (resolvedValue === undefined) {
return;
}
setOutputState(resolvedValue);
if (isMounted()) {
const queryParams = Object.keys(requestedFilters).reduce(
(params, key) => {
const filter = requestedFilters[key as keyof EntityFilters] as
| EntityFilter
| undefined;
if (filter?.toQueryValue) {
params[key] = filter.toQueryValue();
}
return params;
},
{} as Record<string, string | string[]>,
);
const oldParams = qs.parse(location.search, {
ignoreQueryPrefix: true,
});
const newParams = qs.stringify(
{
...oldParams,
filters: queryParams,
...(paginationMode === 'none' ? {} : { cursor, limit, offset }),
},
{ addQueryPrefix: true, arrayFormat: 'repeat' },
);
const newUrl = `${window.location.pathname}${newParams}`;
// We use direct history manipulation since useSearchParams and
// useNavigate in react-router-dom cause unnecessary extra rerenders.
// Also make sure to replace the state rather than pushing, since we
// don't want there to be back/forward slots for every single filter
// change.
window.history?.replaceState(null, document.title, newUrl);
}
}, [
cursor,
isMounted,
limit,
location.search,
offset,
requestedFilters,
resolvedValue,
paginationMode,
]);
const updateFilters = useCallback(
(
update: