From e223f2264d0d3e69f934ac1d8805ee33e690c7b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 22 Nov 2023 17:03:32 +0100 Subject: [PATCH 1/8] implement text based entity card/content filters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Vincenzo Scamporlino Signed-off-by: Fredrik Adelöw --- .changeset/stale-jokes-hunt.md | 5 + .changeset/thick-snails-travel.md | 5 + .changeset/thirty-fireants-cheer.md | 7 + packages/app-next/app-config.yaml | 3 +- plugins/catalog-common/api-report-alpha.md | 6 + plugins/catalog-common/package.json | 1 + plugins/catalog-common/src/alpha.ts | 1 + plugins/catalog-common/src/filter/index.ts | 17 ++ .../src/filter/matrchers/createHasMatcher.ts | 48 +++++ .../src/filter/matrchers/createIsMatcher.ts | 41 +++++ .../src/filter/matrchers/createKindMatcher.ts | 25 +++ .../src/filter/matrchers/createTypeMatcher.ts | 31 ++++ .../src/filter/matrchers/types.ts | 19 ++ .../src/filter/parseFilterExpression.test.ts | 169 ++++++++++++++++++ .../src/filter/parseFilterExpression.ts | 99 ++++++++++ plugins/catalog-react/api-report-alpha.md | 20 +-- plugins/catalog-react/src/alpha.tsx | 70 ++------ .../catalog/src/alpha/EntityOverviewPage.tsx | 42 +++-- plugins/catalog/src/alpha/entityCards.tsx | 4 +- plugins/catalog/src/alpha/entityContents.tsx | 2 +- plugins/catalog/src/alpha/plugin.tsx | 2 +- yarn.lock | 1 + 22 files changed, 529 insertions(+), 89 deletions(-) create mode 100644 .changeset/stale-jokes-hunt.md create mode 100644 .changeset/thick-snails-travel.md create mode 100644 .changeset/thirty-fireants-cheer.md create mode 100644 plugins/catalog-common/src/filter/index.ts create mode 100644 plugins/catalog-common/src/filter/matrchers/createHasMatcher.ts create mode 100644 plugins/catalog-common/src/filter/matrchers/createIsMatcher.ts create mode 100644 plugins/catalog-common/src/filter/matrchers/createKindMatcher.ts create mode 100644 plugins/catalog-common/src/filter/matrchers/createTypeMatcher.ts create mode 100644 plugins/catalog-common/src/filter/matrchers/types.ts create mode 100644 plugins/catalog-common/src/filter/parseFilterExpression.test.ts create mode 100644 plugins/catalog-common/src/filter/parseFilterExpression.ts diff --git a/.changeset/stale-jokes-hunt.md b/.changeset/stale-jokes-hunt.md new file mode 100644 index 0000000000..cbea2dd229 --- /dev/null +++ b/.changeset/stale-jokes-hunt.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-common': minor +--- + +Added a `parseFilterExpression` function to interpret visibility filters diff --git a/.changeset/thick-snails-travel.md b/.changeset/thick-snails-travel.md new file mode 100644 index 0000000000..af27df78e5 --- /dev/null +++ b/.changeset/thick-snails-travel.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog': minor +--- + +Properly support both function- and string-form visibility filter expressions diff --git a/.changeset/thirty-fireants-cheer.md b/.changeset/thirty-fireants-cheer.md new file mode 100644 index 0000000000..abda872570 --- /dev/null +++ b/.changeset/thirty-fireants-cheer.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-catalog-react': patch +--- + +Breaking alpha-API change to entity visibility filter functions to accept a bare entity as their first argument, instead of an object with an entity property. + +Functions that accept such filters now also support the string expression form of filters. diff --git a/packages/app-next/app-config.yaml b/packages/app-next/app-config.yaml index a46e119ac0..a90fe630b5 100644 --- a/packages/app-next/app-config.yaml +++ b/packages/app-next/app-config.yaml @@ -14,8 +14,7 @@ app: - entity.cards.labels - entity.cards.links: config: - filter: - - isKind: component + filter: kind:component # Entity page content - entity.content.techdocs diff --git a/plugins/catalog-common/api-report-alpha.md b/plugins/catalog-common/api-report-alpha.md index 12e0806a9c..573b35d3b3 100644 --- a/plugins/catalog-common/api-report-alpha.md +++ b/plugins/catalog-common/api-report-alpha.md @@ -4,6 +4,7 @@ ```ts import { BasicPermission } from '@backstage/plugin-permission-common'; +import { Entity } from '@backstage/catalog-model'; import { ResourcePermission } from '@backstage/plugin-permission-common'; // @alpha @@ -38,6 +39,11 @@ export const catalogPermissions: ( | ResourcePermission<'catalog-entity'> )[]; +// @alpha +export function parseFilterExpression( + expression: string, +): (entity: Entity) => boolean; + // @alpha export const RESOURCE_TYPE_CATALOG_ENTITY = 'catalog-entity'; diff --git a/plugins/catalog-common/package.json b/plugins/catalog-common/package.json index 45720ee32a..ac21b76c2a 100644 --- a/plugins/catalog-common/package.json +++ b/plugins/catalog-common/package.json @@ -46,6 +46,7 @@ }, "dependencies": { "@backstage/catalog-model": "workspace:^", + "@backstage/errors": "workspace:^", "@backstage/plugin-permission-common": "workspace:^", "@backstage/plugin-search-common": "workspace:^" }, diff --git a/plugins/catalog-common/src/alpha.ts b/plugins/catalog-common/src/alpha.ts index 2dfc028332..015769c8a7 100644 --- a/plugins/catalog-common/src/alpha.ts +++ b/plugins/catalog-common/src/alpha.ts @@ -26,3 +26,4 @@ export { catalogPermissions, } from './permissions'; export type { CatalogEntityPermission } from './permissions'; +export { parseFilterExpression } from './filter'; diff --git a/plugins/catalog-common/src/filter/index.ts b/plugins/catalog-common/src/filter/index.ts new file mode 100644 index 0000000000..c5ad803f4c --- /dev/null +++ b/plugins/catalog-common/src/filter/index.ts @@ -0,0 +1,17 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export { parseFilterExpression } from './parseFilterExpression'; diff --git a/plugins/catalog-common/src/filter/matrchers/createHasMatcher.ts b/plugins/catalog-common/src/filter/matrchers/createHasMatcher.ts new file mode 100644 index 0000000000..7b9eebba1c --- /dev/null +++ b/plugins/catalog-common/src/filter/matrchers/createHasMatcher.ts @@ -0,0 +1,48 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { InputError } from '@backstage/errors'; +import { EntityMatcherFn } from './types'; + +/** + * Matches on the non-empty presence of different parts of the entity + */ +export function createHasMatcher(parameters: string[]): EntityMatcherFn { + const allowedMatchers: Record = { + labels: entity => { + return Object.keys(entity.metadata.labels ?? {}).length > 0; + }, + annotations: entity => { + return Object.keys(entity.metadata.annotations ?? {}).length > 0; + }, + links: entity => { + return (entity.metadata.links ?? []).length > 0; + }, + }; + + const matchers = parameters.map(parameter => { + const matcher = allowedMatchers[parameter.toLocaleLowerCase('en-US')]; + if (!matcher) { + const known = Object.keys(allowedMatchers).map(m => `'${m}'`); + throw new InputError( + `'${parameter}' is not a valid parameter for 'has' filter expressions, expected one of ${known}`, + ); + } + return matcher; + }); + + return entity => matchers.some(matcher => matcher(entity)); +} diff --git a/plugins/catalog-common/src/filter/matrchers/createIsMatcher.ts b/plugins/catalog-common/src/filter/matrchers/createIsMatcher.ts new file mode 100644 index 0000000000..3802d6dbce --- /dev/null +++ b/plugins/catalog-common/src/filter/matrchers/createIsMatcher.ts @@ -0,0 +1,41 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { InputError } from '@backstage/errors'; +import { EntityMatcherFn } from './types'; + +/** + * Matches on different semantic properties of the entity + */ +export function createIsMatcher(parameters: string[]): EntityMatcherFn { + const allowedMatchers: Record = { + orphan: entity => + Boolean(entity.metadata.annotations?.['backstage.io/orphan']), + }; + + const matchers = parameters.map(parameter => { + const matcher = allowedMatchers[parameter.toLocaleLowerCase('en-US')]; + if (!matcher) { + const known = Object.keys(allowedMatchers).map(m => `'${m}'`); + throw new InputError( + `'${parameter}' is not a valid parameter for 'is' filter expressions, expected one of ${known}`, + ); + } + return matcher; + }); + + return entity => matchers.some(matcher => matcher(entity)); +} diff --git a/plugins/catalog-common/src/filter/matrchers/createKindMatcher.ts b/plugins/catalog-common/src/filter/matrchers/createKindMatcher.ts new file mode 100644 index 0000000000..071746453e --- /dev/null +++ b/plugins/catalog-common/src/filter/matrchers/createKindMatcher.ts @@ -0,0 +1,25 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { EntityMatcherFn } from './types'; + +/** + * Matches on kind + */ +export function createKindMatcher(parameters: string[]): EntityMatcherFn { + const items = parameters.map(p => p.toLocaleLowerCase('en-US')); + return entity => items.includes(entity.kind.toLocaleLowerCase('en-US')); +} diff --git a/plugins/catalog-common/src/filter/matrchers/createTypeMatcher.ts b/plugins/catalog-common/src/filter/matrchers/createTypeMatcher.ts new file mode 100644 index 0000000000..d0e0f39c98 --- /dev/null +++ b/plugins/catalog-common/src/filter/matrchers/createTypeMatcher.ts @@ -0,0 +1,31 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { EntityMatcherFn } from './types'; + +/** + * Matches on spec.type + */ +export function createTypeMatcher(parameters: string[]): EntityMatcherFn { + const items = parameters.map(p => p.toLocaleLowerCase('en-US')); + return entity => { + const value = entity.spec?.type; + return ( + typeof value === 'string' && + items.includes(value.toLocaleLowerCase('en-US')) + ); + }; +} diff --git a/plugins/catalog-common/src/filter/matrchers/types.ts b/plugins/catalog-common/src/filter/matrchers/types.ts new file mode 100644 index 0000000000..0fff91b3d5 --- /dev/null +++ b/plugins/catalog-common/src/filter/matrchers/types.ts @@ -0,0 +1,19 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Entity } from '@backstage/catalog-model'; + +export type EntityMatcherFn = (entity: Entity) => boolean; diff --git a/plugins/catalog-common/src/filter/parseFilterExpression.test.ts b/plugins/catalog-common/src/filter/parseFilterExpression.test.ts new file mode 100644 index 0000000000..d8543c4e10 --- /dev/null +++ b/plugins/catalog-common/src/filter/parseFilterExpression.test.ts @@ -0,0 +1,169 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Entity } from '@backstage/catalog-model'; +import { + parseFilterExpression, + splitFilterExpression, +} from './parseFilterExpression'; + +describe('parseFilterExpression', () => { + it('supports "kind" expressions', () => { + const component = { kind: 'Component' } as unknown as Entity; + const user = { kind: 'User' } as unknown as Entity; + const resource = { kind: 'Resource' } as unknown as Entity; + + expect(parseFilterExpression('kind:component')(component)).toBe(true); + expect(parseFilterExpression('kind:componenT')(component)).toBe(true); + expect(parseFilterExpression('kind:user')(component)).toBe(false); + + // match ANY of the parameters + expect(parseFilterExpression('kind:user,component')(user)).toBe(true); + expect(parseFilterExpression('kind:user,component')(component)).toBe(true); + expect(parseFilterExpression('kind:user,component')(resource)).toBe(false); + }); + + it('supports "is" expressions', () => { + const empty = { + metadata: {}, + } as unknown as Entity; + const orphan = { + metadata: { + annotations: { ['backstage.io/orphan']: 'true' }, + }, + } as unknown as Entity; + + expect(parseFilterExpression('is:orphan')(empty)).toBe(false); + expect(parseFilterExpression('is:orphan')(orphan)).toBe(true); + + expect(() => + parseFilterExpression('is:orphan,bar'), + ).toThrowErrorMatchingInlineSnapshot( + `"'bar' is not a valid parameter for 'is' filter expressions, expected one of 'orphan'"`, + ); + }); + + it('supports "has" expressions', () => { + const empty1 = { + metadata: {}, + } as unknown as Entity; + const empty2 = { + metadata: { + labels: {}, + annotations: {}, + links: [], + }, + } as unknown as Entity; + const labels = { + metadata: { + labels: { a: 'b' }, + }, + } as unknown as Entity; + const annotations = { + metadata: { + annotations: { a: 'b' }, + }, + } as unknown as Entity; + const links = { + metadata: { links: [{}] }, + } as unknown as Entity; + + expect(parseFilterExpression('has:labels')(empty1)).toBe(false); + expect(parseFilterExpression('has:labels')(empty2)).toBe(false); + expect(parseFilterExpression('has:labels')(labels)).toBe(true); + + expect(parseFilterExpression('has:annotations')(empty1)).toBe(false); + expect(parseFilterExpression('has:annotations')(empty2)).toBe(false); + expect(parseFilterExpression('has:annotations')(annotations)).toBe(true); + + expect(parseFilterExpression('has:links')(empty1)).toBe(false); + expect(parseFilterExpression('has:links')(empty2)).toBe(false); + expect(parseFilterExpression('has:links')(links)).toBe(true); + + // match ANY of the parameters + expect(parseFilterExpression('has:labels,links')(empty1)).toBe(false); + expect(parseFilterExpression('has:labels,links')(empty2)).toBe(false); + expect(parseFilterExpression('has:labels,links')(labels)).toBe(true); + expect(parseFilterExpression('has:labels,links')(links)).toBe(true); + expect(parseFilterExpression('has:labels,links')(annotations)).toBe(false); + + expect(() => + parseFilterExpression('has:labels,bar'), + ).toThrowErrorMatchingInlineSnapshot( + `"'bar' is not a valid parameter for 'has' filter expressions, expected one of 'labels','annotations','links'"`, + ); + }); + + it('rejects unknown keys', () => { + expect(() => + parseFilterExpression('unknown:foo'), + ).toThrowErrorMatchingInlineSnapshot( + `"'unknown' is not a valid filter expression key, expected one of 'kind','type','is','has'"`, + ); + }); + + it('rejects malformed inputs', () => { + expect(() => parseFilterExpression(':')).toThrowErrorMatchingInlineSnapshot( + `"':' is not a valid filter expression, expected 'key:parameter' form"`, + ); + expect(() => + parseFilterExpression(':a'), + ).toThrowErrorMatchingInlineSnapshot( + `"':a' is not a valid filter expression, expected 'key:parameter' form"`, + ); + expect(() => + parseFilterExpression('a:'), + ).toThrowErrorMatchingInlineSnapshot( + `"'a:' is not a valid filter expression, expected 'key:parameter' form"`, + ); + }); +}); + +describe('splitFilterExpression', () => { + it('properly splits into expression atoms', () => { + expect(splitFilterExpression('')).toEqual([]); + expect(splitFilterExpression(' ')).toEqual([]); + expect(splitFilterExpression('kind:component')).toEqual([ + { key: 'kind', parameters: ['component'] }, + ]); + expect(splitFilterExpression('kind:component,user')).toEqual([ + { key: 'kind', parameters: ['component', 'user'] }, + ]); + expect(splitFilterExpression('kind:component,user type:foo')).toEqual([ + { key: 'kind', parameters: ['component', 'user'] }, + { key: 'type', parameters: ['foo'] }, + ]); + expect(splitFilterExpression('with:multiple:colons')).toEqual([ + { key: 'with', parameters: ['multiple:colons'] }, + ]); + }); + + it('rejects malformed inputs', () => { + expect(() => splitFilterExpression(':')).toThrowErrorMatchingInlineSnapshot( + `"':' is not a valid filter expression, expected 'key:parameter' form"`, + ); + expect(() => + splitFilterExpression(':a'), + ).toThrowErrorMatchingInlineSnapshot( + `"':a' is not a valid filter expression, expected 'key:parameter' form"`, + ); + expect(() => + splitFilterExpression('a:'), + ).toThrowErrorMatchingInlineSnapshot( + `"'a:' is not a valid filter expression, expected 'key:parameter' form"`, + ); + }); +}); diff --git a/plugins/catalog-common/src/filter/parseFilterExpression.ts b/plugins/catalog-common/src/filter/parseFilterExpression.ts new file mode 100644 index 0000000000..d2b46f582e --- /dev/null +++ b/plugins/catalog-common/src/filter/parseFilterExpression.ts @@ -0,0 +1,99 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Entity } from '@backstage/catalog-model'; +import { InputError } from '@backstage/errors'; +import { EntityMatcherFn } from './matrchers/types'; +import { createKindMatcher } from './matrchers/createKindMatcher'; +import { createTypeMatcher } from './matrchers/createTypeMatcher'; +import { createIsMatcher } from './matrchers/createIsMatcher'; +import { createHasMatcher } from './matrchers/createHasMatcher'; + +const rootMatcherFactories: Record< + string, + (parameters: string[]) => EntityMatcherFn +> = { + kind: createKindMatcher, + type: createTypeMatcher, + is: createIsMatcher, + has: createHasMatcher, +}; + +/** + * Parses a filter expression that decides whether to render an entity component + * or not. Returns a function that matches entities based on that expression. + * + * @alpha + * @remarks + * + * Filter strings are on the form `kind:user,group is:orphan`. There's + * effectively an AND between the space separated parts, and an OR between comma + * separated parameters. So the example filter string semantically means + * "entities that are of either User or Group kind, and also are orphans". + */ +export function parseFilterExpression( + expression: string, +): (entity: Entity) => boolean { + const parts = splitFilterExpression(expression); + + const matchers = parts.map(part => { + const factory = rootMatcherFactories[part.key]; + if (!factory) { + const known = Object.keys(rootMatcherFactories).map(m => `'${m}'`); + throw new InputError( + `'${part.key}' is not a valid filter expression key, expected one of ${known}`, + ); + } + return factory(part.parameters); + }); + + return (entity: Entity) => { + return matchers.every(matcher => { + try { + return matcher(entity); + } catch { + return false; + } + }); + }; +} + +export function splitFilterExpression( + expression: string, +): Array<{ key: string; parameters: string[] }> { + const words = expression + .split(' ') + .map(w => w.trim()) + .filter(Boolean); + + const result = new Array<{ key: string; parameters: string[] }>(); + + for (const word of words) { + const match = word.match(/^([^:]+):(.+)$/); + if (!match) { + throw new InputError( + `'${word}' is not a valid filter expression, expected 'key:parameter' form`, + ); + } + + const key = match[1]; + const parameters = match[2].split(','); + + result.push({ key, parameters }); + } + + return result; +} diff --git a/plugins/catalog-react/api-report-alpha.md b/plugins/catalog-react/api-report-alpha.md index ed851f810a..e284f80b8e 100644 --- a/plugins/catalog-react/api-report-alpha.md +++ b/plugins/catalog-react/api-report-alpha.md @@ -24,17 +24,12 @@ export function createEntityCardExtension< }; disabled?: boolean; inputs?: TInputs; - filter?: (ctx: { entity: Entity }) => boolean; + filter?: typeof entityFilterExtensionDataRef.T; loader: (options: { inputs: Expand>; }) => Promise; }): Extension<{ - filter?: - | { - isKind?: string | undefined; - isType?: string | undefined; - }[] - | undefined; + filter?: string | undefined; }>; // @alpha (undocumented) @@ -51,19 +46,14 @@ export function createEntityContentExtension< routeRef?: RouteRef; defaultPath: string; defaultTitle: string; - filter?: (ctx: { entity: Entity }) => boolean; + filter?: typeof entityFilterExtensionDataRef.T; loader: (options: { inputs: Expand>; }) => Promise; }): Extension<{ title: string; path: string; - filter?: - | { - isKind?: string | undefined; - isType?: string | undefined; - }[] - | undefined; + filter?: string | undefined; }>; // @alpha (undocumented) @@ -74,7 +64,7 @@ export const entityContentTitleExtensionDataRef: ConfigurableExtensionDataRef< // @alpha (undocumented) export const entityFilterExtensionDataRef: ConfigurableExtensionDataRef< - (ctx: { entity: Entity }) => boolean, + string | ((entity: Entity) => boolean), {} >; diff --git a/plugins/catalog-react/src/alpha.tsx b/plugins/catalog-react/src/alpha.tsx index a339816742..604c769fe5 100644 --- a/plugins/catalog-react/src/alpha.tsx +++ b/plugins/catalog-react/src/alpha.tsx @@ -14,7 +14,6 @@ * limitations under the License. */ -import React, { lazy } from 'react'; import { AnyExtensionInputMap, ExtensionBoundary, @@ -25,12 +24,13 @@ import { createExtensionDataRef, createSchemaFromZod, } from '@backstage/frontend-plugin-api'; +import React, { lazy } from 'react'; +import { Entity } from '@backstage/catalog-model'; // eslint-disable-next-line @backstage/no-relative-monorepo-imports import { Expand } from '../../../packages/frontend-plugin-api/src/types'; -import { Entity } from '@backstage/catalog-model'; -export { isOwnerOf } from './utils'; export { useEntityPermission } from './hooks/useEntityPermission'; +export { isOwnerOf } from './utils'; /** @alpha */ export const entityContentTitleExtensionDataRef = @@ -38,41 +38,9 @@ export const entityContentTitleExtensionDataRef = /** @alpha */ export const entityFilterExtensionDataRef = createExtensionDataRef< - (ctx: { entity: Entity }) => boolean + string | ((entity: Entity) => boolean) >('plugin.catalog.entity.filter'); -function applyFilter(a?: string, b?: string): boolean { - if (!a) { - return true; - } - return a.toLocaleLowerCase('en-US') === b?.toLocaleLowerCase('en-US'); -} - -// TODO: Only two hardcoded isKind and isType filters are available for now -// This is just an initial config filter implementation and needs to be revisited -function buildFilter( - config: { filter?: { isKind?: string; isType?: string }[] }, - filterFunc?: (ctx: { entity: Entity }) => boolean, -) { - return (ctx: { entity: Entity }) => { - const configuredFilterMatch = config.filter?.some(filter => { - const kindMatch = applyFilter(filter.isKind, ctx.entity.kind); - const typeMatch = applyFilter( - filter.isType, - ctx.entity.spec?.type?.toString(), - ); - return kindMatch && typeMatch; - }); - if (configuredFilterMatch) { - return true; - } - if (filterFunc) { - return filterFunc(ctx); - } - return true; - }; -} - // TODO: Figure out how to merge with provided config schema /** @alpha */ export function createEntityCardExtension< @@ -82,7 +50,7 @@ export function createEntityCardExtension< attachTo?: { id: string; input: string }; disabled?: boolean; inputs?: TInputs; - filter?: (ctx: { entity: Entity }) => boolean; + filter?: typeof entityFilterExtensionDataRef.T; loader: (options: { inputs: Expand>; }) => Promise; @@ -98,19 +66,12 @@ export function createEntityCardExtension< disabled: options.disabled ?? true, output: { element: coreExtensionData.reactElement, - filter: entityFilterExtensionDataRef, + filter: entityFilterExtensionDataRef.optional(), }, inputs: options.inputs, configSchema: createSchemaFromZod(z => z.object({ - filter: z - .array( - z.object({ - isKind: z.string().optional(), - isType: z.string().optional(), - }), - ) - .optional(), + filter: z.string().optional(), }), ), factory({ config, inputs, node }) { @@ -126,7 +87,7 @@ export function createEntityCardExtension< ), - filter: buildFilter(config, options.filter), + filter: config.filter ?? options.filter, }; }, }); @@ -143,7 +104,7 @@ export function createEntityContentExtension< routeRef?: RouteRef; defaultPath: string; defaultTitle: string; - filter?: (ctx: { entity: Entity }) => boolean; + filter?: typeof entityFilterExtensionDataRef.T; loader: (options: { inputs: Expand>; }) => Promise; @@ -162,21 +123,14 @@ export function createEntityContentExtension< path: coreExtensionData.routePath, routeRef: coreExtensionData.routeRef.optional(), title: entityContentTitleExtensionDataRef, - filter: entityFilterExtensionDataRef, + filter: entityFilterExtensionDataRef.optional(), }, inputs: options.inputs, configSchema: createSchemaFromZod(z => z.object({ path: z.string().default(options.defaultPath), title: z.string().default(options.defaultTitle), - filter: z - .array( - z.object({ - isKind: z.string().optional(), - isType: z.string().optional(), - }), - ) - .optional(), + filter: z.string().optional(), }), ), factory({ config, inputs, node }) { @@ -195,7 +149,7 @@ export function createEntityContentExtension< ), - filter: buildFilter(config, options.filter), + filter: config.filter ?? options.filter, }; }, }); diff --git a/plugins/catalog/src/alpha/EntityOverviewPage.tsx b/plugins/catalog/src/alpha/EntityOverviewPage.tsx index baa93845ae..02ec044975 100644 --- a/plugins/catalog/src/alpha/EntityOverviewPage.tsx +++ b/plugins/catalog/src/alpha/EntityOverviewPage.tsx @@ -14,29 +14,51 @@ * limitations under the License. */ -import React from 'react'; -import { useEntity } from '@backstage/plugin-catalog-react'; import { Entity } from '@backstage/catalog-model'; +import { parseFilterExpression } from '@backstage/plugin-catalog-common/alpha'; +import { useEntity } from '@backstage/plugin-catalog-react'; import Grid from '@material-ui/core/Grid'; +import React, { useMemo } from 'react'; interface EntityOverviewPageProps { cards: Array<{ element: React.JSX.Element; - filter: (ctx: { entity: Entity }) => boolean; + filter?: string | ((entity: Entity) => boolean); }>; } +function CardWrapper(props: { + entity: Entity; + element: React.JSX.Element; + filter?: string | ((entity: Entity) => boolean); +}) { + const { entity, element, filter } = props; + + const filterFn = useMemo<(subject: Entity) => boolean>(() => { + if (!filter) { + return () => true; + } else if (typeof filter === 'function') { + return subject => filter(subject); + } + return parseFilterExpression(filter); + }, [filter]); + + return filterFn(entity) ? <>{element} : null; +} + export function EntityOverviewPage(props: EntityOverviewPageProps) { const { entity } = useEntity(); return ( - {props.cards - .filter(card => card.filter({ entity })) - .map(card => ( - - {card.element} - - ))} + {props.cards.map((card, index) => ( + + + + ))} ); } diff --git a/plugins/catalog/src/alpha/entityCards.tsx b/plugins/catalog/src/alpha/entityCards.tsx index c7b75679a7..572e2caa3c 100644 --- a/plugins/catalog/src/alpha/entityCards.tsx +++ b/plugins/catalog/src/alpha/entityCards.tsx @@ -27,7 +27,7 @@ export const EntityAboutCard = createEntityCardExtension({ export const EntityLinksCard = createEntityCardExtension({ id: 'links', - filter: ({ entity }) => Boolean(entity.metadata.links), + filter: 'has:links', loader: async () => import('../components/EntityLinksCard').then(m => { return ; @@ -36,7 +36,7 @@ export const EntityLinksCard = createEntityCardExtension({ export const EntityLabelsCard = createEntityCardExtension({ id: 'labels', - filter: ({ entity }) => Boolean(entity.metadata.labels), + filter: 'has:labels', loader: async () => import('../components/EntityLabelsCard').then(m => ( diff --git a/plugins/catalog/src/alpha/entityContents.tsx b/plugins/catalog/src/alpha/entityContents.tsx index 9bef4a5e0d..0bbee9bb7d 100644 --- a/plugins/catalog/src/alpha/entityContents.tsx +++ b/plugins/catalog/src/alpha/entityContents.tsx @@ -32,7 +32,7 @@ export const OverviewEntityContent = createEntityContentExtension({ inputs: { cards: createExtensionInput({ element: coreExtensionData.reactElement, - filter: entityFilterExtensionDataRef, + filter: entityFilterExtensionDataRef.optional(), }), }, loader: async ({ inputs }) => diff --git a/plugins/catalog/src/alpha/plugin.tsx b/plugins/catalog/src/alpha/plugin.tsx index 7140259df0..99aacd27da 100644 --- a/plugins/catalog/src/alpha/plugin.tsx +++ b/plugins/catalog/src/alpha/plugin.tsx @@ -22,8 +22,8 @@ import { entityRouteRef } from '@backstage/plugin-catalog-react'; import { createComponentRouteRef, createFromTemplateRouteRef, - unregisterRedirectRouteRef, rootRouteRef, + unregisterRedirectRouteRef, viewTechDocRouteRef, } from '../routes'; diff --git a/yarn.lock b/yarn.lock index 2399f06ac5..90be768746 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5831,6 +5831,7 @@ __metadata: dependencies: "@backstage/catalog-model": "workspace:^" "@backstage/cli": "workspace:^" + "@backstage/errors": "workspace:^" "@backstage/plugin-permission-common": "workspace:^" "@backstage/plugin-search-common": "workspace:^" languageName: unknown From 976519c3dc5fafed0788807e783ca88551710d74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Thu, 23 Nov 2023 11:31:03 +0100 Subject: [PATCH 2/8] add tests and error propagation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- plugins/catalog-common/api-report-alpha.md | 15 +++ .../filter/matrchers/createHasMatcher.test.ts | 109 ++++++++++++++++ .../src/filter/matrchers/createHasMatcher.ts | 19 ++- .../filter/matrchers/createIsMatcher.test.ts | 75 +++++++++++ .../src/filter/matrchers/createIsMatcher.ts | 19 ++- .../matrchers/createKindMatcher.test.ts | 46 +++++++ .../src/filter/matrchers/createKindMatcher.ts | 5 +- .../matrchers/createTypeMatcher.test.ts | 54 ++++++++ .../src/filter/matrchers/createTypeMatcher.ts | 5 +- .../src/filter/parseFilterExpression.test.ts | 122 ++++++++---------- .../src/filter/parseFilterExpression.ts | 52 +++++++- .../catalog/src/alpha/EntityOverviewPage.tsx | 28 ++-- 12 files changed, 449 insertions(+), 100 deletions(-) create mode 100644 plugins/catalog-common/src/filter/matrchers/createHasMatcher.test.ts create mode 100644 plugins/catalog-common/src/filter/matrchers/createIsMatcher.test.ts create mode 100644 plugins/catalog-common/src/filter/matrchers/createKindMatcher.test.ts create mode 100644 plugins/catalog-common/src/filter/matrchers/createTypeMatcher.test.ts diff --git a/plugins/catalog-common/api-report-alpha.md b/plugins/catalog-common/api-report-alpha.md index 573b35d3b3..bd36b499fe 100644 --- a/plugins/catalog-common/api-report-alpha.md +++ b/plugins/catalog-common/api-report-alpha.md @@ -42,6 +42,21 @@ export const catalogPermissions: ( // @alpha export function parseFilterExpression( expression: string, + options?: { + onParseError?: ( + error: Error, + context: { + expression: string; + }, + ) => void; + onEvaluateError?: ( + error: Error, + context: { + expression: string; + entity: Entity; + }, + ) => void; + }, ): (entity: Entity) => boolean; // @alpha diff --git a/plugins/catalog-common/src/filter/matrchers/createHasMatcher.test.ts b/plugins/catalog-common/src/filter/matrchers/createHasMatcher.test.ts new file mode 100644 index 0000000000..dbce5025b6 --- /dev/null +++ b/plugins/catalog-common/src/filter/matrchers/createHasMatcher.test.ts @@ -0,0 +1,109 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Entity } from '@backstage/catalog-model'; +import { createHasMatcher } from './createHasMatcher'; + +describe('createHasMatcher', () => { + const empty1 = { + metadata: {}, + } as unknown as Entity; + const empty2 = { + metadata: { + labels: {}, + annotations: {}, + links: [], + }, + } as unknown as Entity; + const labels = { + metadata: { + labels: { a: 'b' }, + }, + } as unknown as Entity; + const annotations = { + metadata: { + annotations: { a: 'b' }, + }, + } as unknown as Entity; + const links = { + metadata: { links: [{}] }, + } as unknown as Entity; + + const err = jest.fn(); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('supports valid parameters', () => { + expect(createHasMatcher(['labels'], err)(empty1)).toBe(false); + expect(createHasMatcher(['labels'], err)(empty2)).toBe(false); + expect(createHasMatcher(['labels'], err)(labels)).toBe(true); + + expect(createHasMatcher(['annotations'], err)(empty1)).toBe(false); + expect(createHasMatcher(['annotations'], err)(empty2)).toBe(false); + expect(createHasMatcher(['annotations'], err)(annotations)).toBe(true); + + expect(createHasMatcher(['links'], err)(empty1)).toBe(false); + expect(createHasMatcher(['links'], err)(empty2)).toBe(false); + expect(createHasMatcher(['links'], err)(links)).toBe(true); + + expect(err).not.toHaveBeenCalled(); + }); + + it('matches if ANY parameter matches', () => { + expect(createHasMatcher(['labels', 'links'], err)(empty1)).toBe(false); + expect(createHasMatcher(['labels', 'links'], err)(empty2)).toBe(false); + expect(createHasMatcher(['labels', 'links'], err)(labels)).toBe(true); + expect(createHasMatcher(['labels', 'links'], err)(links)).toBe(true); + expect(createHasMatcher(['labels', 'links'], err)(annotations)).toBe(false); + + expect(err).not.toHaveBeenCalled(); + }); + + it('emits errors properly, and skips over bad parameters', () => { + // throw if callback throws + expect(() => + createHasMatcher(['labels', 'bar'], e => { + throw e; + }), + ).toThrowErrorMatchingInlineSnapshot( + `"'bar' is not a valid parameter for 'has' filter expressions, expected one of 'labels','annotations','links'"`, + ); + expect(err).not.toHaveBeenCalled(); + + // continue if callback does not throw, and skip over the bad parameter + let matcher = createHasMatcher(['labels', 'foo', 'bar'], err); + expect(err).toHaveBeenCalledTimes(2); + expect(err).toHaveBeenCalledWith( + expect.objectContaining({ + message: `'foo' is not a valid parameter for 'has' filter expressions, expected one of 'labels','annotations','links'`, + }), + ); + expect(err).toHaveBeenCalledWith( + expect.objectContaining({ + message: `'bar' is not a valid parameter for 'has' filter expressions, expected one of 'labels','annotations','links'`, + }), + ); + expect(matcher(empty1)).toBe(false); + expect(matcher(labels)).toBe(true); + + // when no parameters at all match, just return true + matcher = createHasMatcher(['foo', 'bar'], err); + expect(matcher(empty1)).toBe(true); + expect(matcher(labels)).toBe(true); + }); +}); diff --git a/plugins/catalog-common/src/filter/matrchers/createHasMatcher.ts b/plugins/catalog-common/src/filter/matrchers/createHasMatcher.ts index 7b9eebba1c..f57316a65e 100644 --- a/plugins/catalog-common/src/filter/matrchers/createHasMatcher.ts +++ b/plugins/catalog-common/src/filter/matrchers/createHasMatcher.ts @@ -20,7 +20,10 @@ import { EntityMatcherFn } from './types'; /** * Matches on the non-empty presence of different parts of the entity */ -export function createHasMatcher(parameters: string[]): EntityMatcherFn { +export function createHasMatcher( + parameters: string[], + onParseError: (error: Error) => void, +): EntityMatcherFn { const allowedMatchers: Record = { labels: entity => { return Object.keys(entity.metadata.labels ?? {}).length > 0; @@ -33,16 +36,20 @@ export function createHasMatcher(parameters: string[]): EntityMatcherFn { }, }; - const matchers = parameters.map(parameter => { + const matchers = parameters.flatMap(parameter => { const matcher = allowedMatchers[parameter.toLocaleLowerCase('en-US')]; if (!matcher) { const known = Object.keys(allowedMatchers).map(m => `'${m}'`); - throw new InputError( - `'${parameter}' is not a valid parameter for 'has' filter expressions, expected one of ${known}`, + onParseError( + new InputError( + `'${parameter}' is not a valid parameter for 'has' filter expressions, expected one of ${known}`, + ), ); + return []; } - return matcher; + return [matcher]; }); - return entity => matchers.some(matcher => matcher(entity)); + return entity => + matchers.length ? matchers.some(matcher => matcher(entity)) : true; } diff --git a/plugins/catalog-common/src/filter/matrchers/createIsMatcher.test.ts b/plugins/catalog-common/src/filter/matrchers/createIsMatcher.test.ts new file mode 100644 index 0000000000..4cb3094d62 --- /dev/null +++ b/plugins/catalog-common/src/filter/matrchers/createIsMatcher.test.ts @@ -0,0 +1,75 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Entity } from '@backstage/catalog-model'; +import { createIsMatcher } from './createIsMatcher'; + +describe('createIsMatcher', () => { + const empty = { + metadata: {}, + } as unknown as Entity; + const orphan = { + metadata: { + annotations: { ['backstage.io/orphan']: 'true' }, + }, + } as unknown as Entity; + + const err = jest.fn(); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('supports valid parameters', () => { + expect(createIsMatcher(['orphan'], err)(empty)).toBe(false); + expect(createIsMatcher(['orphan'], err)(orphan)).toBe(true); + + expect(err).not.toHaveBeenCalled(); + }); + + it('emits errors properly, and skips over bad parameters', () => { + // throw if callback throws + expect(() => + createIsMatcher(['orphan', 'foo'], e => { + throw e; + }), + ).toThrowErrorMatchingInlineSnapshot( + `"'foo' is not a valid parameter for 'is' filter expressions, expected one of 'orphan'"`, + ); + expect(err).not.toHaveBeenCalled(); + + // continue if callback does not throw, and skip over the bad parameter + let matcher = createIsMatcher(['orphan', 'foo', 'bar'], err); + expect(err).toHaveBeenCalledTimes(2); + expect(err).toHaveBeenCalledWith( + expect.objectContaining({ + message: `'foo' is not a valid parameter for 'is' filter expressions, expected one of 'orphan'`, + }), + ); + expect(err).toHaveBeenCalledWith( + expect.objectContaining({ + message: `'bar' is not a valid parameter for 'is' filter expressions, expected one of 'orphan'`, + }), + ); + expect(matcher(empty)).toBe(false); + expect(matcher(orphan)).toBe(true); + + // when no parameters at all match, just return true + matcher = createIsMatcher(['foo', 'bar'], err); + expect(matcher(empty)).toBe(true); + expect(matcher(orphan)).toBe(true); + }); +}); diff --git a/plugins/catalog-common/src/filter/matrchers/createIsMatcher.ts b/plugins/catalog-common/src/filter/matrchers/createIsMatcher.ts index 3802d6dbce..30bfe4eb15 100644 --- a/plugins/catalog-common/src/filter/matrchers/createIsMatcher.ts +++ b/plugins/catalog-common/src/filter/matrchers/createIsMatcher.ts @@ -20,22 +20,29 @@ import { EntityMatcherFn } from './types'; /** * Matches on different semantic properties of the entity */ -export function createIsMatcher(parameters: string[]): EntityMatcherFn { +export function createIsMatcher( + parameters: string[], + onParseError: (error: Error) => void, +): EntityMatcherFn { const allowedMatchers: Record = { orphan: entity => Boolean(entity.metadata.annotations?.['backstage.io/orphan']), }; - const matchers = parameters.map(parameter => { + const matchers = parameters.flatMap(parameter => { const matcher = allowedMatchers[parameter.toLocaleLowerCase('en-US')]; if (!matcher) { const known = Object.keys(allowedMatchers).map(m => `'${m}'`); - throw new InputError( - `'${parameter}' is not a valid parameter for 'is' filter expressions, expected one of ${known}`, + onParseError( + new InputError( + `'${parameter}' is not a valid parameter for 'is' filter expressions, expected one of ${known}`, + ), ); + return []; } - return matcher; + return [matcher]; }); - return entity => matchers.some(matcher => matcher(entity)); + return entity => + matchers.length ? matchers.some(matcher => matcher(entity)) : true; } diff --git a/plugins/catalog-common/src/filter/matrchers/createKindMatcher.test.ts b/plugins/catalog-common/src/filter/matrchers/createKindMatcher.test.ts new file mode 100644 index 0000000000..f109acccd4 --- /dev/null +++ b/plugins/catalog-common/src/filter/matrchers/createKindMatcher.test.ts @@ -0,0 +1,46 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Entity } from '@backstage/catalog-model'; +import { createKindMatcher } from './createKindMatcher'; + +describe('createKindMatcher', () => { + const component = { kind: 'Component' } as unknown as Entity; + const user = { kind: 'User' } as unknown as Entity; + const resource = { kind: 'Resource' } as unknown as Entity; + + const err = jest.fn(); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('supports valid parameters', () => { + expect(createKindMatcher(['component'], err)(component)).toBe(true); + expect(createKindMatcher(['componenT'], err)(component)).toBe(true); + expect(createKindMatcher(['user'], err)(component)).toBe(false); + + expect(err).not.toHaveBeenCalled(); + }); + + it('matches if ANY parameter matches', () => { + expect(createKindMatcher(['user', 'component'], err)(user)).toBe(true); + expect(createKindMatcher(['user', 'component'], err)(component)).toBe(true); + expect(createKindMatcher(['user', 'component'], err)(resource)).toBe(false); + + expect(err).not.toHaveBeenCalled(); + }); +}); diff --git a/plugins/catalog-common/src/filter/matrchers/createKindMatcher.ts b/plugins/catalog-common/src/filter/matrchers/createKindMatcher.ts index 071746453e..4f7c475ebe 100644 --- a/plugins/catalog-common/src/filter/matrchers/createKindMatcher.ts +++ b/plugins/catalog-common/src/filter/matrchers/createKindMatcher.ts @@ -19,7 +19,10 @@ import { EntityMatcherFn } from './types'; /** * Matches on kind */ -export function createKindMatcher(parameters: string[]): EntityMatcherFn { +export function createKindMatcher( + parameters: string[], + _onParseError: (error: Error) => void, +): EntityMatcherFn { const items = parameters.map(p => p.toLocaleLowerCase('en-US')); return entity => items.includes(entity.kind.toLocaleLowerCase('en-US')); } diff --git a/plugins/catalog-common/src/filter/matrchers/createTypeMatcher.test.ts b/plugins/catalog-common/src/filter/matrchers/createTypeMatcher.test.ts new file mode 100644 index 0000000000..31c9af7e11 --- /dev/null +++ b/plugins/catalog-common/src/filter/matrchers/createTypeMatcher.test.ts @@ -0,0 +1,54 @@ +/* + * Copyright 2023 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Entity } from '@backstage/catalog-model'; +import { createTypeMatcher } from './createTypeMatcher'; + +describe('createTypeMatcher', () => { + const empty1 = {} as unknown as Entity; + const empty2 = { spec: {} } as unknown as Entity; + const service = { spec: { type: 'service' } } as unknown as Entity; + const website = { spec: { type: 'website' } } as unknown as Entity; + const pipeline = { spec: { type: 'pipeline' } } as unknown as Entity; + + const err = jest.fn(); + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('supports valid parameters', () => { + expect(createTypeMatcher(['service'], err)(empty1)).toBe(false); + expect(createTypeMatcher(['service'], err)(empty2)).toBe(false); + + expect(createTypeMatcher(['service'], err)(service)).toBe(true); + expect(createTypeMatcher(['sErViCe'], err)(service)).toBe(true); + expect(createTypeMatcher(['service'], err)(service)).toBe(true); + expect(createTypeMatcher(['website'], err)(service)).toBe(false); + + expect(err).not.toHaveBeenCalled(); + }); + + it('matches if ANY parameter matches', () => { + expect(createTypeMatcher(['service', 'website'], err)(service)).toBe(true); + expect(createTypeMatcher(['service', 'website'], err)(website)).toBe(true); + expect(createTypeMatcher(['service', 'website'], err)(pipeline)).toBe( + false, + ); + + expect(err).not.toHaveBeenCalled(); + }); +}); diff --git a/plugins/catalog-common/src/filter/matrchers/createTypeMatcher.ts b/plugins/catalog-common/src/filter/matrchers/createTypeMatcher.ts index d0e0f39c98..31f7a5a461 100644 --- a/plugins/catalog-common/src/filter/matrchers/createTypeMatcher.ts +++ b/plugins/catalog-common/src/filter/matrchers/createTypeMatcher.ts @@ -19,7 +19,10 @@ import { EntityMatcherFn } from './types'; /** * Matches on spec.type */ -export function createTypeMatcher(parameters: string[]): EntityMatcherFn { +export function createTypeMatcher( + parameters: string[], + _onParseError: (error: Error) => void, +): EntityMatcherFn { const items = parameters.map(p => p.toLocaleLowerCase('en-US')); return entity => { const value = entity.spec?.type; diff --git a/plugins/catalog-common/src/filter/parseFilterExpression.test.ts b/plugins/catalog-common/src/filter/parseFilterExpression.test.ts index d8543c4e10..a53b84646e 100644 --- a/plugins/catalog-common/src/filter/parseFilterExpression.test.ts +++ b/plugins/catalog-common/src/filter/parseFilterExpression.test.ts @@ -21,19 +21,34 @@ import { } from './parseFilterExpression'; describe('parseFilterExpression', () => { + function run(expression: string) { + return parseFilterExpression(expression, { + onParseError(e: Error) { + throw e; + }, + }); + } + it('supports "kind" expressions', () => { const component = { kind: 'Component' } as unknown as Entity; const user = { kind: 'User' } as unknown as Entity; const resource = { kind: 'Resource' } as unknown as Entity; - expect(parseFilterExpression('kind:component')(component)).toBe(true); - expect(parseFilterExpression('kind:componenT')(component)).toBe(true); - expect(parseFilterExpression('kind:user')(component)).toBe(false); + expect(run('kind:user,component')(user)).toBe(true); + expect(run('kind:user,component')(component)).toBe(true); + expect(run('kind:user,component')(resource)).toBe(false); + }); - // match ANY of the parameters - expect(parseFilterExpression('kind:user,component')(user)).toBe(true); - expect(parseFilterExpression('kind:user,component')(component)).toBe(true); - expect(parseFilterExpression('kind:user,component')(resource)).toBe(false); + it('supports "type" expressions', () => { + const empty = {} as unknown as Entity; + const service = { spec: { type: 'service' } } as unknown as Entity; + const website = { spec: { type: 'website' } } as unknown as Entity; + const pipeline = { spec: { type: 'pipeline' } } as unknown as Entity; + + expect(run('type:service,website')(empty)).toBe(false); + expect(run('type:service,website')(service)).toBe(true); + expect(run('type:service,website')(website)).toBe(true); + expect(run('type:service,website')(pipeline)).toBe(false); }); it('supports "is" expressions', () => { @@ -46,123 +61,90 @@ describe('parseFilterExpression', () => { }, } as unknown as Entity; - expect(parseFilterExpression('is:orphan')(empty)).toBe(false); - expect(parseFilterExpression('is:orphan')(orphan)).toBe(true); + expect(run('is:orphan')(empty)).toBe(false); + expect(run('is:orphan')(orphan)).toBe(true); - expect(() => - parseFilterExpression('is:orphan,bar'), - ).toThrowErrorMatchingInlineSnapshot( + expect(() => run('is:orphan,bar')).toThrowErrorMatchingInlineSnapshot( `"'bar' is not a valid parameter for 'is' filter expressions, expected one of 'orphan'"`, ); }); it('supports "has" expressions', () => { - const empty1 = { + const empty = { metadata: {}, } as unknown as Entity; - const empty2 = { - metadata: { - labels: {}, - annotations: {}, - links: [], - }, - } as unknown as Entity; const labels = { - metadata: { - labels: { a: 'b' }, - }, + metadata: { labels: { a: 'b' } }, } as unknown as Entity; const annotations = { - metadata: { - annotations: { a: 'b' }, - }, + metadata: { annotations: { a: 'b' } }, } as unknown as Entity; const links = { metadata: { links: [{}] }, } as unknown as Entity; - expect(parseFilterExpression('has:labels')(empty1)).toBe(false); - expect(parseFilterExpression('has:labels')(empty2)).toBe(false); - expect(parseFilterExpression('has:labels')(labels)).toBe(true); + expect(run('has:labels,links')(empty)).toBe(false); + expect(run('has:labels,links')(labels)).toBe(true); + expect(run('has:labels,links')(links)).toBe(true); + expect(run('has:labels,links')(annotations)).toBe(false); - expect(parseFilterExpression('has:annotations')(empty1)).toBe(false); - expect(parseFilterExpression('has:annotations')(empty2)).toBe(false); - expect(parseFilterExpression('has:annotations')(annotations)).toBe(true); - - expect(parseFilterExpression('has:links')(empty1)).toBe(false); - expect(parseFilterExpression('has:links')(empty2)).toBe(false); - expect(parseFilterExpression('has:links')(links)).toBe(true); - - // match ANY of the parameters - expect(parseFilterExpression('has:labels,links')(empty1)).toBe(false); - expect(parseFilterExpression('has:labels,links')(empty2)).toBe(false); - expect(parseFilterExpression('has:labels,links')(labels)).toBe(true); - expect(parseFilterExpression('has:labels,links')(links)).toBe(true); - expect(parseFilterExpression('has:labels,links')(annotations)).toBe(false); - - expect(() => - parseFilterExpression('has:labels,bar'), - ).toThrowErrorMatchingInlineSnapshot( + expect(() => run('has:labels,bar')).toThrowErrorMatchingInlineSnapshot( `"'bar' is not a valid parameter for 'has' filter expressions, expected one of 'labels','annotations','links'"`, ); }); it('rejects unknown keys', () => { - expect(() => - parseFilterExpression('unknown:foo'), - ).toThrowErrorMatchingInlineSnapshot( + expect(() => run('unknown:foo')).toThrowErrorMatchingInlineSnapshot( `"'unknown' is not a valid filter expression key, expected one of 'kind','type','is','has'"`, ); }); it('rejects malformed inputs', () => { - expect(() => parseFilterExpression(':')).toThrowErrorMatchingInlineSnapshot( + expect(() => run(':')).toThrowErrorMatchingInlineSnapshot( `"':' is not a valid filter expression, expected 'key:parameter' form"`, ); - expect(() => - parseFilterExpression(':a'), - ).toThrowErrorMatchingInlineSnapshot( + expect(() => run(':a')).toThrowErrorMatchingInlineSnapshot( `"':a' is not a valid filter expression, expected 'key:parameter' form"`, ); - expect(() => - parseFilterExpression('a:'), - ).toThrowErrorMatchingInlineSnapshot( + expect(() => run('a:')).toThrowErrorMatchingInlineSnapshot( `"'a:' is not a valid filter expression, expected 'key:parameter' form"`, ); }); }); describe('splitFilterExpression', () => { + function run(expression: string) { + return splitFilterExpression(expression, e => { + throw e; + }); + } + it('properly splits into expression atoms', () => { - expect(splitFilterExpression('')).toEqual([]); - expect(splitFilterExpression(' ')).toEqual([]); - expect(splitFilterExpression('kind:component')).toEqual([ + expect(run('')).toEqual([]); + expect(run(' ')).toEqual([]); + expect(run('kind:component')).toEqual([ { key: 'kind', parameters: ['component'] }, ]); - expect(splitFilterExpression('kind:component,user')).toEqual([ + expect(run('kind:component,user')).toEqual([ { key: 'kind', parameters: ['component', 'user'] }, ]); - expect(splitFilterExpression('kind:component,user type:foo')).toEqual([ + expect(run('kind:component,user type:foo')).toEqual([ { key: 'kind', parameters: ['component', 'user'] }, { key: 'type', parameters: ['foo'] }, ]); - expect(splitFilterExpression('with:multiple:colons')).toEqual([ + expect(run('with:multiple:colons')).toEqual([ { key: 'with', parameters: ['multiple:colons'] }, ]); }); it('rejects malformed inputs', () => { - expect(() => splitFilterExpression(':')).toThrowErrorMatchingInlineSnapshot( + expect(() => run(':')).toThrowErrorMatchingInlineSnapshot( `"':' is not a valid filter expression, expected 'key:parameter' form"`, ); - expect(() => - splitFilterExpression(':a'), - ).toThrowErrorMatchingInlineSnapshot( + expect(() => run(':a')).toThrowErrorMatchingInlineSnapshot( `"':a' is not a valid filter expression, expected 'key:parameter' form"`, ); - expect(() => - splitFilterExpression('a:'), - ).toThrowErrorMatchingInlineSnapshot( + expect(() => run('a:')).toThrowErrorMatchingInlineSnapshot( `"'a:' is not a valid filter expression, expected 'key:parameter' form"`, ); }); diff --git a/plugins/catalog-common/src/filter/parseFilterExpression.ts b/plugins/catalog-common/src/filter/parseFilterExpression.ts index d2b46f582e..ffd25d75d3 100644 --- a/plugins/catalog-common/src/filter/parseFilterExpression.ts +++ b/plugins/catalog-common/src/filter/parseFilterExpression.ts @@ -24,7 +24,10 @@ import { createHasMatcher } from './matrchers/createHasMatcher'; const rootMatcherFactories: Record< string, - (parameters: string[]) => EntityMatcherFn + ( + parameters: string[], + onParseError: (error: Error) => void, + ) => EntityMatcherFn > = { kind: createKindMatcher, type: createTypeMatcher, @@ -43,11 +46,39 @@ const rootMatcherFactories: Record< * effectively an AND between the space separated parts, and an OR between comma * separated parameters. So the example filter string semantically means * "entities that are of either User or Group kind, and also are orphans". + * + * The `onParseError` callback is called whenever an error was encountered + * during initial parsing of the expression. If the callback throws, + * `parseFilterExpression` throws that same error. If the callback does not + * throw, the part of the input expression that had an error is ignored entirely + * and parsing continues. + * + * The `onEvaluateError` callback is called whenever an error was encountered + * during evaluation of the returned function. If the callback throws, the + * evaluation call throws the same error. If the callback does not throw, the + * whole evaluation returns false. */ export function parseFilterExpression( expression: string, + options?: { + onParseError?: ( + error: Error, + context: { + expression: string; + }, + ) => void; + onEvaluateError?: ( + error: Error, + context: { + expression: string; + entity: Entity; + }, + ) => void; + }, ): (entity: Entity) => boolean { - const parts = splitFilterExpression(expression); + const parts = splitFilterExpression(expression, e => + options?.onParseError?.(e, { expression }), + ); const matchers = parts.map(part => { const factory = rootMatcherFactories[part.key]; @@ -57,14 +88,17 @@ export function parseFilterExpression( `'${part.key}' is not a valid filter expression key, expected one of ${known}`, ); } - return factory(part.parameters); + return factory(part.parameters, e => + options?.onParseError?.(e, { expression }), + ); }); return (entity: Entity) => { return matchers.every(matcher => { try { return matcher(entity); - } catch { + } catch (e) { + options?.onEvaluateError?.(e, { expression, entity }); return false; } }); @@ -73,6 +107,7 @@ export function parseFilterExpression( export function splitFilterExpression( expression: string, + onParseError: (error: Error) => void, ): Array<{ key: string; parameters: string[] }> { const words = expression .split(' ') @@ -84,13 +119,16 @@ export function splitFilterExpression( for (const word of words) { const match = word.match(/^([^:]+):(.+)$/); if (!match) { - throw new InputError( - `'${word}' is not a valid filter expression, expected 'key:parameter' form`, + onParseError( + new InputError( + `'${word}' is not a valid filter expression, expected 'key:parameter' form`, + ), ); + continue; } const key = match[1]; - const parameters = match[2].split(','); + const parameters = match[2].split(',').filter(Boolean); // silently ignore double commas result.push({ key, parameters }); } diff --git a/plugins/catalog/src/alpha/EntityOverviewPage.tsx b/plugins/catalog/src/alpha/EntityOverviewPage.tsx index 02ec044975..5ef6b9a2e5 100644 --- a/plugins/catalog/src/alpha/EntityOverviewPage.tsx +++ b/plugins/catalog/src/alpha/EntityOverviewPage.tsx @@ -40,10 +40,21 @@ function CardWrapper(props: { } else if (typeof filter === 'function') { return subject => filter(subject); } - return parseFilterExpression(filter); + return parseFilterExpression(filter, { + onParseError(_error) { + // ignore silently + }, + onEvaluateError(_error) { + // ignore silently + }, + }); }, [filter]); - return filterFn(entity) ? <>{element} : null; + return filterFn(entity) ? ( + + {element} + + ) : null; } export function EntityOverviewPage(props: EntityOverviewPageProps) { @@ -51,13 +62,12 @@ export function EntityOverviewPage(props: EntityOverviewPageProps) { return ( {props.cards.map((card, index) => ( - - - + ))} ); From 3bb23d78f79f5ee94d50d2251a64381dc624daa0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Thu, 23 Nov 2023 14:59:23 +0100 Subject: [PATCH 3/8] Update .changeset/stale-jokes-hunt.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Patrik Oldsberg Signed-off-by: Fredrik Adelöw --- .changeset/stale-jokes-hunt.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/stale-jokes-hunt.md b/.changeset/stale-jokes-hunt.md index cbea2dd229..1e78a3a446 100644 --- a/.changeset/stale-jokes-hunt.md +++ b/.changeset/stale-jokes-hunt.md @@ -2,4 +2,4 @@ '@backstage/plugin-catalog-common': minor --- -Added a `parseFilterExpression` function to interpret visibility filters +Added a `parseFilterExpression` function to interpret visibility filters, exported from the `/alpha` sub-path. From c2dac8cc1ed41964fd991e5eefb7bec8928c59d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Thu, 23 Nov 2023 14:59:39 +0100 Subject: [PATCH 4/8] Update .changeset/thick-snails-travel.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Patrik Oldsberg Signed-off-by: Fredrik Adelöw --- .changeset/thick-snails-travel.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/thick-snails-travel.md b/.changeset/thick-snails-travel.md index af27df78e5..bfd14d55e1 100644 --- a/.changeset/thick-snails-travel.md +++ b/.changeset/thick-snails-travel.md @@ -2,4 +2,4 @@ '@backstage/plugin-catalog': minor --- -Properly support both function- and string-form visibility filter expressions +Properly support both function- and string-form visibility filter expressions in the new extensions exported via `/alpha`. From 423e33d81ffcd9bf514f2e09bf28387209cc107c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Thu, 23 Nov 2023 15:56:20 +0100 Subject: [PATCH 5/8] move over to catalog and make internal, return errors in array MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/stale-jokes-hunt.md | 5 -- packages/app-next/app-config.yaml | 2 +- plugins/catalog-common/api-report-alpha.md | 21 ------ plugins/catalog-common/package.json | 1 - plugins/catalog-common/src/alpha.ts | 1 - plugins/catalog-common/src/filter/index.ts | 17 ----- .../catalog/src/alpha/EntityOverviewPage.tsx | 26 ++++--- .../filter/matrchers/createHasMatcher.test.ts | 0 .../filter/matrchers/createHasMatcher.ts | 0 .../filter/matrchers/createIsMatcher.test.ts | 0 .../filter/matrchers/createIsMatcher.ts | 0 .../matrchers/createKindMatcher.test.ts | 0 .../filter/matrchers/createKindMatcher.ts | 0 .../matrchers/createTypeMatcher.test.ts | 0 .../filter/matrchers/createTypeMatcher.ts | 0 .../src/alpha}/filter/matrchers/types.ts | 0 .../filter/parseFilterExpression.test.ts | 10 +-- .../alpha}/filter/parseFilterExpression.ts | 67 ++++++++----------- yarn.lock | 1 - 19 files changed, 51 insertions(+), 100 deletions(-) delete mode 100644 .changeset/stale-jokes-hunt.md delete mode 100644 plugins/catalog-common/src/filter/index.ts rename plugins/{catalog-common/src => catalog/src/alpha}/filter/matrchers/createHasMatcher.test.ts (100%) rename plugins/{catalog-common/src => catalog/src/alpha}/filter/matrchers/createHasMatcher.ts (100%) rename plugins/{catalog-common/src => catalog/src/alpha}/filter/matrchers/createIsMatcher.test.ts (100%) rename plugins/{catalog-common/src => catalog/src/alpha}/filter/matrchers/createIsMatcher.ts (100%) rename plugins/{catalog-common/src => catalog/src/alpha}/filter/matrchers/createKindMatcher.test.ts (100%) rename plugins/{catalog-common/src => catalog/src/alpha}/filter/matrchers/createKindMatcher.ts (100%) rename plugins/{catalog-common/src => catalog/src/alpha}/filter/matrchers/createTypeMatcher.test.ts (100%) rename plugins/{catalog-common/src => catalog/src/alpha}/filter/matrchers/createTypeMatcher.ts (100%) rename plugins/{catalog-common/src => catalog/src/alpha}/filter/matrchers/types.ts (100%) rename plugins/{catalog-common/src => catalog/src/alpha}/filter/parseFilterExpression.test.ts (96%) rename plugins/{catalog-common/src => catalog/src/alpha}/filter/parseFilterExpression.ts (66%) diff --git a/.changeset/stale-jokes-hunt.md b/.changeset/stale-jokes-hunt.md deleted file mode 100644 index 1e78a3a446..0000000000 --- a/.changeset/stale-jokes-hunt.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@backstage/plugin-catalog-common': minor ---- - -Added a `parseFilterExpression` function to interpret visibility filters, exported from the `/alpha` sub-path. diff --git a/packages/app-next/app-config.yaml b/packages/app-next/app-config.yaml index a90fe630b5..34cd78d480 100644 --- a/packages/app-next/app-config.yaml +++ b/packages/app-next/app-config.yaml @@ -14,7 +14,7 @@ app: - entity.cards.labels - entity.cards.links: config: - filter: kind:component + filter: kind:component has:links # Entity page content - entity.content.techdocs diff --git a/plugins/catalog-common/api-report-alpha.md b/plugins/catalog-common/api-report-alpha.md index bd36b499fe..12e0806a9c 100644 --- a/plugins/catalog-common/api-report-alpha.md +++ b/plugins/catalog-common/api-report-alpha.md @@ -4,7 +4,6 @@ ```ts import { BasicPermission } from '@backstage/plugin-permission-common'; -import { Entity } from '@backstage/catalog-model'; import { ResourcePermission } from '@backstage/plugin-permission-common'; // @alpha @@ -39,26 +38,6 @@ export const catalogPermissions: ( | ResourcePermission<'catalog-entity'> )[]; -// @alpha -export function parseFilterExpression( - expression: string, - options?: { - onParseError?: ( - error: Error, - context: { - expression: string; - }, - ) => void; - onEvaluateError?: ( - error: Error, - context: { - expression: string; - entity: Entity; - }, - ) => void; - }, -): (entity: Entity) => boolean; - // @alpha export const RESOURCE_TYPE_CATALOG_ENTITY = 'catalog-entity'; diff --git a/plugins/catalog-common/package.json b/plugins/catalog-common/package.json index ac21b76c2a..45720ee32a 100644 --- a/plugins/catalog-common/package.json +++ b/plugins/catalog-common/package.json @@ -46,7 +46,6 @@ }, "dependencies": { "@backstage/catalog-model": "workspace:^", - "@backstage/errors": "workspace:^", "@backstage/plugin-permission-common": "workspace:^", "@backstage/plugin-search-common": "workspace:^" }, diff --git a/plugins/catalog-common/src/alpha.ts b/plugins/catalog-common/src/alpha.ts index 015769c8a7..2dfc028332 100644 --- a/plugins/catalog-common/src/alpha.ts +++ b/plugins/catalog-common/src/alpha.ts @@ -26,4 +26,3 @@ export { catalogPermissions, } from './permissions'; export type { CatalogEntityPermission } from './permissions'; -export { parseFilterExpression } from './filter'; diff --git a/plugins/catalog-common/src/filter/index.ts b/plugins/catalog-common/src/filter/index.ts deleted file mode 100644 index c5ad803f4c..0000000000 --- a/plugins/catalog-common/src/filter/index.ts +++ /dev/null @@ -1,17 +0,0 @@ -/* - * Copyright 2023 The Backstage Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -export { parseFilterExpression } from './parseFilterExpression'; diff --git a/plugins/catalog/src/alpha/EntityOverviewPage.tsx b/plugins/catalog/src/alpha/EntityOverviewPage.tsx index 5ef6b9a2e5..a71e789522 100644 --- a/plugins/catalog/src/alpha/EntityOverviewPage.tsx +++ b/plugins/catalog/src/alpha/EntityOverviewPage.tsx @@ -15,10 +15,10 @@ */ import { Entity } from '@backstage/catalog-model'; -import { parseFilterExpression } from '@backstage/plugin-catalog-common/alpha'; import { useEntity } from '@backstage/plugin-catalog-react'; import Grid from '@material-ui/core/Grid'; import React, { useMemo } from 'react'; +import { parseFilterExpression } from './filter/parseFilterExpression'; interface EntityOverviewPageProps { cards: Array<{ @@ -27,6 +27,9 @@ interface EntityOverviewPageProps { }>; } +// Keeps track of what filter expression strings that we've emitted warnings for so far +const seenExpressionStrings = new Set(); + function CardWrapper(props: { entity: Entity; element: React.JSX.Element; @@ -40,14 +43,19 @@ function CardWrapper(props: { } else if (typeof filter === 'function') { return subject => filter(subject); } - return parseFilterExpression(filter, { - onParseError(_error) { - // ignore silently - }, - onEvaluateError(_error) { - // ignore silently - }, - }); + const result = parseFilterExpression(filter); + if ( + result.expressionParseErrors.length && + !seenExpressionStrings.has(filter) + ) { + // eslint-disable-next-line no-console + console.warn( + `Error(s) in entity filter expression '${filter}'`, + result.expressionParseErrors, + ); + seenExpressionStrings.add(filter); + } + return result.filterFn; }, [filter]); return filterFn(entity) ? ( diff --git a/plugins/catalog-common/src/filter/matrchers/createHasMatcher.test.ts b/plugins/catalog/src/alpha/filter/matrchers/createHasMatcher.test.ts similarity index 100% rename from plugins/catalog-common/src/filter/matrchers/createHasMatcher.test.ts rename to plugins/catalog/src/alpha/filter/matrchers/createHasMatcher.test.ts diff --git a/plugins/catalog-common/src/filter/matrchers/createHasMatcher.ts b/plugins/catalog/src/alpha/filter/matrchers/createHasMatcher.ts similarity index 100% rename from plugins/catalog-common/src/filter/matrchers/createHasMatcher.ts rename to plugins/catalog/src/alpha/filter/matrchers/createHasMatcher.ts diff --git a/plugins/catalog-common/src/filter/matrchers/createIsMatcher.test.ts b/plugins/catalog/src/alpha/filter/matrchers/createIsMatcher.test.ts similarity index 100% rename from plugins/catalog-common/src/filter/matrchers/createIsMatcher.test.ts rename to plugins/catalog/src/alpha/filter/matrchers/createIsMatcher.test.ts diff --git a/plugins/catalog-common/src/filter/matrchers/createIsMatcher.ts b/plugins/catalog/src/alpha/filter/matrchers/createIsMatcher.ts similarity index 100% rename from plugins/catalog-common/src/filter/matrchers/createIsMatcher.ts rename to plugins/catalog/src/alpha/filter/matrchers/createIsMatcher.ts diff --git a/plugins/catalog-common/src/filter/matrchers/createKindMatcher.test.ts b/plugins/catalog/src/alpha/filter/matrchers/createKindMatcher.test.ts similarity index 100% rename from plugins/catalog-common/src/filter/matrchers/createKindMatcher.test.ts rename to plugins/catalog/src/alpha/filter/matrchers/createKindMatcher.test.ts diff --git a/plugins/catalog-common/src/filter/matrchers/createKindMatcher.ts b/plugins/catalog/src/alpha/filter/matrchers/createKindMatcher.ts similarity index 100% rename from plugins/catalog-common/src/filter/matrchers/createKindMatcher.ts rename to plugins/catalog/src/alpha/filter/matrchers/createKindMatcher.ts diff --git a/plugins/catalog-common/src/filter/matrchers/createTypeMatcher.test.ts b/plugins/catalog/src/alpha/filter/matrchers/createTypeMatcher.test.ts similarity index 100% rename from plugins/catalog-common/src/filter/matrchers/createTypeMatcher.test.ts rename to plugins/catalog/src/alpha/filter/matrchers/createTypeMatcher.test.ts diff --git a/plugins/catalog-common/src/filter/matrchers/createTypeMatcher.ts b/plugins/catalog/src/alpha/filter/matrchers/createTypeMatcher.ts similarity index 100% rename from plugins/catalog-common/src/filter/matrchers/createTypeMatcher.ts rename to plugins/catalog/src/alpha/filter/matrchers/createTypeMatcher.ts diff --git a/plugins/catalog-common/src/filter/matrchers/types.ts b/plugins/catalog/src/alpha/filter/matrchers/types.ts similarity index 100% rename from plugins/catalog-common/src/filter/matrchers/types.ts rename to plugins/catalog/src/alpha/filter/matrchers/types.ts diff --git a/plugins/catalog-common/src/filter/parseFilterExpression.test.ts b/plugins/catalog/src/alpha/filter/parseFilterExpression.test.ts similarity index 96% rename from plugins/catalog-common/src/filter/parseFilterExpression.test.ts rename to plugins/catalog/src/alpha/filter/parseFilterExpression.test.ts index a53b84646e..d1335524b4 100644 --- a/plugins/catalog-common/src/filter/parseFilterExpression.test.ts +++ b/plugins/catalog/src/alpha/filter/parseFilterExpression.test.ts @@ -22,11 +22,11 @@ import { describe('parseFilterExpression', () => { function run(expression: string) { - return parseFilterExpression(expression, { - onParseError(e: Error) { - throw e; - }, - }); + const result = parseFilterExpression(expression); + if (result.expressionParseErrors.length) { + throw result.expressionParseErrors[0]; + } + return result.filterFn; } it('supports "kind" expressions', () => { diff --git a/plugins/catalog-common/src/filter/parseFilterExpression.ts b/plugins/catalog/src/alpha/filter/parseFilterExpression.ts similarity index 66% rename from plugins/catalog-common/src/filter/parseFilterExpression.ts rename to plugins/catalog/src/alpha/filter/parseFilterExpression.ts index ffd25d75d3..a4c17cb8da 100644 --- a/plugins/catalog-common/src/filter/parseFilterExpression.ts +++ b/plugins/catalog/src/alpha/filter/parseFilterExpression.ts @@ -39,7 +39,6 @@ const rootMatcherFactories: Record< * Parses a filter expression that decides whether to render an entity component * or not. Returns a function that matches entities based on that expression. * - * @alpha * @remarks * * Filter strings are on the form `kind:user,group is:orphan`. There's @@ -47,61 +46,51 @@ const rootMatcherFactories: Record< * separated parameters. So the example filter string semantically means * "entities that are of either User or Group kind, and also are orphans". * - * The `onParseError` callback is called whenever an error was encountered - * during initial parsing of the expression. If the callback throws, - * `parseFilterExpression` throws that same error. If the callback does not - * throw, the part of the input expression that had an error is ignored entirely - * and parsing continues. - * - * The `onEvaluateError` callback is called whenever an error was encountered - * during evaluation of the returned function. If the callback throws, the - * evaluation call throws the same error. If the callback does not throw, the - * whole evaluation returns false. + * The `expressionParseErrors` array contains any errors that were encountered + * during initial parsing of the expression. Note that the parts of the input + * expression that had errors are ignored entirely and parsing continues as if + * they didn't exist. */ -export function parseFilterExpression( - expression: string, - options?: { - onParseError?: ( - error: Error, - context: { - expression: string; - }, - ) => void; - onEvaluateError?: ( - error: Error, - context: { - expression: string; - entity: Entity; - }, - ) => void; - }, -): (entity: Entity) => boolean { +export function parseFilterExpression(expression: string): { + filterFn: (entity: Entity) => boolean; + expressionParseErrors: Error[]; +} { + const expressionParseErrors: Error[] = []; + const parts = splitFilterExpression(expression, e => - options?.onParseError?.(e, { expression }), + expressionParseErrors.push(e), ); - const matchers = parts.map(part => { + const matchers = parts.flatMap(part => { const factory = rootMatcherFactories[part.key]; if (!factory) { const known = Object.keys(rootMatcherFactories).map(m => `'${m}'`); - throw new InputError( - `'${part.key}' is not a valid filter expression key, expected one of ${known}`, + expressionParseErrors.push( + new InputError( + `'${part.key}' is not a valid filter expression key, expected one of ${known}`, + ), ); + return []; } - return factory(part.parameters, e => - options?.onParseError?.(e, { expression }), + + const matcher = factory(part.parameters, e => + expressionParseErrors.push(e), ); + return [matcher]; }); - return (entity: Entity) => { - return matchers.every(matcher => { + const filterFn = (entity: Entity) => + matchers.every(matcher => { try { return matcher(entity); - } catch (e) { - options?.onEvaluateError?.(e, { expression, entity }); + } catch { return false; } }); + + return { + filterFn, + expressionParseErrors, }; } diff --git a/yarn.lock b/yarn.lock index 90be768746..2399f06ac5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5831,7 +5831,6 @@ __metadata: dependencies: "@backstage/catalog-model": "workspace:^" "@backstage/cli": "workspace:^" - "@backstage/errors": "workspace:^" "@backstage/plugin-permission-common": "workspace:^" "@backstage/plugin-search-common": "workspace:^" languageName: unknown From 969d028b93eb72dc70a707df8f945da0f18c9ac4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Thu, 23 Nov 2023 16:36:16 +0100 Subject: [PATCH 6/8] split apart into two filter refs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- plugins/catalog-react/api-report-alpha.md | 18 +++- plugins/catalog-react/src/alpha.tsx | 55 ++++++++++-- .../catalog/src/alpha/EntityOverviewPage.tsx | 88 ++++++++++++------- plugins/catalog/src/alpha/entityContents.tsx | 6 +- 4 files changed, 121 insertions(+), 46 deletions(-) diff --git a/plugins/catalog-react/api-report-alpha.md b/plugins/catalog-react/api-report-alpha.md index e284f80b8e..913055f8b0 100644 --- a/plugins/catalog-react/api-report-alpha.md +++ b/plugins/catalog-react/api-report-alpha.md @@ -24,7 +24,9 @@ export function createEntityCardExtension< }; disabled?: boolean; inputs?: TInputs; - filter?: typeof entityFilterExtensionDataRef.T; + filter?: + | typeof entityFilterFunctionExtensionDataRef.T + | typeof entityFilterExpressionExtensionDataRef.T; loader: (options: { inputs: Expand>; }) => Promise; @@ -46,7 +48,9 @@ export function createEntityContentExtension< routeRef?: RouteRef; defaultPath: string; defaultTitle: string; - filter?: typeof entityFilterExtensionDataRef.T; + filter?: + | typeof entityFilterFunctionExtensionDataRef.T + | typeof entityFilterExpressionExtensionDataRef.T; loader: (options: { inputs: Expand>; }) => Promise; @@ -63,8 +67,14 @@ export const entityContentTitleExtensionDataRef: ConfigurableExtensionDataRef< >; // @alpha (undocumented) -export const entityFilterExtensionDataRef: ConfigurableExtensionDataRef< - string | ((entity: Entity) => boolean), +export const entityFilterExpressionExtensionDataRef: ConfigurableExtensionDataRef< + string, + {} +>; + +// @alpha (undocumented) +export const entityFilterFunctionExtensionDataRef: ConfigurableExtensionDataRef< + (entity: Entity) => boolean, {} >; diff --git a/plugins/catalog-react/src/alpha.tsx b/plugins/catalog-react/src/alpha.tsx index 604c769fe5..43b7156b34 100644 --- a/plugins/catalog-react/src/alpha.tsx +++ b/plugins/catalog-react/src/alpha.tsx @@ -37,9 +37,13 @@ export const entityContentTitleExtensionDataRef = createExtensionDataRef('plugin.catalog.entity.content.title'); /** @alpha */ -export const entityFilterExtensionDataRef = createExtensionDataRef< - string | ((entity: Entity) => boolean) ->('plugin.catalog.entity.filter'); +export const entityFilterFunctionExtensionDataRef = createExtensionDataRef< + (entity: Entity) => boolean +>('plugin.catalog.entity.filter.fn'); + +/** @alpha */ +export const entityFilterExpressionExtensionDataRef = + createExtensionDataRef('plugin.catalog.entity.filter.expression'); // TODO: Figure out how to merge with provided config schema /** @alpha */ @@ -50,7 +54,9 @@ export function createEntityCardExtension< attachTo?: { id: string; input: string }; disabled?: boolean; inputs?: TInputs; - filter?: typeof entityFilterExtensionDataRef.T; + filter?: + | typeof entityFilterFunctionExtensionDataRef.T + | typeof entityFilterExpressionExtensionDataRef.T; loader: (options: { inputs: Expand>; }) => Promise; @@ -66,7 +72,8 @@ export function createEntityCardExtension< disabled: options.disabled ?? true, output: { element: coreExtensionData.reactElement, - filter: entityFilterExtensionDataRef.optional(), + filterFunction: entityFilterFunctionExtensionDataRef.optional(), + filterExpression: entityFilterExpressionExtensionDataRef.optional(), }, inputs: options.inputs, configSchema: createSchemaFromZod(z => @@ -87,7 +94,7 @@ export function createEntityCardExtension< ), - filter: config.filter ?? options.filter, + ...mergeFilters({ config, options }), }; }, }); @@ -104,7 +111,9 @@ export function createEntityContentExtension< routeRef?: RouteRef; defaultPath: string; defaultTitle: string; - filter?: typeof entityFilterExtensionDataRef.T; + filter?: + | typeof entityFilterFunctionExtensionDataRef.T + | typeof entityFilterExpressionExtensionDataRef.T; loader: (options: { inputs: Expand>; }) => Promise; @@ -123,7 +132,8 @@ export function createEntityContentExtension< path: coreExtensionData.routePath, routeRef: coreExtensionData.routeRef.optional(), title: entityContentTitleExtensionDataRef, - filter: entityFilterExtensionDataRef.optional(), + filterFunction: entityFilterFunctionExtensionDataRef.optional(), + filterExpression: entityFilterExpressionExtensionDataRef.optional(), }, inputs: options.inputs, configSchema: createSchemaFromZod(z => @@ -149,8 +159,35 @@ export function createEntityContentExtension< ), - filter: config.filter ?? options.filter, + ...mergeFilters({ config, options }), }; }, }); } + +/** + * Decides what filter outputs to produce, given some options and config + */ +function mergeFilters(inputs: { + options: { + filter?: + | typeof entityFilterFunctionExtensionDataRef.T + | typeof entityFilterExpressionExtensionDataRef.T; + }; + config: { + filter?: string; + }; +}): { + filterFunction?: typeof entityFilterFunctionExtensionDataRef.T; + filterExpression?: typeof entityFilterExpressionExtensionDataRef.T; +} { + const { options, config } = inputs; + if (config.filter) { + return { filterExpression: config.filter }; + } else if (typeof options.filter === 'string') { + return { filterExpression: options.filter }; + } else if (typeof options.filter === 'function') { + return { filterFunction: options.filter }; + } + return {}; +} diff --git a/plugins/catalog/src/alpha/EntityOverviewPage.tsx b/plugins/catalog/src/alpha/EntityOverviewPage.tsx index a71e789522..932e13d962 100644 --- a/plugins/catalog/src/alpha/EntityOverviewPage.tsx +++ b/plugins/catalog/src/alpha/EntityOverviewPage.tsx @@ -23,40 +23,71 @@ import { parseFilterExpression } from './filter/parseFilterExpression'; interface EntityOverviewPageProps { cards: Array<{ element: React.JSX.Element; - filter?: string | ((entity: Entity) => boolean); + filterFunction?: (entity: Entity) => boolean; + filterExpression?: string; }>; } -// Keeps track of what filter expression strings that we've emitted warnings for so far -const seenExpressionStrings = new Set(); +// Keeps track of what filter expression strings that we've seen duplicates of +// with functions, or which emitted parsing errors for so far +const seenParseErrorExpressionStrings = new Set(); +const seenDuplicateExpressionStrings = new Set(); +// Given an optional filter function and an optional filter expression, make +// sure that at most one of them was given, and return a filter function that +// does the right thing. +function buildFilterFn( + filterFunction?: (entity: Entity) => boolean, + filterExpression?: string, +): (entity: Entity) => boolean { + if ( + filterFunction && + filterExpression && + !seenDuplicateExpressionStrings.has(filterExpression) + ) { + // eslint-disable-next-line no-console + console.warn( + `Duplicate entity filter methods found, both '${filterExpression}' as well as a callback function, which is not permitted - using the callback`, + ); + seenDuplicateExpressionStrings.add(filterExpression); + } + + const filter = filterFunction || filterExpression; + if (!filter) { + return () => true; + } else if (typeof filter === 'function') { + return subject => filter(subject); + } + + const result = parseFilterExpression(filter); + if ( + result.expressionParseErrors.length && + !seenParseErrorExpressionStrings.has(filter) + ) { + // eslint-disable-next-line no-console + console.warn( + `Error(s) in entity filter expression '${filter}'`, + result.expressionParseErrors, + ); + seenParseErrorExpressionStrings.add(filter); + } + + return result.filterFn; +} + +// Handles the memoized parsing of filter expressions for each card function CardWrapper(props: { entity: Entity; element: React.JSX.Element; - filter?: string | ((entity: Entity) => boolean); + filterFunction?: (entity: Entity) => boolean; + filterExpression?: string; }) { - const { entity, element, filter } = props; + const { entity, element, filterFunction, filterExpression } = props; - const filterFn = useMemo<(subject: Entity) => boolean>(() => { - if (!filter) { - return () => true; - } else if (typeof filter === 'function') { - return subject => filter(subject); - } - const result = parseFilterExpression(filter); - if ( - result.expressionParseErrors.length && - !seenExpressionStrings.has(filter) - ) { - // eslint-disable-next-line no-console - console.warn( - `Error(s) in entity filter expression '${filter}'`, - result.expressionParseErrors, - ); - seenExpressionStrings.add(filter); - } - return result.filterFn; - }, [filter]); + const filterFn = useMemo( + () => buildFilterFn(filterFunction, filterExpression), + [filterFunction, filterExpression], + ); return filterFn(entity) ? ( @@ -70,12 +101,7 @@ export function EntityOverviewPage(props: EntityOverviewPageProps) { return ( {props.cards.map((card, index) => ( - + ))} ); diff --git a/plugins/catalog/src/alpha/entityContents.tsx b/plugins/catalog/src/alpha/entityContents.tsx index 0bbee9bb7d..e280326b02 100644 --- a/plugins/catalog/src/alpha/entityContents.tsx +++ b/plugins/catalog/src/alpha/entityContents.tsx @@ -21,7 +21,8 @@ import { } from '@backstage/frontend-plugin-api'; import { createEntityContentExtension, - entityFilterExtensionDataRef, + entityFilterFunctionExtensionDataRef, + entityFilterExpressionExtensionDataRef, } from '@backstage/plugin-catalog-react/alpha'; export const OverviewEntityContent = createEntityContentExtension({ @@ -32,7 +33,8 @@ export const OverviewEntityContent = createEntityContentExtension({ inputs: { cards: createExtensionInput({ element: coreExtensionData.reactElement, - filter: entityFilterExtensionDataRef.optional(), + filterFunction: entityFilterFunctionExtensionDataRef.optional(), + filterExpression: entityFilterExpressionExtensionDataRef.optional(), }), }, loader: async ({ inputs }) => From a52d98820162176bb3f2d53347d6b501766274c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Thu, 23 Nov 2023 16:39:41 +0100 Subject: [PATCH 7/8] moved out the matchers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .../filter/matrchers/createHasMatcher.ts | 24 +++++++++---------- .../alpha/filter/matrchers/createIsMatcher.ts | 10 ++++---- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/plugins/catalog/src/alpha/filter/matrchers/createHasMatcher.ts b/plugins/catalog/src/alpha/filter/matrchers/createHasMatcher.ts index f57316a65e..35a3f7baf7 100644 --- a/plugins/catalog/src/alpha/filter/matrchers/createHasMatcher.ts +++ b/plugins/catalog/src/alpha/filter/matrchers/createHasMatcher.ts @@ -17,6 +17,18 @@ import { InputError } from '@backstage/errors'; import { EntityMatcherFn } from './types'; +const allowedMatchers: Record = { + labels: entity => { + return Object.keys(entity.metadata.labels ?? {}).length > 0; + }, + annotations: entity => { + return Object.keys(entity.metadata.annotations ?? {}).length > 0; + }, + links: entity => { + return (entity.metadata.links ?? []).length > 0; + }, +}; + /** * Matches on the non-empty presence of different parts of the entity */ @@ -24,18 +36,6 @@ export function createHasMatcher( parameters: string[], onParseError: (error: Error) => void, ): EntityMatcherFn { - const allowedMatchers: Record = { - labels: entity => { - return Object.keys(entity.metadata.labels ?? {}).length > 0; - }, - annotations: entity => { - return Object.keys(entity.metadata.annotations ?? {}).length > 0; - }, - links: entity => { - return (entity.metadata.links ?? []).length > 0; - }, - }; - const matchers = parameters.flatMap(parameter => { const matcher = allowedMatchers[parameter.toLocaleLowerCase('en-US')]; if (!matcher) { diff --git a/plugins/catalog/src/alpha/filter/matrchers/createIsMatcher.ts b/plugins/catalog/src/alpha/filter/matrchers/createIsMatcher.ts index 30bfe4eb15..49a54349d1 100644 --- a/plugins/catalog/src/alpha/filter/matrchers/createIsMatcher.ts +++ b/plugins/catalog/src/alpha/filter/matrchers/createIsMatcher.ts @@ -17,6 +17,11 @@ import { InputError } from '@backstage/errors'; import { EntityMatcherFn } from './types'; +const allowedMatchers: Record = { + orphan: entity => + Boolean(entity.metadata.annotations?.['backstage.io/orphan']), +}; + /** * Matches on different semantic properties of the entity */ @@ -24,11 +29,6 @@ export function createIsMatcher( parameters: string[], onParseError: (error: Error) => void, ): EntityMatcherFn { - const allowedMatchers: Record = { - orphan: entity => - Boolean(entity.metadata.annotations?.['backstage.io/orphan']), - }; - const matchers = parameters.flatMap(parameter => { const matcher = allowedMatchers[parameter.toLocaleLowerCase('en-US')]; if (!matcher) { From 645c64332ab9f27d2c351762fb6d58849a7c40e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Fri, 24 Nov 2023 09:41:59 +0100 Subject: [PATCH 8/8] removed has:annotations since that makes little sense - they are always there MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .../filter/matrchers/createHasMatcher.test.ts | 17 +++-------------- .../alpha/filter/matrchers/createHasMatcher.ts | 3 --- .../alpha/filter/parseFilterExpression.test.ts | 2 +- 3 files changed, 4 insertions(+), 18 deletions(-) diff --git a/plugins/catalog/src/alpha/filter/matrchers/createHasMatcher.test.ts b/plugins/catalog/src/alpha/filter/matrchers/createHasMatcher.test.ts index dbce5025b6..53524d228e 100644 --- a/plugins/catalog/src/alpha/filter/matrchers/createHasMatcher.test.ts +++ b/plugins/catalog/src/alpha/filter/matrchers/createHasMatcher.test.ts @@ -24,7 +24,6 @@ describe('createHasMatcher', () => { const empty2 = { metadata: { labels: {}, - annotations: {}, links: [], }, } as unknown as Entity; @@ -33,11 +32,6 @@ describe('createHasMatcher', () => { labels: { a: 'b' }, }, } as unknown as Entity; - const annotations = { - metadata: { - annotations: { a: 'b' }, - }, - } as unknown as Entity; const links = { metadata: { links: [{}] }, } as unknown as Entity; @@ -53,10 +47,6 @@ describe('createHasMatcher', () => { expect(createHasMatcher(['labels'], err)(empty2)).toBe(false); expect(createHasMatcher(['labels'], err)(labels)).toBe(true); - expect(createHasMatcher(['annotations'], err)(empty1)).toBe(false); - expect(createHasMatcher(['annotations'], err)(empty2)).toBe(false); - expect(createHasMatcher(['annotations'], err)(annotations)).toBe(true); - expect(createHasMatcher(['links'], err)(empty1)).toBe(false); expect(createHasMatcher(['links'], err)(empty2)).toBe(false); expect(createHasMatcher(['links'], err)(links)).toBe(true); @@ -69,7 +59,6 @@ describe('createHasMatcher', () => { expect(createHasMatcher(['labels', 'links'], err)(empty2)).toBe(false); expect(createHasMatcher(['labels', 'links'], err)(labels)).toBe(true); expect(createHasMatcher(['labels', 'links'], err)(links)).toBe(true); - expect(createHasMatcher(['labels', 'links'], err)(annotations)).toBe(false); expect(err).not.toHaveBeenCalled(); }); @@ -81,7 +70,7 @@ describe('createHasMatcher', () => { throw e; }), ).toThrowErrorMatchingInlineSnapshot( - `"'bar' is not a valid parameter for 'has' filter expressions, expected one of 'labels','annotations','links'"`, + `"'bar' is not a valid parameter for 'has' filter expressions, expected one of 'labels','links'"`, ); expect(err).not.toHaveBeenCalled(); @@ -90,12 +79,12 @@ describe('createHasMatcher', () => { expect(err).toHaveBeenCalledTimes(2); expect(err).toHaveBeenCalledWith( expect.objectContaining({ - message: `'foo' is not a valid parameter for 'has' filter expressions, expected one of 'labels','annotations','links'`, + message: `'foo' is not a valid parameter for 'has' filter expressions, expected one of 'labels','links'`, }), ); expect(err).toHaveBeenCalledWith( expect.objectContaining({ - message: `'bar' is not a valid parameter for 'has' filter expressions, expected one of 'labels','annotations','links'`, + message: `'bar' is not a valid parameter for 'has' filter expressions, expected one of 'labels','links'`, }), ); expect(matcher(empty1)).toBe(false); diff --git a/plugins/catalog/src/alpha/filter/matrchers/createHasMatcher.ts b/plugins/catalog/src/alpha/filter/matrchers/createHasMatcher.ts index 35a3f7baf7..9fd393a561 100644 --- a/plugins/catalog/src/alpha/filter/matrchers/createHasMatcher.ts +++ b/plugins/catalog/src/alpha/filter/matrchers/createHasMatcher.ts @@ -21,9 +21,6 @@ const allowedMatchers: Record = { labels: entity => { return Object.keys(entity.metadata.labels ?? {}).length > 0; }, - annotations: entity => { - return Object.keys(entity.metadata.annotations ?? {}).length > 0; - }, links: entity => { return (entity.metadata.links ?? []).length > 0; }, diff --git a/plugins/catalog/src/alpha/filter/parseFilterExpression.test.ts b/plugins/catalog/src/alpha/filter/parseFilterExpression.test.ts index d1335524b4..616091739e 100644 --- a/plugins/catalog/src/alpha/filter/parseFilterExpression.test.ts +++ b/plugins/catalog/src/alpha/filter/parseFilterExpression.test.ts @@ -89,7 +89,7 @@ describe('parseFilterExpression', () => { expect(run('has:labels,links')(annotations)).toBe(false); expect(() => run('has:labels,bar')).toThrowErrorMatchingInlineSnapshot( - `"'bar' is not a valid parameter for 'has' filter expressions, expected one of 'labels','annotations','links'"`, + `"'bar' is not a valid parameter for 'has' filter expressions, expected one of 'labels','links'"`, ); });