From 78fa85cfa726774be4958dd7a3691f3c84a5c558 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 16 Dec 2021 11:06:57 +0100 Subject: [PATCH 1/6] core-plugin-api: redesign StorageApi to add .snapshot() and new observe behavior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Co-authored-by: Johan Haals Signed-off-by: Patrik Oldsberg --- .../src/apis/definitions/StorageApi.ts | 73 +++++++++++++++---- 1 file changed, 60 insertions(+), 13 deletions(-) diff --git a/packages/core-plugin-api/src/apis/definitions/StorageApi.ts b/packages/core-plugin-api/src/apis/definitions/StorageApi.ts index 73a19a1ceb..25ec5d8131 100644 --- a/packages/core-plugin-api/src/apis/definitions/StorageApi.ts +++ b/packages/core-plugin-api/src/apis/definitions/StorageApi.ts @@ -15,55 +15,102 @@ */ import { ApiRef, createApiRef } from '../system'; -import { Observable } from '@backstage/types'; +import { JsonValue, Observable } from '@backstage/types'; /** - * Describes a value change event. + * A snapshot in time of the current known value of a storage key. * * @public */ -export type StorageValueChange = { - key: string; - newValue?: T; -}; +export type StorageValueSnapshot = + | { + key: string; + presence: 'unknown' | 'absent'; + value?: undefined; + /** @deprecated Use `value` instead */ + newValue?: undefined; + } + | { + key: string; + presence: 'present'; + value: TValue; + /** @deprecated Use `value` instead */ + newValue?: TValue; + }; + +/** @deprecated Use StorageValueSnapshot instead */ +export type StorageValueChange = + StorageValueSnapshot; /** - * Provides key-value persistence API. + * Provides a key-value persistence API. * * @public */ export interface StorageApi { /** * Create a bucket to store data in. + * * @param name - Namespace for the storage to be stored under, - * will inherit previous namespaces too + * will inherit previous namespaces too */ forBucket(name: string): StorageApi; /** * Get the current value for persistent data, use observe$ to be notified of updates. + * + * @deprecated Use `snapshot` instead. * @param key - Unique key associated with the data. */ - get(key: string): T | undefined; + get(key: string): T | undefined; /** * Remove persistent data. + * * @param key - Unique key associated with the data. */ remove(key: string): Promise; /** - * Save persistent data, and emit messages to anyone that is using observe$ for this key + * Save persistent data, and emit messages to anyone that is using + * {@link StorageApi.observe$} for this key. + * * @param key - Unique key associated with the data. * @param data - The data to be stored under the key. */ - set(key: string, data: any): Promise; + set(key: string, data: T): Promise; /** - * Observe changes on a particular key in the bucket + * Observe the value over time for a particular key in the current bucket. + * + * @remarks + * + * The returned observable will immediately emit a value snapshot when + * subscribed to, even if the presence of the value is unknown at that time. + * + * The values emitted by the observe method aim to keep reference equality + * across snapshots if the value is unchanged. This means that it is important + * not to mutate the returned values, and they may be frozen as a precaution. + * * @param key - Unique key associated with the data */ - observe$(key: string): Observable>; + observe$( + key: string, + ): Observable>; + + /** + * Returns an immediate snapshot value for the given key, if possible. + * + * @remarks + * + * Combine with {@link StorageApi.observe$} to get notified of value changes. + * + * Note that this method is synchronous, and some underlying storages may be + * unable to retrieve a value using this method - the result may or may not + * consistently have a presence of 'unknown'. Use {@link StorageApi.observe$} + * to be sure to receive an actual value eventually. + */ + snapshot(key: string): StorageValueSnapshot; } /** From 61fa5d0171042155b7077ebfd782097c32fd3be8 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 16 Dec 2021 11:52:49 +0100 Subject: [PATCH 2/6] core-app-api: add tests for new StorageApi design MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Co-authored-by: blam Co-authored-by: Johan Haals Signed-off-by: Patrik Oldsberg --- .../StorageApi/WebStorage.test.ts | 115 +++++++++++++++--- 1 file changed, 100 insertions(+), 15 deletions(-) diff --git a/packages/core-app-api/src/apis/implementations/StorageApi/WebStorage.test.ts b/packages/core-app-api/src/apis/implementations/StorageApi/WebStorage.test.ts index f8dd85c05c..d81cba48d2 100644 --- a/packages/core-app-api/src/apis/implementations/StorageApi/WebStorage.test.ts +++ b/packages/core-app-api/src/apis/implementations/StorageApi/WebStorage.test.ts @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import { WebStorage } from './WebStorage'; import { ErrorApi, StorageApi } from '@backstage/core-plugin-api'; @@ -33,6 +34,12 @@ describe('WebStorage Storage API', () => { const storage = createWebStorage(); expect(storage.get('myfakekey')).toBeUndefined(); + expect(storage.snapshot('myfakekey')).toEqual({ + key: 'myfakekey', + presence: 'absent', + value: undefined, + newValue: undefined, + }); }); it('should allow the setting and getting of the simple data structures', async () => { @@ -44,9 +51,27 @@ describe('WebStorage Storage API', () => { expect(storage.get('myfakekey')).toBe('helloimastring'); expect(storage.get('mysecondfakekey')).toBe(1234); expect(storage.get('mythirdfakekey')).toBe(true); + expect(storage.snapshot('myfakekey')).toEqual({ + key: 'myfakekey', + presence: 'present', + value: 'helloimastring', + newValue: 'helloimastring', + }); + expect(storage.snapshot('mysecondfakekey')).toEqual({ + key: 'mysecondfakekey', + presence: 'present', + value: 1234, + newValue: 1234, + }); + expect(storage.snapshot('mythirdfakekey')).toEqual({ + key: 'mythirdfakekey', + presence: 'present', + value: true, + newValue: true, + }); }); - it('should allow setting of complex datastructures', async () => { + it('should allow setting of complex data structures', async () => { const storage = createWebStorage(); const mockData = { @@ -57,6 +82,12 @@ describe('WebStorage Storage API', () => { await storage.set('myfakekey', mockData); expect(storage.get('myfakekey')).toEqual(mockData); + expect(storage.snapshot('myfakekey')).toEqual({ + key: 'myfakekey', + presence: 'present', + value: mockData, + newValue: mockData, + }); }); it('should subscribe to key changes when setting a new value', async () => { @@ -67,10 +98,12 @@ describe('WebStorage Storage API', () => { const mockData = { hello: 'im a great new value' }; await new Promise(resolve => { - storage.observe$('correctKey').subscribe({ - next: (...args) => { - selectedKeyNextHandler(...args); - resolve(); + storage.observe$('correctKey').subscribe({ + next: snapshot => { + selectedKeyNextHandler(snapshot); + if (snapshot.presence === 'present') { + resolve(); + } }, }); @@ -79,10 +112,12 @@ describe('WebStorage Storage API', () => { storage.set('correctKey', mockData); }); - expect(wrongKeyNextHandler).not.toHaveBeenCalled(); + expect(wrongKeyNextHandler).toHaveBeenCalledTimes(0); expect(selectedKeyNextHandler).toHaveBeenCalledTimes(1); expect(selectedKeyNextHandler).toHaveBeenCalledWith({ key: 'correctKey', + presence: 'present', + value: mockData, newValue: mockData, }); }); @@ -98,9 +133,11 @@ describe('WebStorage Storage API', () => { await new Promise(resolve => { storage.observe$('correctKey').subscribe({ - next: (...args) => { - selectedKeyNextHandler(...args); - resolve(); + next: snapshot => { + selectedKeyNextHandler(snapshot); + if (snapshot.presence === 'absent') { + resolve(); + } }, }); @@ -109,10 +146,12 @@ describe('WebStorage Storage API', () => { storage.remove('correctKey'); }); - expect(wrongKeyNextHandler).not.toHaveBeenCalled(); + expect(wrongKeyNextHandler).toHaveBeenCalledTimes(0); expect(selectedKeyNextHandler).toHaveBeenCalledTimes(1); expect(selectedKeyNextHandler).toHaveBeenCalledWith({ key: 'correctKey', + presence: 'absent', + value: undefined, newValue: undefined, }); }); @@ -130,9 +169,24 @@ describe('WebStorage Storage API', () => { expect(firstStorage.get(keyName)).not.toBe(secondStorage.get(keyName)); expect(firstStorage.get(keyName)).toBe('boop'); expect(secondStorage.get(keyName)).toBe('deerp'); + expect(firstStorage.snapshot(keyName)).not.toEqual( + secondStorage.snapshot(keyName), + ); + expect(firstStorage.snapshot(keyName)).toEqual({ + key: keyName, + presence: 'present', + value: 'boop', + newValue: 'boop', + }); + expect(secondStorage.snapshot(keyName)).toEqual({ + key: keyName, + presence: 'present', + value: 'deerp', + newValue: 'deerp', + }); }); - it('should not clash with other namesapces when creating buckets', async () => { + it('should not clash with other namespaces when creating buckets', async () => { const rootStorage = createWebStorage(); // when getting key test2 it will translate to /profile/something/deep/test2 @@ -145,7 +199,9 @@ describe('WebStorage Storage API', () => { await firstStorage.set('test2', { error: true }); - expect(secondStorage.get('deep/test2')).toBe(undefined); + expect(secondStorage.snapshot('deep/test2')).toMatchObject({ + presence: 'absent', + }); }); it('should call the error api when the json can not be parsed in local storage', async () => { @@ -155,9 +211,14 @@ describe('WebStorage Storage API', () => { localStorage.setItem('/Test/Mock/Thing/key', '{smd: asdouindA}'); - const value = rootStorage.get('key'); + const value = rootStorage.snapshot('key'); - expect(value).toBe(undefined); + expect(value).toEqual({ + key: 'key', + presence: 'absent', + value: undefined, + newValue: undefined, + }); expect(mockErrorApi.post).toHaveBeenCalledWith(expect.any(Error)); expect(mockErrorApi.post).toHaveBeenCalledWith( expect.objectContaining({ @@ -166,11 +227,35 @@ describe('WebStorage Storage API', () => { ); }); - it('should return a singleton for the same namespace and same bucket', async () => { + it('should return a stable reference for the same namespace and same bucket', async () => { const rootStorage = createWebStorage({ namespace: '/Test/Mock/Thing/Thing ', }); expect(rootStorage.forBucket('test')).toBe(rootStorage.forBucket('test')); }); + + it('should freeze the snapshot value', async () => { + const storage = createWebStorage(); + + const data = { foo: 'bar', baz: [{ foo: 'bar' }] }; + storage.set('foo', data); + + const snapshot = storage.snapshot('foo'); + expect(snapshot.value).not.toBe(data); + + if (snapshot.presence !== 'present') { + throw new Error('Invalid presence'); + } + + 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/); + }); }); From a283b7396f6c0a117e63394edd26ed4892b2a0fb Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 16 Dec 2021 15:23:30 +0100 Subject: [PATCH 3/6] core-app-api: update WebStorage implementation and drop immidiate emit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Co-authored-by: blam Co-authored-by: Johan Haals Signed-off-by: Patrik Oldsberg --- .../implementations/StorageApi/WebStorage.ts | 56 ++++++++++++------- .../src/apis/definitions/StorageApi.ts | 13 +++-- 2 files changed, 43 insertions(+), 26 deletions(-) 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 b252e6b726..c4cffaf184 100644 --- a/packages/core-app-api/src/apis/implementations/StorageApi/WebStorage.ts +++ b/packages/core-app-api/src/apis/implementations/StorageApi/WebStorage.ts @@ -16,10 +16,10 @@ import { StorageApi, - StorageValueChange, + StorageValueSnapshot, ErrorApi, } from '@backstage/core-plugin-api'; -import { Observable } from '@backstage/types'; +import { JsonValue, Observable } from '@backstage/types'; import ObservableImpl from 'zen-observable'; const buckets = new Map(); @@ -43,16 +43,29 @@ export class WebStorage implements StorageApi { } get(key: string): T | undefined { + return this.snapshot(key).value as T | undefined; + } + + snapshot(key: string): StorageValueSnapshot { + let value = undefined; + let presence: 'present' | 'absent' = 'absent'; try { - const storage = JSON.parse(localStorage.getItem(this.getKeyName(key))!); - return storage ?? undefined; + const item = localStorage.getItem(this.getKeyName(key)); + if (item) { + value = JSON.parse(item, (_key, val) => { + if (typeof val === 'object' && val !== null) { + Object.freeze(val); + } + return val; + }); + presence = 'present'; + } } catch (e) { this.errorApi.post( new Error(`Error when parsing JSON config from storage for: ${key}`), ); } - - return undefined; + return { key, value, newValue: value, presence }; } forBucket(name: string): WebStorage { @@ -64,16 +77,16 @@ export class WebStorage implements StorageApi { } async set(key: string, data: T): Promise { - localStorage.setItem(this.getKeyName(key), JSON.stringify(data, null, 2)); - this.notifyChanges({ key, newValue: data }); + localStorage.setItem(this.getKeyName(key), JSON.stringify(data)); + this.notifyChanges(key); } async remove(key: string): Promise { localStorage.removeItem(this.getKeyName(key)); - this.notifyChanges({ key, newValue: undefined }); + this.notifyChanges(key); } - observe$(key: string): Observable> { + observe$(key: string): Observable> { return this.observable.filter(({ key: messageKey }) => messageKey === key); } @@ -81,22 +94,23 @@ export class WebStorage implements StorageApi { return `${this.namespace}/${encodeURIComponent(key)}`; } - private notifyChanges(message: StorageValueChange) { + private notifyChanges(key: string) { + const snapshot = this.snapshot(key); for (const subscription of this.subscribers) { - subscription.next(message); + subscription.next(snapshot); } } private subscribers = new Set< - ZenObservable.SubscriptionObserver + ZenObservable.SubscriptionObserver> >(); - private readonly observable = new ObservableImpl( - subscriber => { - this.subscribers.add(subscriber); - return () => { - this.subscribers.delete(subscriber); - }; - }, - ); + private readonly observable = new ObservableImpl< + StorageValueSnapshot + >(subscriber => { + this.subscribers.add(subscriber); + return () => { + this.subscribers.delete(subscriber); + }; + }); } diff --git a/packages/core-plugin-api/src/apis/definitions/StorageApi.ts b/packages/core-plugin-api/src/apis/definitions/StorageApi.ts index 25ec5d8131..8dcfe6739a 100644 --- a/packages/core-plugin-api/src/apis/definitions/StorageApi.ts +++ b/packages/core-plugin-api/src/apis/definitions/StorageApi.ts @@ -85,12 +85,15 @@ export interface StorageApi { * * @remarks * - * The returned observable will immediately emit a value snapshot when - * subscribed to, even if the presence of the value is unknown at that time. + * The observable will only emit values when the value changes in the underlying + * storage, although multiple values with the same shape may be emitted in a row. * - * The values emitted by the observe method aim to keep reference equality - * across snapshots if the value is unchanged. This means that it is important - * not to mutate the returned values, and they may be frozen as a precaution. + * If a {@link StorageApi.snapshot} of a key is retrieved and the presence is + * `'unknown'`, then you are guaranteed to receive a snapshot with a known + * presence, as long as you observe the key within the same tick. + * + * Since the emitted values are shared across all subscribers, it is important + * not to mutate the returned values. The values may be frozen as a precaution. * * @param key - Unique key associated with the data */ From 0b01baf309bbf9b730c826f394f08272c6806f2f Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 16 Dec 2021 15:26:39 +0100 Subject: [PATCH 4/6] test-utils: update MockStorageApi Signed-off-by: Patrik Oldsberg --- .../apis/StorageApi/MockStorageApi.test.ts | 66 ++++++++++++++++++- .../apis/StorageApi/MockStorageApi.ts | 60 ++++++++++++----- 2 files changed, 109 insertions(+), 17 deletions(-) diff --git a/packages/test-utils/src/testUtils/apis/StorageApi/MockStorageApi.test.ts b/packages/test-utils/src/testUtils/apis/StorageApi/MockStorageApi.test.ts index a3028f8b57..a8187cd849 100644 --- a/packages/test-utils/src/testUtils/apis/StorageApi/MockStorageApi.test.ts +++ b/packages/test-utils/src/testUtils/apis/StorageApi/MockStorageApi.test.ts @@ -25,6 +25,12 @@ describe('WebStorage Storage API', () => { const storage = createMockStorage(); expect(storage.get('myfakekey')).toBeUndefined(); + expect(storage.snapshot('myfakekey')).toEqual({ + key: 'myfakekey', + presence: 'absent', + value: undefined, + newValue: undefined, + }); }); it('should allow the setting and getting of the simple data structures', async () => { @@ -36,6 +42,24 @@ describe('WebStorage Storage API', () => { expect(storage.get('myfakekey')).toBe('helloimastring'); expect(storage.get('mysecondfakekey')).toBe(1234); expect(storage.get('mythirdfakekey')).toBe(true); + expect(storage.snapshot('myfakekey')).toEqual({ + key: 'myfakekey', + presence: 'present', + value: 'helloimastring', + newValue: 'helloimastring', + }); + expect(storage.snapshot('mysecondfakekey')).toEqual({ + key: 'mysecondfakekey', + presence: 'present', + value: 1234, + newValue: 1234, + }); + expect(storage.snapshot('mythirdfakekey')).toEqual({ + key: 'mythirdfakekey', + presence: 'present', + value: true, + newValue: true, + }); }); it('should allow setting of complex datastructures', async () => { @@ -49,6 +73,12 @@ describe('WebStorage Storage API', () => { await storage.set('myfakekey', mockData); expect(storage.get('myfakekey')).toEqual(mockData); + expect(storage.snapshot('myfakekey')).toEqual({ + key: 'myfakekey', + presence: 'present', + value: mockData, + newValue: mockData, + }); }); it('should subscribe to key changes when setting a new value', async () => { @@ -59,7 +89,7 @@ describe('WebStorage Storage API', () => { const mockData = { hello: 'im a great new value' }; await new Promise(resolve => { - storage.observe$('correctKey').subscribe({ + storage.observe$('correctKey').subscribe({ next: (...args) => { selectedKeyNextHandler(...args); resolve(); @@ -75,6 +105,8 @@ describe('WebStorage Storage API', () => { expect(selectedKeyNextHandler).toHaveBeenCalledTimes(1); expect(selectedKeyNextHandler).toHaveBeenCalledWith({ key: 'correctKey', + presence: 'present', + value: mockData, newValue: mockData, }); }); @@ -105,6 +137,8 @@ describe('WebStorage Storage API', () => { expect(selectedKeyNextHandler).toHaveBeenCalledTimes(1); expect(selectedKeyNextHandler).toHaveBeenCalledWith({ key: 'correctKey', + presence: 'absent', + value: undefined, newValue: undefined, }); }); @@ -122,6 +156,21 @@ describe('WebStorage Storage API', () => { expect(firstStorage.get(keyName)).not.toBe(secondStorage.get(keyName)); expect(firstStorage.get(keyName)).toBe('boop'); expect(secondStorage.get(keyName)).toBe('deerp'); + expect(firstStorage.snapshot(keyName)).not.toEqual( + secondStorage.snapshot(keyName), + ); + expect(firstStorage.snapshot(keyName)).toEqual({ + key: keyName, + presence: 'present', + value: 'boop', + newValue: 'boop', + }); + expect(secondStorage.snapshot(keyName)).toEqual({ + key: keyName, + presence: 'present', + value: 'deerp', + newValue: 'deerp', + }); }); it('should not clash with other namespaces when creating buckets', async () => { @@ -138,6 +187,9 @@ describe('WebStorage Storage API', () => { await firstStorage.set('test2', { error: true }); expect(secondStorage.get('deep/test2')).toBe(undefined); + expect(secondStorage.snapshot('deep/test2')).toMatchObject({ + presence: 'absent', + }); }); it('should not reuse storage instances between different rootStorages', async () => { @@ -151,5 +203,17 @@ describe('WebStorage Storage API', () => { expect(firstStorage.get('test2')).toBe(true); expect(secondStorage.get('test2')).toBe(undefined); + expect(firstStorage.snapshot('test2')).toEqual({ + key: 'test2', + presence: 'present', + value: true, + newValue: true, + }); + expect(secondStorage.snapshot('test2')).toEqual({ + key: 'test2', + presence: 'absent', + value: undefined, + newValue: undefined, + }); }); }); diff --git a/packages/test-utils/src/testUtils/apis/StorageApi/MockStorageApi.ts b/packages/test-utils/src/testUtils/apis/StorageApi/MockStorageApi.ts index 6378a77474..f71d2602b5 100644 --- a/packages/test-utils/src/testUtils/apis/StorageApi/MockStorageApi.ts +++ b/packages/test-utils/src/testUtils/apis/StorageApi/MockStorageApi.ts @@ -14,8 +14,8 @@ * limitations under the License. */ -import { StorageApi, StorageValueChange } from '@backstage/core-plugin-api'; -import { Observable } from '@backstage/types'; +import { StorageApi, StorageValueSnapshot } from '@backstage/core-plugin-api'; +import { JsonValue, Observable } from '@backstage/types'; import ObservableImpl from 'zen-observable'; /** @@ -62,20 +62,48 @@ export class MockStorageApi implements StorageApi { } get(key: string): T | undefined { - return this.data[this.getKeyName(key)]; + return this.snapshot(key).value as T | undefined; + } + + snapshot(key: string): StorageValueSnapshot { + if (this.data.hasOwnProperty(this.getKeyName(key))) { + const data = this.data[this.getKeyName(key)]; + return { + key, + presence: 'present', + value: data, + newValue: data, + }; + } + return { + key, + presence: 'absent', + value: undefined, + newValue: undefined, + }; } async set(key: string, data: T): Promise { this.data[this.getKeyName(key)] = data; - this.notifyChanges({ key, newValue: data }); + this.notifyChanges({ + key, + presence: 'present', + value: data, + newValue: data, + }); } async remove(key: string): Promise { delete this.data[this.getKeyName(key)]; - this.notifyChanges({ key, newValue: undefined }); + this.notifyChanges({ + key, + presence: 'absent', + value: undefined, + newValue: undefined, + }); } - observe$(key: string): Observable> { + observe$(key: string): Observable> { return this.observable.filter(({ key: messageKey }) => messageKey === key); } @@ -83,22 +111,22 @@ export class MockStorageApi implements StorageApi { return `${this.namespace}/${encodeURIComponent(key)}`; } - private notifyChanges(message: StorageValueChange) { + private notifyChanges(message: StorageValueSnapshot) { for (const subscription of this.subscribers) { subscription.next(message); } } private subscribers = new Set< - ZenObservable.SubscriptionObserver + ZenObservable.SubscriptionObserver> >(); - private readonly observable = new ObservableImpl( - subscriber => { - this.subscribers.add(subscriber); - return () => { - this.subscribers.delete(subscriber); - }; - }, - ); + private readonly observable = new ObservableImpl< + StorageValueSnapshot + >(subscriber => { + this.subscribers.add(subscriber); + return () => { + this.subscribers.delete(subscriber); + }; + }); } From 281bac6f0c2cd538518868d5eb5a5c911af55816 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 16 Dec 2021 17:11:39 +0100 Subject: [PATCH 5/6] catalog-react: explicit type for storageApi.get Signed-off-by: Patrik Oldsberg --- plugins/catalog-react/src/apis/StarredEntitiesApi/migration.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugins/catalog-react/src/apis/StarredEntitiesApi/migration.ts b/plugins/catalog-react/src/apis/StarredEntitiesApi/migration.ts index 7147d449ab..8405d9e08d 100644 --- a/plugins/catalog-react/src/apis/StarredEntitiesApi/migration.ts +++ b/plugins/catalog-react/src/apis/StarredEntitiesApi/migration.ts @@ -41,8 +41,7 @@ export async function performMigrationToTheNewBucket({ // nothing to do return; } - - const targetEntities = new Set(target.get('entityRefs') ?? []); + const targetEntities = new Set(target.get('entityRefs') ?? []); oldStarredEntities .filter(isString) From a195284c7b545d49d326147750e1ce3d151beeb8 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 16 Dec 2021 23:24:18 +0100 Subject: [PATCH 6/6] api report updates and changests for StorageApi refactor Signed-off-by: Patrik Oldsberg --- .changeset/cyan-seahorses-film.md | 11 +++++++ .changeset/gorgeous-beers-teach.md | 5 +++ .changeset/green-timers-film.md | 5 +++ packages/core-app-api/api-report.md | 7 ++-- packages/core-plugin-api/api-report.md | 32 +++++++++++++++---- .../src/apis/definitions/StorageApi.ts | 5 ++- packages/test-utils/api-report.md | 7 ++-- 7 files changed, 60 insertions(+), 12 deletions(-) create mode 100644 .changeset/cyan-seahorses-film.md create mode 100644 .changeset/gorgeous-beers-teach.md create mode 100644 .changeset/green-timers-film.md diff --git a/.changeset/cyan-seahorses-film.md b/.changeset/cyan-seahorses-film.md new file mode 100644 index 0000000000..80a27baec0 --- /dev/null +++ b/.changeset/cyan-seahorses-film.md @@ -0,0 +1,11 @@ +--- +'@backstage/core-plugin-api': minor +--- + +**BREAKING CHANGE** The `StorageApi` has received several updates that fills in gaps for some use-cases and makes it easier to avoid mistakes: + +- The `StorageValueChange` type has been renamed to `StorageValueSnapshot`, the `newValue` property has been renamed to `value`, the stored value type has been narrowed to `JsonValue`, and it has received a new `presence` property that is `'unknown'`, `'absent'`, or `'present'`. +- The `get` method has been deprecated in favor of a new `snapshot` method, which returns a `StorageValueSnapshot`. +- The `observe$` method has had its contract changed. It should now emit values when the `presence` of a key changes, this may for example happen when remotely stored values are requested on page load and the presence switches from `'unknown'` to either `'absent'` or `'present'`. + +The above changes have been made with deprecations in place to maintain much of the backwards compatibility for consumers of the `StorageApi`. The only breaking change is the narrowing of the stored value type, which may in some cases require the addition of an explicit type parameter to the `get` and `observe$` methods. diff --git a/.changeset/gorgeous-beers-teach.md b/.changeset/gorgeous-beers-teach.md new file mode 100644 index 0000000000..014398a5be --- /dev/null +++ b/.changeset/gorgeous-beers-teach.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-app-api': minor +--- + +Updated `WebStorageApi` to reflect the `StorageApi` changes in `@backstage/core-plugin-api`. diff --git a/.changeset/green-timers-film.md b/.changeset/green-timers-film.md new file mode 100644 index 0000000000..be94b13f0d --- /dev/null +++ b/.changeset/green-timers-film.md @@ -0,0 +1,5 @@ +--- +'@backstage/test-utils': minor +--- + +Updated `MockStorageApi` to reflect the `StorageApi` changes in `@backstage/core-plugin-api`. diff --git a/packages/core-app-api/api-report.md b/packages/core-app-api/api-report.md index 8746211a45..f94979f975 100644 --- a/packages/core-app-api/api-report.md +++ b/packages/core-app-api/api-report.md @@ -39,6 +39,7 @@ import { gitlabAuthApiRef } from '@backstage/core-plugin-api'; import { googleAuthApiRef } 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 { microsoftAuthApiRef } from '@backstage/core-plugin-api'; import { OAuthApi } from '@backstage/core-plugin-api'; import { OAuthRequestApi } from '@backstage/core-plugin-api'; @@ -59,7 +60,7 @@ import { RouteRef } from '@backstage/core-plugin-api'; import { SessionApi } from '@backstage/core-plugin-api'; import { SessionState } from '@backstage/core-plugin-api'; import { StorageApi } from '@backstage/core-plugin-api'; -import { StorageValueChange } from '@backstage/core-plugin-api'; +import { StorageValueSnapshot } from '@backstage/core-plugin-api'; import { SubRouteRef } from '@backstage/core-plugin-api'; // @public @@ -606,10 +607,12 @@ 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) set(key: string, data: T): Promise; + // (undocumented) + snapshot(key: string): StorageValueSnapshot; } ``` diff --git a/packages/core-plugin-api/api-report.md b/packages/core-plugin-api/api-report.md index c9045a21d4..928a90cee5 100644 --- a/packages/core-plugin-api/api-report.md +++ b/packages/core-plugin-api/api-report.md @@ -10,6 +10,7 @@ import { ComponentType } from 'react'; import { Config } from '@backstage/config'; import { IconComponent as IconComponent_2 } from '@backstage/core-plugin-api'; import { IdentityApi as IdentityApi_2 } from '@backstage/core-plugin-api'; +import { JsonValue } from '@backstage/types'; import { Observable } from '@backstage/types'; import { ProfileInfo as ProfileInfo_2 } from '@backstage/core-plugin-api'; import { default as React_2 } from 'react'; @@ -762,20 +763,37 @@ export type SignInResult = { // @public export interface StorageApi { forBucket(name: string): StorageApi; - get(key: string): T | undefined; - observe$(key: string): Observable>; + // @deprecated + get(key: string): T | undefined; + observe$( + key: string, + ): Observable>; remove(key: string): Promise; - set(key: string, data: any): Promise; + set(key: string, data: T): Promise; + snapshot(key: string): StorageValueSnapshot; } // @public export const storageApiRef: ApiRef; +// @public @deprecated (undocumented) +export type StorageValueChange = + StorageValueSnapshot; + // @public -export type StorageValueChange = { - key: string; - newValue?: T; -}; +export type StorageValueSnapshot = + | { + key: string; + presence: 'unknown' | 'absent'; + value?: undefined; + newValue?: undefined; + } + | { + key: string; + presence: 'present'; + value: TValue; + newValue?: TValue; + }; // @public export type SubRouteRef = { diff --git a/packages/core-plugin-api/src/apis/definitions/StorageApi.ts b/packages/core-plugin-api/src/apis/definitions/StorageApi.ts index 8dcfe6739a..4bd49f9b83 100644 --- a/packages/core-plugin-api/src/apis/definitions/StorageApi.ts +++ b/packages/core-plugin-api/src/apis/definitions/StorageApi.ts @@ -38,7 +38,10 @@ export type StorageValueSnapshot = newValue?: TValue; }; -/** @deprecated Use StorageValueSnapshot instead */ +/** + * @public + * @deprecated Use StorageValueSnapshot instead + */ export type StorageValueChange = StorageValueSnapshot; diff --git a/packages/test-utils/api-report.md b/packages/test-utils/api-report.md index 24f17e0b84..a75a6d958e 100644 --- a/packages/test-utils/api-report.md +++ b/packages/test-utils/api-report.md @@ -12,13 +12,14 @@ import { ErrorApi } from '@backstage/core-plugin-api'; import { ErrorApiError } from '@backstage/core-plugin-api'; import { ErrorApiErrorContext } from '@backstage/core-plugin-api'; import { ExternalRouteRef } from '@backstage/core-plugin-api'; +import { JsonValue } from '@backstage/types'; import { Observable } from '@backstage/types'; import { ReactElement } from 'react'; import { ReactNode } from 'react'; import { RenderResult } from '@testing-library/react'; import { RouteRef } from '@backstage/core-plugin-api'; import { StorageApi } from '@backstage/core-plugin-api'; -import { StorageValueChange } from '@backstage/core-plugin-api'; +import { StorageValueSnapshot } from '@backstage/core-plugin-api'; // @public export type AsyncLogCollector = () => Promise; @@ -81,11 +82,13 @@ export class MockStorageApi implements StorageApi { // (undocumented) get(key: string): T | undefined; // (undocumented) - observe$(key: string): Observable>; + observe$(key: string): Observable>; // (undocumented) remove(key: string): Promise; // (undocumented) set(key: string, data: T): Promise; + // (undocumented) + snapshot(key: string): StorageValueSnapshot; } // @public