fix: make sure scope filters are used for each fk on includes

If a scope filter is passed, the filter should be applied for each foreign key and not globally.
This implies a query for each fk, and may impact performance. However, is the only way to respect
the scope filter. Example, and include with scope.limit:2 should include 2 instances of the related
model for each fk. Previously scope.limit:2 will cause to "only" include 2 related models.
Signed-off-by:Rodrigo Collao <[email protected]>

fix loopbackio#6832

Signed-off-by: Rodrigo Collao <[email protected]>

packages/repository/src/__tests__/unit/repositories/legacy-juggler-bridge.unit.ts

@@ -475,6 +475,53 @@ describe('DefaultCrudRepository', () => {
         ]);
       });
 
+      it('implements Repository.find() with included models and scope limit', async () => {
+        const createdFolders = await folderRepo.createAll([
+          {name: 'f1', id: 1},
+          {name: 'f2', id: 2},
+          {name: 'f3', id: 3},
+          {name: 'f4', id: 4},
+        ]);
+        const files = await fileRepo.createAll([
+          {id: 1, title: 'file1', folderId: 1},
+          {id: 2, title: 'file3.A', folderId: 3},
+          {id: 3, title: 'file3.D', folderId: 3},
+          {id: 4, title: 'file3.C', folderId: 3},
+          {id: 5, title: 'file3.B', folderId: 3},
+          {id: 6, title: 'file2.D', folderId: 2},
+          {id: 7, title: 'file2.A', folderId: 2},
+          {id: 8, title: 'file2.C', folderId: 2},
+          {id: 9, title: 'file2.B', folderId: 2},
+        ]);
+
+        folderRepo.registerInclusionResolver(
+          'files',
+          folderFiles.inclusionResolver,
+        );
+
+        const folders = await folderRepo.find({
+          include: [
+            {relation: 'files', scope: {limit: 3, order: ['title ASC']}},
+          ],
+        });
+
+        expect(toJSON(folders)).to.deepEqual([
+          {...createdFolders[0].toJSON(), files: [toJSON(files[0])]},
+          {
+            ...createdFolders[1].toJSON(),
+            files: [toJSON(files[6]), toJSON(files[8]), toJSON(files[7])],
+          },
+          {
+            ...createdFolders[2].toJSON(),
+            files: [toJSON(files[1]), toJSON(files[4]), toJSON(files[3])],
+          },
+          {
+            ...createdFolders[3].toJSON(),
+            // files: [], //? I would prefer to have files as an empty array but I think that would be a change to flattenTargetsOfOneToManyRelation?
+          },
+        ]);
+      });
+
       it('implements Repository.findById() with included models', async () => {
         const folders = await folderRepo.createAll([
           {name: 'f1', id: 1},

packages/repository/src/__tests__/unit/repositories/relations-helpers/find-by-foreign-keys.unit.ts

@@ -171,4 +171,37 @@ describe('findByForeignKeys', () => {
     expect(fkValues).to.deepEqual(fkValuesOriginal);
     expect(scope).to.deepEqual(scopeOriginal);
   });
+
+  it('runs a find for each fkValue to properly respect scope filters', async () => {
+    const find = productRepo.stubs.find;
+    const scope = {
+      limit: 4,
+      order: ['name ASC'],
+      where: {name: {like: 'product%'}},
+    };
+    const newproducts = [
+      createProduct({id: 1, name: 'productA', categoryId: 1}),
+      createProduct({id: 2, name: 'productB', categoryId: 2}),
+    ];
+    await productRepo.create(newproducts[0]);
+    await productRepo.create(newproducts[1]);
+    find.resolves([]);
+    await findByForeignKeys(productRepo, 'categoryId', [1, 2], scope);
+    sinon.assert.calledWithMatch(find, {
+      limit: 4,
+      order: ['name ASC'],
+      where: {
+        categoryId: 1,
+        name: {like: 'product%'},
+      },
+    });
+    sinon.assert.calledWithMatch(find, {
+      limit: 4,
+      order: ['name ASC'],
+      where: {
+        categoryId: 2,
+        name: {like: 'product%'},
+      },
+    });
+  });
 });

packages/repository/src/relations/relation.helpers.ts

@@ -39,25 +39,42 @@ export async function findByForeignKeys<
   options?: Options,
 ): Promise<(Target & TargetRelations)[]> {
   let value;
-  scope = cloneDeep(scope);
-
   if (Array.isArray(fkValues)) {
     if (fkValues.length === 0) return [];
     value = fkValues.length === 1 ? fkValues[0] : {inq: fkValues};
   } else {
     value = fkValues;
   }
 
-  const where = {[fkName]: value} as unknown as Where<Target>;
-
-  if (scope && !_.isEmpty(scope)) {
-    // combine where clause to scope filter
-    scope = new FilterBuilder(scope).impose({where}).filter;
+  const isScopeSet = scope && !_.isEmpty(scope);
+  if (isScopeSet && Array.isArray(fkValues)) {
+    // since there is a scope, there could be a where filter, a limit, an order
+    // and we should run the scope in multiple queries so we can respect the
+    // scope filter params
+    const findPromises = fkValues.map(fk => {
+      const where = {[fkName]: fk} as unknown as Where<Target>;
+      let localScope = cloneDeep(scope);
+      // combine where clause to scope filter
+      localScope = new FilterBuilder(localScope).impose({where}).filter;
+      return targetRepository.find(localScope, options);
+    });
+    return Promise.all(findPromises).then(findResults => {
+      //findResults is an array of arrays for each scope result, so we need to flatten it before returning it
+      return findResults.flat();
+    });
   } else {
-    scope = {where} as Filter<Target>;
-  }
+    scope = cloneDeep(scope);
+    const where = {[fkName]: value} as unknown as Where<Target>;
+
+    if (isScopeSet) {
+      // combine where clause to scope filter
+      scope = new FilterBuilder(scope).impose({where}).filter;
+    } else {
+      scope = {where} as Filter<Target>;
+    }
 
-  return targetRepository.find(scope, options);
+    return targetRepository.find(scope, options);
+  }
 }
 
 export type StringKeyOf<T> = Extract<keyof T, string>;