feat: allow nested condition tree

packages/agent/src/routes/capabilities.ts

@@ -61,6 +61,7 @@ export default class Capabilities extends BaseRoute {
         // front offers oldest first on the project page only when it is set.
         canSortAuditTrailTimeline: Boolean(this.options.auditTrail?.store?.listTimeline),
         checksRelationReadPermissions: !this.options.skipRelationReadPermissions,
+        canUseNestedConditionTree: true,
       },
       collections:
         collections?.map(collection => {

packages/agent/src/services/authorization/authorization.ts

@@ -281,7 +281,11 @@ export default class AuthorizationService {
 
     if (!scope) return null;
 
-    return ConditionTreeParser.fromPlainObject(collection, scope);
+    try {
+      return ConditionTreeParser.fromPlainObject(collection, scope);
+    } catch (error) {
+      throw new UnprocessableError(`Invalid scope (${error.message})`);
+    }
   }
 
   public invalidateScopeCache(renderingId: number | string) {

packages/agent/src/utils/condition-tree-parser.ts

@@ -25,6 +25,10 @@ const STRING_TO_BOOLEAN = {
 
 export default class ConditionTreeParser {
   static fromPlainObject(collection: Collection, json: unknown): ConditionTree {
+    return ConditionTreeParser.parse(collection, json, true);
+  }
+
+  private static parse(collection: Collection, json: unknown, isRoot: boolean): ConditionTree {
     if (ConditionTreeParser.isLeaf(json)) {
       const operator = ConditionTreeParser.toPascalCase(json.operator);
       const value = ConditionTreeParser.parseValue(collection, { ...json, operator });
@@ -33,9 +37,15 @@ export default class ConditionTreeParser {
     }
 
     if (ConditionTreeParser.isBranch(json)) {
+      // An empty nested Or would match nothing and an empty nested And everything: dropping it
+      // widens the filter, keeping it silently empties the result. Either is wrong under a bulk action.
+      if (!isRoot && json.conditions.length === 0) {
+        throw new Error('A nested group of conditions cannot be empty');
+      }
+
       const aggregator = ConditionTreeParser.toPascalCase(json.aggregator) as Aggregator;
       const conditions = json.conditions.map(subTree =>
-        ConditionTreeParser.fromPlainObject(collection, subTree),
+        ConditionTreeParser.parse(collection, subTree, false),
       );
 
       return conditions.length !== 1
@@ -115,10 +125,15 @@ export default class ConditionTreeParser {
   }
 
   private static isLeaf(raw: unknown): raw is { field: string; operator: string; value: unknown } {
-    return typeof raw === 'object' && 'field' in raw && 'operator' in raw;
+    return typeof raw === 'object' && raw !== null && 'field' in raw && 'operator' in raw;
   }
 
   private static isBranch(raw: unknown): raw is { aggregator: string; conditions: unknown[] } {
-    return typeof raw === 'object' && 'aggregator' in raw && 'conditions' in raw;
+    return (
+      typeof raw === 'object' &&
+      raw !== null &&
+      'aggregator' in raw &&
+      Array.isArray((raw as { conditions?: unknown }).conditions)
+    );
   }
 }

packages/agent/test/routes/capabilities.test.ts

@@ -109,6 +109,7 @@ describe('Capabilities', () => {
             canUseAuditTrailTimeline: false,
             canSortAuditTrailTimeline: false,
             checksRelationReadPermissions: true,
+            canUseNestedConditionTree: true,
           },
           collections: [],
         });
@@ -137,6 +138,7 @@ describe('Capabilities', () => {
             canUseAuditTrailTimeline: false,
             canSortAuditTrailTimeline: false,
             checksRelationReadPermissions: true,
+            canUseNestedConditionTree: true,
           },
           collections: [],
         });
@@ -176,6 +178,7 @@ describe('Capabilities', () => {
             canUseAuditTrailTimeline: false,
             canSortAuditTrailTimeline: false,
             checksRelationReadPermissions: true,
+            canUseNestedConditionTree: true,
           },
           collections: [],
         });
@@ -278,6 +281,7 @@ describe('Capabilities', () => {
             canUseAuditTrailTimeline: false,
             canSortAuditTrailTimeline: false,
             checksRelationReadPermissions: true,
+            canUseNestedConditionTree: true,
           },
           collections: [
             {

packages/agent/test/services/authorization/authorization.test.ts

@@ -1,6 +1,6 @@
 import type { Context } from 'koa';
 
-import { Projection } from '@forestadmin/datasource-toolkit';
+import { Projection, UnprocessableError } from '@forestadmin/datasource-toolkit';
 import {
   ChainedSQLQueryError,
   ChartType,
@@ -205,6 +205,23 @@ describe('AuthorizationService', () => {
       });
       expect(ConditionTreeParser.fromPlainObject).toHaveBeenCalledWith(collection, { foo: 'bar' });
     });
+
+    it('should throw an UnprocessableError when the scope cannot be parsed', async () => {
+      const forestAdminClient = factories.forestAdminClient.build();
+      const authorizationService = new AuthorizationService(forestAdminClient);
+      const context = { state: { user: { id: 666, renderingId: 42 } } } as Context;
+
+      (forestAdminClient.getScope as jest.Mock).mockResolvedValue({ foo: 'bar' });
+      (ConditionTreeParser.fromPlainObject as jest.Mock).mockImplementation(() => {
+        throw new Error('Failed to instantiate condition tree from json');
+      });
+
+      await expect(
+        authorizationService.getScope(factories.collection.build({ name: 'books' }), context),
+      ).rejects.toThrow(
+        new UnprocessableError('Invalid scope (Failed to instantiate condition tree from json)'),
+      );
+    });
   });
 
   describe('assertCanExecuteChart', () => {

packages/agent/test/utils/condition-tree-parser.test.ts

@@ -38,6 +38,91 @@ describe('ConditionTreeParser', () => {
     );
   });
 
+  test('should keep a group of And nested in an Or', () => {
+    const tree = ConditionTreeParser.fromPlainObject(collection, {
+      aggregator: 'or',
+      conditions: [
+        {
+          aggregator: 'and',
+          conditions: [
+            { field: 'number', operator: 'greater_than', value: '1' },
+            { field: 'boolean', operator: 'equal', value: 'true' },
+          ],
+        },
+        { field: 'string', operator: 'equal', value: 'a' },
+      ],
+    });
+
+    expect(tree).toStrictEqual(
+      new ConditionTreeBranch('Or', [
+        new ConditionTreeBranch('And', [
+          new ConditionTreeLeaf('number', 'GreaterThan', 1),
+          new ConditionTreeLeaf('boolean', 'Equal', true),
+        ]),
+        new ConditionTreeLeaf('string', 'Equal', 'a'),
+      ]),
+    );
+  });
+
+  test('should unwrap a nested group holding a single condition', () => {
+    const tree = ConditionTreeParser.fromPlainObject(collection, {
+      aggregator: 'and',
+      conditions: [
+        { aggregator: 'or', conditions: [{ field: 'string', operator: 'equal', value: 'a' }] },
+        { field: 'number', operator: 'equal', value: 2 },
+      ],
+    });
+
+    expect(tree).toStrictEqual(
+      new ConditionTreeBranch('And', [
+        new ConditionTreeLeaf('string', 'Equal', 'a'),
+        new ConditionTreeLeaf('number', 'Equal', 2),
+      ]),
+    );
+  });
+
+  test.each(['and', 'or'])('should reject an empty nested %s group', aggregator => {
+    expect(() =>
+      ConditionTreeParser.fromPlainObject(collection, {
+        aggregator: 'and',
+        conditions: [
+          { field: 'string', operator: 'equal', value: 'a' },
+          { aggregator, conditions: [] },
+        ],
+      }),
+    ).toThrow('A nested group of conditions cannot be empty');
+  });
+
+  test('should reject an empty group nested deeper than one level', () => {
+    expect(() =>
+      ConditionTreeParser.fromPlainObject(collection, {
+        aggregator: 'or',
+        conditions: [
+          { field: 'string', operator: 'equal', value: 'a' },
+          { aggregator: 'and', conditions: [{ aggregator: 'or', conditions: [] }] },
+        ],
+      }),
+    ).toThrow('A nested group of conditions cannot be empty');
+  });
+
+  test.each([
+    ['a null condition', { aggregator: 'and', conditions: [null] }],
+    ['conditions that are not an array', { aggregator: 'or', conditions: 'x' }],
+  ])('should reject a group holding %s', (_, json) => {
+    expect(() => ConditionTreeParser.fromPlainObject(collection, json)).toThrow(
+      'Failed to instantiate condition tree from json',
+    );
+  });
+
+  test('should keep an empty root group as an empty branch', () => {
+    const tree = ConditionTreeParser.fromPlainObject(collection, {
+      aggregator: 'and',
+      conditions: [],
+    });
+
+    expect(tree).toStrictEqual(new ConditionTreeBranch('And', []));
+  });
+
   test('should work with snake case', () => {
     const tree = ConditionTreeParser.fromPlainObject(collection, {
       field: 'id',

packages/agent/test/utils/query-string.test.ts

@@ -109,6 +109,32 @@ describe('QueryStringParser', () => {
 
       expect(fn).toThrow(ValidationError);
     });
+
+    test('should reject a bulk action selection holding an empty nested group', () => {
+      const context = createMockContext({
+        requestBody: {
+          data: {
+            attributes: {
+              all_records_subset_query: {
+                filters: JSON.stringify({
+                  aggregator: 'and',
+                  conditions: [
+                    { field: 'name', operator: 'equal', value: 'Foundation' },
+                    { aggregator: 'or', conditions: [] },
+                  ],
+                }),
+              },
+            },
+          },
+        },
+      });
+
+      const fn = () => QueryStringParser.parseConditionTree(collectionSimple, context);
+
+      expect(fn).toThrow(
+        new ValidationError('Invalid filters (A nested group of conditions cannot be empty)'),
+      );
+    });
   });
 
   describe('parseProjection', () => {

packages/datasource-mongoose/src/utils/pipeline/filter.ts

@@ -109,6 +109,13 @@ export default class FilterGenerator {
     const schema = MongooseSchema.fromModel(model).applyStack(stack, true);
 
     if (tree instanceof ConditionTreeBranch) {
+      // MongoDB refuses `$and: []` and `$or: []`; the toolkit's MatchNone is an empty Or.
+      if (tree.conditions.length === 0 && tree.aggregator === 'Or') {
+        return { $expr: { $literal: false } };
+      }
+
+      if (tree.conditions.length === 0 && tree.aggregator === 'And') return {};
+
       return {
         [`$${tree.aggregator.toLowerCase()}`]: tree.conditions.map(condition =>
           this.computeMatch(model, stack, condition, fields),

packages/datasource-mongoose/test/integration/review/collection_list.test.ts

@@ -4,7 +4,9 @@ import type { Aggregator, ConditionTreeLeaf, Operator } from '@forestadmin/datas
 import type { Connection } from 'mongoose';
 
 import {
+  ConditionTreeBranch as Branch,
   ConditionTreeEquivalent,
+  ConditionTreeFactory,
   ConditionTreeLeaf as Leaf,
   Page,
   PaginatedFilter,
@@ -388,6 +390,85 @@ describe('MongooseCollection', () => {
       ).rejects.toThrow('unknown top level operator: $bad.');
     });
 
+    describe('with branches', () => {
+      const setupRecords = async () => {
+        connection = await setupReview('collection_review_list');
+        const review = new MongooseDatasource(connection).getCollection('review');
+        await review.create(factories.caller.build(), [
+          { message: 'A', rating: 1 },
+          { message: 'B', rating: 2 },
+          { message: 'C', rating: 3 },
+        ]);
+
+        return review;
+      };
+
+      it('returns nothing for MatchNone', async () => {
+        const review = await setupRecords();
+
+        const records = await review.list(
+          factories.caller.build(),
+          factories.filter.build({ conditionTree: ConditionTreeFactory.MatchNone }),
+          new Projection('message'),
+        );
+
+        expect(records).toEqual([]);
+      });
+
+      it('returns nothing for MatchNone intersected with another condition', async () => {
+        const review = await setupRecords();
+
+        const records = await review.list(
+          factories.caller.build(),
+          factories.filter.build({
+            conditionTree: new Branch('And', [
+              new Leaf('rating', 'GreaterThan', 0),
+              ConditionTreeFactory.MatchNone,
+            ]),
+          }),
+          new Projection('message'),
+        );
+
+        expect(records).toEqual([]);
+      });
+
+      it('returns every record for an empty And', async () => {
+        const review = await setupRecords();
+
+        const records = await review.list(
+          factories.caller.build(),
+          factories.filter.build({
+            conditionTree: new Branch('And', []),
+            sort: new Sort({ field: 'message', ascending: true }),
+          }),
+          new Projection('message'),
+        );
+
+        expect(records).toEqual([{ message: 'A' }, { message: 'B' }, { message: 'C' }]);
+      });
+
+      it('applies an And group nested in an Or', async () => {
+        const review = await setupRecords();
+
+        const records = await review.list(
+          factories.caller.build(),
+          factories.filter.build({
+            conditionTree: new Branch('Or', [
+              new Branch('And', [
+                new Leaf('rating', 'GreaterThan', 1),
+                new Leaf('message', 'Equal', 'B'),
+              ]),
+              new Leaf('message', 'Equal', 'A'),
+            ]),
+            sort: new Sort({ field: 'message', ascending: true }),
+          }),
+          new Projection('message'),
+        );
+
+        expect(records).toEqual([{ message: 'A' }, { message: 'B' }]);
+      });
+    });
+
     it('supports default operators when the primary key objectId is given', async () => {
       connection = await setupReview('collection_review_list');
       const dataSource = new MongooseDatasource(connection);