diff --git a/plugins/user-settings-backend/package.json b/plugins/user-settings-backend/package.json index c33b2c1a96..261335b040 100644 --- a/plugins/user-settings-backend/package.json +++ b/plugins/user-settings-backend/package.json @@ -59,7 +59,8 @@ "express": "^4.22.0", "express-promise-router": "^4.1.0", "knex": "^3.0.0", - "p-limit": "^3.1.0" + "p-limit": "^3.1.0", + "zod": "^3.22.4" }, "devDependencies": { "@backstage/backend-defaults": "workspace:^", diff --git a/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.test.ts b/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.test.ts index 2d9ea5b23e..5fb2dde23f 100644 --- a/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.test.ts +++ b/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.test.ts @@ -107,6 +107,81 @@ describe.each(databases.eachSupportedId())( }); }); + describe('multiget', () => { + it('should return empty', async () => { + await expect( + storage.multiget({ + userEntityRef: 'user-1', + items: [ + { + bucket: 'bucket-c', + key: 'key-c', + }, + ], + }), + ).resolves.toEqual([null]); + }); + + it('should handle large inputs and return existing values', async () => { + // Create 30 buckets with 300 keys in each. + const BUCKETS = 30; + const KEYS_PER_BUCKET = 300; + + const buckets = Array.from(Array(BUCKETS)).map( + (_, i) => `mget-bucket-${i}`, + ); + + const items = buckets.flatMap(bucket => { + const keys = Array.from(Array(KEYS_PER_BUCKET)).map( + (_, i) => `mget-key-${i}`, + ); + return keys.map(key => ({ bucket, key })); + }); + + const chunkify = (keys: Array, size: number): T[][] => { + const chunks: T[][] = []; + for (let i = 0; i < keys.length; i += size) { + chunks.push(keys.slice(i, i + size)); + } + return chunks; + }; + + const valueOfKey = (bucket: string, key: string) => ({ + theValue: `Value of ${bucket} / ${key}`, + }); + + const chunkedItems = chunkify(items, 100); + for (const chunk of chunkedItems) { + await insert( + chunk.map(({ bucket, key }) => ({ + user_entity_ref: 'user-1', + bucket, + key, + value: JSON.stringify(valueOfKey(bucket, key)), + })), + ); + } + + const result = await storage.multiget({ + userEntityRef: 'user-1', + // Include a missing key which shouldn't exist in the result + items: [...items, { bucket: 'missing', key: 'missing' }], + }); + expect(result).toEqual([ + ...items.map(({ bucket, key }) => ({ + value: valueOfKey(bucket, key), + })), + null, // The missing key + ]); + + const rows = await knex('user_settings') + .where('user_entity_ref', 'user-1') + .count<{ count: string }[]>('* as count'); + const savedRows = Number(rows[0].count); + expect(savedRows).toEqual(BUCKETS * KEYS_PER_BUCKET); + }); + }); + describe('set', () => { it('should insert a new setting', async () => { await storage.set({ diff --git a/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.ts b/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.ts index 8caa62c4cd..1d53d87162 100644 --- a/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.ts +++ b/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.ts @@ -21,6 +21,7 @@ import { import { NotFoundError } from '@backstage/errors'; import { JsonValue } from '@backstage/types'; import { Knex } from 'knex'; +import pLimit from 'p-limit'; import { UserSettingsStore, type UserSetting } from './UserSettingsStore'; const migrationsDir = resolvePackagePath( @@ -91,6 +92,80 @@ export class DatabaseUserSettingsStore implements UserSettingsStore { }; } + async multiget(options: { + userEntityRef: string; + items: Array<{ bucket: string; key: string }>; + }): Promise<({ value: JsonValue } | null)[]> { + if (options.items.length === 0) { + return []; + } + + // Split the items into a map of bucket -> keys + const bucketMap = new Map>(); + for (const item of options.items) { + let keys = bucketMap.get(item.bucket); + if (!keys) { + keys = new Set(); + bucketMap.set(item.bucket, keys); + } + keys.add(item.key); + } + + const dbLimit = pLimit(10); + + // Chunks the keys per bucket to avoid hitting SQL parameter limits + const chunkKeys = (keys: Array, size: number): string[][] => { + const chunks = []; + for (let i = 0; i < keys.length; i += size) { + chunks.push(keys.slice(i, i + size)); + } + return chunks; + }; + + // Store the database content into a map of bucket -> {key -> value} + const resultsMap = new Map>(); + + await Promise.all( + bucketMap.keys().map(bucket => + dbLimit(async (): Promise => { + const keyMap = new Map(); + resultsMap.set(bucket, keyMap); + + const keyChunks = chunkKeys( + Array.from(bucketMap.get(bucket) || []), + 100, + ); + + for (const keys of keyChunks) { + const rows = await this.db('user_settings') + .where({ + user_entity_ref: options.userEntityRef, + bucket, + }) + .whereIn('key', keys) + .select(['bucket', 'key', 'value']); + + for (const row of rows) { + keyMap.set(row.key, JSON.parse(row.value)); + } + } + }), + ), + ); + + // For each exact bucket/key requested, return either the value or null if + // not found + return options.items.map(({ bucket, key }) => { + const value = resultsMap.get(bucket)?.get(key); + + if (typeof value === 'undefined') { + return null; + } + + return { value }; + }); + } + async set(options: { userEntityRef: string; bucket: string; diff --git a/plugins/user-settings-backend/src/database/UserSettingsStore.ts b/plugins/user-settings-backend/src/database/UserSettingsStore.ts index 289c37523e..9c0a5ebf5b 100644 --- a/plugins/user-settings-backend/src/database/UserSettingsStore.ts +++ b/plugins/user-settings-backend/src/database/UserSettingsStore.ts @@ -35,6 +35,16 @@ export interface UserSettingsStore { key: string; }): Promise; + /** + * Get multiple user settings at once. The result is an array corresponding + * index by index to the elements of the input, where the elements are either + * `null` (not found), or an object `{ value: JsonValue }`. + */ + multiget(options: { + userEntityRef: string; + items: Array<{ bucket: string; key: string }>; + }): Promise<({ value: JsonValue } | null)[]>; + set(options: { userEntityRef: string; bucket: string; diff --git a/plugins/user-settings-backend/src/service/router.test.ts b/plugins/user-settings-backend/src/service/router.test.ts index 1f34806313..3503ba3c80 100644 --- a/plugins/user-settings-backend/src/service/router.test.ts +++ b/plugins/user-settings-backend/src/service/router.test.ts @@ -18,17 +18,17 @@ 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'; +import { JsonValue } from '@backstage/types'; describe('createRouter', () => { const userSettingsStore: jest.Mocked = { + multiget: jest.fn(), get: jest.fn(), set: jest.fn(), delete: jest.fn(), @@ -111,78 +111,72 @@ describe('createRouter', () => { }); describe('GET /multi', () => { + const mockMultiget = (store: Record>) => { + userSettingsStore.multiget.mockImplementation(async ({ items }) => { + return items.map(({ bucket, key }) => { + const value = store[bucket]?.[key]; + if (typeof value !== 'undefined') { + return { value }; + } + return null; + }); + }); + }; + it('returns single value', async () => { const values = { 'key-1': 'a' }; - userSettingsStore.get.mockImplementation(async ({ bucket, key }) => { - if (key in values) { - return { + mockMultiget({ 'my-bucket': values }); + + const responses = await request(app) + .post('/multiget') + .send({ + items: Object.keys(values).map(key => ({ 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(responses.body).toEqual({ items: [{ value: 'a' }] }); - expect(userSettingsStore.get).toHaveBeenCalledTimes(1); - expect(userSettingsStore.get).toHaveBeenCalledWith({ + expect(userSettingsStore.multiget).toHaveBeenCalledTimes(1); + expect(userSettingsStore.multiget).toHaveBeenCalledWith({ userEntityRef: mockUserRef, - bucket: 'my-bucket', - key: 'key-1', + items: [ + { + 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}'`, - ); - }); + mockMultiget({ 'my-bucket': values }); - const url = new URLSearchParams(); - url.append('items', stringifyDataLoaderKey('my-bucket', 'missing-key')); - const responses = await request(app).get(`/multi?${url.toString()}`); + const responses = await request(app) + .post('/multiget') + .send({ + items: [{ bucket: 'my-bucket', key: 'missing-key' }], + }); expect(responses.status).toEqual(200); - expect(responses.body).toEqual([ - { - bucket: 'my-bucket', - key: 'missing-key', - error: { - name: 'NotFoundError', - message: expect.stringContaining('missing-key'), - }, - }, - ]); + expect(responses.body).toEqual({ + items: [null], + }); - expect(userSettingsStore.get).toHaveBeenCalledTimes(1); - expect(userSettingsStore.get).toHaveBeenCalledWith({ + expect(userSettingsStore.multiget).toHaveBeenCalledTimes(1); + expect(userSettingsStore.multiget).toHaveBeenCalledWith({ userEntityRef: mockUserRef, - bucket: 'my-bucket', - key: 'missing-key', + items: [ + { + bucket: 'my-bucket', + key: 'missing-key', + }, + ], }); }); @@ -192,52 +186,35 @@ describe('createRouter', () => { '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}'`, - ); - }); + mockMultiget({ 'my-bucket': values }); - 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()}`); + const responses = await request(app) + .post('/multiget') + .send({ + items: [ + ...Object.keys(values).map(key => ({ bucket: 'my-bucket', key })), + { bucket: 'my-bucket', key: 'missing-key' }, + ], + }); 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(responses.body).toEqual({ + items: [{ value: 'a' }, { value: 'b' }, null], + }); - 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({ + expect(userSettingsStore.multiget).toHaveBeenCalledTimes(1); + expect(userSettingsStore.multiget).toHaveBeenCalledWith({ userEntityRef: mockUserRef, - bucket: 'my-bucket', - key: 'missing-key', + items: [ + ...Object.keys(values).map(key => ({ + bucket: 'my-bucket', + key, + })), + { + bucket: 'my-bucket', + key: 'missing-key', + }, + ], }); }); diff --git a/plugins/user-settings-backend/src/service/router.ts b/plugins/user-settings-backend/src/service/router.ts index 8a999fe0c2..c9971bbb4d 100644 --- a/plugins/user-settings-backend/src/service/router.ts +++ b/plugins/user-settings-backend/src/service/router.ts @@ -14,16 +14,15 @@ * limitations under the License. */ -import { InputError, serializeError } from '@backstage/errors'; +import { InputError } from '@backstage/errors'; import express, { Request } from 'express'; import Router from 'express-promise-router'; -import pLimit from 'p-limit'; +import { z } from 'zod'; import { UserSettingsStore } from '../database/UserSettingsStore'; import { SignalsService } from '@backstage/plugin-signals-node'; import { - MultiUserSetting, + MultiGetResponse, UserSettingsSignal, - parseDataLoaderKey, } from '@backstage/plugin-user-settings-common'; import { HttpAuthService } from '@backstage/backend-plugin-api'; @@ -35,6 +34,10 @@ export async function createRouter(options: { const router = Router(); router.use(express.json()); + const multiGetRequestSchema = z.object({ + items: z.array(z.object({ bucket: z.string(), key: z.string() })), + }); + /** * Helper method to extract the userEntityRef from the request. */ @@ -46,58 +49,17 @@ export async function createRouter(options: { }; // get multiple values - router.get('/multi', async (req, res) => { + router.post('/multiget', async (req, res) => { const userEntityRef = await getUserEntityRef(req); - const bucketsAndKeys: ReturnType[] = []; + const bucketsAndKeys = multiGetRequestSchema.parse(req.body).items; - const items = req.query.items; - if (typeof items === 'string') { - bucketsAndKeys.push(parseDataLoaderKey(items)); - } else if (Array.isArray(items)) { - bucketsAndKeys.push( - ...items.map(item => { - if (typeof item !== 'string') { - throw new InputError( - 'Expected query param "items" to be an array of strings', - ); - } - return parseDataLoaderKey(item); - }), - ); - } else { - throw new InputError('Expected query param "items" to be an array'); - } + const items = await options.userSettingsStore.multiget({ + userEntityRef, + items: bucketsAndKeys, + }); - const limit = pLimit(10); - - 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); + res.json({ items } satisfies MultiGetResponse); }); // get a single value diff --git a/plugins/user-settings-common/package.json b/plugins/user-settings-common/package.json index 0002355afb..92a5b84ecd 100644 --- a/plugins/user-settings-common/package.json +++ b/plugins/user-settings-common/package.json @@ -38,7 +38,6 @@ "test": "backstage-cli package test" }, "dependencies": { - "@backstage/errors": "workspace:^", "@backstage/types": "workspace:^" }, "devDependencies": { diff --git a/plugins/user-settings-common/report.api.md b/plugins/user-settings-common/report.api.md index 03903d8107..42b9bd3f04 100644 --- a/plugins/user-settings-common/report.api.md +++ b/plugins/user-settings-common/report.api.md @@ -4,39 +4,14 @@ ```ts import type { JsonValue } from '@backstage/types'; -import type { SerializedError } from '@backstage/errors'; - -// @public (undocumented) -export function isMultiUserSettingError( - setting: MultiUserSetting, -): setting is MultiUserSettingError; // @public -export type MultiUserSetting = MultiUserSettingError | MultiUserSettingSuccess; - -// @public -export type MultiUserSettingError = { - bucket: string; - key: string; - error: SerializedError; +export type MultiGetResponse = { + items: ({ + value: JsonValue; + } | null)[]; }; -// @public -export type MultiUserSettingSuccess = { - bucket: string; - key: string; - value: JsonValue; -}; - -// @public (undocumented) -export function parseDataLoaderKey(bucketAndKey: string): { - bucket: string; - key: string; -}; - -// @public (undocumented) -export function stringifyDataLoaderKey(bucket: string, key: string): string; - // @public (undocumented) export type UserSettingsSignal = { type: 'key-changed' | 'key-deleted'; diff --git a/plugins/user-settings-common/src/index.ts b/plugins/user-settings-common/src/index.ts index 43f85ada2b..5d542de408 100644 --- a/plugins/user-settings-common/src/index.ts +++ b/plugins/user-settings-common/src/index.ts @@ -15,6 +15,3 @@ */ export * from './types'; -export { isMultiUserSettingError } from './types'; - -export { stringifyDataLoaderKey, parseDataLoaderKey } from './keys'; diff --git a/plugins/user-settings-common/src/keys.ts b/plugins/user-settings-common/src/keys.ts deleted file mode 100644 index 4a319dcd02..0000000000 --- a/plugins/user-settings-common/src/keys.ts +++ /dev/null @@ -1,26 +0,0 @@ -/* - * Copyright 2025 The Backstage Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -/** @public */ -export function stringifyDataLoaderKey(bucket: string, key: string) { - return `${encodeURIComponent(bucket)}/${encodeURIComponent(key)}`; -} - -/** @public */ -export function parseDataLoaderKey(bucketAndKey: string) { - const [bucket, key] = bucketAndKey.split('/'); - return { bucket: decodeURIComponent(bucket), key: decodeURIComponent(key) }; -} diff --git a/plugins/user-settings-common/src/types.ts b/plugins/user-settings-common/src/types.ts index 43fcef8a7d..437d0c8e1c 100644 --- a/plugins/user-settings-common/src/types.ts +++ b/plugins/user-settings-common/src/types.ts @@ -14,7 +14,6 @@ * limitations under the License. */ -import type { SerializedError } from '@backstage/errors'; import type { JsonValue } from '@backstage/types'; /** @public */ @@ -24,37 +23,8 @@ export type UserSettingsSignal = { }; /** - * A failed fetch of a user setting in a bucket + * Response type from /multiget * * @public */ -export type MultiUserSettingError = { - bucket: string; - key: string; - error: SerializedError; -}; - -/** - * A successful value of a user setting in a bucket - * - * @public - */ -export type MultiUserSettingSuccess = { - bucket: string; - key: string; - value: JsonValue; -}; - -/** - * A single setting in a bucket, or an error, used result from the /multi endpoint - * - * @public - */ -export type MultiUserSetting = MultiUserSettingError | MultiUserSettingSuccess; - -/** @public */ -export function isMultiUserSettingError( - setting: MultiUserSetting, -): setting is MultiUserSettingError { - return 'error' in setting; -} +export type MultiGetResponse = { items: ({ value: JsonValue } | null)[] }; diff --git a/plugins/user-settings/src/apis/StorageApi/UserSettingsStorage.test.ts b/plugins/user-settings/src/apis/StorageApi/UserSettingsStorage.test.ts index 8ec9439242..0dd8aa4c36 100644 --- a/plugins/user-settings/src/apis/StorageApi/UserSettingsStorage.test.ts +++ b/plugins/user-settings/src/apis/StorageApi/UserSettingsStorage.test.ts @@ -25,8 +25,6 @@ import { mockApis, registerMswTestHooks, } from '@backstage/test-utils'; -import { NotFoundError } from '@backstage/errors'; -import { parseDataLoaderKey } from '@backstage/plugin-user-settings-common'; import { createDeferred } from '@backstage/types'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; @@ -187,7 +185,7 @@ describe('Persistent Storage API', () => { return res(ctx.json(data)); }, ), - rest.get(`${mockBaseUrl}/multi`, async (_req, res, ctx) => { + rest.post(`${mockBaseUrl}/multiget`, async (_req, res, ctx) => { serverCall.resolve(); return res(ctx.json([])); }), @@ -234,7 +232,7 @@ describe('Persistent Storage API', () => { return res(ctx.status(204)); }, ), - rest.get(`${mockBaseUrl}/multi`, async (_req, res, ctx) => { + rest.post(`${mockBaseUrl}/multiget`, async (_req, res, ctx) => { serverCall.resolve(); return res(ctx.json([])); }), @@ -283,10 +281,9 @@ describe('Persistent Storage API', () => { return res(ctx.json({ value })); }, ), - rest.get(`${mockBaseUrl}/multi`, async (req, res, ctx) => { - const { bucket, key } = parseDataLoaderKey( - req.url.searchParams.get('items') as string, - ); + rest.post(`${mockBaseUrl}/multiget`, async (req, res, ctx) => { + const payload = await req.json(); + const { bucket, key } = payload.items[0]; expect(bucket).toEqual('clash.profile/something'); expect(key).toEqual('deep/test2'); @@ -333,12 +330,11 @@ describe('Persistent Storage API', () => { }); server.use( - rest.get(`${mockBaseUrl}/multi`, async (req, res, ctx) => { - const { bucket, key } = parseDataLoaderKey( - req.url.searchParams.get('items') as string, - ); - expect(bucket).toEqual('Test.Mock.Thing'); - expect(key).toEqual('key'); + rest.post(`${mockBaseUrl}/multiget`, async (req, res, ctx) => { + const payload = await req.json(); + expect(payload).toEqual({ + items: [{ bucket: 'Test.Mock.Thing', key: 'key' }], + }); return res(ctx.text('{ invalid: json string }')); }), ); @@ -369,8 +365,8 @@ describe('Persistent Storage API', () => { const data = { foo: 'bar', baz: [{ foo: 'bar' }] }; server.use( - rest.get(`${mockBaseUrl}/multi`, async (_req, res, ctx) => { - return res(ctx.json([{ bucket: 'freeze', key: 'key', value: data }])); + rest.post(`${mockBaseUrl}/multiget`, async (_req, res, ctx) => { + return res(ctx.json({ items: [{ value: data }] })); }), ); @@ -415,21 +411,21 @@ describe('Persistent Storage API', () => { let serverCalls = 0; server.use( - rest.get(`${mockBaseUrl}/multi`, async (req, res, ctx) => { + rest.post(`${mockBaseUrl}/multiget`, async (req, res, ctx) => { ++serverCalls; - const result = req.url.searchParams - .getAll('items') - .map(item => parseDataLoaderKey(item)) - .map(({ key }) => { - if (key === 'key1') { - return { bucket: 'multiget', key, value: data1 }; - } else if (key === 'key2') { - return { bucket: 'multiget', key, value: data2 }; - } - return { bucket: 'multiget', key, error: new NotFoundError() }; - }); + const payload = (await req.json()) as { + items: { bucket: string; key: string }[]; + }; + const result = payload.items.map(item => { + if (item.key === 'key1') { + return { value: data1 }; + } else if (item.key === 'key2') { + return { value: data2 }; + } + return null; + }); - return res(ctx.json(result)); + return res(ctx.json({ items: result })); }), ); diff --git a/plugins/user-settings/src/apis/StorageApi/UserSettingsStorage.ts b/plugins/user-settings/src/apis/StorageApi/UserSettingsStorage.ts index 141aa0aa1a..9a5a0f22ed 100644 --- a/plugins/user-settings/src/apis/StorageApi/UserSettingsStorage.ts +++ b/plugins/user-settings/src/apis/StorageApi/UserSettingsStorage.ts @@ -23,15 +23,12 @@ import { StorageApi, StorageValueSnapshot, } from '@backstage/core-plugin-api'; -import { deserializeError, ResponseError } from '@backstage/errors'; +import { ResponseError } from '@backstage/errors'; import { JsonValue, Observable } from '@backstage/types'; import { SignalApi, SignalSubscriber } from '@backstage/plugin-signals-react'; import ObservableImpl from 'zen-observable'; import { - isMultiUserSettingError, - MultiUserSetting, - parseDataLoaderKey, - stringifyDataLoaderKey, + MultiGetResponse, UserSettingsSignal, } from '@backstage/plugin-user-settings-common'; import DataLoader from 'dataloader'; @@ -47,6 +44,12 @@ const buckets = new Map(); const DATALOADER_CACHE_TTL_MS = 2 * 1000; // 2 seconds cache const DATALOADER_WINDOW_MS = 10; // 10 ms +type DataLoaderType = DataLoader< + { bucket: string; key: string }, + StorageValueSnapshot, + string +>; + /** * An implementation of the storage API, that uses the user-settings backend to * persist the data in the DB. @@ -70,7 +73,7 @@ export class UserSettingsStorage implements StorageApi { private readonly identityApi: IdentityApi; private readonly fallback: WebStorage; private readonly signalApi?: SignalApi; - private readonly userSettingsLoader: DataLoader; + private readonly userSettingsLoader: DataLoaderType; private constructor( namespace: string, @@ -80,7 +83,7 @@ export class UserSettingsStorage implements StorageApi { identityApi: IdentityApi, fallback: WebStorage, signalApi?: SignalApi, - userSettingsLoader?: DataLoader, + userSettingsLoader?: DataLoaderType, ) { this.namespace = namespace; this.fetchApi = fetchApi; @@ -92,24 +95,32 @@ export class UserSettingsStorage implements StorageApi { this.userSettingsLoader = userSettingsLoader ?? - new DataLoader( + new DataLoader( async bucketAndKeyList => this.getMulti(bucketAndKeyList), { name: 'UserSettingsStorage.userSettingsLoader', - cacheMap: new CacheMap>( - DATALOADER_CACHE_TTL_MS, - ), + cacheMap: new CacheMap< + string, + Promise> + >(DATALOADER_CACHE_TTL_MS), + cacheKeyFn: bucketAndKey => this.stringifyDataLoaderKey(bucketAndKey), maxBatchSize: 100, batchScheduleFn: cb => setTimeout(cb, DATALOADER_WINDOW_MS), }, ); } - private stringifyDataLoaderKey(key: string) { - return stringifyDataLoaderKey(this.namespace, key); + private stringifyDataLoaderKey({ + bucket, + key, + }: { + bucket: string; + key: string; + }) { + return `${encodeURIComponent(bucket)}/${encodeURIComponent(key)}`; } private clearCacheKey(key: string) { - this.userSettingsLoader.clear(this.stringifyDataLoaderKey(key)); + this.userSettingsLoader.clear({ bucket: this.namespace, key }); } static create(options: { @@ -197,11 +208,14 @@ export class UserSettingsStorage implements StorageApi { const { value } = await response.json(); - this.userSettingsLoader.prime(this.stringifyDataLoaderKey(key), { - key, - presence: 'present', - value, - }); + this.userSettingsLoader.prime( + { bucket: this.namespace, key }, + { + key, + presence: 'present', + value, + }, + ); this.notifyChanges({ key, value, presence: 'present' }); } @@ -219,7 +233,7 @@ export class UserSettingsStorage implements StorageApi { const updateSnapshot = () => { Promise.resolve() .then(() => - this.userSettingsLoader.load(this.stringifyDataLoaderKey(key)), + this.userSettingsLoader.load({ bucket: this.namespace, key }), ) .then(snapshot => subscriber.next(snapshot)) .catch(error => this.errorApi.post(error)); @@ -256,53 +270,47 @@ export class UserSettingsStorage implements StorageApi { } private async getMulti( - bucketAndKeyList: readonly string[], + bucketAndKeyList: readonly { bucket: string; key: string }[], ): Promise[]> { if (bucketAndKeyList.length === 0) return []; if (!(await this.isSignedIn())) { // This explicitly uses WebStorage, which we know is synchronous and doesn't return presence: unknown return bucketAndKeyList.map(bucketAndKey => - this.fallback.snapshot(parseDataLoaderKey(bucketAndKey).key), + this.fallback.snapshot(bucketAndKey.key), ); } - const baseUrl = await this.discoveryApi.getBaseUrl('user-settings'); - const url = new URL(`${baseUrl}/multi`); - for (const bucketAndKey of bucketAndKeyList) { - url.searchParams.append('items', bucketAndKey); - } - const response = await this.fetchApi.fetch(url); - - if (response.status === 404) { - return bucketAndKeyList.map(bucketAndKey => ({ - key: parseDataLoaderKey(bucketAndKey).key, - presence: 'absent', - })); - } - - if (!response.ok) { - throw await ResponseError.fromResponse(response); - } - try { - const values = await response.json(); + const baseUrl = await this.discoveryApi.getBaseUrl('user-settings'); + const response = await this.fetchApi.fetch(`${baseUrl}/multiget`, { + method: 'POST', + headers: JSON_HEADERS, + body: JSON.stringify({ items: bucketAndKeyList }), + }); - return (values as MultiUserSetting[]).map( - (setting): StorageValueSnapshot => { - if (isMultiUserSettingError(setting)) { - if (setting.error.name === 'NotFoundError') { - return { - key: setting.key, - presence: 'absent', - }; - } - throw deserializeError(setting.error); + if (response.status === 404) { + return bucketAndKeyList.map(bucketAndKey => ({ + key: bucketAndKey.key, + presence: 'absent', + })); + } + + if (!response.ok) { + throw await ResponseError.fromResponse(response); + } + + const { items: values } = (await response.json()) as MultiGetResponse; + + return bucketAndKeyList.map( + ({ key }, i): StorageValueSnapshot => { + if (!values[i]) { + return { key, presence: 'absent' }; } return { - key: setting.key, + key, presence: 'present', - value: JSON.parse(JSON.stringify(setting.value), (_key, val) => { + value: JSON.parse(JSON.stringify(values[i].value), (_key, val) => { if (typeof val === 'object' && val !== null) { Object.freeze(val); } @@ -311,10 +319,10 @@ export class UserSettingsStorage implements StorageApi { }; }, ); - } catch { - // If the value is not valid JSON, we return an unknown presence. This should never happen + } catch (e) { + this.errorApi.post(new Error(`Failed to fetch user settings, ${e}`)); return bucketAndKeyList.map(bucketAndKey => ({ - key: parseDataLoaderKey(bucketAndKey).key, + key: bucketAndKey.key, presence: 'absent', })); }