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 {};