From 154c77bf7f02b11c3480fcb1194902d8495073a1 Mon Sep 17 00:00:00 2001 From: Joon Park Date: Wed, 27 Oct 2021 13:09:04 +0100 Subject: [PATCH 1/5] EntitiesSearchFilter type refactor The current EntitiesSearchFilter's matchValueExists field is a bit confusing - it's not immediately clear the nuances of its behavior until you closely examine the implementation in NextEntitiesCatalog. By splitting the filter into to distinct types (one for key and another for key + value) as well as renaming the fields, I think this expresses the intent of the filters more cleanly. This also allows filtering by negation for key + value filters, which is not possible with the current implementation). Signed-off-by: Joon Park --- plugins/catalog-backend/src/catalog/index.ts | 2 ++ plugins/catalog-backend/src/catalog/types.ts | 24 +++++++++++++------ .../src/service/request/basicEntityFilter.ts | 8 +++---- .../request/parseEntityFilterParams.ts | 15 +++++++----- 4 files changed, 32 insertions(+), 17 deletions(-) diff --git a/plugins/catalog-backend/src/catalog/index.ts b/plugins/catalog-backend/src/catalog/index.ts index 885a2f5389..460aefddba 100644 --- a/plugins/catalog-backend/src/catalog/index.ts +++ b/plugins/catalog-backend/src/catalog/index.ts @@ -23,6 +23,8 @@ export type { EntityUpsertResponse, PageInfo, EntitiesSearchFilter, + EntitiesKeyFilter, + EntitiesValuesFilter, EntityFilter, EntityPagination, } from './types'; diff --git a/plugins/catalog-backend/src/catalog/types.ts b/plugins/catalog-backend/src/catalog/types.ts index df693bfd42..3f74f2ee98 100644 --- a/plugins/catalog-backend/src/catalog/types.ts +++ b/plugins/catalog-backend/src/catalog/types.ts @@ -39,7 +39,7 @@ export type EntityPagination = { /** * Matches rows in the entities_search table. */ -export type EntitiesSearchFilter = { +export type EntitiesValuesFilter = { /** * The key to match on. * @@ -50,18 +50,28 @@ export type EntitiesSearchFilter = { /** * Match on plain equality of values. * - * If undefined, this factor is not taken into account. Otherwise, match on + * Match on * values that are equal to any of the given array items. Matches are always * case insensitive. */ - matchValueIn?: string[]; + values: string[]; - /** - * Match on existence of key. - */ - matchValueExists?: boolean; + negate?: boolean; }; +export type EntitiesKeyFilter = { + /** + * The key to match on. + * + * Matches are always case insensitive. + */ + key: string; + + negate?: boolean; +}; + +export type EntitiesSearchFilter = EntitiesValuesFilter | EntitiesKeyFilter; + export type PageInfo = | { hasNextPage: false; diff --git a/plugins/catalog-backend/src/service/request/basicEntityFilter.ts b/plugins/catalog-backend/src/service/request/basicEntityFilter.ts index 1cf9ca95f0..cdc736e232 100644 --- a/plugins/catalog-backend/src/service/request/basicEntityFilter.ts +++ b/plugins/catalog-backend/src/service/request/basicEntityFilter.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { EntitiesSearchFilter, EntityFilter } from '../../catalog'; +import { EntitiesValuesFilter, EntityFilter } from '../../catalog'; /** * Forms a full EntityFilter based on a single key-value(s) object. @@ -22,7 +22,7 @@ import { EntitiesSearchFilter, EntityFilter } from '../../catalog'; export function basicEntityFilter( items: Record, ): EntityFilter { - const filtersByKey: Record = {}; + const filtersByKey: Record = {}; for (const [key, value] of Object.entries(items)) { const values = [value].flat(); @@ -30,9 +30,9 @@ export function basicEntityFilter( const f = key in filtersByKey ? filtersByKey[key] - : (filtersByKey[key] = { key, matchValueIn: [] }); + : (filtersByKey[key] = { key, values: [] }); - f.matchValueIn!.push(...values); + f.values!.push(...values); } return { anyOf: [{ allOf: Object.values(filtersByKey) }] }; diff --git a/plugins/catalog-backend/src/service/request/parseEntityFilterParams.ts b/plugins/catalog-backend/src/service/request/parseEntityFilterParams.ts index 452958b7ae..e779c193cb 100644 --- a/plugins/catalog-backend/src/service/request/parseEntityFilterParams.ts +++ b/plugins/catalog-backend/src/service/request/parseEntityFilterParams.ts @@ -15,7 +15,11 @@ */ import { InputError } from '@backstage/errors'; -import { EntitiesSearchFilter, EntityFilter } from '../../catalog'; +import { + EntitiesSearchFilter, + EntitiesValuesFilter, + EntityFilter, +} from '../../catalog'; import { parseStringsParam } from './common'; /** @@ -75,11 +79,10 @@ export function parseEntityFilterString( const f = key in filtersByKey ? filtersByKey[key] : (filtersByKey[key] = { key }); - if (value === undefined) { - f.matchValueExists = true; - } else { - f.matchValueIn = f.matchValueIn || []; - f.matchValueIn.push(value); + if (value !== undefined) { + const valuesFilter = f as EntitiesValuesFilter; + valuesFilter.values = valuesFilter.values || []; + valuesFilter.values.push(value); } } From 5f1de0fc60a07fd258d83d6eb1e07dd993374b1b Mon Sep 17 00:00:00 2001 From: Joon Park Date: Tue, 9 Nov 2021 15:11:07 +0000 Subject: [PATCH 2/5] Allow negation through nesting a "not" object. Signed-off-by: Joon Park --- plugins/catalog-backend/src/catalog/types.ts | 25 ++------- .../src/service/NextEntitiesCatalog.test.ts | 51 ++++++++++++++++--- .../src/service/NextEntitiesCatalog.ts | 40 +++++++++------ 3 files changed, 73 insertions(+), 43 deletions(-) diff --git a/plugins/catalog-backend/src/catalog/types.ts b/plugins/catalog-backend/src/catalog/types.ts index 3f74f2ee98..2a18783679 100644 --- a/plugins/catalog-backend/src/catalog/types.ts +++ b/plugins/catalog-backend/src/catalog/types.ts @@ -25,6 +25,7 @@ import { Entity, EntityRelationSpec } from '@backstage/catalog-model'; export type EntityFilter = | { allOf: EntityFilter[] } | { anyOf: EntityFilter[] } + | { not: EntityFilter } | EntitiesSearchFilter; /** @@ -39,7 +40,7 @@ export type EntityPagination = { /** * Matches rows in the entities_search table. */ -export type EntitiesValuesFilter = { +export type EntitiesSearchFilter = { /** * The key to match on. * @@ -50,28 +51,12 @@ export type EntitiesValuesFilter = { /** * Match on plain equality of values. * - * Match on - * values that are equal to any of the given array items. Matches are always - * case insensitive. + * Match on values that are equal to any of the given array items. Matches are + * always case insensitive. */ - values: string[]; - - negate?: boolean; + values?: string[]; }; -export type EntitiesKeyFilter = { - /** - * The key to match on. - * - * Matches are always case insensitive. - */ - key: string; - - negate?: boolean; -}; - -export type EntitiesSearchFilter = EntitiesValuesFilter | EntitiesKeyFilter; - export type PageInfo = | { hasNextPage: false; diff --git a/plugins/catalog-backend/src/service/NextEntitiesCatalog.test.ts b/plugins/catalog-backend/src/service/NextEntitiesCatalog.test.ts index 32d5fa843e..c968b6de82 100644 --- a/plugins/catalog-backend/src/service/NextEntitiesCatalog.test.ts +++ b/plugins/catalog-backend/src/service/NextEntitiesCatalog.test.ts @@ -282,7 +282,6 @@ describe('NextEntitiesCatalog', () => { const testFilter = { key: 'spec.test', - matchValueExists: true, }; const request = { filter: testFilter }; const { entities } = await catalog.entities(request); @@ -292,6 +291,41 @@ describe('NextEntitiesCatalog', () => { }, ); + it.each(databases.eachSupportedId())( + 'should return correct entity for negation filter', + async databaseId => { + const { knex } = await createDatabase(databaseId); + const entity1: Entity = { + apiVersion: 'a', + kind: 'k', + metadata: { name: 'one' }, + spec: {}, + }; + const entity2: Entity = { + apiVersion: 'a', + kind: 'k', + metadata: { name: 'two' }, + spec: { + test: 'test value', + }, + }; + await addEntityToSearch(knex, entity1); + await addEntityToSearch(knex, entity2); + const catalog = new NextEntitiesCatalog(knex); + + const testFilter = { + not: { + key: 'spec.test', + }, + }; + const request = { filter: testFilter }; + const { entities } = await catalog.entities(request); + + expect(entities.length).toBe(1); + expect(entities[0]).toEqual(entity1); + }, + ); + it.each(databases.eachSupportedId())( 'should return correct entity for nested filter', async databaseId => { @@ -328,24 +362,27 @@ describe('NextEntitiesCatalog', () => { const testFilter1 = { key: 'metadata.org', - matchValueExists: true, - matchValueIn: ['b'], + values: ['b'], }; const testFilter2 = { key: 'metadata.desc', - matchValueExists: true, }; const testFilter3 = { key: 'metadata.color', - matchValueExists: true, - matchValueIn: ['blue'], + values: ['blue'], + }; + const testFilter4 = { + not: { + key: 'metadata.color', + values: ['red'], + }, }; const request = { filter: { allOf: [ testFilter1, { - anyOf: [testFilter2, testFilter3], + anyOf: [testFilter2, testFilter3, testFilter4], }, ], }, diff --git a/plugins/catalog-backend/src/service/NextEntitiesCatalog.ts b/plugins/catalog-backend/src/service/NextEntitiesCatalog.ts index 1c615f862a..ccce1395d7 100644 --- a/plugins/catalog-backend/src/service/NextEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/service/NextEntitiesCatalog.ts @@ -78,33 +78,30 @@ function stringifyPagination(input: { limit: number; offset: number }) { function addCondition( queryBuilder: Knex.QueryBuilder, db: Knex, - { key, matchValueIn, matchValueExists }: EntitiesSearchFilter, + filter: EntitiesSearchFilter, + negate: boolean = false, ) { // NOTE(freben): This used to be a set of OUTER JOIN, which may seem to // make a lot of sense. However, it had abysmal performance on sqlite // when datasets grew large, so we're using IN instead. const matchQuery = db('search') .select('entity_id') - .where(function keyFilter() { - this.andWhere({ key: key.toLowerCase() }); - if (matchValueExists !== false && matchValueIn) { - if (matchValueIn.length === 1) { - this.andWhere({ value: matchValueIn[0].toLowerCase() }); - } else if (matchValueIn.length > 1) { + .where({ key: filter.key.toLowerCase() }) + .andWhere(function keyFilter() { + if (filter.values) { + if (filter.values.length === 1) { + this.where({ value: filter.values[0].toLowerCase() }); + } else if (filter.values.length > 1) { this.andWhere( 'value', 'in', - matchValueIn.map(v => v.toLowerCase()), + filter.values.map(v => v.toLowerCase()), ); } } }); // Explicitly evaluate matchValueExists as a boolean since it may be undefined - queryBuilder.andWhere( - 'entity_id', - matchValueExists === false ? 'not in' : 'in', - matchQuery, - ); + queryBuilder.andWhere('entity_id', negate ? 'not in' : 'in', matchQuery); } function isEntitiesSearchFilter( @@ -125,21 +122,32 @@ function isOrEntityFilter( return filter.hasOwnProperty('anyOf'); } +function isNegationEntityFilter( + filter: { not: EntityFilter } | EntityFilter, +): filter is { not: EntityFilter } { + return filter.hasOwnProperty('not'); +} + function parseFilter( filter: EntityFilter, query: Knex.QueryBuilder, db: Knex, + negate: boolean = false, ): Knex.QueryBuilder { if (isEntitiesSearchFilter(filter)) { return query.andWhere(function filterFunction() { - addCondition(this, db, filter); + addCondition(this, db, filter, negate); }); } + if (isNegationEntityFilter(filter)) { + return parseFilter(filter.not, query, db, true); + } + if (isOrEntityFilter(filter)) { return query.andWhere(function filterFunction() { for (const subFilter of filter.anyOf ?? []) { - this.orWhere(subQuery => parseFilter(subFilter, subQuery, db)); + this.orWhere(subQuery => parseFilter(subFilter, subQuery, db, negate)); } }); } @@ -147,7 +155,7 @@ function parseFilter( if (isAndEntityFilter(filter)) { return query.andWhere(function filterFunction() { for (const subFilter of filter.allOf ?? []) { - this.andWhere(subQuery => parseFilter(subFilter, subQuery, db)); + this.andWhere(subQuery => parseFilter(subFilter, subQuery, db, negate)); } }); } From 8d1309bf7d567752acb21da5f48cda5abfcba19c Mon Sep 17 00:00:00 2001 From: Joon Park Date: Tue, 16 Nov 2021 12:53:37 +0000 Subject: [PATCH 3/5] Fix complex negation bug Signed-off-by: Joon Park --- .../src/service/NextEntitiesCatalog.test.ts | 43 ++++++++++++++++++- .../src/service/NextEntitiesCatalog.ts | 28 ++++-------- 2 files changed, 50 insertions(+), 21 deletions(-) diff --git a/plugins/catalog-backend/src/service/NextEntitiesCatalog.test.ts b/plugins/catalog-backend/src/service/NextEntitiesCatalog.test.ts index c968b6de82..ee86e78c6b 100644 --- a/plugins/catalog-backend/src/service/NextEntitiesCatalog.test.ts +++ b/plugins/catalog-backend/src/service/NextEntitiesCatalog.test.ts @@ -327,7 +327,7 @@ describe('NextEntitiesCatalog', () => { ); it.each(databases.eachSupportedId())( - 'should return correct entity for nested filter', + 'should return correct entities for nested filter', async databaseId => { const { knex } = await createDatabase(databaseId); const entity1: Entity = { @@ -394,5 +394,46 @@ describe('NextEntitiesCatalog', () => { expect(entities).toContainEqual(entity4); }, ); + + it.each(databases.eachSupportedId())( + 'should return correct entities for complex negation filter', + async databaseId => { + const { knex } = await createDatabase(databaseId); + const entity1: Entity = { + apiVersion: 'a', + kind: 'k', + metadata: { name: 'one', org: 'a', desc: 'description' }, + spec: {}, + }; + const entity2: Entity = { + apiVersion: 'a', + kind: 'k', + metadata: { name: 'two', org: 'b', desc: 'description' }, + spec: {}, + }; + await addEntityToSearch(knex, entity1); + await addEntityToSearch(knex, entity2); + const catalog = new NextEntitiesCatalog(knex); + + const testFilter1 = { + key: 'metadata.org', + values: ['b'], + }; + const testFilter2 = { + key: 'metadata.desc', + }; + const request = { + filter: { + not: { + allOf: [testFilter1, testFilter2], + }, + }, + }; + const { entities } = await catalog.entities(request); + + expect(entities.length).toBe(1); + expect(entities).toContainEqual(entity1); + }, + ); }); }); diff --git a/plugins/catalog-backend/src/service/NextEntitiesCatalog.ts b/plugins/catalog-backend/src/service/NextEntitiesCatalog.ts index ccce1395d7..0f2fb9494c 100644 --- a/plugins/catalog-backend/src/service/NextEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/service/NextEntitiesCatalog.ts @@ -110,12 +110,6 @@ function isEntitiesSearchFilter( return filter.hasOwnProperty('key'); } -function isAndEntityFilter( - filter: { allOf: EntityFilter[] } | EntityFilter, -): filter is { allOf: EntityFilter[] } { - return filter.hasOwnProperty('allOf'); -} - function isOrEntityFilter( filter: { anyOf: EntityFilter[] } | EntityFilter, ): filter is { anyOf: EntityFilter[] } { @@ -141,26 +135,20 @@ function parseFilter( } if (isNegationEntityFilter(filter)) { - return parseFilter(filter.not, query, db, true); + return parseFilter(filter.not, query, db, !negate); } - if (isOrEntityFilter(filter)) { - return query.andWhere(function filterFunction() { + return query[negate ? 'andWhereNot' : 'andWhere'](function filterFunction() { + if (isOrEntityFilter(filter)) { for (const subFilter of filter.anyOf ?? []) { - this.orWhere(subQuery => parseFilter(subFilter, subQuery, db, negate)); + this.orWhere(subQuery => parseFilter(subFilter, subQuery, db)); } - }); - } - - if (isAndEntityFilter(filter)) { - return query.andWhere(function filterFunction() { + } else { for (const subFilter of filter.allOf ?? []) { - this.andWhere(subQuery => parseFilter(subFilter, subQuery, db, negate)); + this.andWhere(subQuery => parseFilter(subFilter, subQuery, db)); } - }); - } - - return query; + } + }); } export class NextEntitiesCatalog implements EntitiesCatalog { From b5c54795047f90cf407e92c59e560705c94b0ae4 Mon Sep 17 00:00:00 2001 From: Joon Park Date: Tue, 16 Nov 2021 13:06:31 +0000 Subject: [PATCH 4/5] Fix tests Signed-off-by: Joon Park --- plugins/catalog-backend/src/catalog/index.ts | 2 -- .../legacy/database/CommonDatabase.test.ts | 28 +------------------ .../src/legacy/database/CommonDatabase.ts | 25 ++++++++--------- .../src/legacy/service/router.test.ts | 6 ++-- .../src/service/NextEntitiesCatalog.ts | 1 - .../src/service/NextRouter.test.ts | 6 ++-- .../request/parseEntityFilterParams.test.ts | 24 ++++++++-------- 7 files changed, 30 insertions(+), 62 deletions(-) diff --git a/plugins/catalog-backend/src/catalog/index.ts b/plugins/catalog-backend/src/catalog/index.ts index 460aefddba..885a2f5389 100644 --- a/plugins/catalog-backend/src/catalog/index.ts +++ b/plugins/catalog-backend/src/catalog/index.ts @@ -23,8 +23,6 @@ export type { EntityUpsertResponse, PageInfo, EntitiesSearchFilter, - EntitiesKeyFilter, - EntitiesValuesFilter, EntityFilter, EntityPagination, } from './types'; diff --git a/plugins/catalog-backend/src/legacy/database/CommonDatabase.test.ts b/plugins/catalog-backend/src/legacy/database/CommonDatabase.test.ts index ca2e6c2934..8b72b470a6 100644 --- a/plugins/catalog-backend/src/legacy/database/CommonDatabase.test.ts +++ b/plugins/catalog-backend/src/legacy/database/CommonDatabase.test.ts @@ -536,9 +536,7 @@ describe('CommonDatabase', () => { filter: { anyOf: [ { - allOf: [ - { key: 'metadata.annotations.foo', matchValueExists: true }, - ], + allOf: [{ key: 'metadata.annotations.foo' }], }, ], }, @@ -558,30 +556,6 @@ describe('CommonDatabase', () => { }, ]), ); - - const nonExistRows = await db.transaction(async tx => - db.entities(tx, { - filter: { - anyOf: [ - { - allOf: [ - { key: 'metadata.annotations.foo', matchValueExists: false }, - ], - }, - ], - }, - }), - ); - - expect(nonExistRows.entities.length).toEqual(1); - expect(nonExistRows.entities).toEqual( - expect.arrayContaining([ - { - locationId: undefined, - entity: expect.objectContaining({ kind: 'k3' }), - }, - ]), - ); }); }); diff --git a/plugins/catalog-backend/src/legacy/database/CommonDatabase.ts b/plugins/catalog-backend/src/legacy/database/CommonDatabase.ts index 6a150f7583..3ea8696437 100644 --- a/plugins/catalog-backend/src/legacy/database/CommonDatabase.ts +++ b/plugins/catalog-backend/src/legacy/database/CommonDatabase.ts @@ -224,7 +224,8 @@ export class CommonDatabase implements Database { if ( request?.filter && (request.filter.hasOwnProperty('key') || - request.filter.hasOwnProperty('allOf')) + request.filter.hasOwnProperty('allOf') || + request.filter.hasOwnProperty('not')) ) { throw new Error( 'Filters for the legacy CommonDatabase must obey the { anyOf: [{ allOf: [] }] } format.', @@ -236,13 +237,14 @@ export class CommonDatabase implements Database { for (const filter of singleFilter.allOf) { if ( filter.hasOwnProperty('anyOf') || - filter.hasOwnProperty('allOf') + filter.hasOwnProperty('allOf') || + filter.hasOwnProperty('not') ) { throw new Error( 'Nested filters are not supported in the legacy CommonDatabase', ); } - const { key, matchValueIn, matchValueExists } = filter; + const { key, values } = filter; // NOTE(freben): This used to be a set of OUTER JOIN, which may seem to // make a lot of sense. However, it had abysmal performance on sqlite // when datasets grew large, so we're using IN instead. @@ -250,24 +252,19 @@ export class CommonDatabase implements Database { .select('entity_id') .where(function keyFilter() { this.andWhere({ key: key.toLowerCase() }); - if (matchValueExists !== false && matchValueIn) { - if (matchValueIn.length === 1) { - this.andWhere({ value: matchValueIn[0].toLowerCase() }); - } else if (matchValueIn.length > 1) { + if (values) { + if (values.length === 1) { + this.andWhere({ value: values[0].toLowerCase() }); + } else if (values.length > 1) { this.andWhere( 'value', 'in', - matchValueIn.map(v => v.toLowerCase()), + values.map(v => v.toLowerCase()), ); } } }); - // Explicitly evaluate matchValueExists as a boolean since it may be undefined - this.andWhere( - 'id', - matchValueExists === false ? 'not in' : 'in', - matchQuery, - ); + this.andWhere('id', 'in', matchQuery); } }); } diff --git a/plugins/catalog-backend/src/legacy/service/router.test.ts b/plugins/catalog-backend/src/legacy/service/router.test.ts index edc7d852ac..0ff5a44ed0 100644 --- a/plugins/catalog-backend/src/legacy/service/router.test.ts +++ b/plugins/catalog-backend/src/legacy/service/router.test.ts @@ -115,11 +115,11 @@ describe('createRouter readonly disabled', () => { anyOf: [ { allOf: [ - { key: 'a', matchValueIn: ['1', '2'] }, - { key: 'b', matchValueIn: ['3'] }, + { key: 'a', values: ['1', '2'] }, + { key: 'b', values: ['3'] }, ], }, - { allOf: [{ key: 'c', matchValueIn: ['4'] }] }, + { allOf: [{ key: 'c', values: ['4'] }] }, ], }, }); diff --git a/plugins/catalog-backend/src/service/NextEntitiesCatalog.ts b/plugins/catalog-backend/src/service/NextEntitiesCatalog.ts index 0f2fb9494c..8c2de9d7c1 100644 --- a/plugins/catalog-backend/src/service/NextEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/service/NextEntitiesCatalog.ts @@ -100,7 +100,6 @@ function addCondition( } } }); - // Explicitly evaluate matchValueExists as a boolean since it may be undefined queryBuilder.andWhere('entity_id', negate ? 'not in' : 'in', matchQuery); } diff --git a/plugins/catalog-backend/src/service/NextRouter.test.ts b/plugins/catalog-backend/src/service/NextRouter.test.ts index 57911c74fb..6bc4481425 100644 --- a/plugins/catalog-backend/src/service/NextRouter.test.ts +++ b/plugins/catalog-backend/src/service/NextRouter.test.ts @@ -104,11 +104,11 @@ describe('createNextRouter readonly disabled', () => { anyOf: [ { allOf: [ - { key: 'a', matchValueIn: ['1', '2'] }, - { key: 'b', matchValueIn: ['3'] }, + { key: 'a', values: ['1', '2'] }, + { key: 'b', values: ['3'] }, ], }, - { allOf: [{ key: 'c', matchValueIn: ['4'] }] }, + { allOf: [{ key: 'c', values: ['4'] }] }, ], }, }); diff --git a/plugins/catalog-backend/src/service/request/parseEntityFilterParams.test.ts b/plugins/catalog-backend/src/service/request/parseEntityFilterParams.test.ts index fc22a638de..f9fc596cb9 100644 --- a/plugins/catalog-backend/src/service/request/parseEntityFilterParams.test.ts +++ b/plugins/catalog-backend/src/service/request/parseEntityFilterParams.test.ts @@ -28,7 +28,7 @@ describe('parseEntityFilterParams', () => { it('supports single-string format', () => { const result = parseEntityFilterParams({ filter: 'a=1' })!; expect(result).toEqual({ - anyOf: [{ allOf: [{ key: 'a', matchValueIn: ['1'] }] }], + anyOf: [{ allOf: [{ key: 'a', values: ['1'] }] }], }); }); @@ -38,8 +38,8 @@ describe('parseEntityFilterParams', () => { }); expect(result).toEqual({ anyOf: [ - { allOf: [{ key: 'a', matchValueIn: ['1'] }] }, - { allOf: [{ key: 'b', matchValueIn: ['2'] }] }, + { allOf: [{ key: 'a', values: ['1'] }] }, + { allOf: [{ key: 'b', values: ['2'] }] }, ], }); }); @@ -50,11 +50,11 @@ describe('parseEntityFilterParams', () => { }); expect(result).toEqual({ anyOf: [ - { allOf: [{ key: 'a', matchValueIn: ['1'] }] }, + { allOf: [{ key: 'a', values: ['1'] }] }, { allOf: [ - { key: 'b', matchValueIn: ['2', '3'] }, - { key: 'c', matchValueIn: ['4'] }, + { key: 'b', values: ['2', '3'] }, + { key: 'c', values: ['4'] }, ], }, ], @@ -70,17 +70,17 @@ describe('parseEntityFilterString', () => { it('works for the happy path', () => { expect(parseEntityFilterString('')).toBeUndefined(); expect(parseEntityFilterString('a=1,b=2,a=3,c,d=')).toEqual([ - { key: 'a', matchValueIn: ['1', '3'] }, - { key: 'b', matchValueIn: ['2'] }, - { key: 'c', matchValueExists: true }, - { key: 'd', matchValueIn: [''] }, + { key: 'a', values: ['1', '3'] }, + { key: 'b', values: ['2'] }, + { key: 'c' }, + { key: 'd', values: [''] }, ]); }); it('trims values', () => { expect(parseEntityFilterString(' a = 1 , b = 2 , a = 3 ')).toEqual([ - { key: 'a', matchValueIn: ['1', '3'] }, - { key: 'b', matchValueIn: ['2'] }, + { key: 'a', values: ['1', '3'] }, + { key: 'b', values: ['2'] }, ]); }); From 7f82ce9f511878b98946c59e97e1c64c3168be73 Mon Sep 17 00:00:00 2001 From: Joon Park Date: Tue, 16 Nov 2021 13:54:55 +0000 Subject: [PATCH 5/5] API reports and Changelog Signed-off-by: Joon Park --- .changeset/smart-fans-complain.md | 42 +++++++++++++++++++ plugins/catalog-backend/api-report.md | 12 +++--- .../src/service/request/basicEntityFilter.ts | 4 +- .../request/parseEntityFilterParams.ts | 11 ++--- 4 files changed, 54 insertions(+), 15 deletions(-) create mode 100644 .changeset/smart-fans-complain.md diff --git a/.changeset/smart-fans-complain.md b/.changeset/smart-fans-complain.md new file mode 100644 index 0000000000..db404f110d --- /dev/null +++ b/.changeset/smart-fans-complain.md @@ -0,0 +1,42 @@ +--- +'@backstage/plugin-catalog-backend': minor +--- + +**BREAKING** EntitiesSearchFilter fields have changed. + +EntitiesSearchFilter now has only two fields: `key` and `value`. The `matchValueIn` and `matchValueExists` fields are no longer are supported. Previous filters written using the `matchValueIn` and `matchValueExists` fields can be rewritten as follows: + +Filtering by existence of key only: + +```diff + filter: { + { + key: 'abc', +- matchValueExists: true, + }, + } +``` + +Filtering by key and values: + +```diff + filter: { + { + key: 'abc', +- matchValueExists: true, +- matchValueIn: ['xyz'], ++ values: ['xyz'], + }, + } +``` + +Negation of filters can now be achieved through a `not` object: + +``` +filter: { + not: { + key: 'abc', + values: ['xyz'], + }, +} +``` diff --git a/plugins/catalog-backend/api-report.md b/plugins/catalog-backend/api-report.md index 6660912dda..5008cf02f1 100644 --- a/plugins/catalog-backend/api-report.md +++ b/plugins/catalog-backend/api-report.md @@ -846,8 +846,7 @@ export type EntitiesResponse = { // @public export type EntitiesSearchFilter = { key: string; - matchValueIn?: string[]; - matchValueExists?: boolean; + values?: string[]; }; // Warning: (ae-missing-release-tag) "entity" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) @@ -877,6 +876,9 @@ export type EntityFilter = | { anyOf: EntityFilter[]; } + | { + not: EntityFilter; + } | EntitiesSearchFilter; // Warning: (ae-missing-release-tag) "EntityPagination" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) @@ -1546,9 +1548,9 @@ export class UrlReaderProcessor implements CatalogProcessor { // Warnings were encountered during analysis: // -// src/catalog/types.d.ts:97:8 - (tsdoc-param-tag-with-invalid-name) The @param block should be followed by a valid parameter name: The identifier cannot non-word characters -// src/catalog/types.d.ts:98:8 - (tsdoc-param-tag-with-invalid-name) The @param block should be followed by a valid parameter name: The identifier cannot non-word characters -// src/catalog/types.d.ts:99:8 - (tsdoc-param-tag-with-invalid-name) The @param block should be followed by a valid parameter name: The identifier cannot non-word characters +// src/catalog/types.d.ts:94:8 - (tsdoc-param-tag-with-invalid-name) The @param block should be followed by a valid parameter name: The identifier cannot non-word characters +// src/catalog/types.d.ts:95:8 - (tsdoc-param-tag-with-invalid-name) The @param block should be followed by a valid parameter name: The identifier cannot non-word characters +// src/catalog/types.d.ts:96:8 - (tsdoc-param-tag-with-invalid-name) The @param block should be followed by a valid parameter name: The identifier cannot non-word characters // src/ingestion/processors/GithubMultiOrgReaderProcessor.d.ts:23:9 - (ae-forgotten-export) The symbol "GithubMultiOrgConfig" needs to be exported by the entry point index.d.ts // src/ingestion/types.d.ts:8:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen // src/legacy/database/types.d.ts:98:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen diff --git a/plugins/catalog-backend/src/service/request/basicEntityFilter.ts b/plugins/catalog-backend/src/service/request/basicEntityFilter.ts index cdc736e232..36448e0304 100644 --- a/plugins/catalog-backend/src/service/request/basicEntityFilter.ts +++ b/plugins/catalog-backend/src/service/request/basicEntityFilter.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { EntitiesValuesFilter, EntityFilter } from '../../catalog'; +import { EntitiesSearchFilter, EntityFilter } from '../../catalog'; /** * Forms a full EntityFilter based on a single key-value(s) object. @@ -22,7 +22,7 @@ import { EntitiesValuesFilter, EntityFilter } from '../../catalog'; export function basicEntityFilter( items: Record, ): EntityFilter { - const filtersByKey: Record = {}; + const filtersByKey: Record = {}; for (const [key, value] of Object.entries(items)) { const values = [value].flat(); diff --git a/plugins/catalog-backend/src/service/request/parseEntityFilterParams.ts b/plugins/catalog-backend/src/service/request/parseEntityFilterParams.ts index e779c193cb..1ea5d44b2c 100644 --- a/plugins/catalog-backend/src/service/request/parseEntityFilterParams.ts +++ b/plugins/catalog-backend/src/service/request/parseEntityFilterParams.ts @@ -15,11 +15,7 @@ */ import { InputError } from '@backstage/errors'; -import { - EntitiesSearchFilter, - EntitiesValuesFilter, - EntityFilter, -} from '../../catalog'; +import { EntitiesSearchFilter, EntityFilter } from '../../catalog'; import { parseStringsParam } from './common'; /** @@ -80,9 +76,8 @@ export function parseEntityFilterString( key in filtersByKey ? filtersByKey[key] : (filtersByKey[key] = { key }); if (value !== undefined) { - const valuesFilter = f as EntitiesValuesFilter; - valuesFilter.values = valuesFilter.values || []; - valuesFilter.values.push(value); + f.values = f.values || []; + f.values.push(value); } }