move over to catalog and make internal, return errors in array

Signed-off-by: Fredrik Adelöw <freben@gmail.com>

.changeset/stale-jokes-hunt.md

@@ -1,5 +0,0 @@
----
-'@backstage/plugin-catalog-common': minor
----
-
-Added a `parseFilterExpression` function to interpret visibility filters, exported from the `/alpha` sub-path.

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

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';
 

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:^"
   },

plugins/catalog-common/src/alpha.ts

@@ -26,4 +26,3 @@ export {
   catalogPermissions,
 } from './permissions';
 export type { CatalogEntityPermission } from './permissions';
-export { parseFilterExpression } from './filter';

plugins/catalog-common/src/filter/index.ts

@@ -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';

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<string>();
+
 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) ? (

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', () => {

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,
   };
 }
 

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