Fix debounce issue when book toc is updated (#14392)

* pass function to debounce

* remove debounce decorator and move watch methods to bookmodel

* Move the vscode tree change data event to book model

* address pr comments

* fix book tests
This commit is contained in:
Barbara Valdez
2021-02-24 14:24:50 -08:00
committed by GitHub
parent 48c456709e
commit 3d5ff25d13
4 changed files with 41 additions and 50 deletions

View File

@@ -13,7 +13,7 @@ import * as fs from 'fs-extra';
import * as loc from '../common/localizedConstants';
import { IJupyterBookToc, JupyterBookSection } from '../contracts/content';
import { convertFrom, getContentPath, BookVersion } from './bookVersionHandler';
import { debounce } from '../common/utils';
const fsPromises = fileServices.promises;
const content = 'content';
@@ -32,43 +32,62 @@ export class BookModel {
public readonly openAsUntitled: boolean,
public readonly isNotebook: boolean,
private _extensionContext: vscode.ExtensionContext,
private _onDidChangeTreeData: vscode.EventEmitter<BookTreeItem | undefined>,
public readonly notebookRootPath?: string) {
this._bookItems = [];
}
public unwatchTOC(): void {
fs.unwatchFile(this.tableOfContentsPath);
}
public watchTOC(): void {
fs.watchFile(this.tableOfContentsPath, async (curr, prev) => {
if (curr.mtime > prev.mtime) {
this.reinitializeContents();
}
});
}
@debounce(1500)
public async reinitializeContents(): Promise<void> {
await this.initializeContents();
this._onDidChangeTreeData.fire(undefined);
}
public async initializeContents(): Promise<void> {
this._bookItems = [];
this._allNotebooks = new Map<string, BookTreeItem>();
if (this.isNotebook) {
this.readNotebook();
} else {
await this.readBookStructure(this.bookPath);
await this.readBookStructure();
await this.loadTableOfContentFiles();
await this.readBooks();
}
}
public async readBookStructure(folderPath: string): Promise<void> {
public async readBookStructure(): Promise<void> {
// check book structure to determine version
let isOlderVersion: boolean;
this._configPath = path.posix.join(folderPath, '_config.yml');
this._configPath = path.posix.join(this.bookPath, '_config.yml');
try {
isOlderVersion = (await fs.stat(path.posix.join(folderPath, '_data'))).isDirectory() && (await fs.stat(path.posix.join(folderPath, content))).isDirectory();
isOlderVersion = (await fs.stat(path.posix.join(this.bookPath, '_data'))).isDirectory() && (await fs.stat(path.posix.join(this.bookPath, content))).isDirectory();
} catch {
isOlderVersion = false;
}
if (isOlderVersion) {
let isTocFile = (await fs.stat(path.posix.join(folderPath, '_data', 'toc.yml'))).isFile();
let isTocFile = (await fs.stat(path.posix.join(this.bookPath, '_data', 'toc.yml'))).isFile();
if (isTocFile) {
this._tableOfContentsPath = path.posix.join(folderPath, '_data', 'toc.yml');
this._tableOfContentsPath = path.posix.join(this.bookPath, '_data', 'toc.yml');
}
this._bookVersion = BookVersion.v1;
this._contentFolderPath = path.posix.join(folderPath, content, '');
this._contentFolderPath = path.posix.join(this.bookPath, content, '');
this._rootPath = path.dirname(path.dirname(this._tableOfContentsPath));
} else {
this._contentFolderPath = folderPath;
this._tableOfContentsPath = path.posix.join(folderPath, '_toc.yml');
this._contentFolderPath = this.bookPath;
this._tableOfContentsPath = path.posix.join(this.bookPath, '_toc.yml');
this._rootPath = path.dirname(this._tableOfContentsPath);
this._bookVersion = BookVersion.v2;
}
@@ -89,6 +108,7 @@ export class BookModel {
if (await fs.pathExists(this._tableOfContentsPath)) {
vscode.commands.executeCommand('setContext', 'bookOpened', true);
this.watchTOC();
} else {
this._errorMessage = loc.missingTocError;
throw new Error(loc.missingTocError);

View File

@@ -16,7 +16,7 @@ import { Deferred } from '../common/promise';
import { IBookTrustManager, BookTrustManager } from './bookTrustManager';
import * as loc from '../common/localizedConstants';
import * as glob from 'fast-glob';
import { debounce, getPinnedNotebooks, confirmReplace } from '../common/utils';
import { getPinnedNotebooks, confirmReplace } from '../common/utils';
import { IBookPinManager, BookPinManager } from './bookPinManager';
import { BookTocManager, IBookTocManager, quickPickResults } from './bookTocManager';
import { CreateBookDialog } from '../dialog/createBookDialog';
@@ -92,14 +92,6 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
this._extensionContext.globalState.update(constants.visitedNotebooksMementoKey, value);
}
setFileWatcher(book: BookModel): void {
fs.watchFile(book.tableOfContentsPath, async (curr, prev) => {
if (curr.mtime > prev.mtime) {
await this.initializeBookContents(book);
}
});
}
trustBook(bookTreeItem?: BookTreeItem): void {
let bookPathToTrust: string = bookTreeItem ? bookTreeItem.root : this.currentBook?.bookPath;
if (bookPathToTrust) {
@@ -216,7 +208,7 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
this.bookTocManager = new BookTocManager(targetBook, sourceBook);
// remove watch on toc file from source book.
if (sourceBook) {
fs.unwatchFile(sourceBook.tableOfContentsPath);
sourceBook.unwatchTOC();
}
try {
await this.bookTocManager.updateBook(movingElement, updateBook, targetSection);
@@ -225,16 +217,14 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
vscode.window.showErrorMessage(loc.editBookError(updateBook.book.contentPath, e instanceof Error ? e.message : e));
} finally {
try {
await targetBook.initializeContents();
await targetBook.reinitializeContents();
} finally {
if (sourceBook && sourceBook.bookPath !== targetBook.bookPath) {
// refresh source book model to pick up latest changes
await sourceBook.initializeContents();
await sourceBook.reinitializeContents();
}
this._onDidChangeTreeData.fire(undefined);
// even if it fails, we still need to watch the toc file again.
if (sourceBook) {
this.setFileWatcher(sourceBook);
sourceBook.watchTOC();
}
}
}
@@ -261,17 +251,6 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
}
TelemetryReporter.createActionEvent(BookTelemetryView, NbTelemetryActions.OpenBook).send();
// add file watcher on toc file.
if (!isNotebook) {
fs.watchFile(this.currentBook.tableOfContentsPath, async (curr, prev) => {
if (curr.mtime > prev.mtime) {
let book = this.books.find(book => book.bookPath === bookPath);
if (book) {
await this.initializeBookContents(book);
}
}
});
}
} catch (e) {
// if there is an error remove book from context
const index = this.books.findIndex(book => book.bookPath === bookPath);
@@ -297,13 +276,6 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
}
}
@debounce(1500)
async initializeBookContents(book: BookModel): Promise<void> {
await book.initializeContents().then(() => {
this._onDidChangeTreeData.fire(undefined);
});
}
async closeBook(book: BookTreeItem): Promise<void> {
// remove book from the saved books
let deletedBook: BookModel;
@@ -324,7 +296,7 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
} finally {
// remove watch on toc file.
if (deletedBook && !deletedBook.isNotebook) {
fs.unwatchFile(deletedBook.tableOfContentsPath);
deletedBook.unwatchTOC();
}
}
}
@@ -338,7 +310,7 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
*/
private async createAndAddBookModel(bookPath: string, isNotebook: boolean, notebookBookRoot?: string): Promise<void> {
if (!this.books.find(x => x.bookPath === bookPath)) {
const book: BookModel = new BookModel(bookPath, this._openAsUntitled, isNotebook, this._extensionContext, notebookBookRoot);
const book: BookModel = new BookModel(bookPath, this._openAsUntitled, isNotebook, this._extensionContext, this._onDidChangeTreeData, notebookBookRoot);
await book.initializeContents();
this.books.push(book);
if (!this.currentBook) {

View File

@@ -485,14 +485,14 @@ describe('BooksTreeViewTests', function () {
if (run.it === 'v1') {
it('should ignore toc.yml files not in _data folder', async () => {
await bookTreeViewProvider.currentBook.readBookStructure(rootFolderPath);
await bookTreeViewProvider.currentBook.readBookStructure();
await bookTreeViewProvider.currentBook.loadTableOfContentFiles();
let path = bookTreeViewProvider.currentBook.tableOfContentsPath;
should(vscode.Uri.file(path).fsPath).equal(vscode.Uri.file(run.folderPaths.tableOfContentsFile).fsPath);
});
} else if (run.it === 'v2') {
it('should ignore toc.yml files not under the root book folder', async () => {
await bookTreeViewProvider.currentBook.readBookStructure(rootFolderPath);
await bookTreeViewProvider.currentBook.readBookStructure();
await bookTreeViewProvider.currentBook.loadTableOfContentFiles();
let path = bookTreeViewProvider.currentBook.tableOfContentsPath;
should(vscode.Uri.file(path).fsPath).equal(vscode.Uri.file(run.folderPaths.tableOfContentsFile).fsPath);

View File

@@ -20,7 +20,6 @@ import { BookTreeViewProvider } from '../../book/bookTreeView';
import { NavigationProviders } from '../../common/constants';
import * as loc from '../../common/localizedConstants';
export function equalTOC(actualToc: IJupyterBookSectionV2[], expectedToc: IJupyterBookSectionV2[]): boolean {
for (let [i, section] of actualToc.entries()) {
if (section.title !== expectedToc[i].title || section.file !== expectedToc[i].file) {
@@ -475,8 +474,8 @@ describe('BookTocManagerTests', function () {
const mockExtensionContext = new MockExtensionContext();
sourceBookModel = new BookModel(run.sourceBook.rootBookFolderPath, false, false, mockExtensionContext);
targetBookModel = new BookModel(run.targetBook.rootBookFolderPath, false, false, mockExtensionContext);
sourceBookModel = new BookModel(run.sourceBook.rootBookFolderPath, false, false, mockExtensionContext, undefined);
targetBookModel = new BookModel(run.targetBook.rootBookFolderPath, false, false, mockExtensionContext, undefined);
// create book model mock objects
sinon.stub(sourceBookModel, 'bookItems').value([sectionA]);
sinon.stub(targetBookModel, 'bookItems').value([targetBook]);