Add some tests & fix a couple issues with local storage events

Author: avolkovi

packages-exp/auth-exp/src/core/user/reload.test.ts

@@ -31,7 +31,7 @@ import {
   ProviderUserInfo
 } from '../../api/account_management/account';
 import { _reloadWithoutSaving, reload } from './reload';
-import { UserMetadata } from './user_impl';
+import { UserMetadata } from './user_metadata';
 
 use(chaiAsPromised);
 use(sinonChai);

packages-exp/auth-exp/src/platform_browser/persistence/local_storage.test.ts

@@ -15,8 +15,9 @@
  * limitations under the License.
  */
 
-import { expect } from 'chai';
+import { expect, use } from 'chai';
 import * as sinon from 'sinon';
+import * as sinonChai from 'sinon-chai';
 import { testAuth, testUser } from '../../../test/helpers/mock_auth';
 import {
   PersistedBlob,
@@ -26,53 +27,175 @@ import {
 import { _getInstance } from '../../core/util/instantiator';
 import { browserLocalPersistence } from './local_storage';
 
-describe('core/persistence/browser', () => {
+use(sinonChai);
+
+describe('browserLocalPersistence', () => {
+  const persistence: Persistence = _getInstance(browserLocalPersistence);
+
   beforeEach(() => {
     localStorage.clear();
-    sessionStorage.clear();
   });
 
   afterEach(() => sinon.restore());
-  describe('browserLocalPersistence', () => {
-    const persistence: Persistence = _getInstance(browserLocalPersistence);
-
-    it('should work with persistence type', async () => {
-      const key = 'my-super-special-persistence-type';
-      const value = PersistenceType.LOCAL;
-      expect(await persistence.get(key)).to.be.null;
-      await persistence.set(key, value);
-      expect(await persistence.get(key)).to.be.eq(value);
-      expect(await persistence.get('other-key')).to.be.null;
-      await persistence.remove(key);
-      expect(await persistence.get(key)).to.be.null;
+
+  it('should work with persistence type', async () => {
+    const key = 'my-super-special-persistence-type';
+    const value = PersistenceType.LOCAL;
+    expect(await persistence.get(key)).to.be.null;
+    await persistence.set(key, value);
+    expect(await persistence.get(key)).to.be.eq(value);
+    expect(await persistence.get('other-key')).to.be.null;
+    await persistence.remove(key);
+    expect(await persistence.get(key)).to.be.null;
+  });
+
+  it('should return persistedblob from user', async () => {
+    const key = 'my-super-special-user';
+    const auth = await testAuth();
+    const value = testUser(auth, 'some-uid');
+
+    expect(await persistence.get(key)).to.be.null;
+    await persistence.set(key, value.toJSON());
+    const out = await persistence.get<PersistedBlob>(key);
+    expect(out!['uid']).to.eql(value.uid);
+    await persistence.remove(key);
+    expect(await persistence.get(key)).to.be.null;
+  });
+
+  describe('#isAvailable', () => {
+    it('should emit false if localStorage setItem throws', async () => {
+      sinon.stub(localStorage, 'setItem').throws(new Error('nope'));
+      expect(await persistence.isAvailable()).to.be.false;
+    });
+
+    it('should emit false if localStorage removeItem throws', async () => {
+      sinon.stub(localStorage, 'removeItem').throws(new Error('nope'));
+      expect(await persistence.isAvailable()).to.be.false;
+    });
+
+    it('should emit true if everything works properly', async () => {
+      expect(await persistence.isAvailable()).to.be.true;
+    });
+  });
+
+  describe('#addEventListener', () => {
+    const key = 'my-key';
+    const newValue = 'new-value';
+
+    let callback: sinon.SinonSpy;
+
+    beforeEach(() => {
+      callback = sinon.spy();
+      persistence.addListener(key, callback);
+    });
+
+    afterEach(() => {
+      persistence.removeListener(key, callback);
+    });
+
+    context('with multiple listeners', () => {
+      let otherCallback: sinon.SinonSpy;
+
+      beforeEach(() => {
+        otherCallback = sinon.spy();
+        persistence.addListener(key, otherCallback);
+        localStorage.setItem(key, JSON.stringify(newValue));
+      });
+
+      afterEach(() => {
+        persistence.removeListener(key, otherCallback);
+      });
+
+      it('should trigger both listeners if multiple listeners are registered', () => {
+        window.dispatchEvent(
+          new StorageEvent('storage', {
+            key,
+            oldValue: null,
+            newValue: JSON.stringify(newValue)
+          })
+        );
+
+        expect(callback).to.have.been.calledWith(newValue);
+        expect(otherCallback).to.have.been.calledWith(newValue);
+      });
     });
 
-    it('should return persistedblob from user', async () => {
-      const key = 'my-super-special-user';
-      const auth = await testAuth();
-      const value = testUser(auth, 'some-uid');
-
-      expect(await persistence.get(key)).to.be.null;
-      await persistence.set(key, value.toJSON());
-      const out = await persistence.get<PersistedBlob>(key);
-      expect(out!['uid']).to.eql(value.uid);
-      await persistence.remove(key);
-      expect(await persistence.get(key)).to.be.null;
+    context('with a change in the underlying storage', () => {
+      beforeEach(() => {
+        localStorage.setItem(key, JSON.stringify(newValue));
+      });
+
+      it('should trigger on storage event for the same key', () => {
+        window.dispatchEvent(
+          new StorageEvent('storage', {
+            key,
+            oldValue: null,
+            newValue: JSON.stringify(newValue)
+          })
+        );
+
+        expect(callback).to.have.been.calledWith(newValue);
+      });
+
+      it('should not trigger after unsubscribe', () => {
+        persistence.removeListener(key, callback);
+        window.dispatchEvent(
+          new StorageEvent('storage', {
+            key,
+            oldValue: null,
+            newValue: JSON.stringify(newValue)
+          })
+        );
+
+        expect(callback).not.to.have.been.called;
+      });
+
+      it('should trigger even if the event had no key', () => {
+        window.dispatchEvent(new StorageEvent('storage', {}));
+
+        expect(callback).to.have.been.calledWith(newValue);
+      });
     });
 
-    describe('#isAvailable', () => {
-      it('should emit false if localStorage setItem throws', async () => {
-        sinon.stub(localStorage, 'setItem').throws(new Error('nope'));
-        expect(await persistence.isAvailable()).to.be.false;
+    context('without a change in the underlying storage', () => {
+      it('should not trigger', () => {
+        window.dispatchEvent(
+          new StorageEvent('storage', {
+            key,
+            oldValue: null,
+            newValue: JSON.stringify(newValue)
+          })
+        );
+
+        expect(callback).not.to.have.been.called;
       });
 
-      it('should emit false if localStorage removeItem throws', async () => {
-        sinon.stub(localStorage, 'removeItem').throws(new Error('nope'));
-        expect(await persistence.isAvailable()).to.be.false;
+      it('should not trigger on storage event for a different key', () => {
+        localStorage.setItem('other-key', JSON.stringify(newValue));
+        window.dispatchEvent(
+          new StorageEvent('storage', {
+            key: 'other-key',
+            oldValue: null,
+            newValue: JSON.stringify(newValue)
+          })
+        );
+
+        expect(callback).not.to.have.been.called;
       });
 
-      it('should emit true if everything works properly', async () => {
-        expect(await persistence.isAvailable()).to.be.true;
+      it('should not trigger if the listener was added after the storage was updated', () => {
+        const otherCallback = sinon.spy();
+        persistence.addListener(key, otherCallback);
+
+        window.dispatchEvent(
+          new StorageEvent('storage', {
+            key,
+            oldValue: null,
+            newValue: JSON.stringify(newValue)
+          })
+        );
+
+        expect(otherCallback).not.to.have.been.called;
       });
     });
   });

packages-exp/auth-exp/src/platform_browser/persistence/local_storage.ts

@@ -192,7 +192,7 @@ class BrowserLocalPersistence extends BrowserPersistenceClass
   }
 
   private _attachListener(): void {
-    window.addEventListener('storage', this._onStorageEvent);
+    window.addEventListener('storage', this._onStorageEvent.bind(this));
   }
 
   private _detachListener(): void {
@@ -212,7 +212,7 @@ class BrowserLocalPersistence extends BrowserPersistenceClass
         this._attachListener();
       }
     }
-    this.listeners[key] = this.listeners[key] || [];
+    this.listeners[key] = this.listeners[key] || new Set();
     this.listeners[key].add(listener);
   }