fix(Async Limit): Fix queuing for bulk requests

Fixes #642
ideal-postcodes · Dec 11, 2020 · 99d9ee0 · 99d9ee0
1 parent 5fbcdb0
commit 99d9ee0
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 40 deletions.
diff --git a/src/app/controllers/postcodes_controller.ts b/src/app/controllers/postcodes_controller.ts
@@ -1,6 +1,7 @@
 import { isEmpty, qToString } from "../lib/string";
 import { Postcode } from "../models/postcode";
 import { isValid } from "postcode";
+import { chunk } from "../lib/chunk";
 import { getConfig } from "../../config/config";
 import {
   InvalidPostcodeError,
@@ -85,31 +86,33 @@ const bulkGeocode: Handler = async (request, response, next) => {
   try {
     const { geolocations } = request.body;
 
-    let globalLimit;
+    let globalLimit: string | undefined;
     if (request.query.limit) globalLimit = qToString(request.query.limit);
 
-    let globalRadius;
+    let globalRadius: string | undefined;
     if (request.query.radius) globalRadius = qToString(request.query.radius);
 
-    let globalWidesearch;
+    let globalWidesearch: string | boolean;
     if (request.query.widesearch) globalWidesearch = true;
 
     if (!Array.isArray(geolocations)) return next(new JsonArrayRequiredError());
     if (geolocations.length > MAX_GEOLOCATIONS)
       return next(new ExceedMaxGeolocationsError());
 
+    const data: LookupGeolocationResult[] = [];
+
     const lookupGeolocation = async (
       location: NearestPostcodesOptions
-    ): Promise<LookupGeolocationResult> => {
+    ): Promise<void> => {
       const postcodes = await Postcode.nearestPostcodes(location);
       let result = null;
       if (postcodes && postcodes.length > 0) {
         result = postcodes.map((postcode) => Postcode.toJson(postcode));
       }
-      return {
+      data.push({
         query: sanitizeQuery(location),
         result,
-      };
+      });
     };
 
     const whitelist = [
@@ -130,23 +133,22 @@ const bulkGeocode: Handler = async (request, response, next) => {
       return result;
     };
 
-    const result = [];
-    for (let i = 0; i < GEO_ASYNC_LIMIT; i += 1) {
-      if (geolocations[i]) {
-        result.push(
-          await lookupGeolocation({
-            ...(globalLimit && { limit: globalLimit }),
-            ...(globalRadius && { radius: globalRadius }),
-            ...(globalWidesearch && { widesearch: true }),
-            ...geolocations[i],
-          })
-        );
-      } else {
-        break;
-      }
-    }
+    // for (let i = 0; i < GEO_ASYNC_LIMIT; i += 1) {
+    const queue = chunk(
+      geolocations.map((geolocation) => {
+        return lookupGeolocation({
+          ...(globalLimit && { limit: globalLimit }),
+          ...(globalRadius && { radius: globalRadius }),
+          ...(globalWidesearch && { widesearch: true }),
+          ...geolocation,
+        });
+      }),
+      GEO_ASYNC_LIMIT
+    );
+
+    for (const q of queue) await Promise.all(q);
 
-    response.jsonApiResponse = { status: 200, result };
+    response.jsonApiResponse = { status: 200, result: data };
     next();
   } catch (error) {
     next(error);
@@ -169,27 +171,33 @@ const bulkLookupPostcodes: Handler = async (request, response, next) => {
     if (postcodes.length > MAX_POSTCODES)
       return next(new ExceedMaxPostcodesError());
 
-    const lookupPostcode = async (
-      postcode: string
-    ): Promise<BulkLookupPostcodesResult> => {
+    const result: BulkLookupPostcodesResult[] = [];
+
+    const lookupPostcode = async (postcode: string): Promise<void> => {
       const postcodeInfo = await Postcode.find(postcode);
-      if (!postcodeInfo) return { query: postcode, result: null };
-      return {
+      if (!postcodeInfo) {
+        result.push({ query: postcode, result: null });
+        return;
+      }
+      result.push({
         query: postcode,
         result: Postcode.toJson(postcodeInfo),
-      };
+      });
     };
 
-    const data = [];
-    for (let i = 0; i < BULK_ASYNC_LIMIT; i += 1) {
-      if (postcodes[i] && postcodes[i] !== null) {
-        data.push(await lookupPostcode(postcodes[i]));
-      } else {
-        break;
-      }
-    }
+    const queue: Promise<void>[][] = chunk(
+      postcodes
+        .map((p) => {
+          if (typeof p === "string") return lookupPostcode(p);
+          return;
+        })
+        .filter((p) => p !== null),
+      BULK_ASYNC_LIMIT
+    );
 
-    response.jsonApiResponse = { status: 200, result: data };
+    for (const queries of queue) await Promise.all(queries);
+
+    response.jsonApiResponse = { status: 200, result };
     next();
   } catch (error) {
     next(error);

diff --git a/src/config/defaults.ts b/src/config/defaults.ts
@@ -47,14 +47,14 @@ export const defaults = {
   bulkGeocode: {
     geolocations: {
       MAX: parseEnv(BULKGEOCODE_GEOLOCATIONS_MAX, 100), // Maximum number of geolocations per request
-      ASYNC_LIMIT: parseEnv(BULKGEOCODE_GEOLOCATIONS_ASYNC_LIMIT, null), // Maximum number of parallel DB queries per request
+      ASYNC_LIMIT: parseEnv(BULKGEOCODE_GEOLOCATIONS_ASYNC_LIMIT, 3), // Maximum number of parallel DB queries per request
       TIMEOUT: parseEnv(BULKGEOCODE_GEOLOCATIONS_TIMEOUT, 30000), // Maximum interval to run a single bulk request
     },
   },
   bulkLookups: {
     postcodes: {
       MAX: parseEnv(BULKLOOKUPS_POSTCODES_MAX, 100), // Maximum number of postcodes per request
-      ASYNC_LIMIT: parseEnv(BULKLOOKUPS_POSTCODES_ASYNC_LIMIT, null), // Maximum number of parallel DB queries per request
+      ASYNC_LIMIT: parseEnv(BULKLOOKUPS_POSTCODES_ASYNC_LIMIT, 3), // Maximum number of parallel DB queries per request
       TIMEOUT: parseEnv(BULKLOOKUPS_POSTCODES_TIMEOUT, 30000), // Maximum interval to run a single bulk request
     },
   },

diff --git a/test/chunk.unit.ts b/test/chunk.unit.ts
@@ -13,5 +13,6 @@ describe("chunk", () => {
       [2, 2, 2],
       [3, 3],
     ]);
+    assert.deepEqual(chunk([1, 1], 3), [[1, 1]]);
   });
 });
diff --git a/test/postcodes.bulk.integration.ts b/test/postcodes.bulk.integration.ts
@@ -18,7 +18,7 @@ describe("Postcodes routes", () => {
   after(async () => helper.clearPostcodeDb);
 
   describe("POST /postcodes", () => {
-    const bulkLength = 10;
+    const bulkLength = 12;
     let testPostcodes: any, testLocations: any;
 
     describe("Invalid JSON submission", () => {
-Original file line number
+Diff line change
@@ Expand Up / @@ -13,5 +13,6 @@ describe("chunk", () => { @@
           [2, 2, 2],
           [3, 3],
         ]);
+        assert.deepEqual(chunk([1, 1], 3), [[1, 1]]);
       });
     });