Skip to content

Commit

Permalink
Await pending directory change in filebrowser model. (#5224)
Browse files Browse the repository at this point in the history
* Await pending directory change in filebrowser model.

* Add test for slow initial fetch of filebrowser model.
  • Loading branch information
ian-r-rose authored and blink1073 committed Aug 29, 2018
1 parent f6ee668 commit 3489763
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 6 deletions.
14 changes: 9 additions & 5 deletions packages/filebrowser/src/model.ts
Expand Up @@ -238,7 +238,7 @@ export class FileBrowserModel implements IDisposable {
*
* @returns A promise with the contents of the directory.
*/
cd(newValue = '.'): Promise<void> {
async cd(newValue = '.'): Promise<void> {
if (newValue !== '.') {
newValue = Private.normalizePath(
this.manager.services.contents,
Expand All @@ -248,9 +248,13 @@ export class FileBrowserModel implements IDisposable {
} else {
newValue = this._pendingPath || this._model.path;
}
// Collapse requests to the same directory.
if (newValue === this._pendingPath && this._pending) {
return this._pending;
if (this._pending) {
// Collapse requests to the same directory.
if (newValue === this._pendingPath) {
return this._pending;
}
// Otherwise wait for the pending request to complete before continuing.
await this._pending;
}
let oldValue = this.path;
let options: Contents.IFetchOptions = { content: true };
Expand Down Expand Up @@ -289,7 +293,7 @@ export class FileBrowserModel implements IDisposable {
error.message = `Directory not found: "${this._model.path}"`;
console.error(error);
this._connectionFailure.emit(error);
this.cd('/');
return this.cd('/');
} else {
this._refreshDuration = this._baseRefreshDuration * 10;
this._connectionFailure.emit(error);
Expand Down
56 changes: 55 additions & 1 deletion tests/test-filebrowser/src/model.spec.ts
Expand Up @@ -11,7 +11,11 @@ import { DocumentManager, IDocumentManager } from '@jupyterlab/docmanager';

import { DocumentRegistry, TextModelFactory } from '@jupyterlab/docregistry';

import { ServiceManager } from '@jupyterlab/services';
import {
Contents,
ContentsManager,
ServiceManager
} from '@jupyterlab/services';

import {
FileBrowserModel,
Expand All @@ -26,6 +30,29 @@ import {
} from '@jupyterlab/testutils';
import { toArray } from '@phosphor/algorithm';

/**
* A contents manager that delays requests by less each time it is called
* in order to simulate out-of-order responses from the server.
*/
class DelayedContentsManager extends ContentsManager {
get(
path: string,
options?: Contents.IFetchOptions
): Promise<Contents.IModel> {
return new Promise<Contents.IModel>(resolve => {
const delay = this._delay;
this._delay -= 500;
super.get(path, options).then(contents => {
setTimeout(() => {
resolve(contents);
}, Math.max(delay, 0));
});
});
}

private _delay = 1000;
}

describe('filebrowser/model', () => {
let manager: IDocumentManager;
let serviceManager: ServiceManager.IManager;
Expand Down Expand Up @@ -223,6 +250,33 @@ describe('filebrowser/model', () => {
await model.cd('..');
expect(model.path).to.equal('');
});

it('should be resilient to a slow initial fetch', async () => {
let delayedServiceManager = new ServiceManager();
(delayedServiceManager as any).contents = new DelayedContentsManager();
let manager = new DocumentManager({
registry,
opener,
manager: delayedServiceManager
});
model = new FileBrowserModel({ manager, state });

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]);
expect(model.path).to.equal('src');
expect(paths).to.eql(['', 'src']);

manager.dispose();
delayedServiceManager.contents.dispose();
delayedServiceManager.dispose();
model.dispose();
});
});

describe('#restore()', () => {
Expand Down

0 comments on commit 3489763

Please sign in to comment.