Merge pull request #6186 from afshin/api-requests

Update Poll#schedule() semantics.

packages/coreutils/src/poll.ts

@@ -175,17 +175,17 @@ export class Poll<T = any, U = any> implements IDisposable, IPoll<T, U> {
       ...(options.frequency || {})
     };
     this.name = options.name || Private.DEFAULT_NAME;
-    this.ready = (options.when || Promise.resolve())
-      .then(() => {
+
+    // Schedule poll ticks after `when` promise is settled.
+    (options.when || Promise.resolve())
+      .then(_ => {
         if (this.isDisposed) {
           return;
         }
 
-        this.schedule({
+        return this.schedule({
           interval: Private.IMMEDIATE,
-          payload: null,
-          phase: 'when-resolved',
-          timestamp: new Date().getTime()
+          phase: 'when-resolved'
         });
       })
       .catch(reason => {
@@ -193,14 +193,12 @@ export class Poll<T = any, U = any> implements IDisposable, IPoll<T, U> {
           return;
         }
 
-        this.schedule({
+        console.warn(`Poll (${this.name}) started despite rejection.`, reason);
+
+        return this.schedule({
           interval: Private.IMMEDIATE,
-          payload: null,
-          phase: 'when-rejected',
-          timestamp: new Date().getTime()
+          phase: 'when-rejected'
         });
-
-        console.warn(`Poll (${this.name}) started despite rejection.`, reason);
       });
   }
 
@@ -312,23 +310,12 @@ export class Poll<T = any, U = any> implements IDisposable, IPoll<T, U> {
    *
    * @returns A promise that resolves after tick is scheduled and never rejects.
    */
-  async refresh(): Promise<void> {
-    if (this.isDisposed) {
-      return;
-    }
-
-    if (this.state.phase === 'constructed') {
-      await this.ready;
-    }
-
-    if (this.state.phase !== 'refreshed') {
-      this.schedule({
-        interval: Private.IMMEDIATE,
-        payload: null,
-        phase: 'refreshed',
-        timestamp: new Date().getTime()
-      });
-    }
+  refresh(): Promise<void> {
+    return this.schedule({
+      cancel: last => last.phase === 'refreshed',
+      interval: Private.IMMEDIATE,
+      phase: 'refreshed'
+    });
   }
 
   /**
@@ -336,23 +323,12 @@ export class Poll<T = any, U = any> implements IDisposable, IPoll<T, U> {
    *
    * @returns A promise that resolves after tick is scheduled and never rejects.
    */
-  async start(): Promise<void> {
-    if (this.isDisposed) {
-      return;
-    }
-
-    if (this.state.phase === 'constructed') {
-      await this.ready;
-    }
-
-    if (this.state.phase === 'standby' || this.state.phase === 'stopped') {
-      this.schedule({
-        interval: Private.IMMEDIATE,
-        payload: null,
-        phase: 'started',
-        timestamp: new Date().getTime()
-      });
-    }
+  start(): Promise<void> {
+    return this.schedule({
+      cancel: last => last.phase !== 'standby' && last.phase !== 'stopped',
+      interval: Private.IMMEDIATE,
+      phase: 'started'
+    });
   }
 
   /**
@@ -360,59 +336,58 @@ export class Poll<T = any, U = any> implements IDisposable, IPoll<T, U> {
    *
    * @returns A promise that resolves after tick is scheduled and never rejects.
    */
-  async stop(): Promise<void> {
-    if (this.isDisposed) {
-      return;
-    }
-
-    if (this.state.phase === 'constructed') {
-      await this.ready;
-    }
-
-    if (this.state.phase !== 'stopped') {
-      this.schedule({
-        interval: Private.NEVER,
-        payload: null,
-        phase: 'stopped',
-        timestamp: new Date().getTime()
-      });
-    }
+  stop(): Promise<void> {
+    return this.schedule({
+      cancel: last => last.phase === 'stopped',
+      interval: Private.NEVER,
+      phase: 'stopped'
+    });
   }
 
   /**
-   * A promise that resolves after the poll has scheduled its first tick.
+   * Schedule the next poll tick.
    *
-   * #### Notes
-   * This accessor is protected to allow sub-classes to implement methods that
-   * can `await this.ready` if `this.state.phase === 'constructed'`.
+   * @param next - The next poll state data to schedule. Defaults to standby.
    *
-   * A poll should handle ready state without needing to expose it to clients.
-   */
-  protected readonly ready: Promise<void>;
-
-  /**
-   * Schedule the next poll tick.
+   * @param next.cancel - Cancels state transition if function returns `true`.
    *
-   * @param state - The new poll state data to schedule.
+   * @returns A promise that resolves when the next poll state is active.
    *
    * #### Notes
    * This method is protected to allow sub-classes to implement methods that can
    * schedule poll ticks.
-   *
-   * This method synchronously modifies poll state so it should be invoked with
-   * consideration given to the poll state that will be overwritten. It should
-   * be invoked only once in any synchronous context, otherwise the `ticked`
-   * signal and the `tick` promise will fall out of sync with each other.
-   *
-   * It will typically be invoked in a method that returns a promise, allowing
-   * the caller e.g. to `await this.ready` before scheduling a new tick.
    */
-  protected schedule(state: IPoll.State<T, U>): void {
-    const last = this.state;
+  protected async schedule(
+    next: Partial<IPoll.State & { cancel: (last: IPoll.State) => boolean }> = {}
+  ): Promise<void> {
+    if (this.isDisposed) {
+      return;
+    }
+
     const pending = this._tick;
-    const scheduled = new PromiseDelegate<this>();
+
+    // The `when` promise in the constructor options acts as a gate.
+    if (this.state.phase === 'constructed') {
+      if (next.phase !== 'when-rejected' && next.phase !== 'when-resolved') {
+        await pending.promise;
+      }
+    }
+
+    // Check if the phase transition should be canceled.
+    if (next.cancel && next.cancel(this.state)) {
+      return;
+    }
 
     // Update poll state.
+    const last = this.state;
+    const scheduled = new PromiseDelegate<this>();
+    const state: IPoll.State<T, U> = {
+      interval: this.frequency.interval,
+      payload: null,
+      phase: 'standby',
+      timestamp: new Date().getTime(),
+      ...next
+    };
     this._state = state;
     this._tick = scheduled;
 
@@ -423,9 +398,10 @@ export class Poll<T = any, U = any> implements IDisposable, IPoll<T, U> {
       clearTimeout(this._timeout);
     }
 
-    // Emit the ticked signal and resolve the pending promise.
+    // Emit ticked signal, resolve pending promise, and await its settlement.
     this._ticked.emit(this.state);
     pending.resolve(this);
+    await pending.promise;
 
     // Schedule next execution and cache its timeout handle.
     const execute = () => {
@@ -458,12 +434,7 @@ export class Poll<T = any, U = any> implements IDisposable, IPoll<T, U> {
 
     // If in standby mode schedule next tick without calling the factory.
     if (standby) {
-      this.schedule({
-        interval: this.frequency.interval,
-        payload: null,
-        phase: 'standby',
-        timestamp: new Date().getTime()
-      });
+      void this.schedule();
       return;
     }
 
@@ -475,11 +446,9 @@ export class Poll<T = any, U = any> implements IDisposable, IPoll<T, U> {
           return;
         }
 
-        this.schedule({
-          interval: this.frequency.interval,
+        void this.schedule({
           payload: resolved,
-          phase: this.state.phase === 'rejected' ? 'reconnected' : 'resolved',
-          timestamp: new Date().getTime()
+          phase: this.state.phase === 'rejected' ? 'reconnected' : 'resolved'
         });
       })
       .catch((rejected: U) => {
@@ -487,11 +456,10 @@ export class Poll<T = any, U = any> implements IDisposable, IPoll<T, U> {
           return;
         }
 
-        this.schedule({
+        void this.schedule({
           interval: Private.sleep(this.frequency, this.state),
           payload: rejected,
-          phase: 'rejected',
-          timestamp: new Date().getTime()
+          phase: 'rejected'
         });
       });
   }

tests/test-coreutils/src/poll.spec.ts

@@ -7,13 +7,12 @@ import { IPoll, Poll } from '@jupyterlab/coreutils';
 
 import { sleep } from '@jupyterlab/testutils';
 
-class TestPoll<T = any, U = any> extends Poll<T, U> {
-  protected schedule(state: IPoll.State<T, U>): void {
-    super.schedule(state);
-    this.scheduled.push(state.phase);
+class TestPoll extends Poll {
+  schedule(
+    next: Partial<IPoll.State & { cancel: (last: IPoll.State) => boolean }> = {}
+  ): Promise<void> {
+    return super.schedule(next);
   }
-
-  scheduled: string[] = [];
 }
 
 describe('Poll', () => {
@@ -228,16 +227,16 @@ describe('Poll', () => {
       });
       const ticker: IPoll.Phase[] = [];
       const tocker: IPoll.Phase[] = [];
-      poll.ticked.connect((_, state) => {
+      poll.ticked.connect(async (_, state) => {
         ticker.push(state.phase);
         expect(ticker.length).to.equal(tocker.length + 1);
       });
-      const tock = (poll: IPoll) => {
+      const tock = async (poll: IPoll) => {
         tocker.push(poll.state.phase);
         expect(ticker.join(' ')).to.equal(tocker.join(' '));
         poll.tick.then(tock).catch(() => undefined);
       };
-      void poll.tick.then(tock);
+      await poll.tick.then(tock);
       await poll.stop();
       await poll.start();
       await poll.tick;
@@ -509,21 +508,57 @@ describe('Poll', () => {
       poll.dispose();
     });
 
-    it('should schedule a poll tick', async () => {
-      const expected = 'when-resolved stopped started resolved';
+    it('should schedule the next poll state', async () => {
       poll = new TestPoll({
-        name: '@jupyterlab/test-coreutils:Poll#schedule()-1',
+        factory: () => Promise.resolve(),
         frequency: { interval: 100 },
-        factory: () => Promise.resolve()
+        name: '@jupyterlab/test-coreutils:Poll#schedule()-1'
       });
       expect(poll.state.phase).to.equal('constructed');
-      await poll.stop();
-      expect(poll.state.phase).to.equal('stopped');
-      await poll.start();
-      expect(poll.state.phase).to.equal('started');
+      await poll.tick;
+      expect(poll.state.phase).to.equal('when-resolved');
+      await poll.tick;
+      expect(poll.state.phase).to.equal('resolved');
+      await poll.schedule({ phase: 'refreshed' });
+      expect(poll.state.phase).to.equal('refreshed');
+      return;
+    });
+
+    it('should default to standby state', async () => {
+      poll = new TestPoll({
+        factory: () => Promise.resolve(),
+        frequency: { interval: 100 },
+        name: '@jupyterlab/test-coreutils:Poll#schedule()-2'
+      });
+      expect(poll.state.phase).to.equal('constructed');
+      await poll.tick;
+      expect(poll.state.phase).to.equal('when-resolved');
+      await poll.tick;
+      expect(poll.state.phase).to.equal('resolved');
+      await poll.schedule();
+      expect(poll.state.phase).to.equal('standby');
+      return;
+    });
+
+    it('should support phase transition cancellation', async () => {
+      poll = new TestPoll({
+        factory: () => Promise.resolve(),
+        frequency: { interval: 100 },
+        name: '@jupyterlab/test-coreutils:Poll#schedule()-3'
+      });
+      expect(poll.state.phase).to.equal('constructed');
+      await poll.tick;
+      expect(poll.state.phase).to.equal('when-resolved');
       await poll.tick;
       expect(poll.state.phase).to.equal('resolved');
-      expect(poll.scheduled.join(' ')).to.equal(expected);
+      await poll.schedule();
+      expect(poll.state.phase).to.equal('standby');
+      await poll.schedule({
+        cancel: last => last.phase === 'standby',
+        phase: 'refreshed'
+      });
+      expect(poll.state.phase).to.equal('standby');
+      return;
     });
   });
 });