EntitiesSearchFilter type refactor

The current EntitiesSearchFilter's matchValueExists field is a bit confusing - it's not immediately clear the nuances of its behavior until you closely examine the implementation in NextEntitiesCatalog. By splitting the filter into to distinct types (one for key and another for key + value) as well as renaming the fields, I think this expresses the intent of the filters more cleanly. This also allows filtering by negation for key + value filters, which is not possible with the current implementation).

Signed-off-by: Joon Park <joonp@spotify.com>

plugins/catalog-backend/src/catalog/index.ts

@@ -23,6 +23,8 @@ export type {
   EntityUpsertResponse,
   PageInfo,
   EntitiesSearchFilter,
+  EntitiesKeyFilter,
+  EntitiesValuesFilter,
   EntityFilter,
   EntityPagination,
 } from './types';

plugins/catalog-backend/src/catalog/types.ts

@@ -39,7 +39,7 @@ export type EntityPagination = {
 /**
  * Matches rows in the entities_search table.
  */
-export type EntitiesSearchFilter = {
+export type EntitiesValuesFilter = {
   /**
    * The key to match on.
    *
@@ -50,18 +50,28 @@ export type EntitiesSearchFilter = {
   /**
    * Match on plain equality of values.
    *
-   * If undefined, this factor is not taken into account. Otherwise, match on
+   * Match on
    * values that are equal to any of the given array items. Matches are always
    * case insensitive.
    */
-  matchValueIn?: string[];
+  values: string[];
 
-  /**
-   * Match on existence of key.
-   */
-  matchValueExists?: boolean;
+  negate?: boolean;
 };
 
+export type EntitiesKeyFilter = {
+  /**
+   * The key to match on.
+   *
+   * Matches are always case insensitive.
+   */
+  key: string;
+
+  negate?: boolean;
+};
+
+export type EntitiesSearchFilter = EntitiesValuesFilter | EntitiesKeyFilter;
+
 export type PageInfo =
   | {
       hasNextPage: false;

plugins/catalog-backend/src/service/request/basicEntityFilter.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { EntitiesSearchFilter, EntityFilter } from '../../catalog';
+import { EntitiesValuesFilter, EntityFilter } from '../../catalog';
 
 /**
  * Forms a full EntityFilter based on a single key-value(s) object.
@@ -22,7 +22,7 @@ import { EntitiesSearchFilter, EntityFilter } from '../../catalog';
 export function basicEntityFilter(
   items: Record<string, string | string[]>,
 ): EntityFilter {
-  const filtersByKey: Record<string, EntitiesSearchFilter> = {};
+  const filtersByKey: Record<string, EntitiesValuesFilter> = {};
 
   for (const [key, value] of Object.entries(items)) {
     const values = [value].flat();
@@ -30,9 +30,9 @@ export function basicEntityFilter(
     const f =
       key in filtersByKey
         ? filtersByKey[key]
-        : (filtersByKey[key] = { key, matchValueIn: [] });
+        : (filtersByKey[key] = { key, values: [] });
 
-    f.matchValueIn!.push(...values);
+    f.values!.push(...values);
   }
 
   return { anyOf: [{ allOf: Object.values(filtersByKey) }] };

plugins/catalog-backend/src/service/request/parseEntityFilterParams.ts

@@ -15,7 +15,11 @@
  */
 
 import { InputError } from '@backstage/errors';
-import { EntitiesSearchFilter, EntityFilter } from '../../catalog';
+import {
+  EntitiesSearchFilter,
+  EntitiesValuesFilter,
+  EntityFilter,
+} from '../../catalog';
 import { parseStringsParam } from './common';
 
 /**
@@ -75,11 +79,10 @@ export function parseEntityFilterString(
     const f =
       key in filtersByKey ? filtersByKey[key] : (filtersByKey[key] = { key });
 
-    if (value === undefined) {
-      f.matchValueExists = true;
-    } else {
-      f.matchValueIn = f.matchValueIn || [];
-      f.matchValueIn.push(value);
+    if (value !== undefined) {
+      const valuesFilter = f as EntitiesValuesFilter;
+      valuesFilter.values = valuesFilter.values || [];
+      valuesFilter.values.push(value);
     }
   }