diff --git a/packages/catalog-client/src/CatalogClient.test.ts b/packages/catalog-client/src/CatalogClient.test.ts index 7ce9e94396..819fa70381 100644 --- a/packages/catalog-client/src/CatalogClient.test.ts +++ b/packages/catalog-client/src/CatalogClient.test.ts @@ -594,17 +594,6 @@ describe('CatalogClient', () => { expect(response.totalItems).toBe(2); }); - it('should throw error when both filter and query are provided', async () => { - await expect( - client.queryEntities({ - filter: { kind: 'component' }, - query: { kind: 'component' }, - } as any), - ).rejects.toThrow( - 'Cannot specify both "filter" and "query" in the same request', - ); - }); - it('should support $all operator', async () => { const mockedEndpoint = jest.fn().mockImplementation((req, res, ctx) => { expect(req.body).toMatchObject({ diff --git a/packages/catalog-client/src/CatalogClient.ts b/packages/catalog-client/src/CatalogClient.ts index 62111abf9e..bd557cfdd2 100644 --- a/packages/catalog-client/src/CatalogClient.ts +++ b/packages/catalog-client/src/CatalogClient.ts @@ -274,13 +274,6 @@ export class CatalogClient implements CatalogApi { ): Promise { const isInitialRequest = isQueryEntitiesInitialRequest(request); - // Validate that filter and query are mutually exclusive - if (isInitialRequest && request.filter && request.query) { - throw new Error( - 'Cannot specify both "filter" and "query" in the same request. Use "filter" for traditional key-value filtering or "query" for predicate-based filtering.', - ); - } - // Route to POST endpoint if query predicate is provided (initial request) if (isInitialRequest && request.query) { return this.queryEntitiesByPredicate(request, options); diff --git a/packages/catalog-client/src/types/api.ts b/packages/catalog-client/src/types/api.ts index 8bf13271b2..2dffb35570 100644 --- a/packages/catalog-client/src/types/api.ts +++ b/packages/catalog-client/src/types/api.ts @@ -593,6 +593,7 @@ export interface CatalogApi { * const response = await catalogClient.queryEntities({ * filter: [{ kind: 'group' }], * limit: 20, + * fields: ['metadata', 'kind'], * fullTextFilter: { * term: 'A', * }, @@ -609,11 +610,15 @@ export interface CatalogApi { * * ``` * const secondBatchResponse = await catalogClient - * .queryEntities({ cursor: response.nextCursor }); + * .queryEntities({ + * cursor: response.nextCursor, + * limit: 20, + * fields: ['metadata', 'kind'], + * }); * ``` * - * secondBatchResponse will contain the next batch of (maximum) 20 entities, - * together with a prevCursor property, useful to fetch the previous batch. + * `secondBatchResponse` will contain the next batch of (maximum) 20 entities, + * together with a `prevCursor` property, useful to fetch the previous batch. * * @public * diff --git a/plugins/catalog-backend/src/catalog/types.ts b/plugins/catalog-backend/src/catalog/types.ts index dda7e9a811..5d733ad14c 100644 --- a/plugins/catalog-backend/src/catalog/types.ts +++ b/plugins/catalog-backend/src/catalog/types.ts @@ -215,7 +215,6 @@ export interface QueryEntitiesInitialRequest { filter?: EntityFilter; /** * Predicate-based query for filtering entities. - * Mutually exclusive with filter. */ query?: FilterPredicate; orderFields?: EntityOrder[]; @@ -281,7 +280,6 @@ export type Cursor = { filter?: EntityFilter; /** * A predicate-based query to be applied to the full list of entities. - * Mutually exclusive with filter. */ query?: FilterPredicate; /** diff --git a/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.test.ts b/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.test.ts index f074f25397..85fbd16eeb 100644 --- a/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.test.ts +++ b/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.test.ts @@ -308,7 +308,7 @@ describe('AuthorizedEntitiesCatalog', () => { }); }); - it('combines permission filter into query field using $all on CONDITIONAL with initial request', async () => { + it('passes through query alongside permission filter on CONDITIONAL with initial request', async () => { fakePermissionApi.authorizeConditional.mockResolvedValue([ { result: AuthorizeResult.CONDITIONAL, @@ -335,7 +335,8 @@ describe('AuthorizedEntitiesCatalog', () => { nextCursor: { isPrevious: false, orderFieldValues: ['xxx', null], - query: { $all: [{ kind: 'b' }, userQuery] }, + query: userQuery, + filter: { key: 'kind', values: ['b'] }, orderFields: [{ field: 'name', order: 'asc' }], }, }, @@ -351,8 +352,8 @@ describe('AuthorizedEntitiesCatalog', () => { expect(fakeCatalog.queryEntities).toHaveBeenCalledWith({ credentials: mockCredentials.none(), - query: { $all: [{ kind: 'b' }, userQuery] }, - filter: undefined, + query: userQuery, + filter: { key: 'kind', values: ['b'] }, }); expect(response.pageInfo.nextCursor).toEqual({ @@ -364,7 +365,7 @@ describe('AuthorizedEntitiesCatalog', () => { }); }); - it('combines permission filter into cursor query field using $all on CONDITIONAL with cursor request', async () => { + it('passes through cursor query alongside permission filter on CONDITIONAL with cursor request', async () => { fakePermissionApi.authorizeConditional.mockResolvedValue([ { result: AuthorizeResult.CONDITIONAL, @@ -391,13 +392,15 @@ describe('AuthorizedEntitiesCatalog', () => { nextCursor: { isPrevious: false, orderFieldValues: ['yyy', null], - query: { $all: [{ kind: 'b' }, userQuery] }, + query: userQuery, + filter: { key: 'kind', values: ['b'] }, orderFields: [{ field: 'name', order: 'asc' }], }, prevCursor: { isPrevious: true, orderFieldValues: ['aaa', null], - query: { $all: [{ kind: 'b' }, userQuery] }, + query: userQuery, + filter: { key: 'kind', values: ['b'] }, orderFields: [{ field: 'name', order: 'asc' }], }, }, @@ -422,8 +425,7 @@ describe('AuthorizedEntitiesCatalog', () => { credentials: mockCredentials.none(), cursor: { ...cursor, - query: { $all: [{ kind: 'b' }, userQuery] }, - filter: undefined, + filter: { key: 'kind', values: ['b'] }, }, }); @@ -443,41 +445,6 @@ describe('AuthorizedEntitiesCatalog', () => { orderFields: [{ field: 'name', order: 'asc' }], }); }); - - it('converts multi-value permission filter with $in when converting to predicate', async () => { - fakePermissionApi.authorizeConditional.mockResolvedValue([ - { - result: AuthorizeResult.CONDITIONAL, - conditions: { - rule: 'IS_ENTITY_KIND', - params: { kinds: ['component', 'api'] }, - }, - }, - ]); - - const userQuery: FilterPredicate = { 'metadata.name': 'my-entity' }; - - fakeCatalog.queryEntities.mockResolvedValue({ - items: { type: 'object', entities: [] }, - pageInfo: {}, - totalItems: 0, - } as QueryEntitiesResponse); - - const catalog = createCatalog(isEntityKind); - - await catalog.queryEntities({ - credentials: mockCredentials.none(), - query: userQuery, - }); - - expect(fakeCatalog.queryEntities).toHaveBeenCalledWith({ - credentials: mockCredentials.none(), - query: { - $all: [{ kind: { $in: ['component', 'api'] } }, userQuery], - }, - filter: undefined, - }); - }); }); describe('removeEntityByUid', () => { diff --git a/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.ts b/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.ts index bb81144f25..910188b036 100644 --- a/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/service/AuthorizedEntitiesCatalog.ts @@ -44,30 +44,6 @@ import { PermissionsService, } from '@backstage/backend-plugin-api'; -function entityFilterToFilterPredicate(filter: EntityFilter): FilterPredicate { - if ('allOf' in filter) { - return { $all: filter.allOf.map(entityFilterToFilterPredicate) }; - } - - if ('anyOf' in filter) { - return { $any: filter.anyOf.map(entityFilterToFilterPredicate) }; - } - - if ('not' in filter) { - return { $not: entityFilterToFilterPredicate(filter.not) }; - } - - if (!filter.values) { - return { [filter.key]: { $exists: true } } as FilterPredicate; - } - - if (filter.values.length === 1) { - return { [filter.key]: filter.values[0] } as FilterPredicate; - } - - return { [filter.key]: { $in: filter.values } } as FilterPredicate; -} - export class AuthorizedEntitiesCatalog implements EntitiesCatalog { private readonly entitiesCatalog: EntitiesCatalog; private readonly permissionApi: PermissionsService; @@ -178,45 +154,25 @@ export class AuthorizedEntitiesCatalog implements EntitiesCatalog { requestFilter = request.cursor.filter; requestQuery = request.cursor.query; - if (request.cursor.query) { - const permissionPredicate = - entityFilterToFilterPredicate(permissionFilter); - permissionedRequest = { - ...request, - cursor: { - ...request.cursor, - query: { $all: [permissionPredicate, request.cursor.query] }, - filter: undefined, - }, - }; - } else { - permissionedRequest = { - ...request, - cursor: { - ...request.cursor, - filter: request.cursor.filter - ? { allOf: [permissionFilter, request.cursor.filter] } - : permissionFilter, - }, - }; - } - } else if (request.query) { - const permissionPredicate = - entityFilterToFilterPredicate(permissionFilter); - requestQuery = request.query; permissionedRequest = { ...request, - query: { $all: [permissionPredicate, request.query] }, - filter: undefined, + cursor: { + ...request.cursor, + filter: request.cursor.filter + ? { allOf: [permissionFilter, request.cursor.filter] } + : permissionFilter, + }, }; } else { + requestFilter = request.filter; + requestQuery = request.query; + permissionedRequest = { ...request, filter: request.filter ? { allOf: [permissionFilter, request.filter] } : permissionFilter, }; - requestFilter = request.filter; } const response = await this.entitiesCatalog.queryEntities( diff --git a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts index 7012c2e505..f9e41b679c 100644 --- a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts +++ b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.test.ts @@ -2054,6 +2054,38 @@ describe('DefaultEntitiesCatalog', () => { ]); }, ); + + it.each(databases.eachSupportedId())( + 'should apply both filter and query when both are given, %p', + async databaseId => { + await createDatabase(databaseId); + + // Add entities with different kinds and names + await addEntityToSearch(entityFrom('A', { kind: 'component' })); + await addEntityToSearch(entityFrom('B', { kind: 'component' })); + await addEntityToSearch(entityFrom('C', { kind: 'api' })); + await addEntityToSearch(entityFrom('D', { kind: 'api' })); + + const catalog = new DefaultEntitiesCatalog({ + database: knex, + logger: mockServices.logger.mock(), + stitcher, + }); + + // Use filter to restrict to kind=component, and query to restrict to name=A + const response = await catalog.queryEntities({ + filter: { key: 'kind', values: ['component'] }, + query: { 'metadata.name': 'a' }, + orderFields: [{ field: 'metadata.name', order: 'asc' }], + credentials: mockCredentials.none(), + }); + + const resultEntities = entitiesResponseToObjects(response.items); + expect(resultEntities).toEqual([ + entityFrom('A', { kind: 'component' }), + ]); + }, + ); }); describe('removeEntityByUid', () => {