diff --git a/packages/filebrowser/src/model.ts b/packages/filebrowser/src/model.ts index ea74128c6802..f205cdda064e 100644 --- a/packages/filebrowser/src/model.ts +++ b/packages/filebrowser/src/model.ts @@ -238,7 +238,7 @@ export class FileBrowserModel implements IDisposable { * * @returns A promise with the contents of the directory. */ - cd(newValue = '.'): Promise { + async cd(newValue = '.'): Promise { if (newValue !== '.') { newValue = Private.normalizePath( this.manager.services.contents, @@ -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 }; @@ -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); diff --git a/tests/test-filebrowser/src/model.spec.ts b/tests/test-filebrowser/src/model.spec.ts index 12ec722536b2..dcf87a01a158 100644 --- a/tests/test-filebrowser/src/model.spec.ts +++ b/tests/test-filebrowser/src/model.spec.ts @@ -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, @@ -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 { + return new Promise(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; @@ -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()', () => {