Fix complex negation bug

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

plugins/catalog-backend/src/service/NextEntitiesCatalog.test.ts

@@ -327,7 +327,7 @@ describe('NextEntitiesCatalog', () => {
     );
 
     it.each(databases.eachSupportedId())(
-      'should return correct entity for nested filter',
+      'should return correct entities for nested filter',
       async databaseId => {
         const { knex } = await createDatabase(databaseId);
         const entity1: Entity = {
@@ -394,5 +394,46 @@ describe('NextEntitiesCatalog', () => {
         expect(entities).toContainEqual(entity4);
       },
     );
+
+    it.each(databases.eachSupportedId())(
+      'should return correct entities for complex negation filter',
+      async databaseId => {
+        const { knex } = await createDatabase(databaseId);
+        const entity1: Entity = {
+          apiVersion: 'a',
+          kind: 'k',
+          metadata: { name: 'one', org: 'a', desc: 'description' },
+          spec: {},
+        };
+        const entity2: Entity = {
+          apiVersion: 'a',
+          kind: 'k',
+          metadata: { name: 'two', org: 'b', desc: 'description' },
+          spec: {},
+        };
+        await addEntityToSearch(knex, entity1);
+        await addEntityToSearch(knex, entity2);
+        const catalog = new NextEntitiesCatalog(knex);
+
+        const testFilter1 = {
+          key: 'metadata.org',
+          values: ['b'],
+        };
+        const testFilter2 = {
+          key: 'metadata.desc',
+        };
+        const request = {
+          filter: {
+            not: {
+              allOf: [testFilter1, testFilter2],
+            },
+          },
+        };
+        const { entities } = await catalog.entities(request);
+
+        expect(entities.length).toBe(1);
+        expect(entities).toContainEqual(entity1);
+      },
+    );
   });
 });

plugins/catalog-backend/src/service/NextEntitiesCatalog.ts

@@ -110,12 +110,6 @@ function isEntitiesSearchFilter(
   return filter.hasOwnProperty('key');
 }
 
-function isAndEntityFilter(
-  filter: { allOf: EntityFilter[] } | EntityFilter,
-): filter is { allOf: EntityFilter[] } {
-  return filter.hasOwnProperty('allOf');
-}
-
 function isOrEntityFilter(
   filter: { anyOf: EntityFilter[] } | EntityFilter,
 ): filter is { anyOf: EntityFilter[] } {
@@ -141,26 +135,20 @@ function parseFilter(
   }
 
   if (isNegationEntityFilter(filter)) {
-    return parseFilter(filter.not, query, db, true);
+    return parseFilter(filter.not, query, db, !negate);
   }
 
-  if (isOrEntityFilter(filter)) {
-    return query.andWhere(function filterFunction() {
+  return query[negate ? 'andWhereNot' : 'andWhere'](function filterFunction() {
+    if (isOrEntityFilter(filter)) {
       for (const subFilter of filter.anyOf ?? []) {
-        this.orWhere(subQuery => parseFilter(subFilter, subQuery, db, negate));
+        this.orWhere(subQuery => parseFilter(subFilter, subQuery, db));
       }
-    });
-  }
-
-  if (isAndEntityFilter(filter)) {
-    return query.andWhere(function filterFunction() {
+    } else {
       for (const subFilter of filter.allOf ?? []) {
-        this.andWhere(subQuery => parseFilter(subFilter, subQuery, db, negate));
+        this.andWhere(subQuery => parseFilter(subFilter, subQuery, db));
       }
-    });
-  }
-
-  return query;
+    }
+  });
 }
 
 export class NextEntitiesCatalog implements EntitiesCatalog {