Skip to content

Commit

Permalink
Merge pull request #6305 from ian-r-rose/filebrowser-poll
Browse files Browse the repository at this point in the history
Use new Poll class for filebrowser contents polling.
  • Loading branch information
jasongrout committed May 11, 2019
2 parents 5211238 + 6746f7d commit f233cb5
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 62 deletions.
5 changes: 5 additions & 0 deletions packages/coreutils/src/poll.ts
Expand Up @@ -309,6 +309,11 @@ export class Poll<T = any, U = any> implements IDisposable, IPoll<T, U> {
* Refreshes the poll. Schedules `refreshed` tick if necessary.
*
* @returns A promise that resolves after tick is scheduled and never rejects.
*
* #### Notes
* The returned promise resolves after the tick is scheduled, but before
* the polling action is run. To wait until after the poll action executes,
* await the `poll.tick` promise: `await poll.refresh(); await poll.tick;`
*/
refresh(): Promise<void> {
return this.schedule({
Expand Down
74 changes: 19 additions & 55 deletions packages/filebrowser/src/model.ts
Expand Up @@ -5,7 +5,8 @@ import {
IChangedArgs,
IStateDB,
PathExt,
PageConfig
PageConfig,
Poll
} from '@jupyterlab/coreutils';

import { IDocumentManager, shouldOverwrite } from '@jupyterlab/docmanager';
Expand Down Expand Up @@ -34,11 +35,6 @@ import { showDialog, Dialog } from '@jupyterlab/apputils';
*/
const DEFAULT_REFRESH_INTERVAL = 10000;

/**
* The enforced time between refreshes in ms.
*/
const MIN_REFRESH = 1000;

/**
* The maximum upload size (in bytes) for notebook version < 5.1.0
*/
Expand Down Expand Up @@ -87,8 +83,7 @@ export class FileBrowserModel implements IDisposable {
format: 'text'
};
this._state = options.state || null;
this._baseRefreshDuration =
options.refreshInterval || DEFAULT_REFRESH_INTERVAL;
const refreshInterval = options.refreshInterval || DEFAULT_REFRESH_INTERVAL;

const { services } = options.manager;
services.contents.fileChanged.connect(this._onFileChanged, this);
Expand All @@ -103,8 +98,15 @@ export class FileBrowserModel implements IDisposable {
}
};
window.addEventListener('beforeunload', this._unloadEventListener);
this._scheduleUpdate();
this._startTimer();
this._poll = new Poll({
factory: () => this.cd('.'),
frequency: {
interval: refreshInterval,
backoff: true,
max: 300 * 1000
},
standby: 'when-hidden'
});
}

/**
Expand Down Expand Up @@ -198,7 +200,7 @@ export class FileBrowserModel implements IDisposable {
}
window.removeEventListener('beforeunload', this._unloadEventListener);
this._isDisposed = true;
clearTimeout(this._timeoutId);
this._poll.dispose();
this._sessions.length = 0;
this._items.length = 0;
Signal.clearData(this);
Expand All @@ -225,10 +227,9 @@ export class FileBrowserModel implements IDisposable {
/**
* Force a refresh of the directory contents.
*/
refresh(): Promise<void> {
this._lastRefresh = new Date().getTime();
this._requested = false;
return this.cd('.');
async refresh(): Promise<void> {
await this._poll.refresh();
await this._poll.tick;
}

/**
Expand Down Expand Up @@ -269,9 +270,9 @@ export class FileBrowserModel implements IDisposable {
if (this.isDisposed) {
return;
}
this._refreshDuration = this._baseRefreshDuration;
this._handleContents(contents);
this._pendingPath = null;
this._pending = null;
if (oldValue !== newValue) {
// If there is a state database and a unique key, save the new path.
// We don't need to wait on the save to continue.
Expand All @@ -296,7 +297,6 @@ export class FileBrowserModel implements IDisposable {
this._connectionFailure.emit(error);
return this.cd('/');
} else {
this._refreshDuration = this._baseRefreshDuration * 10;
this._connectionFailure.emit(error);
}
});
Expand Down Expand Up @@ -586,7 +586,7 @@ export class FileBrowserModel implements IDisposable {

// If either the old value or the new value is in the current path, update.
if (value) {
this._scheduleUpdate();
void this._poll.refresh();
this._populateSessions(sessions.running());
this._fileChanged.emit(change);
return;
Expand All @@ -605,38 +605,6 @@ export class FileBrowserModel implements IDisposable {
});
}

/**
* Start the internal refresh timer.
*/
private _startTimer(): void {
this._timeoutId = window.setInterval(() => {
if (this._requested) {
void this.refresh();
return;
}
if (document.hidden) {
// Don't poll when nobody's looking.
return;
}
let date = new Date().getTime();
if (date - this._lastRefresh > this._refreshDuration) {
void this.refresh();
}
}, MIN_REFRESH);
}

/**
* Handle internal model refresh logic.
*/
private _scheduleUpdate(): void {
let date = new Date().getTime();
if (date - this._lastRefresh > MIN_REFRESH) {
void this.refresh();
} else {
this._requested = true;
}
}

private _connectionFailure = new Signal<this, Error>(this);
private _fileChanged = new Signal<this, Contents.IChangedArgs>(this);
private _items: Contents.IModel[] = [];
Expand All @@ -647,19 +615,15 @@ export class FileBrowserModel implements IDisposable {
private _pending: Promise<void> | null = null;
private _pendingPath: string | null = null;
private _refreshed = new Signal<this, void>(this);
private _lastRefresh = -1;
private _requested = false;
private _sessions: Session.IModel[] = [];
private _state: IStateDB | null = null;
private _timeoutId = -1;
private _refreshDuration: number;
private _baseRefreshDuration: number;
private _driveName: string;
private _isDisposed = false;
private _restored = new PromiseDelegate<void>();
private _uploads: IUploadModel[] = [];
private _uploadChanged = new Signal<this, IChangedArgs<IUploadModel>>(this);
private _unloadEventListener: (e: Event) => string;
private _poll: Poll;
}

/**
Expand Down
12 changes: 5 additions & 7 deletions tests/test-filebrowser/src/model.spec.ts
Expand Up @@ -26,7 +26,8 @@ import {
import {
acceptDialog,
dismissDialog,
signalToPromises
signalToPromises,
sleep
} from '@jupyterlab/testutils';
import { toArray } from '@phosphor/algorithm';

Expand Down Expand Up @@ -259,18 +260,15 @@ describe('filebrowser/model', () => {
opener,
manager: delayedServiceManager
});
model = new FileBrowserModel({ manager, state });
model = new FileBrowserModel({ manager, state }); // Should delay 1000ms

const paths: string[] = [];
// An initial refresh is called in the constructor.
// If it is too slow, it can come in after the directory change,
// causing a directory set by, e.g., the tree handler to be wrong.
// This checks to make sure we are handling that case correctly.
const refresh = model.refresh().then(() => paths.push(model.path));
const cd = model.cd('src').then(() => paths.push(model.path));
await Promise.all([refresh, cd]);
await model.cd('src'); // should delay 500ms
await sleep(2000);
expect(model.path).to.equal('src');
expect(paths).to.eql(['', 'src']);

manager.dispose();
delayedServiceManager.contents.dispose();
Expand Down

0 comments on commit f233cb5

Please sign in to comment.