Use p-limit instead of already for promise concurrency limit

Signed-off-by: Gustaf Räntilä <g.rantila@gmail.com>

plugins/user-settings-backend/package.json

@@ -58,7 +58,8 @@
     "already": "^2.2.1",
     "express": "^4.22.0",
     "express-promise-router": "^4.1.0",
-    "knex": "^3.0.0"
+    "knex": "^3.0.0",
+    "p-limit": "^3.1.0"
   },
   "devDependencies": {
     "@backstage/backend-defaults": "workspace:^",

plugins/user-settings-backend/src/service/router.test.ts

@@ -18,12 +18,14 @@ import express from 'express';
 import request from 'supertest';
 import { UserSettingsStore } from '../database/UserSettingsStore';
 import { createRouter } from './router';
+import { NotFoundError } from '@backstage/errors';
 import { SignalsService } from '@backstage/plugin-signals-node';
 import {
   mockCredentials,
   mockServices,
   mockErrorHandler,
 } from '@backstage/backend-test-utils';
+import { stringifyDataLoaderKey } from '@backstage/plugin-user-settings-common';
 
 describe('createRouter', () => {
   const userSettingsStore: jest.Mocked<UserSettingsStore> = {
@@ -108,6 +110,147 @@ describe('createRouter', () => {
     });
   });
 
+  describe('GET /multi', () => {
+    it('returns single value', async () => {
+      const values = { 'key-1': 'a' };
+
+      userSettingsStore.get.mockImplementation(async ({ bucket, key }) => {
+        if (key in values) {
+          return {
+            bucket: 'my-bucket',
+            key,
+            value: values[key as keyof typeof values],
+          };
+        }
+        throw new NotFoundError(
+          `Unable to find '${key}' in bucket '${bucket}'`,
+        );
+      });
+
+      const url = new URLSearchParams();
+      for (const key of Object.keys(values)) {
+        url.append('items', stringifyDataLoaderKey('my-bucket', key));
+      }
+      const responses = await request(app).get(`/multi?${url.toString()}`);
+
+      expect(responses.status).toEqual(200);
+      expect(responses.body).toEqual([
+        { bucket: 'my-bucket', key: 'key-1', value: 'a' },
+      ]);
+
+      expect(userSettingsStore.get).toHaveBeenCalledTimes(1);
+      expect(userSettingsStore.get).toHaveBeenCalledWith({
+        userEntityRef: mockUserRef,
+        bucket: 'my-bucket',
+        key: 'key-1',
+      });
+    });
+
+    it('returns single missing', async () => {
+      const values = { 'key-1': 'a' };
+
+      userSettingsStore.get.mockImplementation(async ({ bucket, key }) => {
+        if (key in values) {
+          return {
+            bucket: 'my-bucket',
+            key,
+            value: values[key as keyof typeof values],
+          };
+        }
+        throw new NotFoundError(
+          `Unable to find '${key}' in bucket '${bucket}'`,
+        );
+      });
+
+      const url = new URLSearchParams();
+      url.append('items', stringifyDataLoaderKey('my-bucket', 'missing-key'));
+      const responses = await request(app).get(`/multi?${url.toString()}`);
+
+      expect(responses.status).toEqual(200);
+      expect(responses.body).toEqual([
+        {
+          bucket: 'my-bucket',
+          key: 'missing-key',
+          error: {
+            name: 'NotFoundError',
+            message: expect.stringContaining('missing-key'),
+          },
+        },
+      ]);
+
+      expect(userSettingsStore.get).toHaveBeenCalledTimes(1);
+      expect(userSettingsStore.get).toHaveBeenCalledWith({
+        userEntityRef: mockUserRef,
+        bucket: 'my-bucket',
+        key: 'missing-key',
+      });
+    });
+
+    it('returns existing and missing mixed', async () => {
+      const values = {
+        'key-1': 'a',
+        'key-2': 'b',
+      };
+
+      userSettingsStore.get.mockImplementation(async ({ bucket, key }) => {
+        if (key in values) {
+          return {
+            bucket: 'my-bucket',
+            key,
+            value: values[key as keyof typeof values],
+          };
+        }
+        throw new NotFoundError(
+          `Unable to find '${key}' in bucket '${bucket}'`,
+        );
+      });
+
+      const url = new URLSearchParams();
+      for (const key of Object.keys(values)) {
+        url.append('items', stringifyDataLoaderKey('my-bucket', key));
+      }
+      url.append('items', stringifyDataLoaderKey('my-bucket', 'missing-key'));
+      const responses = await request(app).get(`/multi?${url.toString()}`);
+
+      expect(responses.status).toEqual(200);
+      expect(responses.body).toEqual([
+        { bucket: 'my-bucket', key: 'key-1', value: 'a' },
+        { bucket: 'my-bucket', key: 'key-2', value: 'b' },
+        {
+          bucket: 'my-bucket',
+          key: 'missing-key',
+          error: {
+            name: 'NotFoundError',
+            message: expect.stringContaining('missing-key'),
+          },
+        },
+      ]);
+
+      expect(userSettingsStore.get).toHaveBeenCalledTimes(3);
+      for (const key of Object.keys(values)) {
+        expect(userSettingsStore.get).toHaveBeenCalledWith({
+          userEntityRef: mockUserRef,
+          bucket: 'my-bucket',
+          key,
+        });
+      }
+      expect(userSettingsStore.get).toHaveBeenCalledWith({
+        userEntityRef: mockUserRef,
+        bucket: 'my-bucket',
+        key: 'missing-key',
+      });
+    });
+
+    it('returns an error if the Authorization header is missing', async () => {
+      const responses = await request(app)
+        .get('/buckets/my-bucket/keys/my-key')
+        .set('Authorization', mockCredentials.none.header());
+
+      expect(responses.status).toEqual(401);
+      expect(userSettingsStore.get).not.toHaveBeenCalled();
+    });
+  });
+
   describe('DELETE /buckets/:bucket/keys/:key', () => {
     it('returns ok', async () => {
       userSettingsStore.delete.mockResolvedValue();

plugins/user-settings-backend/src/service/router.ts

@@ -15,9 +15,9 @@
  */
 
 import { InputError, serializeError } from '@backstage/errors';
-import { map } from 'already';
 import express, { Request } from 'express';
 import Router from 'express-promise-router';
+import pLimit from 'p-limit';
 import { UserSettingsStore } from '../database/UserSettingsStore';
 import { SignalsService } from '@backstage/plugin-signals-node';
 import {
@@ -69,30 +69,32 @@ export async function createRouter(options: {
       throw new InputError('Expected query param "items" to be an array');
     }
 
-    const userSettings = await map(
-      bucketsAndKeys,
-      { concurrency: 10 },
-      async ({ bucket, key }): Promise<MultiUserSetting> => {
-        try {
-          const setting = await options.userSettingsStore.get({
-            userEntityRef,
-            bucket,
-            key,
-          });
-          return setting;
-        } catch (e) {
-          if (e instanceof Error) {
-            const serialized = serializeError(e);
-            return { bucket, key, error: serialized };
-          }
+    const limit = pLimit(10);
 
-          return {
-            bucket,
-            key,
-            error: { name: 'Error', message: 'Unknown error' },
-          };
-        }
-      },
+    const userSettings = await Promise.all(
+      bucketsAndKeys.map(({ bucket, key }) =>
+        limit(async (): Promise<MultiUserSetting> => {
+          try {
+            const setting = await options.userSettingsStore.get({
+              userEntityRef,
+              bucket,
+              key,
+            });
+            return setting;
+          } catch (e) {
+            if (e instanceof Error) {
+              const serialized = serializeError(e);
+              return { bucket, key, error: serialized };
+            }
+
+            return {
+              bucket,
+              key,
+              error: { name: 'Error', message: 'Unknown error' },
+            };
+          }
+        }),
+      ),
     );
 
     res.json(userSettings);