feat(core-api/Storage): Made the implementation a litle nicer now so that the error API is passed through to the storage API

packages/app/src/apis.ts

@@ -30,6 +30,8 @@ import {
   OAuthRequestManager,
   googleAuthApiRef,
   githubAuthApiRef,
+  storageApiRef,
+  WebStorage,
 } from '@backstage/core';
 
 import {
@@ -45,8 +47,12 @@ import { catalogApiRef, CatalogClient } from '@backstage/plugin-catalog';
 const builder = ApiRegistry.builder();
 
 const alertApi = builder.add(alertApiRef, new AlertApiForwarder());
+const errorApi = builder.add(
+  errorApiRef,
+  new ErrorAlerter(alertApi, new ErrorApiForwarder()),
+);
 
-builder.add(errorApiRef, new ErrorAlerter(alertApi, new ErrorApiForwarder()));
+builder.add(storageApiRef, WebStorage.create({ errorApi }));
 builder.add(circleCIApiRef, new CircleCIApi());
 builder.add(featureFlagsApiRef, new FeatureFlags());
 

packages/core-api/src/apis/definitions/StorageApi.ts

@@ -16,12 +16,18 @@
 
 import { createApiRef } from '../ApiRef';
 import { Observable } from '../../types';
+import { ErrorApi } from './ErrorApi';
 
 export type StorageValueChange<T = any> = {
   key: string;
   newValue?: T;
 };
 
+export type CreateStorageApiOptions = {
+  errorApi: ErrorApi;
+  namespace?: string;
+};
+
 export interface StorageApi {
   /**
    * Create a bucket to store data in.

packages/core-api/src/apis/implementations/StorageApi/WebStorage.test.ts

@@ -14,15 +14,25 @@
  * limitations under the License.
  */
 import { WebStorage } from './WebStorage';
+import { CreateStorageApiOptions, StorageApi } from '../../definitions';
 describe('WebStorage Storage API', () => {
+  const mockErrorApi = { post: jest.fn(), error$: jest.fn() };
+  const createWebStorage = (
+    args?: Partial<CreateStorageApiOptions>,
+  ): StorageApi => {
+    return WebStorage.create({
+      errorApi: mockErrorApi,
+      ...args,
+    });
+  };
   it('should return undefined for values which are unset', async () => {
-    const storage = new WebStorage();
+    const storage = createWebStorage();
 
     expect(storage.get('myfakekey')).toBeUndefined();
   });
 
   it('should allow the setting and getting of the simple data structures', async () => {
-    const storage = new WebStorage();
+    const storage = createWebStorage();
 
     await storage.set('myfakekey', 'helloimastring');
     await storage.set('mysecondfakekey', 1234);
@@ -33,7 +43,7 @@ describe('WebStorage Storage API', () => {
   });
 
   it('should allow setting of complex datastructures', async () => {
-    const storage = new WebStorage();
+    const storage = createWebStorage();
 
     const mockData = {
       something: 'here',
@@ -46,14 +56,14 @@ describe('WebStorage Storage API', () => {
   });
 
   it('should subscribe to key changes when setting a new value', async () => {
-    const storage = new WebStorage();
+    const storage = createWebStorage();
 
     const wrongKeyNextHandler = jest.fn();
     const selectedKeyNextHandler = jest.fn();
     const mockData = { hello: 'im a great new value' };
 
     await new Promise(resolve => {
-      storage.observe$('correctKey').subscribe({
+      storage.observe$<String>('correctKey').subscribe({
         next: (...args) => {
           selectedKeyNextHandler(...args);
           resolve();
@@ -74,7 +84,7 @@ describe('WebStorage Storage API', () => {
   });
 
   it('should subscribe to key changes when deleting a value', async () => {
-    const storage = new WebStorage();
+    const storage = createWebStorage();
 
     const wrongKeyNextHandler = jest.fn();
     const selectedKeyNextHandler = jest.fn();
@@ -104,7 +114,7 @@ describe('WebStorage Storage API', () => {
   });
 
   it('should be able to create different buckets for different uses', async () => {
-    const rootStorage = new WebStorage();
+    const rootStorage = createWebStorage();
 
     const firstStorage = rootStorage.forBucket('userSettings');
     const secondStorage = rootStorage.forBucket('profileSettings');
@@ -119,7 +129,7 @@ describe('WebStorage Storage API', () => {
   });
 
   it('should not clash with other namesapces when creating buckets', async () => {
-    const rootStorage = new WebStorage();
+    const rootStorage = createWebStorage();
 
     // when getting key test2 it will translate to /profile/something/deep/test2
     const firstStorage = rootStorage
@@ -133,4 +143,22 @@ describe('WebStorage Storage API', () => {
 
     expect(secondStorage.get('deep/test2')).toBe(undefined);
   });
+
+  it('should call the error api when the json can not be parsed in local storage', async () => {
+    const rootStorage = createWebStorage({
+      namespace: '/Test/Mock/Thing',
+    });
+
+    localStorage.setItem('/Test/Mock/Thing/key', '{smd: asdouindA}');
+
+    const value = rootStorage.get('key');
+
+    expect(value).toBe(undefined);
+    expect(mockErrorApi.post).toHaveBeenCalledWith(expect.any(Error));
+    expect(mockErrorApi.post).toHaveBeenCalledWith(
+      expect.objectContaining({
+        message: 'Error when parsing JSON config from storage for: key',
+      }),
+    );
+  });
 });

packages/core-api/src/apis/implementations/StorageApi/WebStorage.ts

@@ -13,15 +13,23 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import { StorageApi, StorageValueChange } from '../../definitions';
+import {
+  StorageApi,
+  StorageValueChange,
+  ErrorApi,
+  CreateStorageApiOptions,
+} from '../../definitions';
 import { Observable } from '../../../types';
 import ObservableImpl from 'zen-observable';
 
 export class WebStorage implements StorageApi {
-  private readonly namespace: string;
+  constructor(
+    private readonly namespace: string,
+    private readonly errorApi: ErrorApi,
+  ) {}
 
-  constructor(namespace: string = '') {
-    this.namespace = namespace ? encodeURIComponent(namespace) : namespace;
+  static create(options: CreateStorageApiOptions): WebStorage {
+    return new WebStorage(options.namespace ?? '', options.errorApi);
   }
 
   get<T>(key: string): T | undefined {
@@ -29,9 +37,8 @@ export class WebStorage implements StorageApi {
       const storage = JSON.parse(localStorage.getItem(this.getKeyName(key))!);
       return storage ?? undefined;
     } catch (e) {
-      window.console.error(
-        `Error when parsing JSON config from storage for: ${key}`,
-        e,
+      this.errorApi.post(
+        new Error(`Error when parsing JSON config from storage for: ${key}`),
       );
     }
 
@@ -39,7 +46,7 @@ export class WebStorage implements StorageApi {
   }
 
   forBucket(name: string): WebStorage {
-    return new WebStorage(`${this.namespace}/${name}`);
+    return new WebStorage(`${this.namespace}/${name}`, this.errorApi);
   }
 
   async set<T>(key: string, data: T): Promise<void> {

packages/core-api/src/apis/implementations/index.ts

@@ -25,3 +25,4 @@ export * from './AppThemeApi';
 export * from './ConfigApi';
 export * from './ErrorApi';
 export * from './OAuthRequestApi';
+export * from './StorageApi';