feat: new IDB storing behavior with no wrapping (#82)

Author: cyrilletuzi

README.md

@@ -60,7 +60,7 @@ import { LocalStorage } from '@ngx-pwa/local-storage';
 @Injectable()
 export class YourService {
 
-  constructor(protected localStorage: LocalStorage) {}
+  constructor(private localStorage: LocalStorage) {}
 
 }
 ```

projects/ngx-pwa/local-storage/src/lib/databases/indexeddb-database.ts

@@ -1,9 +1,12 @@
-import { Injectable, Optional, Inject } from '@angular/core';
+import { Injectable, Inject } from '@angular/core';
 import { Observable, ReplaySubject, fromEvent, of, throwError, race } from 'rxjs';
 import { map, mergeMap, first, tap, filter } from 'rxjs/operators';
 
 import { LocalDatabase } from './local-database';
-import { PREFIX, IDB_DB_NAME, DEFAULT_IDB_DB_NAME, IDB_STORE_NAME, DEFAULT_IDB_STORE_NAME } from '../tokens';
+import {
+  PREFIX, IDB_DB_NAME, DEFAULT_IDB_DB_NAME, IDB_STORE_NAME, DEFAULT_IDB_STORE_NAME,
+  COMPATIBILITY_PRIOR_TO_V8, DEFAULT_PREFIX, DEFAULT_COMPATIBILITY_PRIOR_TO_V8
+} from '../tokens';
 import { IDBBrokenError } from '../exceptions';
 
 @Injectable({
@@ -14,18 +17,23 @@ export class IndexedDBDatabase implements LocalDatabase {
   /**
    * `indexedDB` database name
    */
-  protected dbName: string;
+  private readonly dbName: string;
 
   /**
    * `indexedDB` object store name
    */
-  protected storeName: string;
+  private readonly storeName: string;
 
   /**
    * `indexedDB` data path name for local storage (where items' value will be stored)
    */
   private readonly dataPath = 'value';
 
+  /**
+   * Flag to keep storing behavior prior to version 8.
+   */
+  private readonly compatibilityPriorToV8: boolean;
+
   /**
    * `indexedDB` database connection, wrapped in a RxJS `ReplaySubject` to be able to access the connection
    * even after the connection success event happened
@@ -59,11 +67,13 @@ export class IndexedDBDatabase implements LocalDatabase {
    * @param prefix Optional user prefix to avoid collision for multiple apps on the same subdomain
    * @param dbName `indexedDB` database name
    * @param storeName `indexedDB` store name
+   * @param compatibilityPriorToV8 Flag to keep storing behavior prior to version 8
    */
   constructor(
-    @Optional() @Inject(PREFIX) prefix: string | null = null,
-    @Optional() @Inject(IDB_DB_NAME) dbName = DEFAULT_IDB_DB_NAME,
-    @Optional() @Inject(IDB_STORE_NAME) storeName = DEFAULT_IDB_STORE_NAME,
+    @Inject(PREFIX) prefix = DEFAULT_PREFIX,
+    @Inject(IDB_DB_NAME) dbName = DEFAULT_IDB_DB_NAME,
+    @Inject(IDB_STORE_NAME) storeName = DEFAULT_IDB_STORE_NAME,
+    @Inject(COMPATIBILITY_PRIOR_TO_V8) compatibilityPriorToV8 = DEFAULT_COMPATIBILITY_PRIOR_TO_V8,
   ) {
 
     /* Initialize `indexedDB` database name, with prefix if provided by the user */
@@ -72,8 +82,10 @@ export class IndexedDBDatabase implements LocalDatabase {
     /* Initialize `indexedDB` store name */
     this.storeName = storeName;
 
+    this.compatibilityPriorToV8 = compatibilityPriorToV8;
+
     /* Creating the RxJS ReplaySubject */
-    this.database = new ReplaySubject<IDBDatabase>();
+    this.database = new ReplaySubject<IDBDatabase>(1);
 
     /* Connect to `indexedDB`, with prefix if provided by the user */
     this.connect();
@@ -97,24 +109,20 @@ export class IndexedDBDatabase implements LocalDatabase {
         /* Manage success and error events, and get the result */
         return this.requestEventsAndMapTo(request, () => {
 
-          /* Currently, the lib is wrapping the value in a `{ value: ... }` object, so test this case */
-          // TODO: add a check to see if the object has only one key
-          // TODO: stop wrapping
-          if ((request.result !== undefined)
+          if (!this.compatibilityPriorToV8 && (request.result !== undefined) && (request.result !== null)) {
+
+            /* Cast to the wanted type */
+            return request.result as T;
+
+          } else if (this.compatibilityPriorToV8
+          && (request.result !== undefined)
           && (request.result !== null)
-          && (typeof request.result === 'object')
-          && (this.dataPath in request.result)
           && (request.result[this.dataPath] !== undefined)
           && (request.result[this.dataPath] !== null)) {
 
-            /* If so, unwrap the value and cast it to the wanted type */
+            /* Prior to v8, the value was wrapped in an `{ value: ...}` object */
             return (request.result[this.dataPath] as T);
 
-          } else if ((request.result !== undefined) && (request.result !== null)) {
-
-            /* Otherwise, return the value directly, casted to the wanted type */
-            return request.result as T;
-
           }
 
           /* Return `null` if the value is `null` or `undefined` */
@@ -165,11 +173,13 @@ export class IndexedDBDatabase implements LocalDatabase {
              * otherwise it could lead to concurrency failures
              * Avoid https://github.com/cyrilletuzi/angular-async-local-storage/issues/47 */
 
+             /* Prior to v8, data was wrapped in a `{ value: ... }` object */
+            const dataToStore = !this.compatibilityPriorToV8 ? data : { [this.dataPath]: data };
+
             /* Add if the item is not existing yet, or update otherwise */
-            // TODO: stop wrapping
             const request2 = (existingEntry === undefined) ?
-              store.add({ [this.dataPath]: data }, key) :
-              store.put({ [this.dataPath]: data }, key);
+              store.add(dataToStore, key) :
+              store.put(dataToStore, key);
 
             /* Manage success and error events, and map to `true` */
             return this.requestEventsAndMapTo(request2, () => true);

projects/ngx-pwa/local-storage/src/lib/databases/local-database.ts

@@ -1,4 +1,4 @@
-import { Injectable, PLATFORM_ID, Optional } from '@angular/core';
+import { Injectable, PLATFORM_ID } from '@angular/core';
 import { isPlatformBrowser, isPlatformWorkerApp, isPlatformWorkerUi } from '@angular/common';
 import { Observable } from 'rxjs';
 
@@ -13,7 +13,7 @@ import { PREFIX } from '../tokens';
  * @param prefix Optional user prefix to avoid collision for multiple apps on the same subdomain
  * @see https://github.com/cyrilletuzi/angular-async-local-storage/blob/master/docs/BROWSERS_SUPPORT.md
  */
-export function localDatabaseFactory(platformId: Object, prefix: string | null): LocalDatabase {
+export function localDatabaseFactory(platformId: Object, prefix: string): LocalDatabase {
 
   // Do not explicit `window` here, as the global object is not the same in web workers
   if ((isPlatformBrowser(platformId) || isPlatformWorkerApp(platformId) || isPlatformWorkerUi(platformId))
@@ -63,7 +63,7 @@ export function localDatabaseFactory(platformId: Object, prefix: string | null):
   useFactory: localDatabaseFactory,
   deps: [
     PLATFORM_ID,
-    [new Optional(), PREFIX]
+    PREFIX
   ]
 })
 export abstract class LocalDatabase {

projects/ngx-pwa/local-storage/src/lib/databases/localstorage-database.ts

@@ -1,8 +1,8 @@
-import { Injectable, Optional, Inject } from '@angular/core';
+import { Injectable, Inject } from '@angular/core';
 import { Observable, of, throwError } from 'rxjs';
 
 import { LocalDatabase } from './local-database';
-import { PREFIX } from '../tokens';
+import { PREFIX, DEFAULT_PREFIX } from '../tokens';
 
 @Injectable({
   providedIn: 'root'
@@ -12,7 +12,7 @@ export class LocalStorageDatabase implements LocalDatabase {
   /**
    * Optional user prefix to avoid collision for multiple apps on the same subdomain
    */
-  protected prefix = '';
+  private readonly prefix: string;
 
   /**
    * Number of items in `localStorage`
@@ -28,11 +28,10 @@ export class LocalStorageDatabase implements LocalDatabase {
    * Constructor params are provided by Angular (but can also be passed manually in tests)
    * @param prefix Optional user prefix to avoid collision for multiple apps on the same subdomain
    */
-  constructor(@Optional() @Inject(PREFIX) userPrefix: string | null = null) {
+  constructor(@Inject(PREFIX) prefix = DEFAULT_PREFIX) {
 
-    if (userPrefix) {
-      this.prefix = `${userPrefix}_`;
-    }
+    /* Add `_` after prefix only if not empty */
+    this.prefix = prefix ? `${prefix}_` : prefix;
 
   }
 

projects/ngx-pwa/local-storage/src/lib/lib.service.spec.ts

@@ -738,7 +738,7 @@ describe('IndexedDB', () => {
 
           request.addEventListener('success', () => {
 
-            expect(request.result).toEqual({ value });
+            expect(request.result).toEqual(value);
 
             done();
 
@@ -766,21 +766,32 @@ describe('IndexedDB', () => {
 
 });
 
+describe('IndexedDB with compatibilityPriorToV8', () => {
+
+  const localStorageService = new LocalStorage(new IndexedDBDatabase(undefined, undefined, undefined, true), new JSONValidator());
+
+  beforeEach((done) => {
+
+    /* Clear `localStorage` for some browsers private mode which fallbacks to `localStorage` */
+    localStorage.clear();
+
+    clearIndexedDB(done);
+
+  });
+
+  tests(localStorageService);
+
+});
+
 describe('localStorage and a prefix', () => {
 
   const prefix = 'myapp';
 
   it('check prefix', () => {
 
-    class LocalStorageDatabasePrefix extends LocalStorageDatabase {
-      getPrefix() {
-        return this.prefix;
-      }
-    }
-
-    const localStorageServicePrefix = new LocalStorageDatabasePrefix(prefix);
+    const localStorageServicePrefix = new LocalStorageDatabase(prefix);
 
-    expect(localStorageServicePrefix.getPrefix()).toBe(`${prefix}_`);
+    expect(localStorageServicePrefix['prefix']).toBe(`${prefix}_`);
 
   });
 
@@ -800,33 +811,19 @@ describe('IndexedDB and a prefix', () => {
 
   it('check prefix', () => {
 
-    class IndexedDBDatabasePrefix extends IndexedDBDatabase {
-      getDbBame() {
-        return this.dbName;
-      }
-    }
+    const indexedDBService = new IndexedDBDatabase(prefix);
 
-    const indexedDBService = new IndexedDBDatabasePrefix(prefix);
-
-    expect(indexedDBService.getDbBame()).toBe(`${prefix}_${DEFAULT_IDB_DB_NAME}`);
+    expect(indexedDBService['dbName']).toBe(`${prefix}_${DEFAULT_IDB_DB_NAME}`);
 
   });
 
   it('check prefix with custom database and store names', () => {
 
     const dbName = 'customDb';
 
-    class IndexedDBDatabasePrefix extends IndexedDBDatabase {
-
-      getDbBame() {
-        return this.dbName;
-      }
-
-    }
-
-    const indexedDBService = new IndexedDBDatabasePrefix(prefix, dbName);
+    const indexedDBService = new IndexedDBDatabase(prefix, dbName);
 
-    expect(indexedDBService.getDbBame()).toBe(`${prefix}_${dbName}`);
+    expect(indexedDBService['dbName']).toBe(`${prefix}_${dbName}`);
 
   });
 
@@ -850,7 +847,7 @@ describe('IndexedDB with custom database and store names', () => {
   const dbName = 'dBcustom';
   const storeName = 'storeCustom';
 
-  const localStorageService = new LocalStorage(new IndexedDBDatabase(null, dbName, storeName), new JSONValidator());
+  const localStorageService = new LocalStorage(new IndexedDBDatabase(undefined, dbName, storeName), new JSONValidator());
 
   beforeEach((done) => {
 

projects/ngx-pwa/local-storage/src/lib/lib.service.ts

@@ -1,4 +1,4 @@
-import { Injectable, Optional, Inject } from '@angular/core';
+import { Injectable, Inject } from '@angular/core';
 import { Observable, throwError, of, OperatorFunction } from 'rxjs';
 import { mergeMap, catchError } from 'rxjs/operators';
 
@@ -49,7 +49,7 @@ export class LocalStorage {
   constructor(
     private database: LocalDatabase,
     private jsonValidator: JSONValidator,
-    @Optional() @Inject(PREFIX) protected prefix: string | null = null,
+    @Inject(PREFIX) private prefix = '',
   ) {}
 
   /**

projects/ngx-pwa/local-storage/src/lib/tokens.ts

@@ -1,17 +1,18 @@
 import { InjectionToken, Provider } from '@angular/core';
 
 /**
- * Internal. Use the `localStorageProviders()` helper function to provide options.
+ * Default prefix.
+ * *Use only to avoid conflict in multiple apps on the same subdomain.*
  */
-export const PREFIX = new InjectionToken<string>('localStoragePrefix', {
-  providedIn: 'root',
-  factory: () => ''
-});
+export const DEFAULT_PREFIX = '';
 
 /**
- * @deprecated Use the `localStorageProviders()` helper function to provide options. Will be removed in v9.
+ * Token to provide a prefix to avoid collision when multiple apps on the same subdomain.
  */
-export const LOCAL_STORAGE_PREFIX = PREFIX;
+export const PREFIX = new InjectionToken<string>('localStoragePrefix', {
+  providedIn: 'root',
+  factory: () => DEFAULT_PREFIX
+});
 
 /**
  * Default name used for `indexedDB` database.
@@ -20,9 +21,9 @@ export const LOCAL_STORAGE_PREFIX = PREFIX;
 export const DEFAULT_IDB_DB_NAME = 'ngStorage';
 
 /**
- * Internal. Use the `localStorageProviders()` helper function to provide options.
+ * Token to provide `indexedDB` database name.
  */
-export const IDB_DB_NAME = new InjectionToken<string>('localStorageIndexedDbName', {
+export const IDB_DB_NAME = new InjectionToken<string>('localStorageIDBDBName', {
   providedIn: 'root',
   factory: () => DEFAULT_IDB_DB_NAME
 });
@@ -34,13 +35,27 @@ export const IDB_DB_NAME = new InjectionToken<string>('localStorageIndexedDbName
 export const DEFAULT_IDB_STORE_NAME = 'localStorage';
 
 /**
- * Internal. Use the `localStorageProviders()` helper function to provide options.
+ * Token to provide `indexedDB` store name.
  */
-export const IDB_STORE_NAME = new InjectionToken<string>('localStorageIndexedDbStoreName', {
+export const IDB_STORE_NAME = new InjectionToken<string>('localStorageIDBStoreName', {
   providedIn: 'root',
   factory: () => DEFAULT_IDB_STORE_NAME
 });
 
+// TODO: revert to true by default if ng update is not possible
+/**
+ * Default compatibility mode.
+ */
+export const DEFAULT_COMPATIBILITY_PRIOR_TO_V8 = false;
+
+/**
+ * Token to keep storing behavior prior to version 8.
+ */
+export const COMPATIBILITY_PRIOR_TO_V8 = new InjectionToken<boolean>('localStorageCompatibilityPriorToV8', {
+  providedIn: 'root',
+  factory: () => DEFAULT_COMPATIBILITY_PRIOR_TO_V8
+});
+
 export interface LocalStorageProvidersConfig {
 
   /**
@@ -64,6 +79,13 @@ export interface LocalStorageProvidersConfig {
    */
   IDBStoreName?: string;
 
+  /**
+   * Flag to keep storing behavior prior to version 8.
+   * Not needed for new installs,
+   * **must be `true` for upgrades from versions prior to V8, otherwise previously stored data will be lost.**
+   */
+  compatibilityPriorToV8?: boolean;
+
 }
 
 /**
@@ -77,6 +99,7 @@ export function localStorageProviders(config: LocalStorageProvidersConfig): Prov
     config.prefix ? { provide: PREFIX, useValue: config.prefix } : [],
     config.IDBDBName ? { provide: IDB_DB_NAME, useValue: config.IDBDBName } : [],
     config.IDBStoreName ? { provide: IDB_STORE_NAME, useValue: config.IDBStoreName } : [],
+    config.compatibilityPriorToV8 ? { provide: COMPATIBILITY_PRIOR_TO_V8, useValue: config.compatibilityPriorToV8 } : [],
   ];
 
 }