From 108cdc39124bdb8e011bc614cd513cff695e88a0 Mon Sep 17 00:00:00 2001 From: Dominik Schwank Date: Fri, 26 Aug 2022 10:46:16 +0200 Subject: [PATCH 01/14] feat: add new user settings backend Adding a new user settings backend and PersistentStorage to store user related settings in the database. Signed-off-by: Dominik Schwank --- .changeset/healthy-oranges-fly.md | 8 + .github/CODEOWNERS | 1 + packages/core-app-api/api-report.md | 29 ++ packages/core-app-api/package.json | 2 + .../StorageApi/PersistentStorage.test.ts | 324 +++++++++++++++ .../StorageApi/PersistentStorage.ts | 193 +++++++++ .../apis/implementations/StorageApi/index.ts | 1 + plugins/user-settings-backend/.eslintrc.js | 1 + plugins/user-settings-backend/README.md | 104 +++++ plugins/user-settings-backend/api-report.md | 177 +++++++++ .../migrations/20220824183000_init.js | 41 ++ plugins/user-settings-backend/package.json | 52 +++ .../DatabaseUserSettingsStore.test.ts | 373 ++++++++++++++++++ .../src/database/DatabaseUserSettingsStore.ts | 143 +++++++ .../src/database/UserSettingsStore.ts | 65 +++ .../src/database/index.ts | 20 + plugins/user-settings-backend/src/index.ts | 17 + plugins/user-settings-backend/src/run.ts | 33 ++ .../src/service/index.ts | 20 + .../src/service/router.test.ts | 319 +++++++++++++++ .../src/service/router.ts | 168 ++++++++ .../src/service/standaloneServer.ts | 82 ++++ .../user-settings-backend/src/setupTests.ts | 16 + yarn.lock | 29 +- 24 files changed, 2215 insertions(+), 3 deletions(-) create mode 100644 .changeset/healthy-oranges-fly.md create mode 100644 packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.test.ts create mode 100644 packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.ts create mode 100644 plugins/user-settings-backend/.eslintrc.js create mode 100644 plugins/user-settings-backend/README.md create mode 100644 plugins/user-settings-backend/api-report.md create mode 100644 plugins/user-settings-backend/migrations/20220824183000_init.js create mode 100644 plugins/user-settings-backend/package.json create mode 100644 plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.test.ts create mode 100644 plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.ts create mode 100644 plugins/user-settings-backend/src/database/UserSettingsStore.ts create mode 100644 plugins/user-settings-backend/src/database/index.ts create mode 100644 plugins/user-settings-backend/src/index.ts create mode 100644 plugins/user-settings-backend/src/run.ts create mode 100644 plugins/user-settings-backend/src/service/index.ts create mode 100644 plugins/user-settings-backend/src/service/router.test.ts create mode 100644 plugins/user-settings-backend/src/service/router.ts create mode 100644 plugins/user-settings-backend/src/service/standaloneServer.ts create mode 100644 plugins/user-settings-backend/src/setupTests.ts diff --git a/.changeset/healthy-oranges-fly.md b/.changeset/healthy-oranges-fly.md new file mode 100644 index 0000000000..da6530b903 --- /dev/null +++ b/.changeset/healthy-oranges-fly.md @@ -0,0 +1,8 @@ +--- +'@backstage/core-app-api': minor +'@backstage/plugin-user-settings-backend': minor +--- + +Add new plugin @backstage/user-settings-backend to store user related settings +in the database. Additionally adding a PersistentStorage implementation to use +easily use the new plugin as drop-in replacement for the WebStorage. diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index f00771d29d..599a83cedf 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -56,6 +56,7 @@ yarn.lock @backstage/reviewers @backst /plugins/stack-overflow-backend @backstage/reviewers @backstage/techdocs-core /plugins/techdocs @backstage/reviewers @backstage/techdocs-core /plugins/techdocs-* @backstage/reviewers @backstage/techdocs-core +/plugins/user-settings-backend @backstage/reviewers @backstage/sda-se-reviewers /tech-insights-backend @backstage/reviewers @xantier @iain-b /tech-insights-backend-module-jsonfc @backstage/reviewers @xantier @iain-b /tech-insights-tech-insights-common @backstage/reviewers @xantier @iain-b diff --git a/packages/core-app-api/api-report.md b/packages/core-app-api/api-report.md index f2bc4b6665..de3afdcbd4 100644 --- a/packages/core-app-api/api-report.md +++ b/packages/core-app-api/api-report.md @@ -506,6 +506,35 @@ export type OneLoginAuthCreateOptions = { provider?: AuthProviderInfo; }; +// @public +export class PersistentStorage implements StorageApi { + constructor( + namespace: string, + fetchApi: FetchApi, + discoveryApi: DiscoveryApi, + errorApi: ErrorApi, + ); + // (undocumented) + static create(options: { + fetchApi: FetchApi; + discoveryApi: DiscoveryApi; + errorApi: ErrorApi; + namespace?: string; + }): PersistentStorage; + // (undocumented) + forBucket(name: string): StorageApi; + // (undocumented) + observe$( + key: string, + ): Observable>; + // (undocumented) + remove(key: string): Promise; + // (undocumented) + set(key: string, data: T): Promise; + // (undocumented) + snapshot(key: string): StorageValueSnapshot; +} + // @public export class SamlAuth implements ProfileInfoApi, BackstageIdentityApi, SessionApi diff --git a/packages/core-app-api/package.json b/packages/core-app-api/package.json index 8532997477..632c429361 100644 --- a/packages/core-app-api/package.json +++ b/packages/core-app-api/package.json @@ -34,6 +34,8 @@ "dependencies": { "@backstage/config": "^1.0.2-next.0", "@backstage/core-plugin-api": "^1.0.6-next.3", + "@backstage/errors": "^1.1.0", + "@backstage/plugin-permission-common": "^0.6.4-next.0", "@backstage/types": "^1.0.0", "@backstage/version-bridge": "^1.0.1", "@types/prop-types": "^15.7.3", diff --git a/packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.test.ts b/packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.test.ts new file mode 100644 index 0000000000..6cea8dde5b --- /dev/null +++ b/packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.test.ts @@ -0,0 +1,324 @@ +/* + * Copyright 2022 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. + */ + +import { PersistentStorage } from './PersistentStorage'; +import { ErrorApi, FetchApi, StorageApi } from '@backstage/core-plugin-api'; +import { DiscoveryApi } from '@backstage/plugin-permission-common'; +import { MockFetchApi, setupRequestMockHandlers } from '@backstage/test-utils'; + +import { rest } from 'msw'; +import { setupServer } from 'msw/node'; + +const server = setupServer(); + +describe('Persistent Storage API', () => { + setupRequestMockHandlers(server); + const mockBaseUrl = 'http://backstage:9191/api'; + const mockErrorApi = { post: jest.fn(), error$: jest.fn() }; + const mockDiscoveryApi = { getBaseUrl: async () => mockBaseUrl }; + + const createPersistentStorage = ( + args?: Partial<{ + fetchApi: FetchApi; + discoveryApi: DiscoveryApi; + errorApi: ErrorApi; + namespace?: string; + }>, + ): StorageApi => { + return PersistentStorage.create({ + errorApi: mockErrorApi, + fetchApi: new MockFetchApi(), + discoveryApi: mockDiscoveryApi, + ...args, + }); + }; + + beforeEach(() => { + server.use( + rest.get(`${mockBaseUrl}/`, async (_req, res, ctx) => { + return res(ctx.json([])); + }), + ); + }); + + afterEach(() => jest.resetAllMocks()); + + it('should return undefined for values which are unset', async () => { + const storage = createPersistentStorage(); + + server.use( + rest.get(`${mockBaseUrl}/:bucket/:key`, async (_req, res, ctx) => { + return res(ctx.json({ value: 'a' })); + }), + ); + + expect(storage.snapshot('myfakekey').value).toBeUndefined(); + expect(storage.snapshot('myfakekey')).toEqual({ + key: 'myfakekey', + presence: 'unknown', + value: undefined, + }); + }); + + it('should allow setting of a simple data structure', async () => { + const storage = createPersistentStorage(); + const dummyValue = 'a'; + + server.use( + rest.put(`${mockBaseUrl}/:bucket/:key`, async (req, res, ctx) => { + const body = await req.json(); + const data = { value: JSON.stringify(dummyValue) }; + expect(body).toEqual(data); + + return res(ctx.json(data)); + }), + ); + + await storage.set('my-key', dummyValue); + }); + + it('should allow setting of a complex data structure', async () => { + const storage = createPersistentStorage(); + const dummyValue = { + some: 'nice data', + with: { nested: 'values', nice: true }, + }; + + server.use( + rest.put(`${mockBaseUrl}/:bucket/:key`, async (req, res, ctx) => { + const body = await req.json(); + const data = { value: JSON.stringify(dummyValue) }; + expect(body).toEqual(data); + + return res(ctx.json(data)); + }), + ); + + await storage.set('my-key', dummyValue); + }); + + it('should subscribe to key changes when setting a new value', async () => { + const storage = createPersistentStorage(); + + const wrongKeyNextHandler = jest.fn(); + const selectedKeyNextHandler = jest.fn(); + const mockData = { hello: 'im a great new value' }; + + server.use( + rest.put(`${mockBaseUrl}/:bucket/:key`, async (req, res, ctx) => { + const body = await req.json(); + const data = { value: JSON.stringify(mockData) }; + expect(body).toEqual(data); + + return res(ctx.json(data)); + }), + ); + + await new Promise(resolve => { + storage.observe$('correctKey').subscribe({ + next: snapshot => { + selectedKeyNextHandler(snapshot); + if (snapshot.presence === 'present') { + resolve(); + } + }, + }); + + storage.observe$('wrongKey').subscribe({ next: wrongKeyNextHandler }); + + storage.set('correctKey', mockData); + }); + + expect(wrongKeyNextHandler).toHaveBeenCalledTimes(0); + expect(selectedKeyNextHandler).toHaveBeenCalledTimes(1); + expect(selectedKeyNextHandler).toHaveBeenCalledWith({ + key: 'correctKey', + presence: 'present', + value: mockData, + }); + }); + + it('should subscribe to key changes when deleting a value', async () => { + const storage = createPersistentStorage(); + + const wrongKeyNextHandler = jest.fn(); + const selectedKeyNextHandler = jest.fn(); + + server.use( + rest.delete(`${mockBaseUrl}/:bucket/:key`, async (_req, res, ctx) => { + return res(ctx.status(204)); + }), + ); + + await new Promise(resolve => { + storage.observe$('correctKey').subscribe({ + next: snapshot => { + selectedKeyNextHandler(snapshot); + if (snapshot.presence === 'absent') { + resolve(); + } + }, + }); + + storage.observe$('wrongKey').subscribe({ next: wrongKeyNextHandler }); + + storage.remove('correctKey'); + }); + + expect(wrongKeyNextHandler).toHaveBeenCalledTimes(0); + expect(selectedKeyNextHandler).toHaveBeenCalledTimes(1); + expect(selectedKeyNextHandler).toHaveBeenCalledWith({ + key: 'correctKey', + presence: 'absent', + value: undefined, + }); + }); + + it('should not clash with other namespaces when creating buckets', async () => { + const rootStorage = createPersistentStorage(); + const selectedKeyNextHandler = jest.fn(); + + server.use( + rest.put(`${mockBaseUrl}/:bucket/:key`, async (req, res, ctx) => { + const { bucket, key } = req.params; + const { value } = await req.json(); + + expect(bucket).toEqual('default.profile.something.deep'); + expect(key).toEqual('test2'); + + return res(ctx.json({ value })); + }), + rest.get(`${mockBaseUrl}/:bucket/:key`, async (req, res, ctx) => { + const { bucket, key } = req.params; + + expect(bucket).toEqual('default.profile/something'); + expect(key).toEqual('deep/test2'); + + return res(ctx.status(404)); + }), + ); + + // when getting key test2 it will translate to default.profile.something.deep/test2 + const firstStorage = rootStorage + .forBucket('profile') + .forBucket('something') + .forBucket('deep'); + // when getting key deep/test2 it will translate to default.profile.something/deep/test2 + const secondStorage = rootStorage.forBucket('profile/something'); + + await firstStorage.set('test2', { error: true }); + + await new Promise(resolve => { + secondStorage.observe$('deep/test2').subscribe({ + next: snapshot => { + selectedKeyNextHandler(snapshot); + if (snapshot.presence === 'absent') { + resolve(); + } + }, + }); + + secondStorage.snapshot('deep/test2'); + }); + + expect(selectedKeyNextHandler).toHaveBeenCalledWith({ + key: 'deep/test2', + presence: 'absent', + value: undefined, + }); + expect(mockErrorApi.post).not.toHaveBeenCalled(); + }); + + it('should call the error api when the json can not be parsed in local storage', async () => { + const selectedKeyNextHandler = jest.fn(); + const rootStorage = createPersistentStorage({ + namespace: 'Test.Mock.Thing', + }); + + server.use( + rest.get(`${mockBaseUrl}/:bucket/:key`, async (req, res, ctx) => { + const { bucket, key } = req.params; + + expect(bucket).toEqual('Test.Mock.Thing'); + expect(key).toEqual('key'); + + return res(ctx.text('{ invalid: json string }')); + }), + ); + + await new Promise(resolve => { + rootStorage.observe$('key').subscribe({ + next: snapshot => { + selectedKeyNextHandler(snapshot); + if (snapshot.presence === 'absent') { + resolve(); + } + }, + }); + + rootStorage.snapshot('key'); + }); + + expect(selectedKeyNextHandler).toHaveBeenCalledWith({ + key: 'key', + presence: 'absent', + value: undefined, + }); + expect(mockErrorApi.post).toHaveBeenCalledWith(expect.any(Error)); + }); + + it('should freeze the snapshot value', async () => { + const storage = createPersistentStorage(); + const selectedKeyNextHandler = jest.fn(); + const data = { foo: 'bar', baz: [{ foo: 'bar' }] }; + + server.use( + rest.get(`${mockBaseUrl}/:bucket/:key`, async (_req, res, ctx) => { + return res(ctx.text(JSON.stringify({ value: JSON.stringify(data) }))); + }), + ); + + await new Promise(resolve => { + storage.observe$('key').subscribe({ + next: snapshot => { + selectedKeyNextHandler(snapshot); + if (snapshot.presence === 'present') { + resolve(); + } + }, + }); + + storage.snapshot('key'); + }); + + expect(selectedKeyNextHandler).toHaveBeenCalledWith({ + key: 'key', + presence: 'present', + value: { baz: [{ foo: 'bar' }], foo: 'bar' }, + }); + + const snapshot = selectedKeyNextHandler.mock.calls[0][0]; + expect(() => { + snapshot.value.foo = 'buzz'; + }).toThrow(/Cannot assign to read only property/); + expect(() => { + snapshot.value.baz[0].foo = 'buzz'; + }).toThrow(/Cannot assign to read only property/); + expect(() => { + snapshot.value.baz.push({ foo: 'buzz' }); + }).toThrow(/Cannot add property 1, object is not extensible/); + }); +}); diff --git a/packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.ts b/packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.ts new file mode 100644 index 0000000000..4ebf635cd9 --- /dev/null +++ b/packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.ts @@ -0,0 +1,193 @@ +/* + * Copyright 2022 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. + */ + +import { + DiscoveryApi, + ErrorApi, + FetchApi, + StorageApi, + StorageValueSnapshot, +} from '@backstage/core-plugin-api'; +import { NotFoundError } from '@backstage/errors'; +import { JsonValue, Observable } from '@backstage/types'; +import ObservableImpl from 'zen-observable'; + +const buckets = new Map(); + +/** + * An implementation of the storage API, that uses the user-settings backend to + * persist the data in the DB. + * + * @public + */ +export class PersistentStorage implements StorageApi { + private subscribers = new Set< + ZenObservable.SubscriptionObserver> + >(); + + private readonly observable = new ObservableImpl< + StorageValueSnapshot + >(subscriber => { + this.subscribers.add(subscriber); + return () => { + this.subscribers.delete(subscriber); + }; + }); + + constructor( + private readonly namespace: string, + private readonly fetchApi: FetchApi, + private readonly discoveryApi: DiscoveryApi, + private readonly errorApi: ErrorApi, + ) {} + + static create(options: { + fetchApi: FetchApi; + discoveryApi: DiscoveryApi; + errorApi: ErrorApi; + namespace?: string; + }): PersistentStorage { + return new PersistentStorage( + options.namespace ?? 'default', + options.fetchApi, + options.discoveryApi, + options.errorApi, + ); + } + + forBucket(name: string): StorageApi { + // use dot instead of slash separator to have nicer URLs + const bucketPath = `${this.namespace}.${name}`; + + if (!buckets.has(bucketPath)) { + buckets.set( + bucketPath, + new PersistentStorage( + bucketPath, + this.fetchApi, + this.discoveryApi, + this.errorApi, + ), + ); + } + + return buckets.get(bucketPath)!; + } + + async remove(key: string): Promise { + const fetchUrl = await this.getFetchUrl(key); + + await this.fetchApi.fetch(fetchUrl, { + method: 'DELETE', + }); + + this.notifyChanges({ + key, + presence: 'absent', + }); + } + + async set(key: string, data: T): Promise { + const fetchUrl = await this.getFetchUrl(key); + const body = JSON.stringify({ value: JSON.stringify(data) }); + + const response = await this.fetchApi.fetch(fetchUrl, { + method: 'PUT', + body, + }); + + const { value } = await response.json(); + + this.notifyChanges({ + key, + value: JSON.parse(value), + presence: 'present', + }); + } + + observe$( + key: string, + ): Observable> { + return this.observable.filter(({ key: messageKey }) => messageKey === key); + } + + snapshot(key: string): StorageValueSnapshot { + // trigger a reload + this.get(key).then(snapshot => this.notifyChanges(snapshot)); + + return { + key, + presence: 'unknown', + }; + } + + private async get( + key: string, + ): Promise> { + try { + const fetchUrl = await this.getFetchUrl(key); + const response = await this.fetchApi.fetch(fetchUrl); + + if (response.status === 404) { + throw new NotFoundError( + `Setting '${key}' is not set in bucket '${this.namespace}'`, + ); + } else if (!response.ok) { + throw new Error( + `Unable to fetch '${key}' from bucket '${this.namespace}'`, + ); + } + + const { value: rawValue } = await response.json(); + const value = JSON.parse(rawValue, (_key, val) => { + if (typeof val === 'object' && val !== null) { + Object.freeze(val); + } + return val; + }); + + return { + key, + presence: 'present', + value, + }; + } catch (error) { + // NotFoundError shouldn't be recorded + if (error && !(error instanceof NotFoundError)) { + this.errorApi.post(error); + } + + return { + key, + presence: 'absent', + value: undefined, + }; + } + } + + private async getFetchUrl(key: string) { + const baseUrl = await this.discoveryApi.getBaseUrl('user-settings'); + const encodedNamespace = encodeURIComponent(this.namespace); + const encodedKey = encodeURIComponent(key); + return `${baseUrl}/${encodedNamespace}/${encodedKey}`; + } + + private async notifyChanges(snapshot: StorageValueSnapshot) { + for (const subscription of this.subscribers) { + subscription.next(snapshot); + } + } +} diff --git a/packages/core-app-api/src/apis/implementations/StorageApi/index.ts b/packages/core-app-api/src/apis/implementations/StorageApi/index.ts index 2f941381fd..70a8c9cbe1 100644 --- a/packages/core-app-api/src/apis/implementations/StorageApi/index.ts +++ b/packages/core-app-api/src/apis/implementations/StorageApi/index.ts @@ -15,3 +15,4 @@ */ export { WebStorage } from './WebStorage'; +export { PersistentStorage } from './PersistentStorage'; diff --git a/plugins/user-settings-backend/.eslintrc.js b/plugins/user-settings-backend/.eslintrc.js new file mode 100644 index 0000000000..e2a53a6ad2 --- /dev/null +++ b/plugins/user-settings-backend/.eslintrc.js @@ -0,0 +1 @@ +module.exports = require('@backstage/cli/config/eslint-factory')(__dirname); diff --git a/plugins/user-settings-backend/README.md b/plugins/user-settings-backend/README.md new file mode 100644 index 0000000000..05a8c15c6d --- /dev/null +++ b/plugins/user-settings-backend/README.md @@ -0,0 +1,104 @@ +# User settings backend + +This backend allows to save user specific settings. All requests need to be +authorized, as the user identifier (`userEntityRef`) is resolved using the +authorization token. + +## Setup backend + +1. Install the backend plugin: + +```bash +# From your Backstage root directory +yarn --cwd packages/backend add @backstage/plugin-user-settings-backend +``` + +1. Configure the routes by adding a new `userSettings.ts` file in + `packages/backend/src/plugins/`: + +```ts +// packages/backend/src/plugins/userSettings.ts +import { IdentityClient } from '@backstage/plugin-auth-node'; +import { + createRouter, + createUserSettingsStore, +} from '@backstage/plugin-user-settings-backend'; +import { Router } from 'express'; + +import { PluginEnvironment } from '../types'; + +export default async function createPlugin({ + database, + discovery, +}: PluginEnvironment): Promise { + const identity = IdentityClient.create({ + discovery, + issuer: await discovery.getExternalBaseUrl('auth'), + }); + + return await createRouter({ + userSettingsStore: await createUserSettingsStore(database), + identity, + }); +} +``` + +3. Add the new routes to your backend by modifying the + `packages/backend/src/index.ts`: + +```diff +// packages/backend/src/index.ts ++ import userSettings from './plugins/userSettings'; +async function main() { ++ const userSettingsEnv = useHotMemoize(module, () => ++ createEnv('userSettings'), ++ ); + const apiRouter = Router(); ++ apiRouter.use('/user-settings', await userSettings(userSettingsEnv)); +} +``` + +## Setup app + +To make use of the user settings backend, replace the `WebStorage` with the +`PersistentStorage` by using the `storageApiRef`. + +```diff +// packages/app/src/apis.ts +import { + AnyApiFactory, + createApiFactory, ++ discoveryApiRef, ++ fetchApiRef + errorApiRef, ++ storageApiRef, +} from '@backstage/core-plugin-api'; ++ import { PersistentStorage } from '@backstage/core-app-api'; + +export const apis: AnyApiFactory[] = [ ++ createApiFactory({ ++ api: storageApiRef, ++ deps: { ++ discoveryApi: discoveryApiRef, ++ errorApi: errorApiRef, ++ fetchApi: fetchApiRef, ++ }, ++ factory: ({ discoveryApi, errorApi, fetchApi }) => ++ PersistentStorage.create({ discoveryApi, errorApi, fetchApi }), ++ }), +]; +``` + +## Development + +Use `yarn start` to start the local dev environment. To simplify the access to +the API, the token will automatically be set to `user:default/john_doe`. + +You can change the user by setting a custom `Authorization` header. To keep +things simple, the raw `Bearer` value will directly be used as user. + +_Example:_ + +```bash +curl --request GET 'http://localhost:7007/user-settings' --header 'Authorization: Bearer user:default/custom-user' +``` diff --git a/plugins/user-settings-backend/api-report.md b/plugins/user-settings-backend/api-report.md new file mode 100644 index 0000000000..b3b40970c9 --- /dev/null +++ b/plugins/user-settings-backend/api-report.md @@ -0,0 +1,177 @@ +## API Report File for "@backstage/plugin-user-settings-backend" + +> Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/). + +```ts +import express from 'express'; +import { IdentityClient } from '@backstage/plugin-auth-node'; +import { Knex } from 'knex'; +import { PluginDatabaseManager } from '@backstage/backend-common'; + +// @public +export function createRouter( + options: RouterOptions, +): Promise; + +// @public (undocumented) +export function createUserSettingsStore( + database: PluginDatabaseManager, +): Promise; + +// @public +export class DatabaseUserSettingsStore + implements UserSettingsStore +{ + // (undocumented) + static create(knex: Knex): Promise; + // (undocumented) + delete( + tx: Knex.Transaction, + opts: { + userEntityRef: string; + bucket: string; + key: string; + }, + ): Promise; + // (undocumented) + deleteAll( + tx: Knex.Transaction, + opts: { + userEntityRef: string; + }, + ): Promise; + // (undocumented) + deleteBucket( + tx: Knex.Transaction, + opts: { + userEntityRef: string; + bucket: string; + }, + ): Promise; + // (undocumented) + get( + tx: Knex.Transaction, + opts: { + userEntityRef: string; + bucket: string; + key: string; + }, + ): Promise; + // (undocumented) + getAll( + tx: Knex.Transaction, + opts: { + userEntityRef: string; + }, + ): Promise[]>; + // (undocumented) + getBucket( + tx: Knex.Transaction, + opts: { + userEntityRef: string; + bucket: string; + }, + ): Promise; + // (undocumented) + set( + tx: Knex.Transaction, + opts: { + userEntityRef: string; + bucket: string; + key: string; + value: string; + }, + ): Promise; + // (undocumented) + transaction(fn: (tx: Knex.Transaction) => Promise): Promise; +} + +// @public (undocumented) +export type RawDbUserSettingsRow = { + user_entity_ref: string; + bucket: string; + key: string; + value: string; +}; + +// @public (undocumented) +export interface RouterOptions { + // (undocumented) + identity: IdentityClient; + // (undocumented) + userSettingsStore: UserSettingsStore; +} + +// @public (undocumented) +export type UserSetting = { + bucket: string; + key: string; + value: string; +}; + +// @public +export interface UserSettingsStore { + // (undocumented) + delete( + tx: Transaction, + opts: { + userEntityRef: string; + bucket: string; + key: string; + }, + ): Promise; + // (undocumented) + deleteAll( + tx: Transaction, + opts: { + userEntityRef: string; + }, + ): Promise; + // (undocumented) + deleteBucket( + tx: Transaction, + opts: { + userEntityRef: string; + bucket: string; + }, + ): Promise; + // (undocumented) + get( + tx: Transaction, + opts: { + userEntityRef: string; + bucket: string; + key: string; + }, + ): Promise; + // (undocumented) + getAll( + tx: Transaction, + opts: { + userEntityRef: string; + }, + ): Promise; + // (undocumented) + getBucket( + tx: Transaction, + opts: { + userEntityRef: string; + bucket: string; + }, + ): Promise; + // (undocumented) + set( + tx: Transaction, + opts: { + userEntityRef: string; + bucket: string; + key: string; + value: string; + }, + ): Promise; + // (undocumented) + transaction(fn: (tx: Transaction) => Promise): Promise; +} + +// (No @packageDocumentation comment for this package) +``` diff --git a/plugins/user-settings-backend/migrations/20220824183000_init.js b/plugins/user-settings-backend/migrations/20220824183000_init.js new file mode 100644 index 0000000000..688d8495b5 --- /dev/null +++ b/plugins/user-settings-backend/migrations/20220824183000_init.js @@ -0,0 +1,41 @@ +/* + * Copyright 2022 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. + */ + +/** + * @param {import('knex').Knex} knex + */ +exports.up = async function up(knex) { + await knex.schema.createTable('user_settings', table => { + table.comment('The table of user related settings'); + + table + .text('user_entity_ref') + .notNullable() + .comment('The entityRef of the user'); + table.text('bucket').notNullable().comment('Name of the bucket'); + table.text('key').notNullable().comment('Key of a bucket value'); + table.text('value').notNullable().comment('The value'); + + table.primary(['user_entity_ref', 'bucket', 'key']); + }); +}; + +/** + * @param {import('knex').Knex} knex + */ +exports.down = async function down(knex) { + await knex.schema.dropTable('user_settings'); +}; diff --git a/plugins/user-settings-backend/package.json b/plugins/user-settings-backend/package.json new file mode 100644 index 0000000000..f20180cac3 --- /dev/null +++ b/plugins/user-settings-backend/package.json @@ -0,0 +1,52 @@ +{ + "name": "@backstage/plugin-user-settings-backend", + "description": "The Backstage backend plugin to manage user settings", + "version": "0.1.0", + "main": "src/index.ts", + "types": "src/index.ts", + "license": "Apache-2.0", + "backstage": { + "role": "backend-plugin" + }, + "publishConfig": { + "access": "public", + "main": "dist/index.cjs.js", + "types": "dist/index.d.ts" + }, + "repository": { + "type": "git", + "url": "https://github.com/backstage/backstage", + "directory": "plugins/user-settings-backend" + }, + "scripts": { + "start": "backstage-cli package start", + "build": "backstage-cli package build", + "lint": "backstage-cli package lint", + "test": "backstage-cli package test", + "prepack": "backstage-cli package prepack", + "postpack": "backstage-cli package postpack", + "clean": "backstage-cli package clean" + }, + "dependencies": { + "@backstage/backend-common": "^0.15.1-next.1", + "@backstage/catalog-model": "^1.1.0", + "@backstage/errors": "^1.1.0", + "@backstage/plugin-auth-node": "^0.2.5-next.1", + "@types/express": "^4.17.6", + "express": "^4.17.1", + "express-promise-router": "^4.1.0", + "knex": "^2.0.0", + "winston": "^3.2.1", + "yn": "^4.0.0" + }, + "devDependencies": { + "@backstage/backend-test-utils": "^0.1.28-next.1", + "@backstage/cli": "^0.19.0-next.1", + "@types/supertest": "^2.0.8", + "supertest": "^6.1.3" + }, + "files": [ + "dist", + "migrations" + ] +} diff --git a/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.test.ts b/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.test.ts new file mode 100644 index 0000000000..717707f034 --- /dev/null +++ b/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.test.ts @@ -0,0 +1,373 @@ +/* + * Copyright 2022 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. + */ +import { TestDatabaseId, TestDatabases } from '@backstage/backend-test-utils'; +import { Knex } from 'knex'; + +import { + DatabaseUserSettingsStore, + RawDbUserSettingsRow, +} from './DatabaseUserSettingsStore'; + +jest.setTimeout(60_000); + +const databases = TestDatabases.create({ + ids: ['POSTGRES_13', 'SQLITE_3'], +}); + +async function createStore(databaseId: TestDatabaseId) { + const knex = await databases.init(databaseId); + return { + knex, + storage: await DatabaseUserSettingsStore.create(knex), + }; +} + +describe.each(databases.eachSupportedId())( + 'DatabaseUserSettingsStore (%s)', + databaseId => { + let storage: DatabaseUserSettingsStore; + let knex: Knex; + + beforeAll(async () => { + ({ storage, knex } = await createStore(databaseId)); + }); + + afterEach(async () => { + jest.resetAllMocks(); + + await knex('user_settings').del(); + }); + + const insert = (data: RawDbUserSettingsRow[]) => + knex('user_settings').insert(data); + const query = () => + knex('user_settings') + .orderBy('user_entity_ref') + .select(); + + describe('getAll', () => { + it('should return empty user settings', async () => { + expect( + await storage.transaction(tx => + storage.getAll(tx, { userEntityRef: 'user-1' }), + ), + ).toEqual([]); + }); + + it('should return all user settings', async () => { + await insert([ + { + user_entity_ref: 'user-1', + bucket: 'bucket-a', + key: 'key-a', + value: 'value-a', + }, + { + user_entity_ref: 'user-1', + bucket: 'bucket-a', + key: 'key-b', + value: 'value-b', + }, + { + user_entity_ref: 'user-1', + bucket: 'bucket-c', + key: 'key-c', + value: 'value-c', + }, + ]); + + expect( + await storage.transaction(tx => + storage.getAll(tx, { userEntityRef: 'user-1' }), + ), + ).toEqual([ + { bucket: 'bucket-a', key: 'key-a', value: 'value-a' }, + { bucket: 'bucket-a', key: 'key-b', value: 'value-b' }, + { bucket: 'bucket-c', key: 'key-c', value: 'value-c' }, + ]); + }); + }); + + describe('deleteAll', () => { + it('should delete all user settings', async () => { + await insert([ + { + user_entity_ref: 'user-1', + bucket: 'bucket-a', + key: 'key-a', + value: 'value-a', + }, + { + user_entity_ref: 'user-1', + bucket: 'bucket-c', + key: 'key-c', + value: 'value-c', + }, + { + user_entity_ref: 'user-2', + bucket: 'bucket-c', + key: 'key-c', + value: 'value-c', + }, + ]); + + await storage.transaction(tx => + storage.deleteAll(tx, { userEntityRef: 'user-1' }), + ); + + expect(await query()).toEqual([ + { + user_entity_ref: 'user-2', + bucket: 'bucket-c', + key: 'key-c', + value: 'value-c', + }, + ]); + }); + }); + + describe('getBucket', () => { + it('should return an empty bucket', async () => { + expect( + await storage.transaction(tx => + storage.getBucket(tx, { + userEntityRef: 'user-1', + bucket: 'bucket-c', + }), + ), + ).toEqual([]); + }); + + it('should return the settings of the bucket', async () => { + await insert([ + { + user_entity_ref: 'user-1', + bucket: 'bucket-a', + key: 'key-a', + value: 'value-a', + }, + { + user_entity_ref: 'user-1', + bucket: 'bucket-c', + key: 'key-c', + value: 'value-c', + }, + { + user_entity_ref: 'user-2', + bucket: 'bucket-c', + key: 'key-c', + value: 'value-c', + }, + ]); + + expect( + await storage.transaction(tx => + storage.getBucket(tx, { + userEntityRef: 'user-1', + bucket: 'bucket-a', + }), + ), + ).toEqual([{ bucket: 'bucket-a', key: 'key-a', value: 'value-a' }]); + }); + }); + + describe('deleteBucket', () => { + it('should delete a bucket', async () => { + await insert([ + { + user_entity_ref: 'user-1', + bucket: 'bucket-a', + key: 'key-a', + value: 'value-a', + }, + { + user_entity_ref: 'user-1', + bucket: 'bucket-c', + key: 'key-c', + value: 'value-c', + }, + { + user_entity_ref: 'user-2', + bucket: 'bucket-c', + key: 'key-c', + value: 'value-c', + }, + ]); + + await storage.transaction(tx => + storage.deleteBucket(tx, { + userEntityRef: 'user-1', + bucket: 'bucket-a', + }), + ); + + expect(await query()).toEqual([ + { + user_entity_ref: 'user-1', + bucket: 'bucket-c', + key: 'key-c', + value: 'value-c', + }, + { + user_entity_ref: 'user-2', + bucket: 'bucket-c', + key: 'key-c', + value: 'value-c', + }, + ]); + }); + }); + + describe('get', () => { + it('should throw an error', async () => { + await expect(() => + storage.transaction(tx => + storage.get(tx, { + userEntityRef: 'user-1', + bucket: 'bucket-c', + key: 'key-c', + }), + ), + ).rejects.toThrow(`Unable to find 'key-c' in bucket 'bucket-c'`); + }); + + it('should return the setting', async () => { + await insert([ + { + user_entity_ref: 'user-1', + bucket: 'bucket-a', + key: 'key-a', + value: 'value-a', + }, + { + user_entity_ref: 'user-2', + bucket: 'bucket-c', + key: 'key-c', + value: 'value-c', + }, + ]); + + expect( + await storage.transaction(tx => + storage.get(tx, { + userEntityRef: 'user-1', + bucket: 'bucket-a', + key: 'key-a', + }), + ), + ).toEqual({ bucket: 'bucket-a', key: 'key-a', value: 'value-a' }); + }); + }); + + describe('set', () => { + it('should insert a new setting', async () => { + await storage.transaction(tx => + storage.set(tx, { + userEntityRef: 'user-1', + bucket: 'bucket-a', + key: 'key-a', + value: 'value-a', + }), + ); + + expect(await query()).toEqual([ + { + user_entity_ref: 'user-1', + bucket: 'bucket-a', + key: 'key-a', + value: 'value-a', + }, + ]); + }); + + it('should overwrite an existing setting', async () => { + await storage.transaction(tx => + storage.set(tx, { + userEntityRef: 'user-1', + bucket: 'bucket-a', + key: 'key-a', + value: 'value-a', + }), + ); + + await storage.transaction(tx => + storage.set(tx, { + userEntityRef: 'user-1', + bucket: 'bucket-a', + key: 'key-a', + value: 'value-b', + }), + ); + + expect(await query()).toEqual([ + { + user_entity_ref: 'user-1', + bucket: 'bucket-a', + key: 'key-a', + value: 'value-b', + }, + ]); + }); + }); + + describe('delete', () => { + it('should not throw an error if the entry does not exist', async () => { + await expect(() => + storage.transaction(tx => + storage.delete(tx, { + userEntityRef: 'user-1', + bucket: 'bucket-c', + key: 'key-c', + }), + ), + ).not.toThrow(); + }); + + it('should return the setting', async () => { + await insert([ + { + user_entity_ref: 'user-1', + bucket: 'bucket-a', + key: 'key-a', + value: 'value-a', + }, + { + user_entity_ref: 'user-2', + bucket: 'bucket-c', + key: 'key-c', + value: 'value-c', + }, + ]); + + await storage.transaction(tx => + storage.delete(tx, { + userEntityRef: 'user-1', + bucket: 'bucket-a', + key: 'key-a', + }), + ); + expect(await query()).toEqual([ + { + user_entity_ref: 'user-2', + bucket: 'bucket-c', + key: 'key-c', + value: 'value-c', + }, + ]); + }); + }); + }, +); diff --git a/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.ts b/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.ts new file mode 100644 index 0000000000..fd24a6f3e5 --- /dev/null +++ b/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.ts @@ -0,0 +1,143 @@ +/* + * Copyright 2022 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. + */ +import { resolvePackagePath } from '@backstage/backend-common'; +import { NotFoundError } from '@backstage/errors'; +import { Knex } from 'knex'; + +import { type UserSetting, UserSettingsStore } from './UserSettingsStore'; + +const migrationsDir = resolvePackagePath( + '@backstage/plugin-user-settings-backend', + 'migrations', +); + +/** + * @public + */ +export type RawDbUserSettingsRow = { + user_entity_ref: string; + bucket: string; + key: string; + value: string; +}; + +/** + * Store to manage the user settings. + * + * @public + */ +export class DatabaseUserSettingsStore + implements UserSettingsStore +{ + static async create(knex: Knex): Promise { + await knex.migrate.latest({ + directory: migrationsDir, + }); + return new DatabaseUserSettingsStore(knex); + } + + private constructor(private readonly db: Knex) {} + + async getAll(tx: Knex.Transaction, opts: { userEntityRef: string }) { + const settings = await tx('user_settings') + .where({ user_entity_ref: opts.userEntityRef }) + .select('bucket', 'key', 'value'); + + return settings; + } + + async deleteAll( + tx: Knex.Transaction, + opts: { userEntityRef: string }, + ): Promise { + await tx('user_settings') + .where({ user_entity_ref: opts.userEntityRef }) + .delete(); + } + + async getBucket( + tx: Knex.Transaction, + opts: { userEntityRef: string; bucket: string }, + ): Promise { + const settings = await tx('user_settings') + .where({ user_entity_ref: opts.userEntityRef, bucket: opts.bucket }) + .select(['bucket', 'key', 'value']); + + return settings; + } + + async deleteBucket( + tx: Knex.Transaction, + opts: { userEntityRef: string; bucket: string }, + ): Promise { + await tx('user_settings') + .where({ user_entity_ref: opts.userEntityRef, bucket: opts.bucket }) + .delete(); + } + + async get( + tx: Knex.Transaction, + opts: { userEntityRef: string; bucket: string; key: string }, + ): Promise { + const setting = await tx('user_settings') + .where({ + user_entity_ref: opts.userEntityRef, + bucket: opts.bucket, + key: opts.key, + }) + .select(['bucket', 'key', 'value']); + + if (!setting.length) { + throw new NotFoundError( + `Unable to find '${opts.key}' in bucket '${opts.bucket}'`, + ); + } + + return setting[0]; + } + + async set( + tx: Knex.Transaction, + opts: { userEntityRef: string; bucket: string; key: string; value: string }, + ): Promise { + await tx('user_settings') + .insert({ + user_entity_ref: opts.userEntityRef, + bucket: opts.bucket, + key: opts.key, + value: opts.value, + }) + .onConflict(['user_entity_ref', 'bucket', 'key']) + .merge({ value: opts.value }); + } + + async delete( + tx: Knex.Transaction, + opts: { userEntityRef: string; bucket: string; key: string }, + ): Promise { + await tx('user_settings') + .where({ + user_entity_ref: opts.userEntityRef, + bucket: opts.bucket, + key: opts.key, + }) + .delete(); + } + + async transaction(fn: (tx: Knex.Transaction) => Promise) { + return await this.db.transaction(fn); + } +} diff --git a/plugins/user-settings-backend/src/database/UserSettingsStore.ts b/plugins/user-settings-backend/src/database/UserSettingsStore.ts new file mode 100644 index 0000000000..b86673270f --- /dev/null +++ b/plugins/user-settings-backend/src/database/UserSettingsStore.ts @@ -0,0 +1,65 @@ +/* + * Copyright 2022 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 type UserSetting = { + bucket: string; + key: string; + value: string; +}; + +/** + * Store definition for the user settings. + * + * @public + */ +export interface UserSettingsStore { + transaction(fn: (tx: Transaction) => Promise): Promise; + + get( + tx: Transaction, + opts: { userEntityRef: string; bucket: string; key: string }, + ): Promise; + + set( + tx: Transaction, + opts: { userEntityRef: string; bucket: string; key: string; value: string }, + ): Promise; + + delete( + tx: Transaction, + opts: { userEntityRef: string; bucket: string; key: string }, + ): Promise; + + getBucket( + tx: Transaction, + opts: { userEntityRef: string; bucket: string }, + ): Promise; + + deleteBucket( + tx: Transaction, + opts: { userEntityRef: string; bucket: string }, + ): Promise; + + getAll( + tx: Transaction, + opts: { userEntityRef: string }, + ): Promise; + + deleteAll(tx: Transaction, opts: { userEntityRef: string }): Promise; +} diff --git a/plugins/user-settings-backend/src/database/index.ts b/plugins/user-settings-backend/src/database/index.ts new file mode 100644 index 0000000000..23023ceb2d --- /dev/null +++ b/plugins/user-settings-backend/src/database/index.ts @@ -0,0 +1,20 @@ +/* + * Copyright 2022 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. + */ +export { + DatabaseUserSettingsStore, + type RawDbUserSettingsRow, +} from './DatabaseUserSettingsStore'; +export type { UserSettingsStore, UserSetting } from './UserSettingsStore'; diff --git a/plugins/user-settings-backend/src/index.ts b/plugins/user-settings-backend/src/index.ts new file mode 100644 index 0000000000..9d9fe923e4 --- /dev/null +++ b/plugins/user-settings-backend/src/index.ts @@ -0,0 +1,17 @@ +/* + * Copyright 2022 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. + */ +export * from './service'; +export * from './database'; diff --git a/plugins/user-settings-backend/src/run.ts b/plugins/user-settings-backend/src/run.ts new file mode 100644 index 0000000000..a1ba29a339 --- /dev/null +++ b/plugins/user-settings-backend/src/run.ts @@ -0,0 +1,33 @@ +/* + * Copyright 2022 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. + */ +import { getRootLogger } from '@backstage/backend-common'; +import yn from 'yn'; + +import { startStandaloneServer } from './service/standaloneServer'; + +const port = process.env.PLUGIN_PORT ? Number(process.env.PLUGIN_PORT) : 7007; +const enableCors = yn(process.env.PLUGIN_CORS, { default: false }); +const logger = getRootLogger(); + +startStandaloneServer({ port, enableCors, logger }).catch(err => { + logger.error(err); + process.exit(1); +}); + +process.on('SIGINT', () => { + logger.info('CTRL+C pressed; exiting.'); + process.exit(0); +}); diff --git a/plugins/user-settings-backend/src/service/index.ts b/plugins/user-settings-backend/src/service/index.ts new file mode 100644 index 0000000000..5f4020dc68 --- /dev/null +++ b/plugins/user-settings-backend/src/service/index.ts @@ -0,0 +1,20 @@ +/* + * Copyright 2022 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. + */ +export { + createRouter, + createUserSettingsStore, + type RouterOptions, +} from './router'; diff --git a/plugins/user-settings-backend/src/service/router.test.ts b/plugins/user-settings-backend/src/service/router.test.ts new file mode 100644 index 0000000000..ba23ad11c0 --- /dev/null +++ b/plugins/user-settings-backend/src/service/router.test.ts @@ -0,0 +1,319 @@ +/* + * Copyright 2022 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. + */ +import { AuthenticationError } from '@backstage/errors'; +import { IdentityClient } from '@backstage/plugin-auth-node'; +import express from 'express'; +import request from 'supertest'; + +import { UserSettingsStore } from '../database'; +import { createRouter } from './router'; + +describe('createRouter', () => { + const userSettingsStore: jest.Mocked> = { + transaction: jest.fn(), + deleteAll: jest.fn(), + getAll: jest.fn(), + getBucket: jest.fn(), + deleteBucket: jest.fn(), + get: jest.fn(), + set: jest.fn(), + delete: jest.fn(), + }; + const authenticateMock = jest.fn(); + const identityClient: jest.Mocked> = { + authenticate: authenticateMock, + }; + + let app: express.Express; + + beforeEach(async () => { + userSettingsStore.transaction.mockImplementation(fn => fn('tx')); + + const router = await createRouter({ + userSettingsStore, + identity: identityClient as IdentityClient, + }); + + app = express().use(router); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + describe('GET /', () => { + it('returns ok', async () => { + const settings = [ + { bucket: 'a', key: 'a', value: 'a' }, + { bucket: 'b', key: 'b', value: 'b' }, + ]; + authenticateMock.mockResolvedValue({ + identity: { userEntityRef: 'user-1' }, + }); + + userSettingsStore.getAll.mockResolvedValue(settings); + + const responses = await request(app) + .get('/') + .set('Authorization', 'Bearer foo'); + + expect(responses.status).toEqual(200); + expect(responses.body).toEqual(settings); + + expect(authenticateMock).toHaveBeenCalledWith('foo'); + expect(userSettingsStore.getAll).toBeCalledTimes(1); + expect(userSettingsStore.getAll).toBeCalledWith('tx', { + userEntityRef: 'user-1', + }); + }); + + it('returns an error if the Authorization header is missing', async () => { + const responses = await request(app).get('/'); + + expect(responses.status).toEqual(401); + expect(userSettingsStore.getAll).not.toHaveBeenCalled(); + }); + + it('returns an error if the token is not valid', async () => { + authenticateMock.mockRejectedValue( + new AuthenticationError('Invalid token'), + ); + + const responses = await request(app) + .get('/') + .set('Authorization', 'Bearer foo'); + + expect(responses.status).toEqual(401); + expect(userSettingsStore.getAll).not.toHaveBeenCalled(); + }); + }); + + describe('DELETE /', () => { + it('returns ok', async () => { + authenticateMock.mockResolvedValue({ + identity: { userEntityRef: 'user-1' }, + }); + + userSettingsStore.deleteAll.mockResolvedValue(); + + const responses = await request(app) + .delete('/') + .set('Authorization', 'Bearer foo'); + + expect(responses.status).toEqual(204); + + expect(authenticateMock).toHaveBeenCalledWith('foo'); + expect(userSettingsStore.deleteAll).toBeCalledTimes(1); + expect(userSettingsStore.deleteAll).toBeCalledWith('tx', { + userEntityRef: 'user-1', + }); + }); + + it('returns an error if the Authorization header is missing', async () => { + const responses = await request(app).delete('/'); + + expect(responses.status).toEqual(401); + expect(userSettingsStore.getAll).not.toHaveBeenCalled(); + }); + }); + + describe('GET /:bucket', () => { + it('returns ok', async () => { + const settings = [ + { bucket: 'my-bucket', key: 'a', value: 'a' }, + { bucket: 'my-bucket', key: 'b', value: 'b' }, + ]; + authenticateMock.mockResolvedValue({ + identity: { userEntityRef: 'user-1' }, + }); + + userSettingsStore.getBucket.mockResolvedValue(settings); + + const responses = await request(app) + .get('/my-bucket') + .set('Authorization', 'Bearer foo'); + + expect(responses.status).toEqual(200); + expect(responses.body).toEqual(settings); + + expect(authenticateMock).toHaveBeenCalledWith('foo'); + expect(userSettingsStore.getBucket).toBeCalledTimes(1); + expect(userSettingsStore.getBucket).toBeCalledWith('tx', { + userEntityRef: 'user-1', + bucket: 'my-bucket', + }); + }); + + it('returns an error if the Authorization header is missing', async () => { + const responses = await request(app).get('/my-bucket'); + + expect(responses.status).toEqual(401); + expect(userSettingsStore.getBucket).not.toHaveBeenCalled(); + }); + }); + + describe('DELETE /:bucket', () => { + it('returns ok', async () => { + authenticateMock.mockResolvedValue({ + identity: { userEntityRef: 'user-1' }, + }); + + userSettingsStore.deleteBucket.mockResolvedValue(); + + const responses = await request(app) + .delete('/my-bucket') + .set('Authorization', 'Bearer foo'); + + expect(responses.status).toEqual(204); + + expect(authenticateMock).toHaveBeenCalledWith('foo'); + expect(userSettingsStore.deleteBucket).toBeCalledTimes(1); + expect(userSettingsStore.deleteBucket).toBeCalledWith('tx', { + userEntityRef: 'user-1', + bucket: 'my-bucket', + }); + }); + + it('returns an error if the Authorization header is missing', async () => { + const responses = await request(app).delete('/my-bucket'); + + expect(responses.status).toEqual(401); + expect(userSettingsStore.deleteBucket).not.toHaveBeenCalled(); + }); + }); + + describe('GET /:bucket/:key', () => { + it('returns ok', async () => { + const setting = { bucket: 'my-bucket', key: 'my-key', value: 'a' }; + authenticateMock.mockResolvedValue({ + identity: { userEntityRef: 'user-1' }, + }); + + userSettingsStore.get.mockResolvedValue(setting); + + const responses = await request(app) + .get('/my-bucket/my-key') + .set('Authorization', 'Bearer foo'); + + expect(responses.status).toEqual(200); + expect(responses.body).toEqual(setting); + + expect(authenticateMock).toHaveBeenCalledWith('foo'); + expect(userSettingsStore.get).toBeCalledTimes(1); + expect(userSettingsStore.get).toBeCalledWith('tx', { + userEntityRef: 'user-1', + bucket: 'my-bucket', + key: 'my-key', + }); + }); + + it('returns an error if the Authorization header is missing', async () => { + const responses = await request(app).get('/my-bucket/my-key'); + + expect(responses.status).toEqual(401); + expect(userSettingsStore.get).not.toHaveBeenCalled(); + }); + }); + + describe('DELETE /:bucket/:key', () => { + it('returns ok', async () => { + authenticateMock.mockResolvedValue({ + identity: { userEntityRef: 'user-1' }, + }); + + userSettingsStore.delete.mockResolvedValue(); + + const responses = await request(app) + .delete('/my-bucket/my-key') + .set('Authorization', 'Bearer foo'); + + expect(responses.status).toEqual(204); + + expect(authenticateMock).toHaveBeenCalledWith('foo'); + expect(userSettingsStore.delete).toBeCalledTimes(1); + expect(userSettingsStore.delete).toBeCalledWith('tx', { + userEntityRef: 'user-1', + bucket: 'my-bucket', + key: 'my-key', + }); + }); + + it('returns an error if the Authorization header is missing', async () => { + const responses = await request(app).delete('/my-bucket/my-key'); + + expect(responses.status).toEqual(401); + expect(userSettingsStore.delete).not.toHaveBeenCalled(); + }); + }); + + describe('PUT /:bucket/:key', () => { + it('returns ok', async () => { + const setting = { bucket: 'my-bucket', key: 'my-key', value: 'a' }; + authenticateMock.mockResolvedValue({ + identity: { userEntityRef: 'user-1' }, + }); + + userSettingsStore.set.mockResolvedValue(); + userSettingsStore.get.mockResolvedValue(setting); + + const responses = await request(app) + .put('/my-bucket/my-key') + .set('Authorization', 'Bearer foo') + .send({ value: 'a' }); + + expect(responses.status).toEqual(200); + expect(responses.body).toEqual(setting); + + expect(authenticateMock).toHaveBeenCalledWith('foo'); + expect(userSettingsStore.set).toBeCalledTimes(1); + expect(userSettingsStore.set).toHaveBeenCalledWith('tx', { + userEntityRef: 'user-1', + bucket: 'my-bucket', + key: 'my-key', + value: 'a', + }); + expect(userSettingsStore.get).toBeCalledTimes(1); + expect(userSettingsStore.get).toBeCalledWith('tx', { + userEntityRef: 'user-1', + bucket: 'my-bucket', + key: 'my-key', + }); + }); + + it('returns an error if the value is not a string', async () => { + authenticateMock.mockResolvedValue({ + identity: { userEntityRef: 'user-1' }, + }); + + const responses = await request(app) + .put('/my-bucket/my-key') + .set('Authorization', 'Bearer foo') + .send({ value: { invalid: 'because not a string' } }); + + expect(responses.status).toEqual(400); + + expect(authenticateMock).toHaveBeenCalledWith('foo'); + expect(userSettingsStore.set).not.toHaveBeenCalled(); + }); + + it('returns an error if the Authorization header is missing', async () => { + const responses = await request(app).get('/my-bucket/my-key'); + + expect(responses.status).toEqual(401); + expect(userSettingsStore.get).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/plugins/user-settings-backend/src/service/router.ts b/plugins/user-settings-backend/src/service/router.ts new file mode 100644 index 0000000000..a715433dcb --- /dev/null +++ b/plugins/user-settings-backend/src/service/router.ts @@ -0,0 +1,168 @@ +/* + * Copyright 2022 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. + */ +import { PluginDatabaseManager, errorHandler } from '@backstage/backend-common'; +import { AuthenticationError, InputError } from '@backstage/errors'; +import { + getBearerTokenFromAuthorizationHeader, + IdentityClient, +} from '@backstage/plugin-auth-node'; +import express, { Request } from 'express'; +import Router from 'express-promise-router'; + +import { DatabaseUserSettingsStore, UserSettingsStore } from '../database'; + +/** + * @public + */ +export async function createUserSettingsStore(database: PluginDatabaseManager) { + return await DatabaseUserSettingsStore.create(await database.getClient()); +} + +/** + * @public + */ +export interface RouterOptions { + userSettingsStore: UserSettingsStore; + identity: IdentityClient; +} + +/** + * Create the user settings backend routes. + * + * @public + */ +export async function createRouter( + options: RouterOptions, +): Promise { + const { userSettingsStore, identity } = options; + const router = Router(); + router.use(express.json()); + + /** + * Helper method to extract the userEntityRef from the request. + */ + const getUserEntityRef = async (req: Request): Promise => { + const token = getBearerTokenFromAuthorizationHeader( + req.header('authorization'), + ); + + if (!token) { + throw new AuthenticationError(`Missing token in 'authorization' header`); + } + + // throws an AuthenticationError in case the token is invalid + const user = await identity.authenticate(token); + + return user.identity.userEntityRef; + }; + + // get all user related settings + router.get('/', async (req, res) => { + const userEntityRef = await getUserEntityRef(req); + + const settings = await userSettingsStore.transaction(tx => + userSettingsStore.getAll(tx, { userEntityRef }), + ); + + res.json(settings); + }); + + // remove all user related settings + router.delete('/', async (req, res) => { + const userEntityRef = await getUserEntityRef(req); + + await userSettingsStore.transaction(tx => + userSettingsStore.deleteAll(tx, { userEntityRef }), + ); + + res.send(204).end(); + }); + + // get a single bucket + router.get('/:bucket', async (req, res) => { + const userEntityRef = await getUserEntityRef(req); + const { bucket } = req.params; + + const settings = await userSettingsStore.transaction(tx => + userSettingsStore.getBucket(tx, { userEntityRef, bucket }), + ); + + res.json(settings); + }); + + // delete a whole bucket + router.delete('/:bucket', async (req, res) => { + const userEntityRef = await getUserEntityRef(req); + const { bucket } = req.params; + + await userSettingsStore.transaction(tx => + userSettingsStore.deleteBucket(tx, { userEntityRef, bucket }), + ); + + res.status(204).end(); + }); + + // get a single value + router.get('/:bucket/:key', async (req, res) => { + const userEntityRef = await getUserEntityRef(req); + const { bucket, key } = req.params; + + const setting = await userSettingsStore.transaction(tx => + userSettingsStore.get(tx, { userEntityRef, bucket, key }), + ); + + res.json(setting); + }); + + // set a single value + router.put('/:bucket/:key', async (req, res) => { + const userEntityRef = await getUserEntityRef(req); + const { bucket, key } = req.params; + const { value } = req.body; + + if (typeof value !== 'string') { + throw new InputError('Value must be a string'); + } + + const setting = await userSettingsStore.transaction(async tx => { + await userSettingsStore.set(tx, { + userEntityRef, + bucket, + key, + value, + }); + return userSettingsStore.get(tx, { userEntityRef, bucket, key }); + }); + + res.json(setting); + }); + + // get a single value + router.delete('/:bucket/:key', async (req, res) => { + const userEntityRef = await getUserEntityRef(req); + const { bucket, key } = req.params; + + await userSettingsStore.transaction(tx => + userSettingsStore.delete(tx, { userEntityRef, bucket, key }), + ); + + res.send(204).end(); + }); + + router.use(errorHandler()); + + return router; +} diff --git a/plugins/user-settings-backend/src/service/standaloneServer.ts b/plugins/user-settings-backend/src/service/standaloneServer.ts new file mode 100644 index 0000000000..39ef0ba098 --- /dev/null +++ b/plugins/user-settings-backend/src/service/standaloneServer.ts @@ -0,0 +1,82 @@ +/* + * Copyright 2022 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. + */ +import { Server } from 'http'; + +import { createServiceBuilder, useHotMemoize } from '@backstage/backend-common'; +import Knex from 'knex'; +import { Logger } from 'winston'; + +import { DatabaseUserSettingsStore } from '../database'; +import { createRouter } from './router'; +import { IdentityClient } from '@backstage/plugin-auth-node'; +import { Router } from 'express'; + +export interface ServerOptions { + port: number; + enableCors: boolean; + logger: Logger; +} + +export async function startStandaloneServer( + options: ServerOptions, +): Promise { + const logger = options.logger.child({ service: 'storage-backend' }); + + const database = useHotMemoize(module, () => { + return Knex({ + client: 'better-sqlite3', + connection: ':memory:', + useNullAsDefault: true, + }); + }); + + logger.debug('Starting application server...'); + + const identityMock = { + authenticate: async token => ({ + identity: { userEntityRef: token ?? 'user:default/john_doe' }, + }), + } as IdentityClient; + + const router = await createRouter({ + userSettingsStore: await DatabaseUserSettingsStore.create(database), + identity: identityMock, + }); + + // set a custom authorization header to simplify the development + const authWrapper = Router(); + authWrapper.use((req, _res, next) => { + req.headers.authorization = + req.headers.authorization ?? 'Bearer user:default/john_doe'; + next(); + }); + authWrapper.use(router); + + let service = createServiceBuilder(module) + .setPort(options.port) + .addRouter('/user-settings', authWrapper); + + if (options.enableCors) { + service = service.enableCors({ origin: 'http://localhost:3000' }); + } + + return await service.start().catch(err => { + logger.error(err); + process.exit(1); + }); +} + +module.hot?.accept(); diff --git a/plugins/user-settings-backend/src/setupTests.ts b/plugins/user-settings-backend/src/setupTests.ts new file mode 100644 index 0000000000..8b9b6bd586 --- /dev/null +++ b/plugins/user-settings-backend/src/setupTests.ts @@ -0,0 +1,16 @@ +/* + * Copyright 2022 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. + */ +export {}; diff --git a/yarn.lock b/yarn.lock index afe89bf424..a51757dcde 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2855,7 +2855,7 @@ __metadata: languageName: unknown linkType: soft -"@backstage/backend-common@^0.15.1-next.3, @backstage/backend-common@workspace:packages/backend-common": +"@backstage/backend-common@^0.15.1-next.1, @backstage/backend-common@^0.15.1-next.3, @backstage/backend-common@workspace:packages/backend-common": version: 0.0.0-use.local resolution: "@backstage/backend-common@workspace:packages/backend-common" dependencies: @@ -2991,7 +2991,7 @@ __metadata: languageName: unknown linkType: soft -"@backstage/backend-test-utils@^0.1.28-next.3, @backstage/backend-test-utils@workspace:packages/backend-test-utils": +"@backstage/backend-test-utils@^0.1.28-next.1, @backstage/backend-test-utils@^0.1.28-next.3, @backstage/backend-test-utils@workspace:packages/backend-test-utils": version: 0.0.0-use.local resolution: "@backstage/backend-test-utils@workspace:packages/backend-test-utils" dependencies: @@ -3291,6 +3291,8 @@ __metadata: "@backstage/cli": ^0.19.0-next.3 "@backstage/config": ^1.0.2-next.0 "@backstage/core-plugin-api": ^1.0.6-next.3 + "@backstage/errors": ^1.1.0 + "@backstage/plugin-permission-common": ^0.6.4-next.0 "@backstage/test-utils": ^1.2.0-next.3 "@backstage/types": ^1.0.0 "@backstage/version-bridge": ^1.0.1 @@ -4027,7 +4029,7 @@ __metadata: languageName: unknown linkType: soft -"@backstage/plugin-auth-node@^0.2.5-next.3, @backstage/plugin-auth-node@workspace:plugins/auth-node": +"@backstage/plugin-auth-node@^0.2.5-next.1, @backstage/plugin-auth-node@^0.2.5-next.3, @backstage/plugin-auth-node@workspace:plugins/auth-node": version: 0.0.0-use.local resolution: "@backstage/plugin-auth-node@workspace:plugins/auth-node" dependencies: @@ -7310,6 +7312,27 @@ __metadata: languageName: unknown linkType: soft +"@backstage/plugin-user-settings-backend@workspace:plugins/user-settings-backend": + version: 0.0.0-use.local + resolution: "@backstage/plugin-user-settings-backend@workspace:plugins/user-settings-backend" + dependencies: + "@backstage/backend-common": ^0.15.1-next.1 + "@backstage/backend-test-utils": ^0.1.28-next.1 + "@backstage/catalog-model": ^1.1.0 + "@backstage/cli": ^0.19.0-next.1 + "@backstage/errors": ^1.1.0 + "@backstage/plugin-auth-node": ^0.2.5-next.1 + "@types/express": ^4.17.6 + "@types/supertest": ^2.0.8 + express: ^4.17.1 + express-promise-router: ^4.1.0 + knex: ^2.0.0 + supertest: ^6.1.3 + winston: ^3.2.1 + yn: ^4.0.0 + languageName: unknown + linkType: soft + "@backstage/plugin-user-settings@^0.4.8-next.3, @backstage/plugin-user-settings@workspace:plugins/user-settings": version: 0.0.0-use.local resolution: "@backstage/plugin-user-settings@workspace:plugins/user-settings" From 32fa6ca2d8a4912cd6d1b7a228fc20bad0f5ce4d Mon Sep 17 00:00:00 2001 From: Dominik Schwank Date: Thu, 1 Sep 2022 10:30:02 +0200 Subject: [PATCH 02/14] fix: change imports Signed-off-by: Dominik Schwank --- packages/core-app-api/package.json | 1 - .../implementations/StorageApi/PersistentStorage.test.ts | 8 ++++++-- yarn.lock | 1 - 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/core-app-api/package.json b/packages/core-app-api/package.json index 632c429361..e1c24997ac 100644 --- a/packages/core-app-api/package.json +++ b/packages/core-app-api/package.json @@ -35,7 +35,6 @@ "@backstage/config": "^1.0.2-next.0", "@backstage/core-plugin-api": "^1.0.6-next.3", "@backstage/errors": "^1.1.0", - "@backstage/plugin-permission-common": "^0.6.4-next.0", "@backstage/types": "^1.0.0", "@backstage/version-bridge": "^1.0.1", "@types/prop-types": "^15.7.3", diff --git a/packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.test.ts b/packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.test.ts index 6cea8dde5b..dfa0008e32 100644 --- a/packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.test.ts +++ b/packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.test.ts @@ -15,8 +15,12 @@ */ import { PersistentStorage } from './PersistentStorage'; -import { ErrorApi, FetchApi, StorageApi } from '@backstage/core-plugin-api'; -import { DiscoveryApi } from '@backstage/plugin-permission-common'; +import { + ErrorApi, + FetchApi, + StorageApi, + DiscoveryApi, +} from '@backstage/core-plugin-api'; import { MockFetchApi, setupRequestMockHandlers } from '@backstage/test-utils'; import { rest } from 'msw'; diff --git a/yarn.lock b/yarn.lock index a51757dcde..9df599d106 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3292,7 +3292,6 @@ __metadata: "@backstage/config": ^1.0.2-next.0 "@backstage/core-plugin-api": ^1.0.6-next.3 "@backstage/errors": ^1.1.0 - "@backstage/plugin-permission-common": ^0.6.4-next.0 "@backstage/test-utils": ^1.2.0-next.3 "@backstage/types": ^1.0.0 "@backstage/version-bridge": ^1.0.1 From 3250830e6ec3b8006bbd86e252634cf0c9e6fde9 Mon Sep 17 00:00:00 2001 From: Dominik Schwank Date: Thu, 1 Sep 2022 10:31:22 +0200 Subject: [PATCH 03/14] feat: use string in DB instead of text Signed-off-by: Dominik Schwank --- .../user-settings-backend/migrations/20220824183000_init.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/user-settings-backend/migrations/20220824183000_init.js b/plugins/user-settings-backend/migrations/20220824183000_init.js index 688d8495b5..a519969a98 100644 --- a/plugins/user-settings-backend/migrations/20220824183000_init.js +++ b/plugins/user-settings-backend/migrations/20220824183000_init.js @@ -22,11 +22,11 @@ exports.up = async function up(knex) { table.comment('The table of user related settings'); table - .text('user_entity_ref') + .string('user_entity_ref') .notNullable() .comment('The entityRef of the user'); - table.text('bucket').notNullable().comment('Name of the bucket'); - table.text('key').notNullable().comment('Key of a bucket value'); + table.string('bucket').notNullable().comment('Name of the bucket'); + table.string('key').notNullable().comment('Key of a bucket value'); table.text('value').notNullable().comment('The value'); table.primary(['user_entity_ref', 'bucket', 'key']); From 81f0945f6e724427abd522f6f74544d26e471a3a Mon Sep 17 00:00:00 2001 From: Dominik Schwank Date: Thu, 1 Sep 2022 10:31:45 +0200 Subject: [PATCH 04/14] fix: change initial version to 0.0.0 Signed-off-by: Dominik Schwank --- plugins/user-settings-backend/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/user-settings-backend/package.json b/plugins/user-settings-backend/package.json index f20180cac3..41178d49b8 100644 --- a/plugins/user-settings-backend/package.json +++ b/plugins/user-settings-backend/package.json @@ -1,7 +1,7 @@ { "name": "@backstage/plugin-user-settings-backend", "description": "The Backstage backend plugin to manage user settings", - "version": "0.1.0", + "version": "0.0.0", "main": "src/index.ts", "types": "src/index.ts", "license": "Apache-2.0", From 1e12c7f5f188cdf221d876e2b6e4faf6908d9b83 Mon Sep 17 00:00:00 2001 From: Dominik Schwank Date: Thu, 1 Sep 2022 11:03:32 +0200 Subject: [PATCH 05/14] feat: support skip migration Signed-off-by: Dominik Schwank --- plugins/user-settings-backend/api-report.md | 9 +++----- .../DatabaseUserSettingsStore.test.ts | 11 ++++++++- .../src/database/DatabaseUserSettingsStore.ts | 23 ++++++++++++++----- .../src/service/index.ts | 6 +---- .../src/service/router.ts | 11 ++------- .../src/service/standaloneServer.ts | 4 +++- 6 files changed, 36 insertions(+), 28 deletions(-) diff --git a/plugins/user-settings-backend/api-report.md b/plugins/user-settings-backend/api-report.md index b3b40970c9..47e59637d6 100644 --- a/plugins/user-settings-backend/api-report.md +++ b/plugins/user-settings-backend/api-report.md @@ -13,17 +13,14 @@ export function createRouter( options: RouterOptions, ): Promise; -// @public (undocumented) -export function createUserSettingsStore( - database: PluginDatabaseManager, -): Promise; - // @public export class DatabaseUserSettingsStore implements UserSettingsStore { // (undocumented) - static create(knex: Knex): Promise; + static create(options: { + database: PluginDatabaseManager; + }): Promise; // (undocumented) delete( tx: Knex.Transaction, diff --git a/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.test.ts b/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.test.ts index 717707f034..547beaa6b6 100644 --- a/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.test.ts +++ b/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.test.ts @@ -29,9 +29,18 @@ const databases = TestDatabases.create({ async function createStore(databaseId: TestDatabaseId) { const knex = await databases.init(databaseId); + const databaseManager = { + getClient: async () => knex, + migrations: { + skip: false, + }, + }; + return { knex, - storage: await DatabaseUserSettingsStore.create(knex), + storage: await DatabaseUserSettingsStore.create({ + database: databaseManager, + }), }; } diff --git a/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.ts b/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.ts index fd24a6f3e5..2e2eb6318a 100644 --- a/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.ts +++ b/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.ts @@ -13,7 +13,10 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { resolvePackagePath } from '@backstage/backend-common'; +import { + PluginDatabaseManager, + resolvePackagePath, +} from '@backstage/backend-common'; import { NotFoundError } from '@backstage/errors'; import { Knex } from 'knex'; @@ -42,11 +45,19 @@ export type RawDbUserSettingsRow = { export class DatabaseUserSettingsStore implements UserSettingsStore { - static async create(knex: Knex): Promise { - await knex.migrate.latest({ - directory: migrationsDir, - }); - return new DatabaseUserSettingsStore(knex); + static async create(options: { + database: PluginDatabaseManager; + }): Promise { + const { database } = options; + const client = await database.getClient(); + + if (!database.migrations?.skip) { + await client.migrate.latest({ + directory: migrationsDir, + }); + } + + return new DatabaseUserSettingsStore(client); } private constructor(private readonly db: Knex) {} diff --git a/plugins/user-settings-backend/src/service/index.ts b/plugins/user-settings-backend/src/service/index.ts index 5f4020dc68..8f3e084d24 100644 --- a/plugins/user-settings-backend/src/service/index.ts +++ b/plugins/user-settings-backend/src/service/index.ts @@ -13,8 +13,4 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -export { - createRouter, - createUserSettingsStore, - type RouterOptions, -} from './router'; +export { createRouter, type RouterOptions } from './router'; diff --git a/plugins/user-settings-backend/src/service/router.ts b/plugins/user-settings-backend/src/service/router.ts index a715433dcb..0a8c52b156 100644 --- a/plugins/user-settings-backend/src/service/router.ts +++ b/plugins/user-settings-backend/src/service/router.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { PluginDatabaseManager, errorHandler } from '@backstage/backend-common'; +import { errorHandler } from '@backstage/backend-common'; import { AuthenticationError, InputError } from '@backstage/errors'; import { getBearerTokenFromAuthorizationHeader, @@ -22,14 +22,7 @@ import { import express, { Request } from 'express'; import Router from 'express-promise-router'; -import { DatabaseUserSettingsStore, UserSettingsStore } from '../database'; - -/** - * @public - */ -export async function createUserSettingsStore(database: PluginDatabaseManager) { - return await DatabaseUserSettingsStore.create(await database.getClient()); -} +import { UserSettingsStore } from '../database'; /** * @public diff --git a/plugins/user-settings-backend/src/service/standaloneServer.ts b/plugins/user-settings-backend/src/service/standaloneServer.ts index 39ef0ba098..1e079f02c0 100644 --- a/plugins/user-settings-backend/src/service/standaloneServer.ts +++ b/plugins/user-settings-backend/src/service/standaloneServer.ts @@ -52,7 +52,9 @@ export async function startStandaloneServer( } as IdentityClient; const router = await createRouter({ - userSettingsStore: await DatabaseUserSettingsStore.create(database), + userSettingsStore: await DatabaseUserSettingsStore.create({ + database: { getClient: async () => database }, + }), identity: identityMock, }); From 31a240304444d11e18e3d64f3bef24d877685681 Mon Sep 17 00:00:00 2001 From: Dominik Schwank Date: Thu, 1 Sep 2022 11:12:44 +0200 Subject: [PATCH 06/14] feat: use buckets prefix for routes Signed-off-by: Dominik Schwank --- .../StorageApi/PersistentStorage.test.ts | 41 ++++++---- .../StorageApi/PersistentStorage.ts | 2 +- .../src/service/router.test.ts | 76 +++++++++---------- .../src/service/router.ts | 14 ++-- 4 files changed, 71 insertions(+), 62 deletions(-) diff --git a/packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.test.ts b/packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.test.ts index dfa0008e32..3157395bcf 100644 --- a/packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.test.ts +++ b/packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.test.ts @@ -52,7 +52,7 @@ describe('Persistent Storage API', () => { beforeEach(() => { server.use( - rest.get(`${mockBaseUrl}/`, async (_req, res, ctx) => { + rest.get(`${mockBaseUrl}/buckets/`, async (_req, res, ctx) => { return res(ctx.json([])); }), ); @@ -64,9 +64,12 @@ describe('Persistent Storage API', () => { const storage = createPersistentStorage(); server.use( - rest.get(`${mockBaseUrl}/:bucket/:key`, async (_req, res, ctx) => { - return res(ctx.json({ value: 'a' })); - }), + rest.get( + `${mockBaseUrl}/buckets/:bucket/:key`, + async (_req, res, ctx) => { + return res(ctx.json({ value: 'a' })); + }, + ), ); expect(storage.snapshot('myfakekey').value).toBeUndefined(); @@ -82,7 +85,7 @@ describe('Persistent Storage API', () => { const dummyValue = 'a'; server.use( - rest.put(`${mockBaseUrl}/:bucket/:key`, async (req, res, ctx) => { + rest.put(`${mockBaseUrl}/buckets/:bucket/:key`, async (req, res, ctx) => { const body = await req.json(); const data = { value: JSON.stringify(dummyValue) }; expect(body).toEqual(data); @@ -102,7 +105,7 @@ describe('Persistent Storage API', () => { }; server.use( - rest.put(`${mockBaseUrl}/:bucket/:key`, async (req, res, ctx) => { + rest.put(`${mockBaseUrl}/buckets/:bucket/:key`, async (req, res, ctx) => { const body = await req.json(); const data = { value: JSON.stringify(dummyValue) }; expect(body).toEqual(data); @@ -122,7 +125,7 @@ describe('Persistent Storage API', () => { const mockData = { hello: 'im a great new value' }; server.use( - rest.put(`${mockBaseUrl}/:bucket/:key`, async (req, res, ctx) => { + rest.put(`${mockBaseUrl}/buckets/:bucket/:key`, async (req, res, ctx) => { const body = await req.json(); const data = { value: JSON.stringify(mockData) }; expect(body).toEqual(data); @@ -162,9 +165,12 @@ describe('Persistent Storage API', () => { const selectedKeyNextHandler = jest.fn(); server.use( - rest.delete(`${mockBaseUrl}/:bucket/:key`, async (_req, res, ctx) => { - return res(ctx.status(204)); - }), + rest.delete( + `${mockBaseUrl}/buckets/:bucket/:key`, + async (_req, res, ctx) => { + return res(ctx.status(204)); + }, + ), ); await new Promise(resolve => { @@ -196,7 +202,7 @@ describe('Persistent Storage API', () => { const selectedKeyNextHandler = jest.fn(); server.use( - rest.put(`${mockBaseUrl}/:bucket/:key`, async (req, res, ctx) => { + rest.put(`${mockBaseUrl}/buckets/:bucket/:key`, async (req, res, ctx) => { const { bucket, key } = req.params; const { value } = await req.json(); @@ -205,7 +211,7 @@ describe('Persistent Storage API', () => { return res(ctx.json({ value })); }), - rest.get(`${mockBaseUrl}/:bucket/:key`, async (req, res, ctx) => { + rest.get(`${mockBaseUrl}/buckets/:bucket/:key`, async (req, res, ctx) => { const { bucket, key } = req.params; expect(bucket).toEqual('default.profile/something'); @@ -253,7 +259,7 @@ describe('Persistent Storage API', () => { }); server.use( - rest.get(`${mockBaseUrl}/:bucket/:key`, async (req, res, ctx) => { + rest.get(`${mockBaseUrl}/buckets/:bucket/:key`, async (req, res, ctx) => { const { bucket, key } = req.params; expect(bucket).toEqual('Test.Mock.Thing'); @@ -290,9 +296,12 @@ describe('Persistent Storage API', () => { const data = { foo: 'bar', baz: [{ foo: 'bar' }] }; server.use( - rest.get(`${mockBaseUrl}/:bucket/:key`, async (_req, res, ctx) => { - return res(ctx.text(JSON.stringify({ value: JSON.stringify(data) }))); - }), + rest.get( + `${mockBaseUrl}/buckets/:bucket/:key`, + async (_req, res, ctx) => { + return res(ctx.text(JSON.stringify({ value: JSON.stringify(data) }))); + }, + ), ); await new Promise(resolve => { diff --git a/packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.ts b/packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.ts index 4ebf635cd9..38a986863e 100644 --- a/packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.ts +++ b/packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.ts @@ -182,7 +182,7 @@ export class PersistentStorage implements StorageApi { const baseUrl = await this.discoveryApi.getBaseUrl('user-settings'); const encodedNamespace = encodeURIComponent(this.namespace); const encodedKey = encodeURIComponent(key); - return `${baseUrl}/${encodedNamespace}/${encodedKey}`; + return `${baseUrl}/buckets/${encodedNamespace}/${encodedKey}`; } private async notifyChanges(snapshot: StorageValueSnapshot) { diff --git a/plugins/user-settings-backend/src/service/router.test.ts b/plugins/user-settings-backend/src/service/router.test.ts index ba23ad11c0..21aff9f164 100644 --- a/plugins/user-settings-backend/src/service/router.test.ts +++ b/plugins/user-settings-backend/src/service/router.test.ts @@ -54,7 +54,7 @@ describe('createRouter', () => { jest.resetAllMocks(); }); - describe('GET /', () => { + describe('GET /buckets/', () => { it('returns ok', async () => { const settings = [ { bucket: 'a', key: 'a', value: 'a' }, @@ -67,21 +67,21 @@ describe('createRouter', () => { userSettingsStore.getAll.mockResolvedValue(settings); const responses = await request(app) - .get('/') + .get('/buckets/') .set('Authorization', 'Bearer foo'); expect(responses.status).toEqual(200); expect(responses.body).toEqual(settings); expect(authenticateMock).toHaveBeenCalledWith('foo'); - expect(userSettingsStore.getAll).toBeCalledTimes(1); - expect(userSettingsStore.getAll).toBeCalledWith('tx', { + expect(userSettingsStore.getAll).toHaveBeenCalledTimes(1); + expect(userSettingsStore.getAll).toHaveBeenCalledWith('tx', { userEntityRef: 'user-1', }); }); it('returns an error if the Authorization header is missing', async () => { - const responses = await request(app).get('/'); + const responses = await request(app).get('/buckets/'); expect(responses.status).toEqual(401); expect(userSettingsStore.getAll).not.toHaveBeenCalled(); @@ -93,7 +93,7 @@ describe('createRouter', () => { ); const responses = await request(app) - .get('/') + .get('/buckets/') .set('Authorization', 'Bearer foo'); expect(responses.status).toEqual(401); @@ -101,7 +101,7 @@ describe('createRouter', () => { }); }); - describe('DELETE /', () => { + describe('DELETE /buckets/', () => { it('returns ok', async () => { authenticateMock.mockResolvedValue({ identity: { userEntityRef: 'user-1' }, @@ -110,27 +110,27 @@ describe('createRouter', () => { userSettingsStore.deleteAll.mockResolvedValue(); const responses = await request(app) - .delete('/') + .delete('/buckets/') .set('Authorization', 'Bearer foo'); expect(responses.status).toEqual(204); expect(authenticateMock).toHaveBeenCalledWith('foo'); - expect(userSettingsStore.deleteAll).toBeCalledTimes(1); - expect(userSettingsStore.deleteAll).toBeCalledWith('tx', { + expect(userSettingsStore.deleteAll).toHaveBeenCalledTimes(1); + expect(userSettingsStore.deleteAll).toHaveBeenCalledWith('tx', { userEntityRef: 'user-1', }); }); it('returns an error if the Authorization header is missing', async () => { - const responses = await request(app).delete('/'); + const responses = await request(app).delete('/buckets/'); expect(responses.status).toEqual(401); expect(userSettingsStore.getAll).not.toHaveBeenCalled(); }); }); - describe('GET /:bucket', () => { + describe('GET /buckets/:bucket', () => { it('returns ok', async () => { const settings = [ { bucket: 'my-bucket', key: 'a', value: 'a' }, @@ -143,29 +143,29 @@ describe('createRouter', () => { userSettingsStore.getBucket.mockResolvedValue(settings); const responses = await request(app) - .get('/my-bucket') + .get('/buckets/my-bucket') .set('Authorization', 'Bearer foo'); expect(responses.status).toEqual(200); expect(responses.body).toEqual(settings); expect(authenticateMock).toHaveBeenCalledWith('foo'); - expect(userSettingsStore.getBucket).toBeCalledTimes(1); - expect(userSettingsStore.getBucket).toBeCalledWith('tx', { + expect(userSettingsStore.getBucket).toHaveBeenCalledTimes(1); + expect(userSettingsStore.getBucket).toHaveBeenCalledWith('tx', { userEntityRef: 'user-1', bucket: 'my-bucket', }); }); it('returns an error if the Authorization header is missing', async () => { - const responses = await request(app).get('/my-bucket'); + const responses = await request(app).get('/buckets/my-bucket'); expect(responses.status).toEqual(401); expect(userSettingsStore.getBucket).not.toHaveBeenCalled(); }); }); - describe('DELETE /:bucket', () => { + describe('DELETE /buckets/:bucket', () => { it('returns ok', async () => { authenticateMock.mockResolvedValue({ identity: { userEntityRef: 'user-1' }, @@ -174,28 +174,28 @@ describe('createRouter', () => { userSettingsStore.deleteBucket.mockResolvedValue(); const responses = await request(app) - .delete('/my-bucket') + .delete('/buckets/my-bucket') .set('Authorization', 'Bearer foo'); expect(responses.status).toEqual(204); expect(authenticateMock).toHaveBeenCalledWith('foo'); - expect(userSettingsStore.deleteBucket).toBeCalledTimes(1); - expect(userSettingsStore.deleteBucket).toBeCalledWith('tx', { + expect(userSettingsStore.deleteBucket).toHaveBeenCalledTimes(1); + expect(userSettingsStore.deleteBucket).toHaveBeenCalledWith('tx', { userEntityRef: 'user-1', bucket: 'my-bucket', }); }); it('returns an error if the Authorization header is missing', async () => { - const responses = await request(app).delete('/my-bucket'); + const responses = await request(app).delete('/buckets/my-bucket'); expect(responses.status).toEqual(401); expect(userSettingsStore.deleteBucket).not.toHaveBeenCalled(); }); }); - describe('GET /:bucket/:key', () => { + describe('GET /buckets/:bucket/:key', () => { it('returns ok', async () => { const setting = { bucket: 'my-bucket', key: 'my-key', value: 'a' }; authenticateMock.mockResolvedValue({ @@ -205,15 +205,15 @@ describe('createRouter', () => { userSettingsStore.get.mockResolvedValue(setting); const responses = await request(app) - .get('/my-bucket/my-key') + .get('/buckets/my-bucket/my-key') .set('Authorization', 'Bearer foo'); expect(responses.status).toEqual(200); expect(responses.body).toEqual(setting); expect(authenticateMock).toHaveBeenCalledWith('foo'); - expect(userSettingsStore.get).toBeCalledTimes(1); - expect(userSettingsStore.get).toBeCalledWith('tx', { + expect(userSettingsStore.get).toHaveBeenCalledTimes(1); + expect(userSettingsStore.get).toHaveBeenCalledWith('tx', { userEntityRef: 'user-1', bucket: 'my-bucket', key: 'my-key', @@ -221,14 +221,14 @@ describe('createRouter', () => { }); it('returns an error if the Authorization header is missing', async () => { - const responses = await request(app).get('/my-bucket/my-key'); + const responses = await request(app).get('/buckets/my-bucket/my-key'); expect(responses.status).toEqual(401); expect(userSettingsStore.get).not.toHaveBeenCalled(); }); }); - describe('DELETE /:bucket/:key', () => { + describe('DELETE /buckets/:bucket/:key', () => { it('returns ok', async () => { authenticateMock.mockResolvedValue({ identity: { userEntityRef: 'user-1' }, @@ -237,14 +237,14 @@ describe('createRouter', () => { userSettingsStore.delete.mockResolvedValue(); const responses = await request(app) - .delete('/my-bucket/my-key') + .delete('/buckets/my-bucket/my-key') .set('Authorization', 'Bearer foo'); expect(responses.status).toEqual(204); expect(authenticateMock).toHaveBeenCalledWith('foo'); - expect(userSettingsStore.delete).toBeCalledTimes(1); - expect(userSettingsStore.delete).toBeCalledWith('tx', { + expect(userSettingsStore.delete).toHaveBeenCalledTimes(1); + expect(userSettingsStore.delete).toHaveBeenCalledWith('tx', { userEntityRef: 'user-1', bucket: 'my-bucket', key: 'my-key', @@ -252,14 +252,14 @@ describe('createRouter', () => { }); it('returns an error if the Authorization header is missing', async () => { - const responses = await request(app).delete('/my-bucket/my-key'); + const responses = await request(app).delete('/buckets/my-bucket/my-key'); expect(responses.status).toEqual(401); expect(userSettingsStore.delete).not.toHaveBeenCalled(); }); }); - describe('PUT /:bucket/:key', () => { + describe('PUT /buckets/:bucket/:key', () => { it('returns ok', async () => { const setting = { bucket: 'my-bucket', key: 'my-key', value: 'a' }; authenticateMock.mockResolvedValue({ @@ -270,7 +270,7 @@ describe('createRouter', () => { userSettingsStore.get.mockResolvedValue(setting); const responses = await request(app) - .put('/my-bucket/my-key') + .put('/buckets/my-bucket/my-key') .set('Authorization', 'Bearer foo') .send({ value: 'a' }); @@ -278,15 +278,15 @@ describe('createRouter', () => { expect(responses.body).toEqual(setting); expect(authenticateMock).toHaveBeenCalledWith('foo'); - expect(userSettingsStore.set).toBeCalledTimes(1); + expect(userSettingsStore.set).toHaveBeenCalledTimes(1); expect(userSettingsStore.set).toHaveBeenCalledWith('tx', { userEntityRef: 'user-1', bucket: 'my-bucket', key: 'my-key', value: 'a', }); - expect(userSettingsStore.get).toBeCalledTimes(1); - expect(userSettingsStore.get).toBeCalledWith('tx', { + expect(userSettingsStore.get).toHaveBeenCalledTimes(1); + expect(userSettingsStore.get).toHaveBeenCalledWith('tx', { userEntityRef: 'user-1', bucket: 'my-bucket', key: 'my-key', @@ -299,7 +299,7 @@ describe('createRouter', () => { }); const responses = await request(app) - .put('/my-bucket/my-key') + .put('/buckets/my-bucket/my-key') .set('Authorization', 'Bearer foo') .send({ value: { invalid: 'because not a string' } }); @@ -310,7 +310,7 @@ describe('createRouter', () => { }); it('returns an error if the Authorization header is missing', async () => { - const responses = await request(app).get('/my-bucket/my-key'); + const responses = await request(app).get('/buckets/my-bucket/my-key'); expect(responses.status).toEqual(401); expect(userSettingsStore.get).not.toHaveBeenCalled(); diff --git a/plugins/user-settings-backend/src/service/router.ts b/plugins/user-settings-backend/src/service/router.ts index 0a8c52b156..588759c929 100644 --- a/plugins/user-settings-backend/src/service/router.ts +++ b/plugins/user-settings-backend/src/service/router.ts @@ -63,7 +63,7 @@ export async function createRouter( }; // get all user related settings - router.get('/', async (req, res) => { + router.get('/buckets', async (req, res) => { const userEntityRef = await getUserEntityRef(req); const settings = await userSettingsStore.transaction(tx => @@ -74,7 +74,7 @@ export async function createRouter( }); // remove all user related settings - router.delete('/', async (req, res) => { + router.delete('/buckets', async (req, res) => { const userEntityRef = await getUserEntityRef(req); await userSettingsStore.transaction(tx => @@ -85,7 +85,7 @@ export async function createRouter( }); // get a single bucket - router.get('/:bucket', async (req, res) => { + router.get('/buckets/:bucket', async (req, res) => { const userEntityRef = await getUserEntityRef(req); const { bucket } = req.params; @@ -97,7 +97,7 @@ export async function createRouter( }); // delete a whole bucket - router.delete('/:bucket', async (req, res) => { + router.delete('/buckets/:bucket', async (req, res) => { const userEntityRef = await getUserEntityRef(req); const { bucket } = req.params; @@ -109,7 +109,7 @@ export async function createRouter( }); // get a single value - router.get('/:bucket/:key', async (req, res) => { + router.get('/buckets/:bucket/:key', async (req, res) => { const userEntityRef = await getUserEntityRef(req); const { bucket, key } = req.params; @@ -121,7 +121,7 @@ export async function createRouter( }); // set a single value - router.put('/:bucket/:key', async (req, res) => { + router.put('/buckets/:bucket/:key', async (req, res) => { const userEntityRef = await getUserEntityRef(req); const { bucket, key } = req.params; const { value } = req.body; @@ -144,7 +144,7 @@ export async function createRouter( }); // get a single value - router.delete('/:bucket/:key', async (req, res) => { + router.delete('/buckets/:bucket/:key', async (req, res) => { const userEntityRef = await getUserEntityRef(req); const { bucket, key } = req.params; From 8f7e5a7517ed25d9ff34cb1a37d367698c321885 Mon Sep 17 00:00:00 2001 From: Dominik Schwank Date: Thu, 1 Sep 2022 11:22:42 +0200 Subject: [PATCH 07/14] feat: add keys path support Signed-off-by: Dominik Schwank --- .../StorageApi/PersistentStorage.test.ts | 98 +++++++++++-------- .../StorageApi/PersistentStorage.ts | 2 +- .../src/service/router.test.ts | 26 +++-- .../src/service/router.ts | 6 +- 4 files changed, 78 insertions(+), 54 deletions(-) diff --git a/packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.test.ts b/packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.test.ts index 3157395bcf..79da01ca41 100644 --- a/packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.test.ts +++ b/packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.test.ts @@ -65,7 +65,7 @@ describe('Persistent Storage API', () => { server.use( rest.get( - `${mockBaseUrl}/buckets/:bucket/:key`, + `${mockBaseUrl}/buckets/:bucket/keys/:key`, async (_req, res, ctx) => { return res(ctx.json({ value: 'a' })); }, @@ -85,13 +85,16 @@ describe('Persistent Storage API', () => { const dummyValue = 'a'; server.use( - rest.put(`${mockBaseUrl}/buckets/:bucket/:key`, async (req, res, ctx) => { - const body = await req.json(); - const data = { value: JSON.stringify(dummyValue) }; - expect(body).toEqual(data); + rest.put( + `${mockBaseUrl}/buckets/:bucket/keys/:key`, + async (req, res, ctx) => { + const body = await req.json(); + const data = { value: JSON.stringify(dummyValue) }; + expect(body).toEqual(data); - return res(ctx.json(data)); - }), + return res(ctx.json(data)); + }, + ), ); await storage.set('my-key', dummyValue); @@ -105,13 +108,16 @@ describe('Persistent Storage API', () => { }; server.use( - rest.put(`${mockBaseUrl}/buckets/:bucket/:key`, async (req, res, ctx) => { - const body = await req.json(); - const data = { value: JSON.stringify(dummyValue) }; - expect(body).toEqual(data); + rest.put( + `${mockBaseUrl}/buckets/:bucket/keys/:key`, + async (req, res, ctx) => { + const body = await req.json(); + const data = { value: JSON.stringify(dummyValue) }; + expect(body).toEqual(data); - return res(ctx.json(data)); - }), + return res(ctx.json(data)); + }, + ), ); await storage.set('my-key', dummyValue); @@ -125,13 +131,16 @@ describe('Persistent Storage API', () => { const mockData = { hello: 'im a great new value' }; server.use( - rest.put(`${mockBaseUrl}/buckets/:bucket/:key`, async (req, res, ctx) => { - const body = await req.json(); - const data = { value: JSON.stringify(mockData) }; - expect(body).toEqual(data); + rest.put( + `${mockBaseUrl}/buckets/:bucket/keys/:key`, + async (req, res, ctx) => { + const body = await req.json(); + const data = { value: JSON.stringify(mockData) }; + expect(body).toEqual(data); - return res(ctx.json(data)); - }), + return res(ctx.json(data)); + }, + ), ); await new Promise(resolve => { @@ -166,7 +175,7 @@ describe('Persistent Storage API', () => { server.use( rest.delete( - `${mockBaseUrl}/buckets/:bucket/:key`, + `${mockBaseUrl}/buckets/:bucket/keys/:key`, async (_req, res, ctx) => { return res(ctx.status(204)); }, @@ -202,23 +211,29 @@ describe('Persistent Storage API', () => { const selectedKeyNextHandler = jest.fn(); server.use( - rest.put(`${mockBaseUrl}/buckets/:bucket/:key`, async (req, res, ctx) => { - const { bucket, key } = req.params; - const { value } = await req.json(); + rest.put( + `${mockBaseUrl}/buckets/:bucket/keys/:key`, + async (req, res, ctx) => { + const { bucket, key } = req.params; + const { value } = await req.json(); - expect(bucket).toEqual('default.profile.something.deep'); - expect(key).toEqual('test2'); + expect(bucket).toEqual('default.profile.something.deep'); + expect(key).toEqual('test2'); - return res(ctx.json({ value })); - }), - rest.get(`${mockBaseUrl}/buckets/:bucket/:key`, async (req, res, ctx) => { - const { bucket, key } = req.params; + return res(ctx.json({ value })); + }, + ), + rest.get( + `${mockBaseUrl}/buckets/:bucket/keys/:key`, + async (req, res, ctx) => { + const { bucket, key } = req.params; - expect(bucket).toEqual('default.profile/something'); - expect(key).toEqual('deep/test2'); + expect(bucket).toEqual('default.profile/something'); + expect(key).toEqual('deep/test2'); - return res(ctx.status(404)); - }), + return res(ctx.status(404)); + }, + ), ); // when getting key test2 it will translate to default.profile.something.deep/test2 @@ -259,14 +274,17 @@ describe('Persistent Storage API', () => { }); server.use( - rest.get(`${mockBaseUrl}/buckets/:bucket/:key`, async (req, res, ctx) => { - const { bucket, key } = req.params; + rest.get( + `${mockBaseUrl}/buckets/:bucket/keys/:key`, + async (req, res, ctx) => { + const { bucket, key } = req.params; - expect(bucket).toEqual('Test.Mock.Thing'); - expect(key).toEqual('key'); + expect(bucket).toEqual('Test.Mock.Thing'); + expect(key).toEqual('key'); - return res(ctx.text('{ invalid: json string }')); - }), + return res(ctx.text('{ invalid: json string }')); + }, + ), ); await new Promise(resolve => { @@ -297,7 +315,7 @@ describe('Persistent Storage API', () => { server.use( rest.get( - `${mockBaseUrl}/buckets/:bucket/:key`, + `${mockBaseUrl}/buckets/:bucket/keys/:key`, async (_req, res, ctx) => { return res(ctx.text(JSON.stringify({ value: JSON.stringify(data) }))); }, diff --git a/packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.ts b/packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.ts index 38a986863e..b23ecd2b3f 100644 --- a/packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.ts +++ b/packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.ts @@ -182,7 +182,7 @@ export class PersistentStorage implements StorageApi { const baseUrl = await this.discoveryApi.getBaseUrl('user-settings'); const encodedNamespace = encodeURIComponent(this.namespace); const encodedKey = encodeURIComponent(key); - return `${baseUrl}/buckets/${encodedNamespace}/${encodedKey}`; + return `${baseUrl}/buckets/${encodedNamespace}/keys/${encodedKey}`; } private async notifyChanges(snapshot: StorageValueSnapshot) { diff --git a/plugins/user-settings-backend/src/service/router.test.ts b/plugins/user-settings-backend/src/service/router.test.ts index 21aff9f164..a0e5403612 100644 --- a/plugins/user-settings-backend/src/service/router.test.ts +++ b/plugins/user-settings-backend/src/service/router.test.ts @@ -195,7 +195,7 @@ describe('createRouter', () => { }); }); - describe('GET /buckets/:bucket/:key', () => { + describe('GET /buckets/:bucket/keys/:key', () => { it('returns ok', async () => { const setting = { bucket: 'my-bucket', key: 'my-key', value: 'a' }; authenticateMock.mockResolvedValue({ @@ -205,7 +205,7 @@ describe('createRouter', () => { userSettingsStore.get.mockResolvedValue(setting); const responses = await request(app) - .get('/buckets/my-bucket/my-key') + .get('/buckets/my-bucket/keys/my-key') .set('Authorization', 'Bearer foo'); expect(responses.status).toEqual(200); @@ -221,14 +221,16 @@ describe('createRouter', () => { }); it('returns an error if the Authorization header is missing', async () => { - const responses = await request(app).get('/buckets/my-bucket/my-key'); + const responses = await request(app).get( + '/buckets/my-bucket/keys/my-key', + ); expect(responses.status).toEqual(401); expect(userSettingsStore.get).not.toHaveBeenCalled(); }); }); - describe('DELETE /buckets/:bucket/:key', () => { + describe('DELETE /buckets/:bucket/keys/:key', () => { it('returns ok', async () => { authenticateMock.mockResolvedValue({ identity: { userEntityRef: 'user-1' }, @@ -237,7 +239,7 @@ describe('createRouter', () => { userSettingsStore.delete.mockResolvedValue(); const responses = await request(app) - .delete('/buckets/my-bucket/my-key') + .delete('/buckets/my-bucket/keys/my-key') .set('Authorization', 'Bearer foo'); expect(responses.status).toEqual(204); @@ -252,14 +254,16 @@ describe('createRouter', () => { }); it('returns an error if the Authorization header is missing', async () => { - const responses = await request(app).delete('/buckets/my-bucket/my-key'); + const responses = await request(app).delete( + '/buckets/my-bucket/keys/my-key', + ); expect(responses.status).toEqual(401); expect(userSettingsStore.delete).not.toHaveBeenCalled(); }); }); - describe('PUT /buckets/:bucket/:key', () => { + describe('PUT /buckets/:bucket/keys/:key', () => { it('returns ok', async () => { const setting = { bucket: 'my-bucket', key: 'my-key', value: 'a' }; authenticateMock.mockResolvedValue({ @@ -270,7 +274,7 @@ describe('createRouter', () => { userSettingsStore.get.mockResolvedValue(setting); const responses = await request(app) - .put('/buckets/my-bucket/my-key') + .put('/buckets/my-bucket/keys/my-key') .set('Authorization', 'Bearer foo') .send({ value: 'a' }); @@ -299,7 +303,7 @@ describe('createRouter', () => { }); const responses = await request(app) - .put('/buckets/my-bucket/my-key') + .put('/buckets/my-bucket/keys/my-key') .set('Authorization', 'Bearer foo') .send({ value: { invalid: 'because not a string' } }); @@ -310,7 +314,9 @@ describe('createRouter', () => { }); it('returns an error if the Authorization header is missing', async () => { - const responses = await request(app).get('/buckets/my-bucket/my-key'); + const responses = await request(app).get( + '/buckets/my-bucket/keys/my-key', + ); expect(responses.status).toEqual(401); expect(userSettingsStore.get).not.toHaveBeenCalled(); diff --git a/plugins/user-settings-backend/src/service/router.ts b/plugins/user-settings-backend/src/service/router.ts index 588759c929..f51e38c10a 100644 --- a/plugins/user-settings-backend/src/service/router.ts +++ b/plugins/user-settings-backend/src/service/router.ts @@ -109,7 +109,7 @@ export async function createRouter( }); // get a single value - router.get('/buckets/:bucket/:key', async (req, res) => { + router.get('/buckets/:bucket/keys/:key', async (req, res) => { const userEntityRef = await getUserEntityRef(req); const { bucket, key } = req.params; @@ -121,7 +121,7 @@ export async function createRouter( }); // set a single value - router.put('/buckets/:bucket/:key', async (req, res) => { + router.put('/buckets/:bucket/keys/:key', async (req, res) => { const userEntityRef = await getUserEntityRef(req); const { bucket, key } = req.params; const { value } = req.body; @@ -144,7 +144,7 @@ export async function createRouter( }); // get a single value - router.delete('/buckets/:bucket/:key', async (req, res) => { + router.delete('/buckets/:bucket/keys/:key', async (req, res) => { const userEntityRef = await getUserEntityRef(req); const { bucket, key } = req.params; From 9895e6bfb2707eaee73c198d36db9268ed1afa6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Tue, 6 Sep 2022 10:50:49 +0200 Subject: [PATCH 08/14] rename to UserSettingsStorage and adjust error handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/healthy-oranges-fly.md | 7 +- packages/core-app-api/api-report.md | 52 ++++++------- ...ge.test.ts => UserSettingsStorage.test.ts} | 20 +++-- ...stentStorage.ts => UserSettingsStorage.ts} | 78 +++++++++++-------- .../apis/implementations/StorageApi/index.ts | 2 +- .../DatabaseUserSettingsStore.test.ts | 2 +- .../src/database/DatabaseUserSettingsStore.ts | 2 +- .../src/database/index.ts | 1 + plugins/user-settings-backend/src/index.ts | 1 + plugins/user-settings-backend/src/run.ts | 1 + .../src/service/index.ts | 1 + .../src/service/router.test.ts | 2 +- .../src/service/router.ts | 2 +- .../src/service/standaloneServer.ts | 11 ++- .../user-settings-backend/src/setupTests.ts | 1 + 15 files changed, 97 insertions(+), 86 deletions(-) rename packages/core-app-api/src/apis/implementations/StorageApi/{PersistentStorage.test.ts => UserSettingsStorage.test.ts} (96%) rename packages/core-app-api/src/apis/implementations/StorageApi/{PersistentStorage.ts => UserSettingsStorage.ts} (73%) diff --git a/.changeset/healthy-oranges-fly.md b/.changeset/healthy-oranges-fly.md index da6530b903..c6b6f6ff93 100644 --- a/.changeset/healthy-oranges-fly.md +++ b/.changeset/healthy-oranges-fly.md @@ -3,6 +3,7 @@ '@backstage/plugin-user-settings-backend': minor --- -Add new plugin @backstage/user-settings-backend to store user related settings -in the database. Additionally adding a PersistentStorage implementation to use -easily use the new plugin as drop-in replacement for the WebStorage. +Add new plugin `@backstage/user-settings-backend` to store user related settings +in the database. Additionally adding a `UserSettingsStorage` implementation of +the `StorageApi` to easily use the new plugin as drop-in replacement for the +`WebStorage`. diff --git a/packages/core-app-api/api-report.md b/packages/core-app-api/api-report.md index de3afdcbd4..90e18df723 100644 --- a/packages/core-app-api/api-report.md +++ b/packages/core-app-api/api-report.md @@ -506,35 +506,6 @@ export type OneLoginAuthCreateOptions = { provider?: AuthProviderInfo; }; -// @public -export class PersistentStorage implements StorageApi { - constructor( - namespace: string, - fetchApi: FetchApi, - discoveryApi: DiscoveryApi, - errorApi: ErrorApi, - ); - // (undocumented) - static create(options: { - fetchApi: FetchApi; - discoveryApi: DiscoveryApi; - errorApi: ErrorApi; - namespace?: string; - }): PersistentStorage; - // (undocumented) - forBucket(name: string): StorageApi; - // (undocumented) - observe$( - key: string, - ): Observable>; - // (undocumented) - remove(key: string): Promise; - // (undocumented) - set(key: string, data: T): Promise; - // (undocumented) - snapshot(key: string): StorageValueSnapshot; -} - // @public export class SamlAuth implements ProfileInfoApi, BackstageIdentityApi, SessionApi @@ -572,6 +543,29 @@ export class UrlPatternDiscovery implements DiscoveryApi { getBaseUrl(pluginId: string): Promise; } +// @public +export class UserSettingsStorage implements StorageApi { + // (undocumented) + static create(options: { + fetchApi: FetchApi; + discoveryApi: DiscoveryApi; + errorApi: ErrorApi; + namespace?: string; + }): UserSettingsStorage; + // (undocumented) + forBucket(name: string): StorageApi; + // (undocumented) + observe$( + key: string, + ): Observable>; + // (undocumented) + remove(key: string): Promise; + // (undocumented) + set(key: string, data: T): Promise; + // (undocumented) + snapshot(key: string): StorageValueSnapshot; +} + // @public export class WebStorage implements StorageApi { constructor(namespace: string, errorApi: ErrorApi); diff --git a/packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.test.ts b/packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.test.ts similarity index 96% rename from packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.test.ts rename to packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.test.ts index 79da01ca41..cc48fc925c 100644 --- a/packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.test.ts +++ b/packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.test.ts @@ -14,22 +14,21 @@ * limitations under the License. */ -import { PersistentStorage } from './PersistentStorage'; import { + DiscoveryApi, ErrorApi, FetchApi, StorageApi, - DiscoveryApi, } from '@backstage/core-plugin-api'; import { MockFetchApi, setupRequestMockHandlers } from '@backstage/test-utils'; - import { rest } from 'msw'; import { setupServer } from 'msw/node'; - -const server = setupServer(); +import { UserSettingsStorage } from './UserSettingsStorage'; describe('Persistent Storage API', () => { + const server = setupServer(); setupRequestMockHandlers(server); + const mockBaseUrl = 'http://backstage:9191/api'; const mockErrorApi = { post: jest.fn(), error$: jest.fn() }; const mockDiscoveryApi = { getBaseUrl: async () => mockBaseUrl }; @@ -42,7 +41,7 @@ describe('Persistent Storage API', () => { namespace?: string; }>, ): StorageApi => { - return PersistentStorage.create({ + return UserSettingsStorage.create({ errorApi: mockErrorApi, fetchApi: new MockFetchApi(), discoveryApi: mockDiscoveryApi, @@ -58,7 +57,9 @@ describe('Persistent Storage API', () => { ); }); - afterEach(() => jest.resetAllMocks()); + afterEach(() => { + jest.resetAllMocks(); + }); it('should return undefined for values which are unset', async () => { const storage = createPersistentStorage(); @@ -267,7 +268,7 @@ describe('Persistent Storage API', () => { expect(mockErrorApi.post).not.toHaveBeenCalled(); }); - it('should call the error api when the json can not be parsed in local storage', async () => { + it('should silently treat the value as absent when the json can not be parsed', async () => { const selectedKeyNextHandler = jest.fn(); const rootStorage = createPersistentStorage({ namespace: 'Test.Mock.Thing', @@ -278,10 +279,8 @@ describe('Persistent Storage API', () => { `${mockBaseUrl}/buckets/:bucket/keys/:key`, async (req, res, ctx) => { const { bucket, key } = req.params; - expect(bucket).toEqual('Test.Mock.Thing'); expect(key).toEqual('key'); - return res(ctx.text('{ invalid: json string }')); }, ), @@ -305,7 +304,6 @@ describe('Persistent Storage API', () => { presence: 'absent', value: undefined, }); - expect(mockErrorApi.post).toHaveBeenCalledWith(expect.any(Error)); }); it('should freeze the snapshot value', async () => { diff --git a/packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.ts b/packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.ts similarity index 73% rename from packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.ts rename to packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.ts index b23ecd2b3f..0763ad8187 100644 --- a/packages/core-app-api/src/apis/implementations/StorageApi/PersistentStorage.ts +++ b/packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.ts @@ -21,11 +21,11 @@ import { StorageApi, StorageValueSnapshot, } from '@backstage/core-plugin-api'; -import { NotFoundError } from '@backstage/errors'; +import { ResponseError } from '@backstage/errors'; import { JsonValue, Observable } from '@backstage/types'; import ObservableImpl from 'zen-observable'; -const buckets = new Map(); +const buckets = new Map(); /** * An implementation of the storage API, that uses the user-settings backend to @@ -33,7 +33,7 @@ const buckets = new Map(); * * @public */ -export class PersistentStorage implements StorageApi { +export class UserSettingsStorage implements StorageApi { private subscribers = new Set< ZenObservable.SubscriptionObserver> >(); @@ -47,7 +47,7 @@ export class PersistentStorage implements StorageApi { }; }); - constructor( + private constructor( private readonly namespace: string, private readonly fetchApi: FetchApi, private readonly discoveryApi: DiscoveryApi, @@ -59,8 +59,8 @@ export class PersistentStorage implements StorageApi { discoveryApi: DiscoveryApi; errorApi: ErrorApi; namespace?: string; - }): PersistentStorage { - return new PersistentStorage( + }): UserSettingsStorage { + return new UserSettingsStorage( options.namespace ?? 'default', options.fetchApi, options.discoveryApi, @@ -75,7 +75,7 @@ export class PersistentStorage implements StorageApi { if (!buckets.has(bucketPath)) { buckets.set( bucketPath, - new PersistentStorage( + new UserSettingsStorage( bucketPath, this.fetchApi, this.discoveryApi, @@ -90,10 +90,14 @@ export class PersistentStorage implements StorageApi { async remove(key: string): Promise { const fetchUrl = await this.getFetchUrl(key); - await this.fetchApi.fetch(fetchUrl, { + const response = await this.fetchApi.fetch(fetchUrl, { method: 'DELETE', }); + if (!response.ok && response.status !== 404) { + throw ResponseError.fromResponse(response); + } + this.notifyChanges({ key, presence: 'absent', @@ -109,6 +113,10 @@ export class PersistentStorage implements StorageApi { body, }); + if (!response.ok) { + throw ResponseError.fromResponse(response); + } + const { value } = await response.json(); this.notifyChanges({ @@ -121,12 +129,16 @@ export class PersistentStorage implements StorageApi { observe$( key: string, ): Observable> { + // TODO(freben): Introduce server polling or similar, to ensure that different devices update when values change return this.observable.filter(({ key: messageKey }) => messageKey === key); } snapshot(key: string): StorageValueSnapshot { - // trigger a reload - this.get(key).then(snapshot => this.notifyChanges(snapshot)); + // trigger a reload, ensure it happens on the next tick (after returning) + Promise.resolve() + .then(() => this.get(key)) + .then(snapshot => this.notifyChanges(snapshot)) + .catch(error => this.errorApi.post(error)); return { key, @@ -137,20 +149,21 @@ export class PersistentStorage implements StorageApi { private async get( key: string, ): Promise> { + const fetchUrl = await this.getFetchUrl(key); + const response = await this.fetchApi.fetch(fetchUrl); + + if (response.status === 404) { + return { + key, + presence: 'absent', + }; + } + + if (!response.ok) { + throw ResponseError.fromResponse(response); + } + try { - const fetchUrl = await this.getFetchUrl(key); - const response = await this.fetchApi.fetch(fetchUrl); - - if (response.status === 404) { - throw new NotFoundError( - `Setting '${key}' is not set in bucket '${this.namespace}'`, - ); - } else if (!response.ok) { - throw new Error( - `Unable to fetch '${key}' from bucket '${this.namespace}'`, - ); - } - const { value: rawValue } = await response.json(); const value = JSON.parse(rawValue, (_key, val) => { if (typeof val === 'object' && val !== null) { @@ -164,16 +177,11 @@ export class PersistentStorage implements StorageApi { presence: 'present', value, }; - } catch (error) { - // NotFoundError shouldn't be recorded - if (error && !(error instanceof NotFoundError)) { - this.errorApi.post(error); - } - + } catch { + // If the value is not valid JSON, we return an unknown presence. This should never happen return { key, presence: 'absent', - value: undefined, }; } } @@ -185,9 +193,15 @@ export class PersistentStorage implements StorageApi { return `${baseUrl}/buckets/${encodedNamespace}/keys/${encodedKey}`; } - private async notifyChanges(snapshot: StorageValueSnapshot) { + private async notifyChanges( + snapshot: StorageValueSnapshot, + ) { for (const subscription of this.subscribers) { - subscription.next(snapshot); + try { + subscription.next(snapshot); + } catch { + // ignore + } } } } diff --git a/packages/core-app-api/src/apis/implementations/StorageApi/index.ts b/packages/core-app-api/src/apis/implementations/StorageApi/index.ts index 70a8c9cbe1..e4162d120d 100644 --- a/packages/core-app-api/src/apis/implementations/StorageApi/index.ts +++ b/packages/core-app-api/src/apis/implementations/StorageApi/index.ts @@ -15,4 +15,4 @@ */ export { WebStorage } from './WebStorage'; -export { PersistentStorage } from './PersistentStorage'; +export { UserSettingsStorage } from './UserSettingsStorage'; diff --git a/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.test.ts b/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.test.ts index 547beaa6b6..ed69bdcb59 100644 --- a/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.test.ts +++ b/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.test.ts @@ -13,9 +13,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import { TestDatabaseId, TestDatabases } from '@backstage/backend-test-utils'; import { Knex } from 'knex'; - import { DatabaseUserSettingsStore, RawDbUserSettingsRow, diff --git a/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.ts b/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.ts index 2e2eb6318a..961149c9ea 100644 --- a/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.ts +++ b/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.ts @@ -13,13 +13,13 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import { PluginDatabaseManager, resolvePackagePath, } from '@backstage/backend-common'; import { NotFoundError } from '@backstage/errors'; import { Knex } from 'knex'; - import { type UserSetting, UserSettingsStore } from './UserSettingsStore'; const migrationsDir = resolvePackagePath( diff --git a/plugins/user-settings-backend/src/database/index.ts b/plugins/user-settings-backend/src/database/index.ts index 23023ceb2d..2552b7db13 100644 --- a/plugins/user-settings-backend/src/database/index.ts +++ b/plugins/user-settings-backend/src/database/index.ts @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + export { DatabaseUserSettingsStore, type RawDbUserSettingsRow, diff --git a/plugins/user-settings-backend/src/index.ts b/plugins/user-settings-backend/src/index.ts index 9d9fe923e4..04d8accdaf 100644 --- a/plugins/user-settings-backend/src/index.ts +++ b/plugins/user-settings-backend/src/index.ts @@ -13,5 +13,6 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + export * from './service'; export * from './database'; diff --git a/plugins/user-settings-backend/src/run.ts b/plugins/user-settings-backend/src/run.ts index a1ba29a339..178de0f786 100644 --- a/plugins/user-settings-backend/src/run.ts +++ b/plugins/user-settings-backend/src/run.ts @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import { getRootLogger } from '@backstage/backend-common'; import yn from 'yn'; diff --git a/plugins/user-settings-backend/src/service/index.ts b/plugins/user-settings-backend/src/service/index.ts index 8f3e084d24..8ec00d13c8 100644 --- a/plugins/user-settings-backend/src/service/index.ts +++ b/plugins/user-settings-backend/src/service/index.ts @@ -13,4 +13,5 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + export { createRouter, type RouterOptions } from './router'; diff --git a/plugins/user-settings-backend/src/service/router.test.ts b/plugins/user-settings-backend/src/service/router.test.ts index a0e5403612..c1feb284a0 100644 --- a/plugins/user-settings-backend/src/service/router.test.ts +++ b/plugins/user-settings-backend/src/service/router.test.ts @@ -13,11 +13,11 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import { AuthenticationError } from '@backstage/errors'; import { IdentityClient } from '@backstage/plugin-auth-node'; import express from 'express'; import request from 'supertest'; - import { UserSettingsStore } from '../database'; import { createRouter } from './router'; diff --git a/plugins/user-settings-backend/src/service/router.ts b/plugins/user-settings-backend/src/service/router.ts index f51e38c10a..58580eddda 100644 --- a/plugins/user-settings-backend/src/service/router.ts +++ b/plugins/user-settings-backend/src/service/router.ts @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import { errorHandler } from '@backstage/backend-common'; import { AuthenticationError, InputError } from '@backstage/errors'; import { @@ -21,7 +22,6 @@ import { } from '@backstage/plugin-auth-node'; import express, { Request } from 'express'; import Router from 'express-promise-router'; - import { UserSettingsStore } from '../database'; /** diff --git a/plugins/user-settings-backend/src/service/standaloneServer.ts b/plugins/user-settings-backend/src/service/standaloneServer.ts index 1e079f02c0..3cd3704ca8 100644 --- a/plugins/user-settings-backend/src/service/standaloneServer.ts +++ b/plugins/user-settings-backend/src/service/standaloneServer.ts @@ -13,16 +13,15 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { Server } from 'http'; import { createServiceBuilder, useHotMemoize } from '@backstage/backend-common'; -import Knex from 'knex'; -import { Logger } from 'winston'; - -import { DatabaseUserSettingsStore } from '../database'; -import { createRouter } from './router'; import { IdentityClient } from '@backstage/plugin-auth-node'; import { Router } from 'express'; +import { Server } from 'http'; +import Knex from 'knex'; +import { Logger } from 'winston'; +import { DatabaseUserSettingsStore } from '../database'; +import { createRouter } from './router'; export interface ServerOptions { port: number; diff --git a/plugins/user-settings-backend/src/setupTests.ts b/plugins/user-settings-backend/src/setupTests.ts index 8b9b6bd586..813cdeaae3 100644 --- a/plugins/user-settings-backend/src/setupTests.ts +++ b/plugins/user-settings-backend/src/setupTests.ts @@ -13,4 +13,5 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + export {}; From 46cda8068d8ae75a6eb40827e851e9572ccc2477 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Tue, 6 Sep 2022 17:11:01 +0200 Subject: [PATCH 09/14] simplify router and db interface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- plugins/user-settings-backend/api-report.md | 160 +--------- .../DatabaseUserSettingsStore.test.ts | 281 +++--------------- .../src/database/DatabaseUserSettingsStore.ts | 107 +++---- .../src/database/UserSettingsStore.ts | 53 ++-- .../src/database/index.ts | 6 +- .../src/service/router.test.ts | 165 +--------- .../src/service/router.ts | 106 +++---- .../src/service/standaloneServer.ts | 6 +- 8 files changed, 155 insertions(+), 729 deletions(-) diff --git a/plugins/user-settings-backend/api-report.md b/plugins/user-settings-backend/api-report.md index 47e59637d6..15e8b5eb5b 100644 --- a/plugins/user-settings-backend/api-report.md +++ b/plugins/user-settings-backend/api-report.md @@ -5,169 +5,17 @@ ```ts import express from 'express'; import { IdentityClient } from '@backstage/plugin-auth-node'; -import { Knex } from 'knex'; import { PluginDatabaseManager } from '@backstage/backend-common'; // @public -export function createRouter( - options: RouterOptions, -): Promise; - -// @public -export class DatabaseUserSettingsStore - implements UserSettingsStore -{ - // (undocumented) - static create(options: { - database: PluginDatabaseManager; - }): Promise; - // (undocumented) - delete( - tx: Knex.Transaction, - opts: { - userEntityRef: string; - bucket: string; - key: string; - }, - ): Promise; - // (undocumented) - deleteAll( - tx: Knex.Transaction, - opts: { - userEntityRef: string; - }, - ): Promise; - // (undocumented) - deleteBucket( - tx: Knex.Transaction, - opts: { - userEntityRef: string; - bucket: string; - }, - ): Promise; - // (undocumented) - get( - tx: Knex.Transaction, - opts: { - userEntityRef: string; - bucket: string; - key: string; - }, - ): Promise; - // (undocumented) - getAll( - tx: Knex.Transaction, - opts: { - userEntityRef: string; - }, - ): Promise[]>; - // (undocumented) - getBucket( - tx: Knex.Transaction, - opts: { - userEntityRef: string; - bucket: string; - }, - ): Promise; - // (undocumented) - set( - tx: Knex.Transaction, - opts: { - userEntityRef: string; - bucket: string; - key: string; - value: string; - }, - ): Promise; - // (undocumented) - transaction(fn: (tx: Knex.Transaction) => Promise): Promise; -} +export function createRouter(options: RouterOptions): Promise; // @public (undocumented) -export type RawDbUserSettingsRow = { - user_entity_ref: string; - bucket: string; - key: string; - value: string; -}; - -// @public (undocumented) -export interface RouterOptions { +export interface RouterOptions { + // (undocumented) + database: PluginDatabaseManager; // (undocumented) identity: IdentityClient; - // (undocumented) - userSettingsStore: UserSettingsStore; -} - -// @public (undocumented) -export type UserSetting = { - bucket: string; - key: string; - value: string; -}; - -// @public -export interface UserSettingsStore { - // (undocumented) - delete( - tx: Transaction, - opts: { - userEntityRef: string; - bucket: string; - key: string; - }, - ): Promise; - // (undocumented) - deleteAll( - tx: Transaction, - opts: { - userEntityRef: string; - }, - ): Promise; - // (undocumented) - deleteBucket( - tx: Transaction, - opts: { - userEntityRef: string; - bucket: string; - }, - ): Promise; - // (undocumented) - get( - tx: Transaction, - opts: { - userEntityRef: string; - bucket: string; - key: string; - }, - ): Promise; - // (undocumented) - getAll( - tx: Transaction, - opts: { - userEntityRef: string; - }, - ): Promise; - // (undocumented) - getBucket( - tx: Transaction, - opts: { - userEntityRef: string; - bucket: string; - }, - ): Promise; - // (undocumented) - set( - tx: Transaction, - opts: { - userEntityRef: string; - bucket: string; - key: string; - value: string; - }, - ): Promise; - // (undocumented) - transaction(fn: (tx: Transaction) => Promise): Promise; } // (No @packageDocumentation comment for this package) diff --git a/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.test.ts b/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.test.ts index ed69bdcb59..e98ee0010e 100644 --- a/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.test.ts +++ b/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.test.ts @@ -67,189 +67,14 @@ describe.each(databases.eachSupportedId())( .orderBy('user_entity_ref') .select(); - describe('getAll', () => { - it('should return empty user settings', async () => { - expect( - await storage.transaction(tx => - storage.getAll(tx, { userEntityRef: 'user-1' }), - ), - ).toEqual([]); - }); - - it('should return all user settings', async () => { - await insert([ - { - user_entity_ref: 'user-1', - bucket: 'bucket-a', - key: 'key-a', - value: 'value-a', - }, - { - user_entity_ref: 'user-1', - bucket: 'bucket-a', - key: 'key-b', - value: 'value-b', - }, - { - user_entity_ref: 'user-1', - bucket: 'bucket-c', - key: 'key-c', - value: 'value-c', - }, - ]); - - expect( - await storage.transaction(tx => - storage.getAll(tx, { userEntityRef: 'user-1' }), - ), - ).toEqual([ - { bucket: 'bucket-a', key: 'key-a', value: 'value-a' }, - { bucket: 'bucket-a', key: 'key-b', value: 'value-b' }, - { bucket: 'bucket-c', key: 'key-c', value: 'value-c' }, - ]); - }); - }); - - describe('deleteAll', () => { - it('should delete all user settings', async () => { - await insert([ - { - user_entity_ref: 'user-1', - bucket: 'bucket-a', - key: 'key-a', - value: 'value-a', - }, - { - user_entity_ref: 'user-1', - bucket: 'bucket-c', - key: 'key-c', - value: 'value-c', - }, - { - user_entity_ref: 'user-2', - bucket: 'bucket-c', - key: 'key-c', - value: 'value-c', - }, - ]); - - await storage.transaction(tx => - storage.deleteAll(tx, { userEntityRef: 'user-1' }), - ); - - expect(await query()).toEqual([ - { - user_entity_ref: 'user-2', - bucket: 'bucket-c', - key: 'key-c', - value: 'value-c', - }, - ]); - }); - }); - - describe('getBucket', () => { - it('should return an empty bucket', async () => { - expect( - await storage.transaction(tx => - storage.getBucket(tx, { - userEntityRef: 'user-1', - bucket: 'bucket-c', - }), - ), - ).toEqual([]); - }); - - it('should return the settings of the bucket', async () => { - await insert([ - { - user_entity_ref: 'user-1', - bucket: 'bucket-a', - key: 'key-a', - value: 'value-a', - }, - { - user_entity_ref: 'user-1', - bucket: 'bucket-c', - key: 'key-c', - value: 'value-c', - }, - { - user_entity_ref: 'user-2', - bucket: 'bucket-c', - key: 'key-c', - value: 'value-c', - }, - ]); - - expect( - await storage.transaction(tx => - storage.getBucket(tx, { - userEntityRef: 'user-1', - bucket: 'bucket-a', - }), - ), - ).toEqual([{ bucket: 'bucket-a', key: 'key-a', value: 'value-a' }]); - }); - }); - - describe('deleteBucket', () => { - it('should delete a bucket', async () => { - await insert([ - { - user_entity_ref: 'user-1', - bucket: 'bucket-a', - key: 'key-a', - value: 'value-a', - }, - { - user_entity_ref: 'user-1', - bucket: 'bucket-c', - key: 'key-c', - value: 'value-c', - }, - { - user_entity_ref: 'user-2', - bucket: 'bucket-c', - key: 'key-c', - value: 'value-c', - }, - ]); - - await storage.transaction(tx => - storage.deleteBucket(tx, { - userEntityRef: 'user-1', - bucket: 'bucket-a', - }), - ); - - expect(await query()).toEqual([ - { - user_entity_ref: 'user-1', - bucket: 'bucket-c', - key: 'key-c', - value: 'value-c', - }, - { - user_entity_ref: 'user-2', - bucket: 'bucket-c', - key: 'key-c', - value: 'value-c', - }, - ]); - }); - }); - describe('get', () => { it('should throw an error', async () => { await expect(() => - storage.transaction(tx => - storage.get(tx, { - userEntityRef: 'user-1', - bucket: 'bucket-c', - key: 'key-c', - }), - ), + storage.get({ + userEntityRef: 'user-1', + bucket: 'bucket-c', + key: 'key-c', + }), ).rejects.toThrow(`Unable to find 'key-c' in bucket 'bucket-c'`); }); @@ -269,30 +94,30 @@ describe.each(databases.eachSupportedId())( }, ]); - expect( - await storage.transaction(tx => - storage.get(tx, { - userEntityRef: 'user-1', - bucket: 'bucket-a', - key: 'key-a', - }), - ), - ).toEqual({ bucket: 'bucket-a', key: 'key-a', value: 'value-a' }); + await expect( + storage.get({ + userEntityRef: 'user-1', + bucket: 'bucket-a', + key: 'key-a', + }), + ).resolves.toEqual({ + bucket: 'bucket-a', + key: 'key-a', + value: 'value-a', + }); }); }); describe('set', () => { it('should insert a new setting', async () => { - await storage.transaction(tx => - storage.set(tx, { - userEntityRef: 'user-1', - bucket: 'bucket-a', - key: 'key-a', - value: 'value-a', - }), - ); + await storage.set({ + userEntityRef: 'user-1', + bucket: 'bucket-a', + key: 'key-a', + value: 'value-a', + }); - expect(await query()).toEqual([ + await expect(query()).resolves.toEqual([ { user_entity_ref: 'user-1', bucket: 'bucket-a', @@ -303,25 +128,21 @@ describe.each(databases.eachSupportedId())( }); it('should overwrite an existing setting', async () => { - await storage.transaction(tx => - storage.set(tx, { - userEntityRef: 'user-1', - bucket: 'bucket-a', - key: 'key-a', - value: 'value-a', - }), - ); + await storage.set({ + userEntityRef: 'user-1', + bucket: 'bucket-a', + key: 'key-a', + value: 'value-a', + }); - await storage.transaction(tx => - storage.set(tx, { - userEntityRef: 'user-1', - bucket: 'bucket-a', - key: 'key-a', - value: 'value-b', - }), - ); + await storage.set({ + userEntityRef: 'user-1', + bucket: 'bucket-a', + key: 'key-a', + value: 'value-b', + }); - expect(await query()).toEqual([ + await expect(query()).resolves.toEqual([ { user_entity_ref: 'user-1', bucket: 'bucket-a', @@ -334,15 +155,13 @@ describe.each(databases.eachSupportedId())( describe('delete', () => { it('should not throw an error if the entry does not exist', async () => { - await expect(() => - storage.transaction(tx => - storage.delete(tx, { - userEntityRef: 'user-1', - bucket: 'bucket-c', - key: 'key-c', - }), - ), - ).not.toThrow(); + await expect( + storage.delete({ + userEntityRef: 'user-1', + bucket: 'bucket-c', + key: 'key-c', + }), + ).resolves.toBeUndefined(); }); it('should return the setting', async () => { @@ -361,14 +180,12 @@ describe.each(databases.eachSupportedId())( }, ]); - await storage.transaction(tx => - storage.delete(tx, { - userEntityRef: 'user-1', - bucket: 'bucket-a', - key: 'key-a', - }), - ); - expect(await query()).toEqual([ + await storage.delete({ + userEntityRef: 'user-1', + bucket: 'bucket-a', + key: 'key-a', + }); + await expect(query()).resolves.toEqual([ { user_entity_ref: 'user-2', bucket: 'bucket-c', diff --git a/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.ts b/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.ts index 961149c9ea..4b46b6383f 100644 --- a/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.ts +++ b/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.ts @@ -42,9 +42,7 @@ export type RawDbUserSettingsRow = { * * @public */ -export class DatabaseUserSettingsStore - implements UserSettingsStore -{ +export class DatabaseUserSettingsStore implements UserSettingsStore { static async create(options: { database: PluginDatabaseManager; }): Promise { @@ -62,93 +60,56 @@ export class DatabaseUserSettingsStore private constructor(private readonly db: Knex) {} - async getAll(tx: Knex.Transaction, opts: { userEntityRef: string }) { - const settings = await tx('user_settings') - .where({ user_entity_ref: opts.userEntityRef }) - .select('bucket', 'key', 'value'); - - return settings; - } - - async deleteAll( - tx: Knex.Transaction, - opts: { userEntityRef: string }, - ): Promise { - await tx('user_settings') - .where({ user_entity_ref: opts.userEntityRef }) - .delete(); - } - - async getBucket( - tx: Knex.Transaction, - opts: { userEntityRef: string; bucket: string }, - ): Promise { - const settings = await tx('user_settings') - .where({ user_entity_ref: opts.userEntityRef, bucket: opts.bucket }) - .select(['bucket', 'key', 'value']); - - return settings; - } - - async deleteBucket( - tx: Knex.Transaction, - opts: { userEntityRef: string; bucket: string }, - ): Promise { - await tx('user_settings') - .where({ user_entity_ref: opts.userEntityRef, bucket: opts.bucket }) - .delete(); - } - - async get( - tx: Knex.Transaction, - opts: { userEntityRef: string; bucket: string; key: string }, - ): Promise { - const setting = await tx('user_settings') + async get(options: { + userEntityRef: string; + bucket: string; + key: string; + }): Promise { + const rows = await this.db('user_settings') .where({ - user_entity_ref: opts.userEntityRef, - bucket: opts.bucket, - key: opts.key, + user_entity_ref: options.userEntityRef, + bucket: options.bucket, + key: options.key, }) .select(['bucket', 'key', 'value']); - if (!setting.length) { + if (!rows.length) { throw new NotFoundError( - `Unable to find '${opts.key}' in bucket '${opts.bucket}'`, + `Unable to find '${options.key}' in bucket '${options.bucket}'`, ); } - return setting[0]; + return rows[0]; } - async set( - tx: Knex.Transaction, - opts: { userEntityRef: string; bucket: string; key: string; value: string }, - ): Promise { - await tx('user_settings') + async set(options: { + userEntityRef: string; + bucket: string; + key: string; + value: string; + }): Promise { + await this.db('user_settings') .insert({ - user_entity_ref: opts.userEntityRef, - bucket: opts.bucket, - key: opts.key, - value: opts.value, + user_entity_ref: options.userEntityRef, + bucket: options.bucket, + key: options.key, + value: options.value, }) .onConflict(['user_entity_ref', 'bucket', 'key']) - .merge({ value: opts.value }); + .merge(['value']); } - async delete( - tx: Knex.Transaction, - opts: { userEntityRef: string; bucket: string; key: string }, - ): Promise { - await tx('user_settings') + async delete(options: { + userEntityRef: string; + bucket: string; + key: string; + }): Promise { + await this.db('user_settings') .where({ - user_entity_ref: opts.userEntityRef, - bucket: opts.bucket, - key: opts.key, + user_entity_ref: options.userEntityRef, + bucket: options.bucket, + key: options.key, }) .delete(); } - - async transaction(fn: (tx: Knex.Transaction) => Promise) { - return await this.db.transaction(fn); - } } diff --git a/plugins/user-settings-backend/src/database/UserSettingsStore.ts b/plugins/user-settings-backend/src/database/UserSettingsStore.ts index b86673270f..d14af091ef 100644 --- a/plugins/user-settings-backend/src/database/UserSettingsStore.ts +++ b/plugins/user-settings-backend/src/database/UserSettingsStore.ts @@ -15,7 +15,7 @@ */ /** - * @public + * A single setting in a bucket */ export type UserSetting = { bucket: string; @@ -25,41 +25,24 @@ export type UserSetting = { /** * Store definition for the user settings. - * - * @public */ -export interface UserSettingsStore { - transaction(fn: (tx: Transaction) => Promise): Promise; +export interface UserSettingsStore { + get(options: { + userEntityRef: string; + bucket: string; + key: string; + }): Promise; - get( - tx: Transaction, - opts: { userEntityRef: string; bucket: string; key: string }, - ): Promise; + set(options: { + userEntityRef: string; + bucket: string; + key: string; + value: string; + }): Promise; - set( - tx: Transaction, - opts: { userEntityRef: string; bucket: string; key: string; value: string }, - ): Promise; - - delete( - tx: Transaction, - opts: { userEntityRef: string; bucket: string; key: string }, - ): Promise; - - getBucket( - tx: Transaction, - opts: { userEntityRef: string; bucket: string }, - ): Promise; - - deleteBucket( - tx: Transaction, - opts: { userEntityRef: string; bucket: string }, - ): Promise; - - getAll( - tx: Transaction, - opts: { userEntityRef: string }, - ): Promise; - - deleteAll(tx: Transaction, opts: { userEntityRef: string }): Promise; + delete(options: { + userEntityRef: string; + bucket: string; + key: string; + }): Promise; } diff --git a/plugins/user-settings-backend/src/database/index.ts b/plugins/user-settings-backend/src/database/index.ts index 2552b7db13..813cdeaae3 100644 --- a/plugins/user-settings-backend/src/database/index.ts +++ b/plugins/user-settings-backend/src/database/index.ts @@ -14,8 +14,4 @@ * limitations under the License. */ -export { - DatabaseUserSettingsStore, - type RawDbUserSettingsRow, -} from './DatabaseUserSettingsStore'; -export type { UserSettingsStore, UserSetting } from './UserSettingsStore'; +export {}; diff --git a/plugins/user-settings-backend/src/service/router.test.ts b/plugins/user-settings-backend/src/service/router.test.ts index c1feb284a0..7640714477 100644 --- a/plugins/user-settings-backend/src/service/router.test.ts +++ b/plugins/user-settings-backend/src/service/router.test.ts @@ -14,20 +14,14 @@ * limitations under the License. */ -import { AuthenticationError } from '@backstage/errors'; import { IdentityClient } from '@backstage/plugin-auth-node'; import express from 'express'; import request from 'supertest'; -import { UserSettingsStore } from '../database'; -import { createRouter } from './router'; +import { UserSettingsStore } from '../database/UserSettingsStore'; +import { createRouterInternal } from './router'; describe('createRouter', () => { - const userSettingsStore: jest.Mocked> = { - transaction: jest.fn(), - deleteAll: jest.fn(), - getAll: jest.fn(), - getBucket: jest.fn(), - deleteBucket: jest.fn(), + const userSettingsStore: jest.Mocked = { get: jest.fn(), set: jest.fn(), delete: jest.fn(), @@ -40,9 +34,7 @@ describe('createRouter', () => { let app: express.Express; beforeEach(async () => { - userSettingsStore.transaction.mockImplementation(fn => fn('tx')); - - const router = await createRouter({ + const router = await createRouterInternal({ userSettingsStore, identity: identityClient as IdentityClient, }); @@ -54,147 +46,6 @@ describe('createRouter', () => { jest.resetAllMocks(); }); - describe('GET /buckets/', () => { - it('returns ok', async () => { - const settings = [ - { bucket: 'a', key: 'a', value: 'a' }, - { bucket: 'b', key: 'b', value: 'b' }, - ]; - authenticateMock.mockResolvedValue({ - identity: { userEntityRef: 'user-1' }, - }); - - userSettingsStore.getAll.mockResolvedValue(settings); - - const responses = await request(app) - .get('/buckets/') - .set('Authorization', 'Bearer foo'); - - expect(responses.status).toEqual(200); - expect(responses.body).toEqual(settings); - - expect(authenticateMock).toHaveBeenCalledWith('foo'); - expect(userSettingsStore.getAll).toHaveBeenCalledTimes(1); - expect(userSettingsStore.getAll).toHaveBeenCalledWith('tx', { - userEntityRef: 'user-1', - }); - }); - - it('returns an error if the Authorization header is missing', async () => { - const responses = await request(app).get('/buckets/'); - - expect(responses.status).toEqual(401); - expect(userSettingsStore.getAll).not.toHaveBeenCalled(); - }); - - it('returns an error if the token is not valid', async () => { - authenticateMock.mockRejectedValue( - new AuthenticationError('Invalid token'), - ); - - const responses = await request(app) - .get('/buckets/') - .set('Authorization', 'Bearer foo'); - - expect(responses.status).toEqual(401); - expect(userSettingsStore.getAll).not.toHaveBeenCalled(); - }); - }); - - describe('DELETE /buckets/', () => { - it('returns ok', async () => { - authenticateMock.mockResolvedValue({ - identity: { userEntityRef: 'user-1' }, - }); - - userSettingsStore.deleteAll.mockResolvedValue(); - - const responses = await request(app) - .delete('/buckets/') - .set('Authorization', 'Bearer foo'); - - expect(responses.status).toEqual(204); - - expect(authenticateMock).toHaveBeenCalledWith('foo'); - expect(userSettingsStore.deleteAll).toHaveBeenCalledTimes(1); - expect(userSettingsStore.deleteAll).toHaveBeenCalledWith('tx', { - userEntityRef: 'user-1', - }); - }); - - it('returns an error if the Authorization header is missing', async () => { - const responses = await request(app).delete('/buckets/'); - - expect(responses.status).toEqual(401); - expect(userSettingsStore.getAll).not.toHaveBeenCalled(); - }); - }); - - describe('GET /buckets/:bucket', () => { - it('returns ok', async () => { - const settings = [ - { bucket: 'my-bucket', key: 'a', value: 'a' }, - { bucket: 'my-bucket', key: 'b', value: 'b' }, - ]; - authenticateMock.mockResolvedValue({ - identity: { userEntityRef: 'user-1' }, - }); - - userSettingsStore.getBucket.mockResolvedValue(settings); - - const responses = await request(app) - .get('/buckets/my-bucket') - .set('Authorization', 'Bearer foo'); - - expect(responses.status).toEqual(200); - expect(responses.body).toEqual(settings); - - expect(authenticateMock).toHaveBeenCalledWith('foo'); - expect(userSettingsStore.getBucket).toHaveBeenCalledTimes(1); - expect(userSettingsStore.getBucket).toHaveBeenCalledWith('tx', { - userEntityRef: 'user-1', - bucket: 'my-bucket', - }); - }); - - it('returns an error if the Authorization header is missing', async () => { - const responses = await request(app).get('/buckets/my-bucket'); - - expect(responses.status).toEqual(401); - expect(userSettingsStore.getBucket).not.toHaveBeenCalled(); - }); - }); - - describe('DELETE /buckets/:bucket', () => { - it('returns ok', async () => { - authenticateMock.mockResolvedValue({ - identity: { userEntityRef: 'user-1' }, - }); - - userSettingsStore.deleteBucket.mockResolvedValue(); - - const responses = await request(app) - .delete('/buckets/my-bucket') - .set('Authorization', 'Bearer foo'); - - expect(responses.status).toEqual(204); - - expect(authenticateMock).toHaveBeenCalledWith('foo'); - expect(userSettingsStore.deleteBucket).toHaveBeenCalledTimes(1); - expect(userSettingsStore.deleteBucket).toHaveBeenCalledWith('tx', { - userEntityRef: 'user-1', - bucket: 'my-bucket', - }); - }); - - it('returns an error if the Authorization header is missing', async () => { - const responses = await request(app).delete('/buckets/my-bucket'); - - expect(responses.status).toEqual(401); - expect(userSettingsStore.deleteBucket).not.toHaveBeenCalled(); - }); - }); - describe('GET /buckets/:bucket/keys/:key', () => { it('returns ok', async () => { const setting = { bucket: 'my-bucket', key: 'my-key', value: 'a' }; @@ -213,7 +64,7 @@ describe('createRouter', () => { expect(authenticateMock).toHaveBeenCalledWith('foo'); expect(userSettingsStore.get).toHaveBeenCalledTimes(1); - expect(userSettingsStore.get).toHaveBeenCalledWith('tx', { + expect(userSettingsStore.get).toHaveBeenCalledWith({ userEntityRef: 'user-1', bucket: 'my-bucket', key: 'my-key', @@ -246,7 +97,7 @@ describe('createRouter', () => { expect(authenticateMock).toHaveBeenCalledWith('foo'); expect(userSettingsStore.delete).toHaveBeenCalledTimes(1); - expect(userSettingsStore.delete).toHaveBeenCalledWith('tx', { + expect(userSettingsStore.delete).toHaveBeenCalledWith({ userEntityRef: 'user-1', bucket: 'my-bucket', key: 'my-key', @@ -283,14 +134,14 @@ describe('createRouter', () => { expect(authenticateMock).toHaveBeenCalledWith('foo'); expect(userSettingsStore.set).toHaveBeenCalledTimes(1); - expect(userSettingsStore.set).toHaveBeenCalledWith('tx', { + expect(userSettingsStore.set).toHaveBeenCalledWith({ userEntityRef: 'user-1', bucket: 'my-bucket', key: 'my-key', value: 'a', }); expect(userSettingsStore.get).toHaveBeenCalledTimes(1); - expect(userSettingsStore.get).toHaveBeenCalledWith('tx', { + expect(userSettingsStore.get).toHaveBeenCalledWith({ userEntityRef: 'user-1', bucket: 'my-bucket', key: 'my-key', diff --git a/plugins/user-settings-backend/src/service/router.ts b/plugins/user-settings-backend/src/service/router.ts index 58580eddda..d677ee4bfb 100644 --- a/plugins/user-settings-backend/src/service/router.ts +++ b/plugins/user-settings-backend/src/service/router.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { errorHandler } from '@backstage/backend-common'; +import { errorHandler, PluginDatabaseManager } from '@backstage/backend-common'; import { AuthenticationError, InputError } from '@backstage/errors'; import { getBearerTokenFromAuthorizationHeader, @@ -22,13 +22,14 @@ import { } from '@backstage/plugin-auth-node'; import express, { Request } from 'express'; import Router from 'express-promise-router'; -import { UserSettingsStore } from '../database'; +import { DatabaseUserSettingsStore } from '../database/DatabaseUserSettingsStore'; +import { UserSettingsStore } from '../database/UserSettingsStore'; /** * @public */ -export interface RouterOptions { - userSettingsStore: UserSettingsStore; +export interface RouterOptions { + database: PluginDatabaseManager; identity: IdentityClient; } @@ -37,10 +38,23 @@ export interface RouterOptions { * * @public */ -export async function createRouter( - options: RouterOptions, +export async function createRouter( + options: RouterOptions, ): Promise { - const { userSettingsStore, identity } = options; + const userSettingsStore = await DatabaseUserSettingsStore.create({ + database: options.database, + }); + + return await createRouterInternal({ + userSettingsStore, + identity: options.identity, + }); +} + +export async function createRouterInternal(options: { + identity: IdentityClient; + userSettingsStore: UserSettingsStore; +}): Promise { const router = Router(); router.use(express.json()); @@ -57,65 +71,21 @@ export async function createRouter( } // throws an AuthenticationError in case the token is invalid - const user = await identity.authenticate(token); + const user = await options.identity.authenticate(token); return user.identity.userEntityRef; }; - // get all user related settings - router.get('/buckets', async (req, res) => { - const userEntityRef = await getUserEntityRef(req); - - const settings = await userSettingsStore.transaction(tx => - userSettingsStore.getAll(tx, { userEntityRef }), - ); - - res.json(settings); - }); - - // remove all user related settings - router.delete('/buckets', async (req, res) => { - const userEntityRef = await getUserEntityRef(req); - - await userSettingsStore.transaction(tx => - userSettingsStore.deleteAll(tx, { userEntityRef }), - ); - - res.send(204).end(); - }); - - // get a single bucket - router.get('/buckets/:bucket', async (req, res) => { - const userEntityRef = await getUserEntityRef(req); - const { bucket } = req.params; - - const settings = await userSettingsStore.transaction(tx => - userSettingsStore.getBucket(tx, { userEntityRef, bucket }), - ); - - res.json(settings); - }); - - // delete a whole bucket - router.delete('/buckets/:bucket', async (req, res) => { - const userEntityRef = await getUserEntityRef(req); - const { bucket } = req.params; - - await userSettingsStore.transaction(tx => - userSettingsStore.deleteBucket(tx, { userEntityRef, bucket }), - ); - - res.status(204).end(); - }); - // get a single value router.get('/buckets/:bucket/keys/:key', async (req, res) => { const userEntityRef = await getUserEntityRef(req); const { bucket, key } = req.params; - const setting = await userSettingsStore.transaction(tx => - userSettingsStore.get(tx, { userEntityRef, bucket, key }), - ); + const setting = await options.userSettingsStore.get({ + userEntityRef, + bucket, + key, + }); res.json(setting); }); @@ -130,14 +100,16 @@ export async function createRouter( throw new InputError('Value must be a string'); } - const setting = await userSettingsStore.transaction(async tx => { - await userSettingsStore.set(tx, { - userEntityRef, - bucket, - key, - value, - }); - return userSettingsStore.get(tx, { userEntityRef, bucket, key }); + await options.userSettingsStore.set({ + userEntityRef, + bucket, + key, + value, + }); + const setting = await options.userSettingsStore.get({ + userEntityRef, + bucket, + key, }); res.json(setting); @@ -148,9 +120,7 @@ export async function createRouter( const userEntityRef = await getUserEntityRef(req); const { bucket, key } = req.params; - await userSettingsStore.transaction(tx => - userSettingsStore.delete(tx, { userEntityRef, bucket, key }), - ); + await options.userSettingsStore.delete({ userEntityRef, bucket, key }); res.send(204).end(); }); diff --git a/plugins/user-settings-backend/src/service/standaloneServer.ts b/plugins/user-settings-backend/src/service/standaloneServer.ts index 3cd3704ca8..28af3a163f 100644 --- a/plugins/user-settings-backend/src/service/standaloneServer.ts +++ b/plugins/user-settings-backend/src/service/standaloneServer.ts @@ -20,8 +20,8 @@ import { Router } from 'express'; import { Server } from 'http'; import Knex from 'knex'; import { Logger } from 'winston'; -import { DatabaseUserSettingsStore } from '../database'; -import { createRouter } from './router'; +import { DatabaseUserSettingsStore } from '../database/DatabaseUserSettingsStore'; +import { createRouterInternal } from './router'; export interface ServerOptions { port: number; @@ -50,7 +50,7 @@ export async function startStandaloneServer( }), } as IdentityClient; - const router = await createRouter({ + const router = await createRouterInternal({ userSettingsStore: await DatabaseUserSettingsStore.create({ database: { getClient: async () => database }, }), From 9018da59a91126cc07666bb4b77170f962d8425d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 7 Sep 2022 09:45:43 +0200 Subject: [PATCH 10/14] do not double encode the value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .../StorageApi/UserSettingsStorage.test.ts | 17 +++++------------ .../StorageApi/UserSettingsStorage.ts | 6 +++--- plugins/user-settings-backend/package.json | 1 + .../database/DatabaseUserSettingsStore.test.ts | 14 +++++++------- .../src/database/DatabaseUserSettingsStore.ts | 13 +++++++++---- .../src/database/UserSettingsStore.ts | 6 ++++-- .../src/service/router.test.ts | 4 ++-- .../user-settings-backend/src/service/router.ts | 4 ++-- yarn.lock | 1 + 9 files changed, 34 insertions(+), 32 deletions(-) diff --git a/packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.test.ts b/packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.test.ts index cc48fc925c..d8ea87b38d 100644 --- a/packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.test.ts +++ b/packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.test.ts @@ -49,14 +49,6 @@ describe('Persistent Storage API', () => { }); }; - beforeEach(() => { - server.use( - rest.get(`${mockBaseUrl}/buckets/`, async (_req, res, ctx) => { - return res(ctx.json([])); - }), - ); - }); - afterEach(() => { jest.resetAllMocks(); }); @@ -90,7 +82,8 @@ describe('Persistent Storage API', () => { `${mockBaseUrl}/buckets/:bucket/keys/:key`, async (req, res, ctx) => { const body = await req.json(); - const data = { value: JSON.stringify(dummyValue) }; + const data = { value: dummyValue }; + expect(body).toEqual(data); return res(ctx.json(data)); @@ -113,7 +106,7 @@ describe('Persistent Storage API', () => { `${mockBaseUrl}/buckets/:bucket/keys/:key`, async (req, res, ctx) => { const body = await req.json(); - const data = { value: JSON.stringify(dummyValue) }; + const data = { value: dummyValue }; expect(body).toEqual(data); return res(ctx.json(data)); @@ -136,7 +129,7 @@ describe('Persistent Storage API', () => { `${mockBaseUrl}/buckets/:bucket/keys/:key`, async (req, res, ctx) => { const body = await req.json(); - const data = { value: JSON.stringify(mockData) }; + const data = { value: mockData }; expect(body).toEqual(data); return res(ctx.json(data)); @@ -315,7 +308,7 @@ describe('Persistent Storage API', () => { rest.get( `${mockBaseUrl}/buckets/:bucket/keys/:key`, async (_req, res, ctx) => { - return res(ctx.text(JSON.stringify({ value: JSON.stringify(data) }))); + return res(ctx.json({ value: data })); }, ), ); diff --git a/packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.ts b/packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.ts index 0763ad8187..d01d5021ee 100644 --- a/packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.ts +++ b/packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.ts @@ -106,7 +106,7 @@ export class UserSettingsStorage implements StorageApi { async set(key: string, data: T): Promise { const fetchUrl = await this.getFetchUrl(key); - const body = JSON.stringify({ value: JSON.stringify(data) }); + const body = JSON.stringify({ value: data }); const response = await this.fetchApi.fetch(fetchUrl, { method: 'PUT', @@ -121,7 +121,7 @@ export class UserSettingsStorage implements StorageApi { this.notifyChanges({ key, - value: JSON.parse(value), + value, presence: 'present', }); } @@ -165,7 +165,7 @@ export class UserSettingsStorage implements StorageApi { try { const { value: rawValue } = await response.json(); - const value = JSON.parse(rawValue, (_key, val) => { + const value = JSON.parse(JSON.stringify(rawValue), (_key, val) => { if (typeof val === 'object' && val !== null) { Object.freeze(val); } diff --git a/plugins/user-settings-backend/package.json b/plugins/user-settings-backend/package.json index 41178d49b8..690d1d13c9 100644 --- a/plugins/user-settings-backend/package.json +++ b/plugins/user-settings-backend/package.json @@ -32,6 +32,7 @@ "@backstage/catalog-model": "^1.1.0", "@backstage/errors": "^1.1.0", "@backstage/plugin-auth-node": "^0.2.5-next.1", + "@backstage/types": "^1.0.0", "@types/express": "^4.17.6", "express": "^4.17.1", "express-promise-router": "^4.1.0", diff --git a/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.test.ts b/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.test.ts index e98ee0010e..eb50e26179 100644 --- a/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.test.ts +++ b/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.test.ts @@ -84,13 +84,13 @@ describe.each(databases.eachSupportedId())( user_entity_ref: 'user-1', bucket: 'bucket-a', key: 'key-a', - value: 'value-a', + value: JSON.stringify('value-a'), }, { user_entity_ref: 'user-2', bucket: 'bucket-c', key: 'key-c', - value: 'value-c', + value: JSON.stringify('value-c'), }, ]); @@ -122,7 +122,7 @@ describe.each(databases.eachSupportedId())( user_entity_ref: 'user-1', bucket: 'bucket-a', key: 'key-a', - value: 'value-a', + value: JSON.stringify('value-a'), }, ]); }); @@ -147,7 +147,7 @@ describe.each(databases.eachSupportedId())( user_entity_ref: 'user-1', bucket: 'bucket-a', key: 'key-a', - value: 'value-b', + value: JSON.stringify('value-b'), }, ]); }); @@ -170,13 +170,13 @@ describe.each(databases.eachSupportedId())( user_entity_ref: 'user-1', bucket: 'bucket-a', key: 'key-a', - value: 'value-a', + value: JSON.stringify('value-a'), }, { user_entity_ref: 'user-2', bucket: 'bucket-c', key: 'key-c', - value: 'value-c', + value: JSON.stringify('value-c'), }, ]); @@ -190,7 +190,7 @@ describe.each(databases.eachSupportedId())( user_entity_ref: 'user-2', bucket: 'bucket-c', key: 'key-c', - value: 'value-c', + value: JSON.stringify('value-c'), }, ]); }); diff --git a/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.ts b/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.ts index 4b46b6383f..b0fe8fdecc 100644 --- a/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.ts +++ b/plugins/user-settings-backend/src/database/DatabaseUserSettingsStore.ts @@ -19,8 +19,9 @@ import { resolvePackagePath, } from '@backstage/backend-common'; import { NotFoundError } from '@backstage/errors'; +import { JsonValue } from '@backstage/types'; import { Knex } from 'knex'; -import { type UserSetting, UserSettingsStore } from './UserSettingsStore'; +import { UserSettingsStore, type UserSetting } from './UserSettingsStore'; const migrationsDir = resolvePackagePath( '@backstage/plugin-user-settings-backend', @@ -79,21 +80,25 @@ export class DatabaseUserSettingsStore implements UserSettingsStore { ); } - return rows[0]; + return { + bucket: rows[0].bucket, + key: rows[0].key, + value: JSON.parse(rows[0].value), + }; } async set(options: { userEntityRef: string; bucket: string; key: string; - value: string; + value: JsonValue; }): Promise { await this.db('user_settings') .insert({ user_entity_ref: options.userEntityRef, bucket: options.bucket, key: options.key, - value: options.value, + value: JSON.stringify(options.value), }) .onConflict(['user_entity_ref', 'bucket', 'key']) .merge(['value']); diff --git a/plugins/user-settings-backend/src/database/UserSettingsStore.ts b/plugins/user-settings-backend/src/database/UserSettingsStore.ts index d14af091ef..289c37523e 100644 --- a/plugins/user-settings-backend/src/database/UserSettingsStore.ts +++ b/plugins/user-settings-backend/src/database/UserSettingsStore.ts @@ -14,13 +14,15 @@ * limitations under the License. */ +import { JsonValue } from '@backstage/types'; + /** * A single setting in a bucket */ export type UserSetting = { bucket: string; key: string; - value: string; + value: JsonValue; }; /** @@ -37,7 +39,7 @@ export interface UserSettingsStore { userEntityRef: string; bucket: string; key: string; - value: string; + value: JsonValue; }): Promise; delete(options: { diff --git a/plugins/user-settings-backend/src/service/router.test.ts b/plugins/user-settings-backend/src/service/router.test.ts index 7640714477..c8df77a845 100644 --- a/plugins/user-settings-backend/src/service/router.test.ts +++ b/plugins/user-settings-backend/src/service/router.test.ts @@ -148,7 +148,7 @@ describe('createRouter', () => { }); }); - it('returns an error if the value is not a string', async () => { + it('returns an error if the value is not given', async () => { authenticateMock.mockResolvedValue({ identity: { userEntityRef: 'user-1' }, }); @@ -156,7 +156,7 @@ describe('createRouter', () => { const responses = await request(app) .put('/buckets/my-bucket/keys/my-key') .set('Authorization', 'Bearer foo') - .send({ value: { invalid: 'because not a string' } }); + .send({}); expect(responses.status).toEqual(400); diff --git a/plugins/user-settings-backend/src/service/router.ts b/plugins/user-settings-backend/src/service/router.ts index d677ee4bfb..487a6b4fee 100644 --- a/plugins/user-settings-backend/src/service/router.ts +++ b/plugins/user-settings-backend/src/service/router.ts @@ -96,8 +96,8 @@ export async function createRouterInternal(options: { const { bucket, key } = req.params; const { value } = req.body; - if (typeof value !== 'string') { - throw new InputError('Value must be a string'); + if (value === undefined) { + throw new InputError('Missing required field "value"'); } await options.userSettingsStore.set({ diff --git a/yarn.lock b/yarn.lock index 9df599d106..4d821c0e64 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7321,6 +7321,7 @@ __metadata: "@backstage/cli": ^0.19.0-next.1 "@backstage/errors": ^1.1.0 "@backstage/plugin-auth-node": ^0.2.5-next.1 + "@backstage/types": ^1.0.0 "@types/express": ^4.17.6 "@types/supertest": ^2.0.8 express: ^4.17.1 From 0f88eab1b7bca8dc5bc2f3c546aa29da23890b0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 7 Sep 2022 11:53:53 +0200 Subject: [PATCH 11/14] use the IdentityApi properly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- plugins/user-settings-backend/README.md | 61 +++++++------------ plugins/user-settings-backend/api-report.md | 4 +- .../src/service/router.test.ts | 60 ++++++++++++------ .../src/service/router.ts | 22 +++---- .../src/service/standaloneServer.ts | 32 +++++----- 5 files changed, 89 insertions(+), 90 deletions(-) diff --git a/plugins/user-settings-backend/README.md b/plugins/user-settings-backend/README.md index 05a8c15c6d..5a98c86326 100644 --- a/plugins/user-settings-backend/README.md +++ b/plugins/user-settings-backend/README.md @@ -18,64 +18,47 @@ yarn --cwd packages/backend add @backstage/plugin-user-settings-backend ```ts // packages/backend/src/plugins/userSettings.ts -import { IdentityClient } from '@backstage/plugin-auth-node'; -import { - createRouter, - createUserSettingsStore, -} from '@backstage/plugin-user-settings-backend'; -import { Router } from 'express'; - +import { createRouter } from '@backstage/plugin-user-settings-backend'; import { PluginEnvironment } from '../types'; -export default async function createPlugin({ - database, - discovery, -}: PluginEnvironment): Promise { - const identity = IdentityClient.create({ - discovery, - issuer: await discovery.getExternalBaseUrl('auth'), - }); - +export default async function createPlugin(env: PluginEnvironment) { return await createRouter({ - userSettingsStore: await createUserSettingsStore(database), - identity, + database: env.database, + identity: env.identity, }); } ``` -3. Add the new routes to your backend by modifying the - `packages/backend/src/index.ts`: +3. Add the new routes to your backend by modifying `packages/backend/src/index.ts`: ```diff -// packages/backend/src/index.ts -+ import userSettings from './plugins/userSettings'; -async function main() { -+ const userSettingsEnv = useHotMemoize(module, () => -+ createEnv('userSettings'), -+ ); - const apiRouter = Router(); -+ apiRouter.use('/user-settings', await userSettings(userSettingsEnv)); + // packages/backend/src/index.ts ++import userSettings from './plugins/userSettings'; + async function main() { ++ const userSettingsEnv = useHotMemoize(module, () => createEnv('userSettings')); + const apiRouter = Router(); ++ apiRouter.use('/user-settings', await userSettings(userSettingsEnv)); } ``` ## Setup app To make use of the user settings backend, replace the `WebStorage` with the -`PersistentStorage` by using the `storageApiRef`. +`UserSettingsStorage` by using the `storageApiRef`. ```diff -// packages/app/src/apis.ts -import { - AnyApiFactory, - createApiFactory, + // packages/app/src/apis.ts + import { + AnyApiFactory, + createApiFactory, + discoveryApiRef, + fetchApiRef - errorApiRef, + errorApiRef, + storageApiRef, -} from '@backstage/core-plugin-api'; -+ import { PersistentStorage } from '@backstage/core-app-api'; + } from '@backstage/core-plugin-api'; ++import { UserSettingsStorage } from '@backstage/core-app-api'; -export const apis: AnyApiFactory[] = [ + export const apis: AnyApiFactory[] = [ + createApiFactory({ + api: storageApiRef, + deps: { @@ -84,9 +67,9 @@ export const apis: AnyApiFactory[] = [ + fetchApi: fetchApiRef, + }, + factory: ({ discoveryApi, errorApi, fetchApi }) => -+ PersistentStorage.create({ discoveryApi, errorApi, fetchApi }), ++ UserSettingsStorage.create({ discoveryApi, errorApi, fetchApi }), + }), -]; + ]; ``` ## Development diff --git a/plugins/user-settings-backend/api-report.md b/plugins/user-settings-backend/api-report.md index 15e8b5eb5b..1bc37bbec7 100644 --- a/plugins/user-settings-backend/api-report.md +++ b/plugins/user-settings-backend/api-report.md @@ -4,7 +4,7 @@ ```ts import express from 'express'; -import { IdentityClient } from '@backstage/plugin-auth-node'; +import { IdentityApi } from '@backstage/plugin-auth-node'; import { PluginDatabaseManager } from '@backstage/backend-common'; // @public @@ -15,7 +15,7 @@ export interface RouterOptions { // (undocumented) database: PluginDatabaseManager; // (undocumented) - identity: IdentityClient; + identity: IdentityApi; } // (No @packageDocumentation comment for this package) diff --git a/plugins/user-settings-backend/src/service/router.test.ts b/plugins/user-settings-backend/src/service/router.test.ts index c8df77a845..64cdfcc465 100644 --- a/plugins/user-settings-backend/src/service/router.test.ts +++ b/plugins/user-settings-backend/src/service/router.test.ts @@ -14,7 +14,10 @@ * limitations under the License. */ -import { IdentityClient } from '@backstage/plugin-auth-node'; +import { + BackstageIdentityResponse, + IdentityApi, +} from '@backstage/plugin-auth-node'; import express from 'express'; import request from 'supertest'; import { UserSettingsStore } from '../database/UserSettingsStore'; @@ -26,9 +29,12 @@ describe('createRouter', () => { set: jest.fn(), delete: jest.fn(), }; - const authenticateMock = jest.fn(); - const identityClient: jest.Mocked> = { - authenticate: authenticateMock, + const getIdentityMock = jest.fn< + Promise, + any + >(); + const identityApi: jest.Mocked> = { + getIdentity: getIdentityMock, }; let app: express.Express; @@ -36,7 +42,7 @@ describe('createRouter', () => { beforeEach(async () => { const router = await createRouterInternal({ userSettingsStore, - identity: identityClient as IdentityClient, + identity: identityApi as IdentityApi, }); app = express().use(router); @@ -49,8 +55,13 @@ describe('createRouter', () => { describe('GET /buckets/:bucket/keys/:key', () => { it('returns ok', async () => { const setting = { bucket: 'my-bucket', key: 'my-key', value: 'a' }; - authenticateMock.mockResolvedValue({ - identity: { userEntityRef: 'user-1' }, + getIdentityMock.mockResolvedValue({ + token: 'a', + identity: { + type: 'user', + ownershipEntityRefs: [], + userEntityRef: 'user-1', + }, }); userSettingsStore.get.mockResolvedValue(setting); @@ -62,7 +73,7 @@ describe('createRouter', () => { expect(responses.status).toEqual(200); expect(responses.body).toEqual(setting); - expect(authenticateMock).toHaveBeenCalledWith('foo'); + expect(getIdentityMock).toHaveBeenCalledTimes(1); expect(userSettingsStore.get).toHaveBeenCalledTimes(1); expect(userSettingsStore.get).toHaveBeenCalledWith({ userEntityRef: 'user-1', @@ -83,8 +94,13 @@ describe('createRouter', () => { describe('DELETE /buckets/:bucket/keys/:key', () => { it('returns ok', async () => { - authenticateMock.mockResolvedValue({ - identity: { userEntityRef: 'user-1' }, + getIdentityMock.mockResolvedValue({ + token: 'a', + identity: { + type: 'user', + ownershipEntityRefs: [], + userEntityRef: 'user-1', + }, }); userSettingsStore.delete.mockResolvedValue(); @@ -95,7 +111,7 @@ describe('createRouter', () => { expect(responses.status).toEqual(204); - expect(authenticateMock).toHaveBeenCalledWith('foo'); + expect(getIdentityMock).toHaveBeenCalledTimes(1); expect(userSettingsStore.delete).toHaveBeenCalledTimes(1); expect(userSettingsStore.delete).toHaveBeenCalledWith({ userEntityRef: 'user-1', @@ -117,8 +133,13 @@ describe('createRouter', () => { describe('PUT /buckets/:bucket/keys/:key', () => { it('returns ok', async () => { const setting = { bucket: 'my-bucket', key: 'my-key', value: 'a' }; - authenticateMock.mockResolvedValue({ - identity: { userEntityRef: 'user-1' }, + getIdentityMock.mockResolvedValue({ + token: 'a', + identity: { + type: 'user', + ownershipEntityRefs: [], + userEntityRef: 'user-1', + }, }); userSettingsStore.set.mockResolvedValue(); @@ -132,7 +153,7 @@ describe('createRouter', () => { expect(responses.status).toEqual(200); expect(responses.body).toEqual(setting); - expect(authenticateMock).toHaveBeenCalledWith('foo'); + expect(getIdentityMock).toHaveBeenCalledTimes(1); expect(userSettingsStore.set).toHaveBeenCalledTimes(1); expect(userSettingsStore.set).toHaveBeenCalledWith({ userEntityRef: 'user-1', @@ -149,8 +170,13 @@ describe('createRouter', () => { }); it('returns an error if the value is not given', async () => { - authenticateMock.mockResolvedValue({ - identity: { userEntityRef: 'user-1' }, + getIdentityMock.mockResolvedValue({ + token: 'a', + identity: { + type: 'user', + ownershipEntityRefs: [], + userEntityRef: 'user-1', + }, }); const responses = await request(app) @@ -160,7 +186,7 @@ describe('createRouter', () => { expect(responses.status).toEqual(400); - expect(authenticateMock).toHaveBeenCalledWith('foo'); + expect(getIdentityMock).toHaveBeenCalledTimes(1); expect(userSettingsStore.set).not.toHaveBeenCalled(); }); diff --git a/plugins/user-settings-backend/src/service/router.ts b/plugins/user-settings-backend/src/service/router.ts index 487a6b4fee..6792042585 100644 --- a/plugins/user-settings-backend/src/service/router.ts +++ b/plugins/user-settings-backend/src/service/router.ts @@ -16,10 +16,7 @@ import { errorHandler, PluginDatabaseManager } from '@backstage/backend-common'; import { AuthenticationError, InputError } from '@backstage/errors'; -import { - getBearerTokenFromAuthorizationHeader, - IdentityClient, -} from '@backstage/plugin-auth-node'; +import { IdentityApi } from '@backstage/plugin-auth-node'; import express, { Request } from 'express'; import Router from 'express-promise-router'; import { DatabaseUserSettingsStore } from '../database/DatabaseUserSettingsStore'; @@ -30,7 +27,7 @@ import { UserSettingsStore } from '../database/UserSettingsStore'; */ export interface RouterOptions { database: PluginDatabaseManager; - identity: IdentityClient; + identity: IdentityApi; } /** @@ -52,7 +49,7 @@ export async function createRouter( } export async function createRouterInternal(options: { - identity: IdentityClient; + identity: IdentityApi; userSettingsStore: UserSettingsStore; }): Promise { const router = Router(); @@ -62,18 +59,13 @@ export async function createRouterInternal(options: { * Helper method to extract the userEntityRef from the request. */ const getUserEntityRef = async (req: Request): Promise => { - const token = getBearerTokenFromAuthorizationHeader( - req.header('authorization'), - ); - - if (!token) { + // throws an AuthenticationError in case the token exists but is invalid + const identity = await options.identity.getIdentity({ request: req }); + if (!identity) { throw new AuthenticationError(`Missing token in 'authorization' header`); } - // throws an AuthenticationError in case the token is invalid - const user = await options.identity.authenticate(token); - - return user.identity.userEntityRef; + return identity.identity.userEntityRef; }; // get a single value diff --git a/plugins/user-settings-backend/src/service/standaloneServer.ts b/plugins/user-settings-backend/src/service/standaloneServer.ts index 28af3a163f..6dd8c7f72d 100644 --- a/plugins/user-settings-backend/src/service/standaloneServer.ts +++ b/plugins/user-settings-backend/src/service/standaloneServer.ts @@ -15,8 +15,7 @@ */ import { createServiceBuilder, useHotMemoize } from '@backstage/backend-common'; -import { IdentityClient } from '@backstage/plugin-auth-node'; -import { Router } from 'express'; +import { IdentityApi } from '@backstage/plugin-auth-node'; import { Server } from 'http'; import Knex from 'knex'; import { Logger } from 'winston'; @@ -44,11 +43,19 @@ export async function startStandaloneServer( logger.debug('Starting application server...'); - const identityMock = { - authenticate: async token => ({ - identity: { userEntityRef: token ?? 'user:default/john_doe' }, - }), - } as IdentityClient; + const identityMock: IdentityApi = { + async getIdentity({ request }) { + const token = request.headers.authorization?.split(' ')[1]; + return { + identity: { + type: 'user', + ownershipEntityRefs: [], + userEntityRef: token || 'user:default/john_doe', + }, + token: token || 'no-token', + }; + }, + }; const router = await createRouterInternal({ userSettingsStore: await DatabaseUserSettingsStore.create({ @@ -57,18 +64,9 @@ export async function startStandaloneServer( identity: identityMock, }); - // set a custom authorization header to simplify the development - const authWrapper = Router(); - authWrapper.use((req, _res, next) => { - req.headers.authorization = - req.headers.authorization ?? 'Bearer user:default/john_doe'; - next(); - }); - authWrapper.use(router); - let service = createServiceBuilder(module) .setPort(options.port) - .addRouter('/user-settings', authWrapper); + .addRouter('/user-settings', router); if (options.enableCors) { service = service.enableCors({ origin: 'http://localhost:3000' }); From 8a57ffc0fa9c1c57d21439d8c28d4abc98b2db54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Thu, 8 Sep 2022 09:20:22 +0200 Subject: [PATCH 12/14] Ensure that we return stable observer references MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/sixty-apes-wait.md | 5 + packages/core-app-api/api-report.md | 5 +- .../StorageApi/UserSettingsStorage.test.ts | 9 +- .../StorageApi/UserSettingsStorage.ts | 118 +++++++++++------- .../implementations/StorageApi/WebStorage.ts | 4 +- .../shortcuts/src/api/LocalStoredShortcuts.ts | 12 +- plugins/user-settings-backend/README.md | 9 +- 7 files changed, 104 insertions(+), 58 deletions(-) create mode 100644 .changeset/sixty-apes-wait.md diff --git a/.changeset/sixty-apes-wait.md b/.changeset/sixty-apes-wait.md new file mode 100644 index 0000000000..1273307519 --- /dev/null +++ b/.changeset/sixty-apes-wait.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-shortcuts': patch +--- + +Ensure that a stable observable is used in the shortcuts API diff --git a/packages/core-app-api/api-report.md b/packages/core-app-api/api-report.md index 90e18df723..39e5388d68 100644 --- a/packages/core-app-api/api-report.md +++ b/packages/core-app-api/api-report.md @@ -550,6 +550,7 @@ export class UserSettingsStorage implements StorageApi { fetchApi: FetchApi; discoveryApi: DiscoveryApi; errorApi: ErrorApi; + identityApi: IdentityApi; namespace?: string; }): UserSettingsStorage; // (undocumented) @@ -579,7 +580,9 @@ export class WebStorage implements StorageApi { // (undocumented) get(key: string): T | undefined; // (undocumented) - observe$(key: string): Observable>; + observe$( + key: string, + ): Observable>; // (undocumented) remove(key: string): Promise; // (undocumented) diff --git a/packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.test.ts b/packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.test.ts index d8ea87b38d..0ebd96d19c 100644 --- a/packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.test.ts +++ b/packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.test.ts @@ -18,6 +18,7 @@ import { DiscoveryApi, ErrorApi, FetchApi, + IdentityApi, StorageApi, } from '@backstage/core-plugin-api'; import { MockFetchApi, setupRequestMockHandlers } from '@backstage/test-utils'; @@ -31,7 +32,12 @@ describe('Persistent Storage API', () => { const mockBaseUrl = 'http://backstage:9191/api'; const mockErrorApi = { post: jest.fn(), error$: jest.fn() }; - const mockDiscoveryApi = { getBaseUrl: async () => mockBaseUrl }; + const mockDiscoveryApi = { + getBaseUrl: async () => mockBaseUrl, + }; + const mockIdentityApi: Partial = { + getCredentials: async () => ({ token: 'a-token' }), + }; const createPersistentStorage = ( args?: Partial<{ @@ -45,6 +51,7 @@ describe('Persistent Storage API', () => { errorApi: mockErrorApi, fetchApi: new MockFetchApi(), discoveryApi: mockDiscoveryApi, + identityApi: mockIdentityApi as IdentityApi, ...args, }); }; diff --git a/packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.ts b/packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.ts index d01d5021ee..b595c524cf 100644 --- a/packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.ts +++ b/packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.ts @@ -18,12 +18,19 @@ import { DiscoveryApi, ErrorApi, FetchApi, + IdentityApi, StorageApi, StorageValueSnapshot, } from '@backstage/core-plugin-api'; import { ResponseError } from '@backstage/errors'; import { JsonValue, Observable } from '@backstage/types'; import ObservableImpl from 'zen-observable'; +import { WebStorage } from './WebStorage'; + +const JSON_HEADERS = { + 'Content-Type': 'application/json; charset=utf-8', + Accept: 'application/json', +}; const buckets = new Map(); @@ -38,26 +45,25 @@ export class UserSettingsStorage implements StorageApi { ZenObservable.SubscriptionObserver> >(); - private readonly observable = new ObservableImpl< - StorageValueSnapshot - >(subscriber => { - this.subscribers.add(subscriber); - return () => { - this.subscribers.delete(subscriber); - }; - }); + private readonly observables = new Map< + string, + Observable> + >(); private constructor( private readonly namespace: string, private readonly fetchApi: FetchApi, private readonly discoveryApi: DiscoveryApi, private readonly errorApi: ErrorApi, + private readonly identityApi: IdentityApi, + private readonly fallback: WebStorage, ) {} static create(options: { fetchApi: FetchApi; discoveryApi: DiscoveryApi; errorApi: ErrorApi; + identityApi: IdentityApi; namespace?: string; }): UserSettingsStorage { return new UserSettingsStorage( @@ -65,6 +71,11 @@ export class UserSettingsStorage implements StorageApi { options.fetchApi, options.discoveryApi, options.errorApi, + options.identityApi, + WebStorage.create({ + namespace: options.namespace, + errorApi: options.errorApi, + }), ); } @@ -80,6 +91,8 @@ export class UserSettingsStorage implements StorageApi { this.fetchApi, this.discoveryApi, this.errorApi, + this.identityApi, + this.fallback, ), ); } @@ -95,72 +108,81 @@ export class UserSettingsStorage implements StorageApi { }); if (!response.ok && response.status !== 404) { - throw ResponseError.fromResponse(response); + throw await ResponseError.fromResponse(response); } - this.notifyChanges({ - key, - presence: 'absent', - }); + this.notifyChanges({ key, presence: 'absent' }); } async set(key: string, data: T): Promise { + if (!(await this.isSignedIn())) { + await this.fallback.set(key, data); + this.notifyChanges({ key, presence: 'present', value: data }); + } + const fetchUrl = await this.getFetchUrl(key); - const body = JSON.stringify({ value: data }); const response = await this.fetchApi.fetch(fetchUrl, { method: 'PUT', - body, + headers: JSON_HEADERS, + body: JSON.stringify({ value: data }), }); if (!response.ok) { - throw ResponseError.fromResponse(response); + throw await ResponseError.fromResponse(response); } const { value } = await response.json(); - this.notifyChanges({ - key, - value, - presence: 'present', - }); + this.notifyChanges({ key, value, presence: 'present' }); } observe$( key: string, ): Observable> { - // TODO(freben): Introduce server polling or similar, to ensure that different devices update when values change - return this.observable.filter(({ key: messageKey }) => messageKey === key); + if (!this.observables.has(key)) { + this.observables.set( + key, + new ObservableImpl>(subscriber => { + this.subscribers.add(subscriber); + + // TODO(freben): Introduce server polling or similar, to ensure that different devices update when values change + Promise.resolve() + .then(() => this.get(key)) + .then(snapshot => subscriber.next(snapshot)) + .catch(error => this.errorApi.post(error)); + + return () => { + this.subscribers.delete(subscriber); + }; + }).filter(({ key: messageKey }) => messageKey === key), + ); + } + + return this.observables.get(key) as Observable>; } snapshot(key: string): StorageValueSnapshot { - // trigger a reload, ensure it happens on the next tick (after returning) - Promise.resolve() - .then(() => this.get(key)) - .then(snapshot => this.notifyChanges(snapshot)) - .catch(error => this.errorApi.post(error)); - - return { - key, - presence: 'unknown', - }; + return { key, presence: 'unknown' }; } private async get( key: string, ): Promise> { + if (!(await this.isSignedIn())) { + // This explicitly uses WebStorage, which we know is synchronous and doesn't return presence: unknown + return this.fallback.snapshot(key); + } + const fetchUrl = await this.getFetchUrl(key); const response = await this.fetchApi.fetch(fetchUrl); if (response.status === 404) { - return { - key, - presence: 'absent', - }; + return { key, presence: 'absent' }; } if (!response.ok) { - throw ResponseError.fromResponse(response); + throw await ResponseError.fromResponse(response); } try { @@ -172,17 +194,10 @@ export class UserSettingsStorage implements StorageApi { return val; }); - return { - key, - presence: 'present', - value, - }; + return { key, presence: 'present', value }; } catch { // If the value is not valid JSON, we return an unknown presence. This should never happen - return { - key, - presence: 'absent', - }; + return { key, presence: 'absent' }; } } @@ -204,4 +219,13 @@ export class UserSettingsStorage implements StorageApi { } } } + + private async isSignedIn(): Promise { + try { + const credentials = await this.identityApi.getCredentials(); + return credentials?.token ? true : false; + } catch { + return false; + } + } } diff --git a/packages/core-app-api/src/apis/implementations/StorageApi/WebStorage.ts b/packages/core-app-api/src/apis/implementations/StorageApi/WebStorage.ts index ac3d3f20f2..b0cbfbd883 100644 --- a/packages/core-app-api/src/apis/implementations/StorageApi/WebStorage.ts +++ b/packages/core-app-api/src/apis/implementations/StorageApi/WebStorage.ts @@ -86,7 +86,9 @@ export class WebStorage implements StorageApi { this.notifyChanges(key); } - observe$(key: string): Observable> { + observe$( + key: string, + ): Observable> { return this.observable.filter(({ key: messageKey }) => messageKey === key); } diff --git a/plugins/shortcuts/src/api/LocalStoredShortcuts.ts b/plugins/shortcuts/src/api/LocalStoredShortcuts.ts index 9f3561be67..65aeb86e3f 100644 --- a/plugins/shortcuts/src/api/LocalStoredShortcuts.ts +++ b/plugins/shortcuts/src/api/LocalStoredShortcuts.ts @@ -27,12 +27,16 @@ import Observable from 'zen-observable'; * @public */ export class LocalStoredShortcuts implements ShortcutApi { - constructor(private readonly storageApi: StorageApi) {} + private readonly shortcuts: Observable; + + constructor(private readonly storageApi: StorageApi) { + this.shortcuts = Observable.from( + this.storageApi.observe$('items'), + ).map(snapshot => snapshot.value ?? []); + } shortcut$() { - return Observable.from(this.storageApi.observe$('items')).map( - snapshot => snapshot.value ?? [], - ); + return this.shortcuts; } get() { diff --git a/plugins/user-settings-backend/README.md b/plugins/user-settings-backend/README.md index 5a98c86326..45e06731c9 100644 --- a/plugins/user-settings-backend/README.md +++ b/plugins/user-settings-backend/README.md @@ -35,7 +35,7 @@ export default async function createPlugin(env: PluginEnvironment) { // packages/backend/src/index.ts +import userSettings from './plugins/userSettings'; async function main() { -+ const userSettingsEnv = useHotMemoize(module, () => createEnv('userSettings')); ++ const userSettingsEnv = useHotMemoize(module, () => createEnv('user-settings')); const apiRouter = Router(); + apiRouter.use('/user-settings', await userSettings(userSettingsEnv)); } @@ -52,8 +52,9 @@ To make use of the user settings backend, replace the `WebStorage` with the AnyApiFactory, createApiFactory, + discoveryApiRef, -+ fetchApiRef ++ fetchApiRef, errorApiRef, ++ identityApi, + storageApiRef, } from '@backstage/core-plugin-api'; +import { UserSettingsStorage } from '@backstage/core-app-api'; @@ -65,9 +66,9 @@ To make use of the user settings backend, replace the `WebStorage` with the + discoveryApi: discoveryApiRef, + errorApi: errorApiRef, + fetchApi: fetchApiRef, ++ identityApi: identityApiRef + }, -+ factory: ({ discoveryApi, errorApi, fetchApi }) => -+ UserSettingsStorage.create({ discoveryApi, errorApi, fetchApi }), ++ factory: deps => UserSettingsStorage.create(deps), + }), ]; ``` From 294805ed598df10f2e1b090f8bbc1f2f4c0a6956 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Mon, 19 Sep 2022 13:23:48 +0200 Subject: [PATCH 13/14] refresh internal version deps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- packages/core-app-api/package.json | 2 +- plugins/user-settings-backend/package.json | 10 +++++----- yarn.lock | 18 +++++++++--------- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/packages/core-app-api/package.json b/packages/core-app-api/package.json index e1c24997ac..b6e2f64889 100644 --- a/packages/core-app-api/package.json +++ b/packages/core-app-api/package.json @@ -34,7 +34,7 @@ "dependencies": { "@backstage/config": "^1.0.2-next.0", "@backstage/core-plugin-api": "^1.0.6-next.3", - "@backstage/errors": "^1.1.0", + "@backstage/errors": "^1.1.1-next.0", "@backstage/types": "^1.0.0", "@backstage/version-bridge": "^1.0.1", "@types/prop-types": "^15.7.3", diff --git a/plugins/user-settings-backend/package.json b/plugins/user-settings-backend/package.json index 690d1d13c9..f1b4b503e1 100644 --- a/plugins/user-settings-backend/package.json +++ b/plugins/user-settings-backend/package.json @@ -28,10 +28,10 @@ "clean": "backstage-cli package clean" }, "dependencies": { - "@backstage/backend-common": "^0.15.1-next.1", - "@backstage/catalog-model": "^1.1.0", - "@backstage/errors": "^1.1.0", - "@backstage/plugin-auth-node": "^0.2.5-next.1", + "@backstage/backend-common": "^0.15.1-next.3", + "@backstage/catalog-model": "^1.1.1-next.0", + "@backstage/errors": "^1.1.1-next.0", + "@backstage/plugin-auth-node": "^0.2.5-next.3", "@backstage/types": "^1.0.0", "@types/express": "^4.17.6", "express": "^4.17.1", @@ -41,7 +41,7 @@ "yn": "^4.0.0" }, "devDependencies": { - "@backstage/backend-test-utils": "^0.1.28-next.1", + "@backstage/backend-test-utils": "^0.1.28-next.3", "@backstage/cli": "^0.19.0-next.1", "@types/supertest": "^2.0.8", "supertest": "^6.1.3" diff --git a/yarn.lock b/yarn.lock index 4d821c0e64..a9f5a553b9 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2855,7 +2855,7 @@ __metadata: languageName: unknown linkType: soft -"@backstage/backend-common@^0.15.1-next.1, @backstage/backend-common@^0.15.1-next.3, @backstage/backend-common@workspace:packages/backend-common": +"@backstage/backend-common@^0.15.1-next.3, @backstage/backend-common@workspace:packages/backend-common": version: 0.0.0-use.local resolution: "@backstage/backend-common@workspace:packages/backend-common" dependencies: @@ -2991,7 +2991,7 @@ __metadata: languageName: unknown linkType: soft -"@backstage/backend-test-utils@^0.1.28-next.1, @backstage/backend-test-utils@^0.1.28-next.3, @backstage/backend-test-utils@workspace:packages/backend-test-utils": +"@backstage/backend-test-utils@^0.1.28-next.3, @backstage/backend-test-utils@workspace:packages/backend-test-utils": version: 0.0.0-use.local resolution: "@backstage/backend-test-utils@workspace:packages/backend-test-utils" dependencies: @@ -3291,7 +3291,7 @@ __metadata: "@backstage/cli": ^0.19.0-next.3 "@backstage/config": ^1.0.2-next.0 "@backstage/core-plugin-api": ^1.0.6-next.3 - "@backstage/errors": ^1.1.0 + "@backstage/errors": ^1.1.1-next.0 "@backstage/test-utils": ^1.2.0-next.3 "@backstage/types": ^1.0.0 "@backstage/version-bridge": ^1.0.1 @@ -4028,7 +4028,7 @@ __metadata: languageName: unknown linkType: soft -"@backstage/plugin-auth-node@^0.2.5-next.1, @backstage/plugin-auth-node@^0.2.5-next.3, @backstage/plugin-auth-node@workspace:plugins/auth-node": +"@backstage/plugin-auth-node@^0.2.5-next.3, @backstage/plugin-auth-node@workspace:plugins/auth-node": version: 0.0.0-use.local resolution: "@backstage/plugin-auth-node@workspace:plugins/auth-node" dependencies: @@ -7315,12 +7315,12 @@ __metadata: version: 0.0.0-use.local resolution: "@backstage/plugin-user-settings-backend@workspace:plugins/user-settings-backend" dependencies: - "@backstage/backend-common": ^0.15.1-next.1 - "@backstage/backend-test-utils": ^0.1.28-next.1 - "@backstage/catalog-model": ^1.1.0 + "@backstage/backend-common": ^0.15.1-next.3 + "@backstage/backend-test-utils": ^0.1.28-next.3 + "@backstage/catalog-model": ^1.1.1-next.0 "@backstage/cli": ^0.19.0-next.1 - "@backstage/errors": ^1.1.0 - "@backstage/plugin-auth-node": ^0.2.5-next.1 + "@backstage/errors": ^1.1.1-next.0 + "@backstage/plugin-auth-node": ^0.2.5-next.3 "@backstage/types": ^1.0.0 "@types/express": ^4.17.6 "@types/supertest": ^2.0.8 From 8448b53dd6f5366ce27337e3a473b08b1d23b89d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Mon, 19 Sep 2022 15:21:10 +0200 Subject: [PATCH 14/14] move the settings storage to the user settings frontend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/healthy-oranges-fly.md | 7 ++-- .changeset/six-apricots-add.md | 7 ++++ .changeset/strange-geckos-know.md | 5 +++ packages/core-app-api/api-report.md | 24 -------------- packages/core-app-api/package.json | 1 - .../apis/implementations/StorageApi/index.ts | 1 - plugins/user-settings-backend/README.md | 2 +- plugins/user-settings/README.md | 12 ++++--- plugins/user-settings/api-report.md | 32 +++++++++++++++++++ plugins/user-settings/package.json | 7 ++-- .../StorageApi/UserSettingsStorage.test.ts | 0 .../apis}/StorageApi/UserSettingsStorage.ts | 2 +- .../src/apis/StorageApi/index.ts | 17 ++++++++++ plugins/user-settings/src/apis/index.ts | 17 ++++++++++ plugins/user-settings/src/index.ts | 3 +- yarn.lock | 4 ++- 16 files changed, 99 insertions(+), 42 deletions(-) create mode 100644 .changeset/six-apricots-add.md create mode 100644 .changeset/strange-geckos-know.md rename {packages/core-app-api/src/apis/implementations => plugins/user-settings/src/apis}/StorageApi/UserSettingsStorage.test.ts (100%) rename {packages/core-app-api/src/apis/implementations => plugins/user-settings/src/apis}/StorageApi/UserSettingsStorage.ts (99%) create mode 100644 plugins/user-settings/src/apis/StorageApi/index.ts create mode 100644 plugins/user-settings/src/apis/index.ts diff --git a/.changeset/healthy-oranges-fly.md b/.changeset/healthy-oranges-fly.md index c6b6f6ff93..a10c56f4d0 100644 --- a/.changeset/healthy-oranges-fly.md +++ b/.changeset/healthy-oranges-fly.md @@ -1,9 +1,6 @@ --- -'@backstage/core-app-api': minor '@backstage/plugin-user-settings-backend': minor --- -Add new plugin `@backstage/user-settings-backend` to store user related settings -in the database. Additionally adding a `UserSettingsStorage` implementation of -the `StorageApi` to easily use the new plugin as drop-in replacement for the -`WebStorage`. +Added new plugin `@backstage/plugin-user-settings-backend` to store user related +settings in the database. diff --git a/.changeset/six-apricots-add.md b/.changeset/six-apricots-add.md new file mode 100644 index 0000000000..95ec68b1d5 --- /dev/null +++ b/.changeset/six-apricots-add.md @@ -0,0 +1,7 @@ +--- +'@backstage/plugin-user-settings': patch +--- + +Added a `UserSettingsStorage` implementation of the `StorageApi` for use as +drop-in replacement for the `WebStorage`, in conjunction with the newly created +`@backstage/plugin-user-settings-backend`. diff --git a/.changeset/strange-geckos-know.md b/.changeset/strange-geckos-know.md new file mode 100644 index 0000000000..b14762d3e5 --- /dev/null +++ b/.changeset/strange-geckos-know.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-app-api': patch +--- + +Clarify that the `WebStorage` observable returns `JsonValue` items. diff --git a/packages/core-app-api/api-report.md b/packages/core-app-api/api-report.md index 39e5388d68..4d1db6fde1 100644 --- a/packages/core-app-api/api-report.md +++ b/packages/core-app-api/api-report.md @@ -543,30 +543,6 @@ export class UrlPatternDiscovery implements DiscoveryApi { getBaseUrl(pluginId: string): Promise; } -// @public -export class UserSettingsStorage implements StorageApi { - // (undocumented) - static create(options: { - fetchApi: FetchApi; - discoveryApi: DiscoveryApi; - errorApi: ErrorApi; - identityApi: IdentityApi; - namespace?: string; - }): UserSettingsStorage; - // (undocumented) - forBucket(name: string): StorageApi; - // (undocumented) - observe$( - key: string, - ): Observable>; - // (undocumented) - remove(key: string): Promise; - // (undocumented) - set(key: string, data: T): Promise; - // (undocumented) - snapshot(key: string): StorageValueSnapshot; -} - // @public export class WebStorage implements StorageApi { constructor(namespace: string, errorApi: ErrorApi); diff --git a/packages/core-app-api/package.json b/packages/core-app-api/package.json index b6e2f64889..8532997477 100644 --- a/packages/core-app-api/package.json +++ b/packages/core-app-api/package.json @@ -34,7 +34,6 @@ "dependencies": { "@backstage/config": "^1.0.2-next.0", "@backstage/core-plugin-api": "^1.0.6-next.3", - "@backstage/errors": "^1.1.1-next.0", "@backstage/types": "^1.0.0", "@backstage/version-bridge": "^1.0.1", "@types/prop-types": "^15.7.3", diff --git a/packages/core-app-api/src/apis/implementations/StorageApi/index.ts b/packages/core-app-api/src/apis/implementations/StorageApi/index.ts index e4162d120d..2f941381fd 100644 --- a/packages/core-app-api/src/apis/implementations/StorageApi/index.ts +++ b/packages/core-app-api/src/apis/implementations/StorageApi/index.ts @@ -15,4 +15,3 @@ */ export { WebStorage } from './WebStorage'; -export { UserSettingsStorage } from './UserSettingsStorage'; diff --git a/plugins/user-settings-backend/README.md b/plugins/user-settings-backend/README.md index 45e06731c9..d12ee0aa89 100644 --- a/plugins/user-settings-backend/README.md +++ b/plugins/user-settings-backend/README.md @@ -57,7 +57,7 @@ To make use of the user settings backend, replace the `WebStorage` with the + identityApi, + storageApiRef, } from '@backstage/core-plugin-api'; -+import { UserSettingsStorage } from '@backstage/core-app-api'; ++import { UserSettingsStorage } from '@backstage/plugin-user-settings'; export const apis: AnyApiFactory[] = [ + createApiFactory({ diff --git a/plugins/user-settings/README.md b/plugins/user-settings/README.md index 13cfe720c8..abe66120fe 100644 --- a/plugins/user-settings/README.md +++ b/plugins/user-settings/README.md @@ -2,15 +2,17 @@ Welcome to the user-settings plugin! -_This plugin was created through the Backstage CLI_ - ## About the plugin -This plugin provides two components, `` is intended to be used within the [``](https://backstage.io/storybook/?path=/story/sidebar--sample-sidebar) and displays the signed-in users profile picture and name. +This plugin provides two components, `` is intended to be used within the [``](https://backstage.io/storybook/?path=/story/sidebar--sample-sidebar) and displays the signed-in users profile picture and name. The second component is a settings page where the user can control different settings across the App. -The second component is a settings page where the user can control different settings across the App. +It also provides a `UserSettingsStorage` implementation of the `StorageApi`, to +be used in the frontend as a persistent alternative to the builtin `WebStorage`. +Please see [the backend +README](https://github.com/backstage/backstage/tree/master/plugins/user-settings-backend) +for installation instructions. -## Usage +## Components Usage Add the item to the Sidebar: diff --git a/plugins/user-settings/api-report.md b/plugins/user-settings/api-report.md index bc5c3a9c70..76eaed940e 100644 --- a/plugins/user-settings/api-report.md +++ b/plugins/user-settings/api-report.md @@ -8,11 +8,19 @@ import { ApiRef } from '@backstage/core-plugin-api'; import { BackstagePlugin } from '@backstage/core-plugin-api'; import { BackstageUserIdentity } from '@backstage/core-plugin-api'; +import { DiscoveryApi } from '@backstage/core-plugin-api'; +import { ErrorApi } from '@backstage/core-plugin-api'; +import { FetchApi } from '@backstage/core-plugin-api'; import { IconComponent } from '@backstage/core-plugin-api'; +import { IdentityApi } from '@backstage/core-plugin-api'; +import { JsonValue } from '@backstage/types'; +import { Observable } from '@backstage/types'; import { ProfileInfo } from '@backstage/core-plugin-api'; import { PropsWithChildren } from 'react'; import { RouteRef } from '@backstage/core-plugin-api'; import { SessionApi } from '@backstage/core-plugin-api'; +import { StorageApi } from '@backstage/core-plugin-api'; +import { StorageValueSnapshot } from '@backstage/core-plugin-api'; // @public (undocumented) export const DefaultProviderSettings: (props: { @@ -83,6 +91,30 @@ export const UserSettingsSignInAvatar: (props: { size?: number; }) => JSX.Element; +// @public +export class UserSettingsStorage implements StorageApi { + // (undocumented) + static create(options: { + fetchApi: FetchApi; + discoveryApi: DiscoveryApi; + errorApi: ErrorApi; + identityApi: IdentityApi; + namespace?: string; + }): UserSettingsStorage; + // (undocumented) + forBucket(name: string): StorageApi; + // (undocumented) + observe$( + key: string, + ): Observable>; + // (undocumented) + remove(key: string): Promise; + // (undocumented) + set(key: string, data: T): Promise; + // (undocumented) + snapshot(key: string): StorageValueSnapshot; +} + // @public export const UserSettingsTab: (props: UserSettingsTabProps) => JSX.Element; diff --git a/plugins/user-settings/package.json b/plugins/user-settings/package.json index f99c0d206e..b4afc6766b 100644 --- a/plugins/user-settings/package.json +++ b/plugins/user-settings/package.json @@ -32,14 +32,18 @@ "clean": "backstage-cli package clean" }, "dependencies": { + "@backstage/core-app-api": "^1.1.0-next.3", "@backstage/core-components": "^0.11.1-next.3", "@backstage/core-plugin-api": "^1.0.6-next.3", + "@backstage/errors": "^1.1.1-next.0", "@backstage/theme": "^0.2.16", + "@backstage/types": "^1.0.0", "@material-ui/core": "^4.12.2", "@material-ui/icons": "^4.9.1", "@material-ui/lab": "4.0.0-alpha.57", "@types/react": "^16.13.1 || ^17.0.0", - "react-use": "^17.2.4" + "react-use": "^17.2.4", + "zen-observable": "^0.8.15" }, "peerDependencies": { "react": "^16.13.1 || ^17.0.0", @@ -47,7 +51,6 @@ }, "devDependencies": { "@backstage/cli": "^0.19.0-next.3", - "@backstage/core-app-api": "^1.1.0-next.3", "@backstage/dev-utils": "^1.0.6-next.2", "@backstage/test-utils": "^1.2.0-next.3", "@testing-library/jest-dom": "^5.10.1", diff --git a/packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.test.ts b/plugins/user-settings/src/apis/StorageApi/UserSettingsStorage.test.ts similarity index 100% rename from packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.test.ts rename to plugins/user-settings/src/apis/StorageApi/UserSettingsStorage.test.ts diff --git a/packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.ts b/plugins/user-settings/src/apis/StorageApi/UserSettingsStorage.ts similarity index 99% rename from packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.ts rename to plugins/user-settings/src/apis/StorageApi/UserSettingsStorage.ts index b595c524cf..8905b4aeea 100644 --- a/packages/core-app-api/src/apis/implementations/StorageApi/UserSettingsStorage.ts +++ b/plugins/user-settings/src/apis/StorageApi/UserSettingsStorage.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import { WebStorage } from '@backstage/core-app-api'; import { DiscoveryApi, ErrorApi, @@ -25,7 +26,6 @@ import { import { ResponseError } from '@backstage/errors'; import { JsonValue, Observable } from '@backstage/types'; import ObservableImpl from 'zen-observable'; -import { WebStorage } from './WebStorage'; const JSON_HEADERS = { 'Content-Type': 'application/json; charset=utf-8', diff --git a/plugins/user-settings/src/apis/StorageApi/index.ts b/plugins/user-settings/src/apis/StorageApi/index.ts new file mode 100644 index 0000000000..9127cdfef4 --- /dev/null +++ b/plugins/user-settings/src/apis/StorageApi/index.ts @@ -0,0 +1,17 @@ +/* + * Copyright 2020 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. + */ + +export { UserSettingsStorage } from './UserSettingsStorage'; diff --git a/plugins/user-settings/src/apis/index.ts b/plugins/user-settings/src/apis/index.ts new file mode 100644 index 0000000000..1598430773 --- /dev/null +++ b/plugins/user-settings/src/apis/index.ts @@ -0,0 +1,17 @@ +/* + * Copyright 2022 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. + */ + +export * from './StorageApi'; diff --git a/plugins/user-settings/src/index.ts b/plugins/user-settings/src/index.ts index 0654b7fced..bc878ecbb3 100644 --- a/plugins/user-settings/src/index.ts +++ b/plugins/user-settings/src/index.ts @@ -15,11 +15,12 @@ */ /** - * A Backstage plugin that provides a settings page + * A Backstage plugin that provides various per-user settings functionality. * * @packageDocumentation */ +export * from './apis'; export { userSettingsPlugin, userSettingsPlugin as plugin, diff --git a/yarn.lock b/yarn.lock index a9f5a553b9..69e6cb4267 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3291,7 +3291,6 @@ __metadata: "@backstage/cli": ^0.19.0-next.3 "@backstage/config": ^1.0.2-next.0 "@backstage/core-plugin-api": ^1.0.6-next.3 - "@backstage/errors": ^1.1.1-next.0 "@backstage/test-utils": ^1.2.0-next.3 "@backstage/types": ^1.0.0 "@backstage/version-bridge": ^1.0.1 @@ -7342,8 +7341,10 @@ __metadata: "@backstage/core-components": ^0.11.1-next.3 "@backstage/core-plugin-api": ^1.0.6-next.3 "@backstage/dev-utils": ^1.0.6-next.2 + "@backstage/errors": ^1.1.1-next.0 "@backstage/test-utils": ^1.2.0-next.3 "@backstage/theme": ^0.2.16 + "@backstage/types": ^1.0.0 "@material-ui/core": ^4.12.2 "@material-ui/icons": ^4.9.1 "@material-ui/lab": 4.0.0-alpha.57 @@ -7355,6 +7356,7 @@ __metadata: cross-fetch: ^3.1.5 msw: ^0.47.0 react-use: ^17.2.4 + zen-observable: ^0.8.15 peerDependencies: react: ^16.13.1 || ^17.0.0 react-router: 6.0.0-beta.0 || ^6.3.0