feat(list): Optimize limit/offset queries by pre-fetching the primary keys that should be returned.

vincentdesmares · vincentdesmares · commit 74c19593f1e4 · 2024-03-04T16:39:08.000+01:00
diff --git a/lib/associations/inject.js b/lib/associations/inject.js
@@ -16,7 +16,7 @@ var __importDefault = (this && this.__importDefault) || function (mod) {
 exports.__esModule = true;
 var debug_1 = __importDefault(require("debug"));
 var graphql_sequelize_1 = require("graphql-sequelize");
-var createResolver_1 = __importDefault(require("../createResolver"));
+var createListResolver_1 = __importDefault(require("../createListResolver"));
 var field_1 = __importDefault(require("./field"));
 var debug = (0, debug_1["default"])('gsg');
 function injectAssociations(modelGraphQLType, graphqlSchemaDeclaration, outputTypes, models, globalPreCallback, proxyModelName) {
@@ -37,7 +37,7 @@ function injectAssociations(modelGraphQLType, graphqlSchemaDeclaration, outputTy
             debug("Cannot generate the association for model [".concat(associations[associationName].target.name, "] as it wasn't declared in the schema declaration. Skipping it."));
             continue;
         }
-        associationsFields[associationName] = (0, field_1["default"])(associations[associationName], outputTypes, graphqlSchemaDeclaration, models, globalPreCallback, (0, createResolver_1["default"])(graphqlSchemaDeclaration[associations[associationName].target.name
+        associationsFields[associationName] = (0, field_1["default"])(associations[associationName], outputTypes, graphqlSchemaDeclaration, models, globalPreCallback, (0, createListResolver_1["default"])(graphqlSchemaDeclaration[associations[associationName].target.name
         // Models MUST be ModelDeclarationType. GraphQLFieldConfig are for custom endpoints.
         ], models, globalPreCallback, associations[associationName]));
     }
diff --git a/lib/createListResolver.js b/lib/createListResolver.js
@@ -163,7 +163,51 @@ var argsAdvancedProcessing = function (findOptions, args, context, info, model,
     }
     return findOptions;
 };
-function createResolver(graphqlTypeDeclaration, models, globalPreCallback, relation) {
+function trimAndOptimizeFindOptions(_a) {
+    var findOptions = _a.findOptions, graphqlTypeDeclaration = _a.graphqlTypeDeclaration, info = _a.info, models = _a.models;
+    return __awaiter(this, void 0, void 0, function () {
+        var trimedFindOptions, fetchIdsFindOptions, result;
+        var _b;
+        return __generator(this, function (_c) {
+            switch (_c.label) {
+                case 0:
+                    trimedFindOptions = graphqlTypeDeclaration.list &&
+                        graphqlTypeDeclaration.list.removeUnusedAttributes === false
+                        ? findOptions
+                        : (0, removeUnusedAttributes_1["default"])(findOptions, info, graphqlTypeDeclaration.model, models);
+                    if (!
+                    // If we have a list with a limit and an offset
+                    (trimedFindOptions.limit &&
+                        trimedFindOptions.offset &&
+                        // And no explicit instructions to not optimize it.
+                        // In the majority of the case, doubling the number of queries should be either
+                        // faster OR not significantly slower.
+                        // As GSG is made to be "easy-to-use", we optimize by default.
+                        // We expect limit to be small enough to not cause performance issues.
+                        // If you are in a case where you need to fetch a big offset, you should disable the optimization.
+                        (!graphqlTypeDeclaration.list ||
+                            typeof graphqlTypeDeclaration.list.disableOptimizationForLimitOffset ===
+                                'undefined' ||
+                            graphqlTypeDeclaration.list.disableOptimizationForLimitOffset !== true))) 
+                    // If we have a list with a limit and an offset
+                    return [3 /*break*/, 2];
+                    fetchIdsFindOptions = __assign(__assign({}, trimedFindOptions), { 
+                        // We only fetch the primary attribute
+                        attributes: [graphqlTypeDeclaration.model.primaryKeyAttribute] });
+                    return [4 /*yield*/, graphqlTypeDeclaration.model.findAll(fetchIdsFindOptions)];
+                case 1:
+                    result = _c.sent();
+                    return [2 /*return*/, __assign(__assign({}, trimedFindOptions), { offset: undefined, limit: undefined, 
+                            // We override the where to only fetch the rows we want.
+                            where: (_b = {},
+                                _b[graphqlTypeDeclaration.model.primaryKeyAttribute] = result.map(function (r) { return r[graphqlTypeDeclaration.model.primaryKeyAttribute]; }),
+                                _b) })];
+                case 2: return [2 /*return*/, trimedFindOptions];
+            }
+        });
+    });
+}
+function createListResolver(graphqlTypeDeclaration, models, globalPreCallback, relation) {
     var _this = this;
     var _a;
     if (relation === void 0) { relation = null; }
@@ -199,7 +243,7 @@ function createResolver(graphqlTypeDeclaration, models, globalPreCallback, relat
             ? graphqlTypeDeclaration.list.contextToOptions
             : undefined,
         before: function (findOptions, args, context, info) { return __awaiter(_this, void 0, void 0, function () {
-            var processedFindOptions, beforeList, beforeList_1, beforeList_1_1, before, handle, e_1_1, handle, result;
+            var processedFindOptions, beforeList, beforeList_1, beforeList_1_1, before, handle, e_1_1, handle, resultBefore;
             var e_1, _a;
             return __generator(this, function (_b) {
                 switch (_b.label) {
@@ -261,18 +305,22 @@ function createResolver(graphqlTypeDeclaration, models, globalPreCallback, relat
                         handle = globalPreCallback('listBefore');
                         return [4 /*yield*/, listBefore(processedFindOptions, args, context, info)];
                     case 9:
-                        result = _b.sent();
+                        resultBefore = _b.sent();
+                        if (!resultBefore) {
+                            throw new Error('The before hook of the list endpoint must return a value.');
+                        }
+                        // The list overwrite the findOptions
+                        processedFindOptions = resultBefore;
                         if (handle) {
                             handle();
                         }
-                        return [2 /*return*/, graphqlTypeDeclaration.list &&
-                                graphqlTypeDeclaration.list.removeUnusedAttributes === false
-                                ? result
-                                : (0, removeUnusedAttributes_1["default"])(result, info, graphqlTypeDeclaration.model, models)];
-                    case 10: return [2 /*return*/, graphqlTypeDeclaration.list &&
-                            graphqlTypeDeclaration.list.removeUnusedAttributes === false
-                            ? processedFindOptions
-                            : (0, removeUnusedAttributes_1["default"])(processedFindOptions, info, graphqlTypeDeclaration.model, models)];
+                        _b.label = 10;
+                    case 10: return [2 /*return*/, trimAndOptimizeFindOptions({
+                            findOptions: processedFindOptions,
+                            graphqlTypeDeclaration: graphqlTypeDeclaration,
+                            info: info,
+                            models: models
+                        })];
                 }
             });
         }); },
@@ -296,4 +344,4 @@ function createResolver(graphqlTypeDeclaration, models, globalPreCallback, relat
         }); }
     });
 }
-exports["default"] = createResolver;
+exports["default"] = createListResolver;
diff --git a/lib/queryResolvers/list.js b/lib/queryResolvers/list.js
@@ -16,8 +16,8 @@ var __importDefault = (this && this.__importDefault) || function (mod) {
 exports.__esModule = true;
 var graphql_1 = require("graphql");
 var graphql_sequelize_1 = require("graphql-sequelize");
-var createResolver_1 = __importDefault(require("../createResolver"));
 var inject_1 = __importDefault(require("../associations/inject"));
+var createListResolver_1 = __importDefault(require("../createListResolver"));
 /**
  * Returns a root `GraphQLObjectType` used as query for `GraphQLSchema`.
  *
@@ -35,7 +35,7 @@ function generateListResolver(modelType, allSchemaDeclarations, outputTypes, mod
         args: __assign(__assign(__assign({}, (0, graphql_sequelize_1.defaultArgs)(schemaDeclaration.model)), (0, graphql_sequelize_1.defaultListArgs)()), (schemaDeclaration.list && schemaDeclaration.list.extraArg
             ? schemaDeclaration.list.extraArg
             : {})),
-        resolve: (0, createResolver_1["default"])(schemaDeclaration, models, globalPreCallback)
+        resolve: (0, createListResolver_1["default"])(schemaDeclaration, models, globalPreCallback)
     };
 }
 exports["default"] = generateListResolver;
diff --git a/src/associations/inject.ts b/src/associations/inject.ts
@@ -2,7 +2,7 @@ import _debug from 'debug'
 import { GraphQLObjectType } from 'graphql'
 import { attributeFields } from 'graphql-sequelize'
 
-import createResolver from '../createResolver'
+import createResolver from '../createListResolver'
 import {
   GlobalPreCallback,
   GraphqlSchemaDeclarationType,
diff --git a/src/createListResolver.ts b/src/createListResolver.ts
@@ -129,7 +129,70 @@ const argsAdvancedProcessing = (
   return findOptions
 }
 
-export default function createResolver(
+async function trimAndOptimizeFindOptions({
+  findOptions,
+  graphqlTypeDeclaration,
+  info,
+  models,
+}: {
+  findOptions: any
+  graphqlTypeDeclaration: any
+  info: any
+  models: any
+}) {
+  const trimedFindOptions =
+    graphqlTypeDeclaration.list &&
+    graphqlTypeDeclaration.list.removeUnusedAttributes === false
+      ? findOptions
+      : removeUnusedAttributes(
+          findOptions,
+          info,
+          graphqlTypeDeclaration.model,
+          models
+        )
+
+  if (
+    // If we have a list with a limit and an offset
+    trimedFindOptions.limit &&
+    trimedFindOptions.offset &&
+    // And no explicit instructions to not optimize it.
+    // In the majority of the case, doubling the number of queries should be either
+    // faster OR not significantly slower.
+    // As GSG is made to be "easy-to-use", we optimize by default.
+    // We expect limit to be small enough to not cause performance issues.
+    // If you are in a case where you need to fetch a big offset, you should disable the optimization.
+    (!graphqlTypeDeclaration.list ||
+      typeof graphqlTypeDeclaration.list.disableOptimizationForLimitOffset ===
+        'undefined' ||
+      graphqlTypeDeclaration.list.disableOptimizationForLimitOffset !== true)
+  ) {
+    // then we pre-fetch the ids to avoid slowness problems for big offsets.
+    const fetchIdsFindOptions = {
+      ...trimedFindOptions,
+      // We only fetch the primary attribute
+      attributes: [graphqlTypeDeclaration.model.primaryKeyAttribute],
+    }
+    const result = await graphqlTypeDeclaration.model.findAll(
+      fetchIdsFindOptions
+    )
+
+    return {
+      ...trimedFindOptions,
+      offset: undefined,
+      limit: undefined,
+      // We override the where to only fetch the rows we want.
+      where: {
+        [graphqlTypeDeclaration.model.primaryKeyAttribute]: result.map(
+          (r: any) => r[graphqlTypeDeclaration.model.primaryKeyAttribute]
+        ),
+      },
+    }
+  }
+
+  return trimedFindOptions
+}
+
+export default function createListResolver(
   graphqlTypeDeclaration: ModelDeclarationType<any>,
   models: any,
   globalPreCallback: any,
@@ -167,7 +230,7 @@ export default function createResolver(
       ? graphqlTypeDeclaration.list.contextToOptions
       : undefined,
     before: async (findOptions: any, args: any, context: any, info: any) => {
-      const processedFindOptions = argsAdvancedProcessing(
+      let processedFindOptions = argsAdvancedProcessing(
         findOptions,
         args,
         context,
@@ -194,6 +257,8 @@ export default function createResolver(
           findOptions.limit = graphqlTypeDeclaration.list.enforceMaxLimit
         }
       }
+
+      // Global hooks, cannot impact the findOptions
       if (graphqlTypeDeclaration.before) {
         const beforeList: GlobalBeforeHook[] =
           typeof graphqlTypeDeclaration.before.length !== 'undefined'
@@ -211,37 +276,35 @@ export default function createResolver(
         }
       }
 
+      // before hook, can mutate the findOptions
       if (listBefore) {
         const handle = globalPreCallback('listBefore')
-        const result = await listBefore(
+        const resultBefore = await listBefore(
           processedFindOptions,
           args,
           context,
           info
         )
+        if (!resultBefore) {
+          throw new Error(
+            'The before hook of the list endpoint must return a value.'
+          )
+        }
+
+        // The list overwrite the findOptions
+        processedFindOptions = resultBefore
+
         if (handle) {
           handle()
         }
-        return graphqlTypeDeclaration.list &&
-          graphqlTypeDeclaration.list.removeUnusedAttributes === false
-          ? result
-          : removeUnusedAttributes(
-              result,
-              info,
-              graphqlTypeDeclaration.model,
-              models
-            )
       }
 
-      return graphqlTypeDeclaration.list &&
-        graphqlTypeDeclaration.list.removeUnusedAttributes === false
-        ? processedFindOptions
-        : removeUnusedAttributes(
-            processedFindOptions,
-            info,
-            graphqlTypeDeclaration.model,
-            models
-          )
+      return trimAndOptimizeFindOptions({
+        findOptions: processedFindOptions,
+        graphqlTypeDeclaration,
+        info,
+        models,
+      })
     },
     after: async (
       result: Model<any> | Model<any>[],
diff --git a/src/queryResolvers/list.ts b/src/queryResolvers/list.ts
@@ -1,8 +1,8 @@
 import { GraphQLList, GraphQLType } from 'graphql'
 import { defaultArgs, defaultListArgs } from 'graphql-sequelize'
 
-import createResolver from '../createResolver'
 import injectAssociations from '../associations/inject'
+import createListResolver from '../createListResolver'
 /**
  * Returns a root `GraphQLObjectType` used as query for `GraphQLSchema`.
  *
@@ -46,6 +46,6 @@ export default function generateListResolver(
         ? schemaDeclaration.list.extraArg
         : {}),
     },
-    resolve: createResolver(schemaDeclaration, models, globalPreCallback),
+    resolve: createListResolver(schemaDeclaration, models, globalPreCallback),
   }
 }
diff --git a/src/tests/basic.spec.js b/src/tests/basic.spec.js
@@ -282,5 +282,23 @@ describe('Test the API queries', () => {
       .set('userId', 1)
     expect(responseNull.body.errors).toBeUndefined()
     expect(responseNull.body.data.locations.length).toBe(2)
+
+    const responseWorksWithLimitAndOffsetOptimization = await request(server)
+      .get(
+        `/graphql?query=
+          query getLocations {
+            locations: location(limit: 10, offset:10) {
+              id
+            }
+          }
+          &operationName=getLocations`
+      )
+      .set('userId', 1)
+    expect(
+      responseWorksWithLimitAndOffsetOptimization.body.errors
+    ).toBeUndefined()
+    expect(
+      responseWorksWithLimitAndOffsetOptimization.body.data.locations.length
+    ).toBe(2)
   })
 })
diff --git a/src/types/types.d.ts b/src/types/types.d.ts
@@ -190,6 +190,7 @@ export type DeleteFieldDeclarationType<M extends Model<M>> = {
 
 export type ListDeclarationType<M extends Model<any>> = {
   removeUnusedAttributes?: boolean
+  disableOptimizationForLimitOffset?: boolean
   extraArg?: ExtraArg
   before?: QueryBeforeHook<M>
   after?: ListAfterHook<M>

Original file line number	Diff line number	Diff line change
`@@ -1,8 +1,8 @@`
`1`	`1`	`import { GraphQLList, GraphQLType } from 'graphql'`
`2`	`2`	`import { defaultArgs, defaultListArgs } from 'graphql-sequelize'`
`3`	`3`
`4`		`-import createResolver from '../createResolver'`
`5`	`4`	`import injectAssociations from '../associations/inject'`
	`5`	`+import createListResolver from '../createListResolver'`
`6`	`6`	`/**`
`7`	`7`	* Returns a root `GraphQLObjectType` used as query for `GraphQLSchema`.
`8`	`8`	`*`
`@@ -46,6 +46,6 @@ export default function generateListResolver(`
`46`	`46`	`? schemaDeclaration.list.extraArg`
`47`	`47`	`: {}),`
`48`	`48`	`},`
`49`		`- resolve: createResolver(schemaDeclaration, models, globalPreCallback),`
	`49`	`+ resolve: createListResolver(schemaDeclaration, models, globalPreCallback),`
`50`	`50`	`}`
`51`	`51`	`}`