diff --git a/plugins/user-settings-backend/package.json b/plugins/user-settings-backend/package.json index 917fd775b1..c33b2c1a96 100644 --- a/plugins/user-settings-backend/package.json +++ b/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:^", diff --git a/plugins/user-settings-backend/src/service/router.test.ts b/plugins/user-settings-backend/src/service/router.test.ts index 01277a9fd3..1f34806313 100644 --- a/plugins/user-settings-backend/src/service/router.test.ts +++ b/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 = { @@ -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(); diff --git a/plugins/user-settings-backend/src/service/router.ts b/plugins/user-settings-backend/src/service/router.ts index 9e86c062a9..8a999fe0c2 100644 --- a/plugins/user-settings-backend/src/service/router.ts +++ b/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 => { - 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 => { + 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); diff --git a/plugins/user-settings/package.json b/plugins/user-settings/package.json index 4853953f64..cb8777b5aa 100644 --- a/plugins/user-settings/package.json +++ b/plugins/user-settings/package.json @@ -83,7 +83,6 @@ "@testing-library/react": "^16.0.0", "@testing-library/user-event": "^14.0.0", "@types/react": "^18.0.0", - "already": "^2.2.1", "msw": "^1.0.0", "react": "^18.0.2", "react-dom": "^18.0.2", diff --git a/plugins/user-settings/src/apis/StorageApi/UserSettingsStorage.test.ts b/plugins/user-settings/src/apis/StorageApi/UserSettingsStorage.test.ts index 22f7f75ee3..8ec9439242 100644 --- a/plugins/user-settings/src/apis/StorageApi/UserSettingsStorage.test.ts +++ b/plugins/user-settings/src/apis/StorageApi/UserSettingsStorage.test.ts @@ -27,7 +27,7 @@ import { } from '@backstage/test-utils'; import { NotFoundError } from '@backstage/errors'; import { parseDataLoaderKey } from '@backstage/plugin-user-settings-common'; -import { defer } from 'already'; +import { createDeferred } from '@backstage/types'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; import { UserSettingsStorage } from './UserSettingsStorage'; @@ -174,7 +174,7 @@ describe('Persistent Storage API', () => { const selectedKeyNextHandler = jest.fn(); const mockData = { hello: 'im a great new value' }; - const serverCall = defer(undefined); + const serverCall = createDeferred(); server.use( rest.put( @@ -216,7 +216,7 @@ describe('Persistent Storage API', () => { value: mockData, }); - await serverCall.promise; + await serverCall; }); it('should subscribe to key changes when deleting a value', async () => { @@ -225,7 +225,7 @@ describe('Persistent Storage API', () => { const wrongKeyNextHandler = jest.fn(); const selectedKeyNextHandler = jest.fn(); - const serverCall = defer(undefined); + const serverCall = createDeferred(); server.use( rest.delete( @@ -263,7 +263,7 @@ describe('Persistent Storage API', () => { value: undefined, }); - await serverCall.promise; + await serverCall; }); it('should not clash with other namespaces when creating buckets', async () => { diff --git a/yarn.lock b/yarn.lock index 4704f1f38a..0d3f14ac7b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7827,6 +7827,7 @@ __metadata: express: "npm:^4.22.0" express-promise-router: "npm:^4.1.0" knex: "npm:^3.0.0" + p-limit: "npm:^3.1.0" supertest: "npm:^7.0.0" languageName: unknown linkType: soft @@ -7868,7 +7869,6 @@ __metadata: "@testing-library/react": "npm:^16.0.0" "@testing-library/user-event": "npm:^14.0.0" "@types/react": "npm:^18.0.0" - already: "npm:^2.2.1" dataloader: "npm:^2.0.0" msw: "npm:^1.0.0" react: "npm:^18.0.2"