Batch query to-one related items

.changeset/neat-plants-shake.md

@@ -0,0 +1,5 @@
+---
+'@keystone-6/core': patch
+---
+
+Improves performance of querying to-one relationships

docs/pages/docs/config/access-control.md

@@ -193,9 +193,7 @@ Filter based access control cannot be used for `create` operations.
 If you want to limit `create` operations, use either `access.operation.create` or `access.item.create`.
 {% /hint %}
 
-{% hint kind="warn" %}
-Filter based access control can impact the performance of your database queries.
-{% /hint %}
+
 
 ### Item (mutations only)
 

packages/core/package.json

@@ -81,6 +81,7 @@
     "cookie": "^0.5.0",
     "cors": "^2.8.5",
     "cuid": "^2.1.8",
+    "dataloader": "^2.1.0",
     "date-fns": "^2.26.0",
     "decimal.js": "^10.4.1",
     "dumb-passwords": "^0.2.1",

packages/core/src/lib/core/queries/output-field.ts

@@ -1,5 +1,6 @@
 import { CacheHint } from 'apollo-server-types';
 import { GraphQLResolveInfo } from 'graphql';
+import DataLoader from 'dataloader';
 import {
   NextFieldType,
   IndividualFieldAccessControl,
@@ -25,7 +26,7 @@ function getRelationVal(
   foreignList: InitialisedList,
   context: KeystoneContext,
   info: GraphQLResolveInfo,
-  fk?: IdType
+  fk: IdType | null | undefined
 ) {
   const oppositeDbField = foreignList.resolvedDbFields[dbField.field];
   if (oppositeDbField.kind !== 'relation') throw new Error('failed assert');
@@ -47,52 +48,82 @@ function getRelationVal(
         // since we know the related item doesn't exist.
         return null;
       }
-      // Check operation permission to pass into single operation
-      const operationAccess = await getOperationAccess(foreignList, context, 'query');
-      if (!operationAccess) {
-        return null;
-      }
-      const accessFilters = await getAccessFilters(foreignList, context, 'query');
-      if (accessFilters === false) {
-        return null;
-      }
-
-      if (accessFilters === true && fk !== undefined) {
-        // We know the exact item we're looking for, and there are no other filters to apply,
-        // so we can use findUnique to get the item. This allows Prisma to group multiple
-        // findUnique operations into a single database query, which solves the N+1 problem
-        // in this specific case.
-        return runWithPrisma(context, foreignList, model =>
-          model.findUnique({ where: { id: fk } })
-        );
-      } else {
-        // Either we have access filters to apply, or we don't have a foreign key to use.
-        // If we have a foreign key, we'll search directly on this ID, and merge in the access filters.
-        // If we don't have a foreign key, we'll use the general solution, which is a filter based
-        // on the original item's ID, merged with any access control filters.
-        const relationFilter =
-          fk !== undefined
-            ? { id: fk }
-            : { [dbField.field]: oppositeDbField.mode === 'many' ? { some: { id } } : { id } };
-
-        // There's no need to check isFilterable access here (c.f. `findOne()`), as
-        // the filter has been constructed internally, not as part of user input.
-
-        // Apply access control
-        const resolvedWhere = await accessControlledFilter(
-          foreignList,
-          context,
-          relationFilter,
-          accessFilters
-        );
-        return runWithPrisma(context, foreignList, model =>
-          model.findFirst({ where: resolvedWhere })
-        );
-      }
+      const ownsForeignKey = fk !== undefined;
+      return fetchRelatedItem(context)(foreignList)(ownsForeignKey ? 'id' : `${dbField.field}Id`)(
+        ownsForeignKey ? fk : id
+      );
     };
   }
 }
 
+function weakMemoize<Arg extends object, Return>(cb: (arg: Arg) => Return) {
+  const cache = new WeakMap<Arg, Return>();
+  return (arg: Arg) => {
+    if (!cache.has(arg)) {
+      const result = cb(arg);
+      cache.set(arg, result);
+    }
+    return cache.get(arg)!;
+  };
+}
+
+function memoize<Arg, Return>(cb: (arg: Arg) => Return) {
+  const cache = new Map<Arg, Return>();
+  return (arg: Arg) => {
+    if (!cache.has(arg)) {
+      const result = cb(arg);
+      cache.set(arg, result);
+    }
+    return cache.get(arg)!;
+  };
+}
+
+const fetchRelatedItem = weakMemoize((context: KeystoneContext) =>
+  weakMemoize((foreignList: InitialisedList) =>
+    memoize((idFieldKey: string) => {
+      const relatedItemLoader = new DataLoader(
+        (keys: readonly IdType[]) => fetchRelatedItems(context, foreignList, idFieldKey, keys),
+        { cache: false }
+      );
+      return (id: IdType) => relatedItemLoader.load(id);
+    })
+  )
+);
+
+async function fetchRelatedItems(
+  context: KeystoneContext,
+  foreignList: InitialisedList,
+  idFieldKey: string,
+  toFetch: readonly IdType[]
+) {
+  const operationAccess = await getOperationAccess(foreignList, context, 'query');
+  if (!operationAccess) {
+    return [];
+  }
+
+  const accessFilters = await getAccessFilters(foreignList, context, 'query');
+  if (accessFilters === false) {
+    return [];
+  }
+
+  const resolvedWhere = await accessControlledFilter(
+    foreignList,
+    context,
+    { [idFieldKey]: { in: toFetch } },
+    accessFilters
+  );
+
+  const results = await runWithPrisma(context, foreignList, model =>
+    model.findMany({
+      where: resolvedWhere,
+    })
+  );
+
+  const resultsById = new Map(results.map(x => [x[idFieldKey], x]));
+
+  return toFetch.map(id => resultsById.get(id));
+}
+
 function getValueForDBField(
   rootVal: BaseItem,
   dbField: ResolvedDBField,

tests/api-tests/relationships/to-one-query-batching.test.ts

@@ -0,0 +1,90 @@
+import { list } from '@keystone-6/core';
+import { allowAll } from '@keystone-6/core/access';
+import { relationship, text } from '@keystone-6/core/fields';
+import stripAnsi from 'strip-ansi';
+import { setupTestRunner } from '../test-runner';
+import { apiTestConfig, dbProvider, dbName } from '../utils';
+
+const runner = setupTestRunner({
+  config: apiTestConfig({
+    db: { enableLogging: true },
+    lists: {
+      Post: list({
+        access: {
+          operation: allowAll,
+          filter: { query: () => ({ title: { not: { contains: 'Secret' } } }) },
+        },
+        fields: {
+          title: text(),
+          author: relationship({ ref: 'User.posts', many: false }),
+        },
+      }),
+      User: list({
+        access: {
+          operation: allowAll,
+          filter: { query: () => ({ name: { contains: 'User' } }) },
+        },
+        fields: {
+          name: text(),
+          posts: relationship({ ref: 'Post.author', many: true }),
+        },
+      }),
+    },
+  }),
+});
+
+test(
+  'to-one relationship query batching',
+  runner(async ({ context }) => {
+    let prevConsoleLog = console.log;
+    let logs: unknown[][] = [];
+    console.log = (...args) => {
+      logs.push(args.map(x => (typeof x === 'string' ? stripAnsi(x) : x)));
+    };
+    try {
+      await context.query.User.createMany({
+        data: Array.from({ length: 10 }, (_, i) => ({
+          name: `User ${i}`,
+          posts: {
+            create: [{ title: `Post from User ${i}` }, { title: `Secret post from User ${i}` }],
+          },
+        })),
+      });
+      logs = [];
+
+      expect(
+        await context.query.Post.findMany({
+          query: 'title author { name }',
+          orderBy: { title: 'asc' },
+        })
+      ).toEqual(
+        Array.from({ length: 10 }, (_, i) => ({
+          title: `Post from User ${i}`,
+          author: { name: `User ${i}` },
+        }))
+      );
+      expect(logs).toEqual([
+        ['prisma:query', expect.stringContaining('SELECT')],
+        ['prisma:query', expect.stringContaining('SELECT')],
+      ]);
+      const expectedSql = {
+        sqlite: [
+          'SELECT `main`.`Post`.`id`, `main`.`Post`.`title`, `main`.`Post`.`author` FROM `main`.`Post` WHERE `main`.`Post`.`title` NOT LIKE ? ORDER BY `main`.`Post`.`title` ASC LIMIT ? OFFSET ? /* traceparent=00-00-00-00 */',
+          'SELECT `main`.`User`.`id`, `main`.`User`.`name` FROM `main`.`User` WHERE (`main`.`User`.`id` IN (?,?,?,?,?,?,?,?,?,?) AND `main`.`User`.`name` LIKE ?) LIMIT ? OFFSET ? /* traceparent=00-00-00-00 */',
+        ],
+        postgresql: [
+          `SELECT "public"."Post"."id", "public"."Post"."title", "public"."Post"."author" FROM "public"."Post" WHERE "public"."Post"."title"::text NOT LIKE $1 ORDER BY "public"."Post"."title" ASC OFFSET $2 /* traceparent=00-00-00-00 */`,
+          `SELECT "public"."User"."id", "public"."User"."name" FROM "public"."User" WHERE ("public"."User"."id" IN ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10) AND "public"."User"."name"::text LIKE $11) OFFSET $12 /* traceparent=00-00-00-00 */`,
+        ],
+        mysql: [
+          `SELECT \`${dbName}\`.\`Post\`.\`id\`, \`${dbName}\`.\`Post\`.\`title\`, \`${dbName}\`.\`Post\`.\`author\` FROM \`${dbName}\`.\`Post\` WHERE \`${dbName}\`.\`Post\`.\`title\` NOT LIKE ? ORDER BY \`${dbName}\`.\`Post\`.\`title\` ASC /* traceparent=00-00-00-00 */`,
+          `SELECT \`${dbName}\`.\`User\`.\`id\`, \`${dbName}\`.\`User\`.\`name\` FROM \`${dbName}\`.\`User\` WHERE (\`${dbName}\`.\`User\`.\`id\` IN (?,?,?,?,?,?,?,?,?,?) AND \`${dbName}\`.\`User\`.\`name\` LIKE ?) /* traceparent=00-00-00-00 */`,
+        ],
+      }[dbProvider];
+      const sql = logs.map(([, query]) => query);
+      expect(sql).toEqual(expectedSql);
+    } finally {
+      console.log = prevConsoleLog;
+    }
+  })
+);

yarn.lock

@@ -6427,7 +6427,7 @@ dataloader@^1.4.0:
   resolved "https://registry.yarnpkg.com/dataloader/-/dataloader-1.4.0.tgz#bca11d867f5d3f1b9ed9f737bd15970c65dff5c8"
   integrity sha512-68s5jYdlvasItOJnCuI2Q9s4q98g0pCyL3HrcKJu8KNugUl8ahgmZYg38ysLTgQjjXX3H8CJLkAvWrclWfcalw==
 
-dataloader@^2.0.0:
+dataloader@^2.0.0, dataloader@^2.1.0:
   version "2.1.0"
   resolved "https://registry.yarnpkg.com/dataloader/-/dataloader-2.1.0.tgz#c69c538235e85e7ac6c6c444bae8ecabf5de9df7"
   integrity sha512-qTcEYLen3r7ojZNgVUaRggOI+KM7jrKxXeSHhogh/TWxYMeONEMqY+hmkobiYQozsGIyg9OYVzO4ZIfoB4I0pQ==