HARP-12352: subsequent DataProvider.register() returns early

* subsequent register operations should resolve after first connect Signed-off-by: stefan.dachwitz <stefan.dachwitz@here.com>
heremaps · Nov 10, 2020 · c60b6c4 · c60b6c4
1 parent 863722d
commit c60b6c4
Show file tree

Hide file tree

Showing 2 changed files with 88 additions and 2 deletions.
diff --git a/@here/harp-mapview-decoder/lib/DataProvider.ts b/@here/harp-mapview-decoder/lib/DataProvider.ts
@@ -19,6 +19,7 @@ import { EventDispatcher } from "three";
  */
 export abstract class DataProvider extends EventDispatcher {
     private readonly m_clients: Set<Object> = new Set();
+    private m_connectPromise: Promise<void> | undefined;
 
     /**
      * Registers a client to the data provider.
@@ -27,9 +28,11 @@ export abstract class DataProvider extends EventDispatcher {
      * @returns Promise to wait for successful (or failed) connection to the data source.
      */
     register(client: Object): Promise<void> {
-        const result = this.m_clients.size === 0 ? this.connect() : Promise.resolve();
+        if (this.m_clients.size === 0) {
+            this.m_connectPromise = this.connect();
+        }
         this.m_clients.add(client);
-        return result;
+        return this.m_connectPromise!;
     }
 
     /**

diff --git a/@here/harp-mapview-decoder/test/DataProviderTest.ts b/@here/harp-mapview-decoder/test/DataProviderTest.ts
@@ -29,6 +29,17 @@ class FakeDataProvider extends DataProvider {
     }
 }
 
+class TimedFakeDataProvider extends FakeDataProvider {
+    /** @override */ async connect(): Promise<void> {
+        const promise = new Promise<void>(resolve => {
+            setTimeout(() => {
+                resolve();
+            }, 100);
+        });
+        return promise;
+    }
+}
+
 describe("DataProvider", function() {
     const clients = [{}, {}];
 
@@ -60,4 +71,76 @@ describe("DataProvider", function() {
         dataProvider.unregister(clients[1]);
         expect(disposeSpy.calledOnce).is.true;
     });
+
+    it("calls connect only on first registered client", async function() {
+        const dataProvider = new TimedFakeDataProvider();
+        const connectSpy = sinon.spy(dataProvider, "connect");
+
+        await dataProvider.register(clients[0]);
+        expect(connectSpy.calledOnce).is.true;
+
+        await dataProvider.register(clients[0]);
+        await dataProvider.register(clients[1]);
+        expect(connectSpy.calledOnce).is.true;
+    });
+
+    it("calls connect again after unregister", async function() {
+        const dataProvider = new TimedFakeDataProvider();
+        const connectSpy = sinon.spy(dataProvider, "connect");
+
+        dataProvider.register(clients[0]);
+        expect(connectSpy.calledOnce).is.true;
+
+        dataProvider.unregister(clients[0]);
+        expect(connectSpy.calledOnce).is.true;
+
+        dataProvider.register(clients[0]);
+        expect(connectSpy.calledTwice).is.true;
+    });
+
+    it("calls connect after one provider call unregister", async function() {
+        const dataProvider = new TimedFakeDataProvider();
+
+        const promise0 = dataProvider.register(clients[0]);
+        const promise1 = dataProvider.register(clients[1]);
+        dataProvider.unregister(clients[0]);
+
+        let promise0Resolved = false;
+        await promise0.then(() => {
+            promise0Resolved = true;
+        });
+
+        let promise1Resolved = false;
+        await promise1.then(() => {
+            promise1Resolved = true;
+        });
+
+        expect(promise0Resolved, "unregistered provider resolve not called").to.be.true;
+        expect(promise1Resolved, "registered provider resolve not called after an unregister").to.be
+            .true;
+    });
+
+    it("subsequent register operations wait for connect", async function() {
+        const dataProvider = new TimedFakeDataProvider();
+        const connectSpy = sinon.spy(dataProvider, "connect");
+
+        const promise0 = dataProvider.register(clients[0]);
+        const promise1 = dataProvider.register(clients[1]);
+
+        let promise0Resolved = false;
+
+        const _promise2 = promise0.then(() => {
+            promise0Resolved = true;
+        });
+
+        expect(promise0Resolved, "promise resolved immediately").to.be.false;
+
+        // Assert that when promise1 is resolved, promise0 is also resolved.
+        await promise1;
+
+        // Assert that at this stage, promise0Resolved is true. State of promise2 can be ignored
+        // here.
+        expect(promise0Resolved, "subsequent promises resolve before the first").to.be.true;
+        expect(connectSpy.calledOnce).is.true;
+    });
 });