Support trusted books in new version of Jupyter Books (#12874)

* Fix path gor v1 and v2 boooks

* Add tests and address PR comments

* fix failing tests

* Address pr comments

* address pr comments

extensions/notebook/src/book/bookTreeView.ts

@@ -92,7 +92,7 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
 	trustBook(bookTreeItem?: BookTreeItem): void {
 		let bookPathToTrust: string = bookTreeItem ? bookTreeItem.root : this.currentBook?.bookPath;
 		if (bookPathToTrust) {
-			let trustChanged = this._bookTrustManager.setBookAsTrusted(bookPathToTrust);
+			let trustChanged = this._bookTrustManager.setBookAsTrusted(bookPathToTrust, true);
 			if (trustChanged) {
 				let notebookDocuments = azdata.nb.notebookDocuments;
 				if (notebookDocuments) {

extensions/notebook/src/book/bookTrustManager.ts

@@ -6,11 +6,11 @@ import * as path from 'path';
 import * as vscode from 'vscode';
 import * as constants from './../common/constants';
 import { BookTreeItem } from './bookTreeItem';
-import { BookModel } from './bookModel';
+import { BookModel, BookVersion } from './bookModel';
 
 export interface IBookTrustManager {
 	isNotebookTrustedByDefault(notebookUri: string): boolean;
-	setBookAsTrusted(bookRootPath: string): boolean;
+	setBookAsTrusted(bookRootPath: string, isTrusted: boolean): boolean;
 }
 
 enum TrustBookOperation {
@@ -27,7 +27,7 @@ export class BookTrustManager implements IBookTrustManager {
 		let trustableBookPaths = this.getTrustableBookPaths();
 		let hasTrustedBookPath: boolean = treeBookItems
 			.filter(bookItem => trustableBookPaths.some(trustableBookPath => trustableBookPath === path.join(bookItem.book.root, path.sep)))
-			.some(bookItem => normalizedNotebookUri.startsWith(path.join(bookItem.root, 'content', path.sep)));
+			.some(bookItem => normalizedNotebookUri.startsWith(bookItem.version === BookVersion.v1 ? path.join(bookItem.book.root, 'content', path.sep) : path.join(bookItem.book.root, path.sep)));
 		let isNotebookTrusted = hasTrustedBookPath && this.books.some(bookModel => bookModel.getNotebook(normalizedNotebookUri));
 		return isNotebookTrusted;
 	}
@@ -57,9 +57,12 @@ export class BookTrustManager implements IBookTrustManager {
 			.reduce((accumulator, currentBookItemList) => accumulator.concat(currentBookItemList), []);
 	}
 
-	setBookAsTrusted(bookRootPath: string): boolean {
+	setBookAsTrusted(bookRootPath: string, isTrusted: boolean): boolean {
+		if (isTrusted) {
 			return this.updateTrustedBooks(bookRootPath, TrustBookOperation.Add);
 		}
+		return this.updateTrustedBooks(bookRootPath, TrustBookOperation.Remove);
+	}
 
 	getTrustedBookPathsInConfig(): string[] {
 		let config: vscode.WorkspaceConfiguration = vscode.workspace.getConfiguration(constants.notebookConfigKey);

extensions/notebook/src/test/book/bookTrustManager.test.ts

@@ -19,6 +19,39 @@ describe('BookTrustManagerTests', function () {
 		let bookTrustManager: IBookTrustManager;
 		let trustedSubFolders: string[];
 		let books: BookModel[];
+		let runs = [
+			{
+				it: 'using the jupyter-book legacy version < 0.7.0',
+				book1: {
+					'notebook1': path.join(path.sep, 'temp', 'SubFolder', 'content', 'sample', 'notebook.ipynb'),
+					'notebook2': path.join(path.sep, 'temp', 'SubFolder', 'content', 'sample', 'notebook2.ipynb'),
+					'notebook3': path.join(path.sep, 'temp', 'SubFolder2', 'content', 'sample', 'notebook.ipynb'),
+					'notInTocNb': path.join(path.sep, 'temp', 'SubFolder', 'content', 'sample', 'notInToc.ipynb')
+				},
+				book2: {
+					'notebook1': path.join(path.sep, 'temp2', 'SubFolder', 'content', 'sample', 'notebook.ipynb')
+				},
+				unknownBook: {
+					'unknownNotebook': path.join(path.sep, 'randomfolder', 'randomsubfolder', 'content', 'randomnotebook.ipynb')
+				}
+			}, {
+				it: 'using jupyter-book versions >= 0.7.0',
+				book1: {
+					'notebook1': path.join(path.sep, 'temp', 'SubFolder', 'sample', 'notebook.ipynb'),
+					'notebook2': path.join(path.sep, 'temp', 'SubFolder', 'sample', 'notebook2.ipynb'),
+					'notebook3': path.join(path.sep, 'temp', 'SubFolder2', 'sample', 'notebook.ipynb'),
+					'notInTocNb': path.join(path.sep, 'temp', 'SubFolder', 'sample', 'notInToc.ipynb')
+				},
+				book2: {
+					'notebook1': path.join(path.sep, 'temp2', 'SubFolder', 'sample', 'notebook.ipynb')
+				},
+				unknownBook: {
+					'unknownNotebook': path.join(path.sep, 'randomfolder', 'randomsubfolder', 'randomnotebook.ipynb')
+				}
+			}
+		];
+		runs.forEach(function (run) {
+			describe('Trusting in Workspaces ' + run.it, function (): void {
 
 				afterEach(function (): void {
 					sinon.restore();
@@ -108,14 +141,14 @@ describe('BookTrustManagerTests', function () {
 
 					let bookModel1Mock: TypeMoq.IMock<BookModel> = TypeMoq.Mock.ofType<BookModel>();
 					bookModel1Mock.setup(model => model.bookItems).returns(() => [new BookTreeItem(bookTreeItemFormat1, undefined), new BookTreeItem(bookTreeItemFormat2, undefined)]);
-			bookModel1Mock.setup(model => model.getNotebook(TypeMoq.It.isValue(path.join(path.sep, 'temp', 'SubFolder', 'content', 'sample', 'notebook.ipynb')))).returns((uri: string) => TypeMoq.Mock.ofType<BookTreeItem>().object);
+					bookModel1Mock.setup(model => model.getNotebook(TypeMoq.It.isValue(run.book1.notebook1))).returns((uri: string) => TypeMoq.Mock.ofType<BookTreeItem>().object);
-			bookModel1Mock.setup(model => model.getNotebook(TypeMoq.It.isValue(path.join(path.sep, 'temp', 'SubFolder', 'content', 'sample', 'notebook2.ipynb')))).returns((uri: string) => TypeMoq.Mock.ofType<BookTreeItem>().object);
+					bookModel1Mock.setup(model => model.getNotebook(TypeMoq.It.isValue(run.book1.notebook2))).returns((uri: string) => TypeMoq.Mock.ofType<BookTreeItem>().object);
-			bookModel1Mock.setup(model => model.getNotebook(TypeMoq.It.isValue(path.join(path.sep, 'temp', 'SubFolder2', 'content', 'sample', 'notebook.ipynb')))).returns((uri: string) => TypeMoq.Mock.ofType<BookTreeItem>().object);
+					bookModel1Mock.setup(model => model.getNotebook(TypeMoq.It.isValue(run.book1.notebook3))).returns((uri: string) => TypeMoq.Mock.ofType<BookTreeItem>().object);
 					bookModel1Mock.setup(model => model.getNotebook(TypeMoq.It.isAnyString())).returns((uri: string) => undefined);
 
 					let bookModel2Mock: TypeMoq.IMock<BookModel> = TypeMoq.Mock.ofType<BookModel>();
 					bookModel2Mock.setup(model => model.bookItems).returns(() => [new BookTreeItem(bookTreeItemFormat3, undefined)]);
-			bookModel2Mock.setup(model => model.getNotebook(TypeMoq.It.isValue(path.join(path.sep, 'temp2', 'SubFolder', 'content', 'sample', 'notebook.ipynb')))).returns((uri: string) => TypeMoq.Mock.ofType<BookTreeItem>().object);
+					bookModel2Mock.setup(model => model.getNotebook(TypeMoq.It.isValue(run.book2.notebook1))).returns((uri: string) => TypeMoq.Mock.ofType<BookTreeItem>().object);
 					bookModel2Mock.setup(model => model.getNotebook(TypeMoq.It.isAnyString())).returns((uri: string) => undefined);
 
 					books = [bookModel1Mock.object, bookModel2Mock.object];
@@ -124,32 +157,31 @@ describe('BookTrustManagerTests', function () {
 				});
 
 				it('should trust notebooks in a trusted book within a workspace', async () => {
-			let notebookUri1 = path.join(path.sep, 'temp', 'SubFolder', 'content', 'sample', 'notebook.ipynb');
+					let notebookUri1 = run.book1.notebook1;
-			let notebookUri2 = path.join(path.sep, 'temp', 'SubFolder', 'content', 'sample', 'notebook2.ipynb');
+					let notebookUri2 = run.book1.notebook2;
 
 					let isNotebook1Trusted = bookTrustManager.isNotebookTrustedByDefault(notebookUri1);
 					let isNotebook2Trusted = bookTrustManager.isNotebookTrustedByDefault(notebookUri2);
 
 					should(isNotebook1Trusted).be.true('Notebook 1 should be trusted');
 					should(isNotebook2Trusted).be.true('Notebook 2 should be trusted');
-
 				});
 
 				it('should NOT trust a notebook in an untrusted book within a workspace', async () => {
-			let notebookUri = path.join(path.sep, 'temp', 'SubFolder2', 'content', 'sample', 'notebook.ipynb');
+					let notebookUri = run.book1.notebook3;
 					let isNotebookTrusted = bookTrustManager.isNotebookTrustedByDefault(notebookUri);
 
-			should(isNotebookTrusted).be.false('Notebook should be trusted');
+					should(isNotebookTrusted).be.false('Notebook should not be trusted');
 				});
 
 				it('should trust notebook after book has been trusted within a workspace', async () => {
-			let notebookUri = path.join(path.sep, 'temp', 'SubFolder2', 'content', 'sample', 'notebook.ipynb');
+					let notebookUri = run.book1.notebook3;
 					let isNotebookTrustedBeforeChange = bookTrustManager.isNotebookTrustedByDefault(notebookUri);
 
 					should(isNotebookTrustedBeforeChange).be.false('Notebook should NOT be trusted');
 
 					// add another book subfolder
-			bookTrustManager.setBookAsTrusted('/SubFolder2/');
+					bookTrustManager.setBookAsTrusted('/SubFolder2/', true);
 
 					let isNotebookTrustedAfterChange = bookTrustManager.isNotebookTrustedByDefault(notebookUri);
 
@@ -157,7 +189,7 @@ describe('BookTrustManagerTests', function () {
 				});
 
 				it('should NOT trust a notebook when untrusting a book within a workspace', async () => {
-			let notebookUri = path.join(path.sep, 'temp', 'SubFolder', 'content', 'sample', 'notebook.ipynb');
+					let notebookUri = run.book1.notebook1;
 					let isNotebookTrusted = bookTrustManager.isNotebookTrustedByDefault(notebookUri);
 
 					should(isNotebookTrusted).be.true('Notebook should be trusted');
@@ -171,21 +203,25 @@ describe('BookTrustManagerTests', function () {
 				});
 
 				it('should NOT trust an unknown book within a workspace', async () => {
-			let notebookUri = path.join(path.sep, 'randomfolder', 'randomsubfolder', 'content', 'randomnotebook.ipynb');
+					let notebookUri = run.unknownBook.unknownNotebook;
 					let isNotebookTrusted = bookTrustManager.isNotebookTrustedByDefault(notebookUri);
 
 					should(isNotebookTrusted).be.false('Random notebooks should not be trusted');
 				});
 
 				it('should NOT trust notebook inside trusted subfolder when absent in table of contents ', async () => {
-			bookTrustManager.setBookAsTrusted('/temp/SubFolder/');
+					bookTrustManager.setBookAsTrusted('/temp/SubFolder/', true);
 
-			let notebookUri = path.join(path.sep, 'temp', 'SubFolder', 'content', 'sample', 'notInToc.ipynb');
+					let notebookUri = run.book1.notInTocNb;
 					let isNotebookTrusted = bookTrustManager.isNotebookTrustedByDefault(notebookUri);
 
 					should(isNotebookTrusted).be.false('Notebook should NOT be trusted');
 				});
 			});
+		});
+	});
+
+
 
 	describe('TrustingInFolder', () => {
 
@@ -193,7 +229,41 @@ describe('BookTrustManagerTests', function () {
 		let books: BookModel[];
 		let trustedFolders: string[] = [];
 
+		let runs = [
+			{
+				it: 'using the jupyter-book legacy version < 0.7.0',
+				book1: {
+					'notebook1': path.join(path.sep, 'temp', 'SubFolder', 'content', 'sample', 'notebook.ipynb'),
+					'notebook2': path.join(path.sep, 'temp', 'SubFolder', 'content', 'sample', 'notebook2.ipynb'),
+					'notInTocNb': path.join(path.sep, 'temp', 'SubFolder', 'content', 'sample', 'notInToc.ipynb')
+				},
+				book2: {
+					'notebook1': path.join(path.sep, 'temp', 'SubFolder2', 'content', 'sample', 'notebook.ipynb'),
+					'notebook2': path.join(path.sep, 'temp', 'SubFolder2', 'content', 'sample', 'notebook2.ipynb')
+				},
+				unknownBook: {
+					'unknownNotebook': path.join(path.sep, 'randomfolder', 'randomsubfolder', 'content', 'randomnotebook.ipynb')
+				}
+			}, {
+				it: 'using jupyter-book versions >= 0.7.0',
+				book1: {
+					'notebook1': path.join(path.sep, 'temp', 'SubFolder', 'sample', 'notebook.ipynb'),
+					'notebook2': path.join(path.sep, 'temp', 'SubFolder', 'sample', 'notebook2.ipynb'),
+					'notInTocNb': path.join(path.sep, 'temp', 'SubFolder', 'sample', 'notInToc.ipynb')
+				},
+				book2: {
+					'notebook1': path.join(path.sep, 'temp', 'SubFolder2', 'sample', 'notebook.ipynb'),
+					'notebook2': path.join(path.sep, 'temp', 'SubFolder2', 'sample', 'notebook2.ipynb')
+				},
+				unknownBook: {
+					'unknownNotebook': path.join(path.sep, 'randomfolder', 'randomsubfolder', 'randomnotebook.ipynb')
+				}
+			}
+		];
+		runs.forEach(function (run) {
+			describe('Trusting in Workspaces ' + run.it, function (): void {
 				beforeEach(() => {
+					trustedFolders = [];
 					// Mock Workspace Configuration
 					let workspaceConfigurtionMock: TypeMoq.IMock<vscode.WorkspaceConfiguration> = TypeMoq.Mock.ofType<vscode.WorkspaceConfiguration>();
 					workspaceConfigurtionMock.setup(config => config.get(TypeMoq.It.isValue(constants.trustedBooksConfigKey))).returns(() => [].concat(trustedFolders));
@@ -244,14 +314,14 @@ describe('BookTrustManagerTests', function () {
 
 					let bookModel1Mock: TypeMoq.IMock<BookModel> = TypeMoq.Mock.ofType<BookModel>();
 					bookModel1Mock.setup(model => model.bookItems).returns(() => [new BookTreeItem(bookTreeItemFormat1, undefined)]);
-			bookModel1Mock.setup(model => model.getNotebook(TypeMoq.It.isValue(path.join(path.sep, 'temp', 'SubFolder', 'content', 'sample', 'notebook.ipynb')))).returns((uri: string) => TypeMoq.Mock.ofType<BookTreeItem>().object);
+					bookModel1Mock.setup(model => model.getNotebook(TypeMoq.It.isValue(run.book1.notebook1))).returns((uri: string) => TypeMoq.Mock.ofType<BookTreeItem>().object);
-			bookModel1Mock.setup(model => model.getNotebook(TypeMoq.It.isValue(path.join(path.sep, 'temp', 'SubFolder', 'content', 'sample', 'notebook2.ipynb')))).returns((uri: string) => TypeMoq.Mock.ofType<BookTreeItem>().object);
+					bookModel1Mock.setup(model => model.getNotebook(TypeMoq.It.isValue(run.book1.notebook2))).returns((uri: string) => TypeMoq.Mock.ofType<BookTreeItem>().object);
 					bookModel1Mock.setup(model => model.getNotebook(TypeMoq.It.isAnyString())).returns((uri: string) => undefined);
 
 					let bookModel2Mock: TypeMoq.IMock<BookModel> = TypeMoq.Mock.ofType<BookModel>();
 					bookModel2Mock.setup(model => model.bookItems).returns(() => [new BookTreeItem(bookTreeItemFormat2, undefined)]);
-			bookModel2Mock.setup(model => model.getNotebook(TypeMoq.It.isValue(path.join(path.sep, 'temp', 'SubFolder2', 'content', 'sample', 'notebook.ipynb')))).returns((uri: string) => TypeMoq.Mock.ofType<BookTreeItem>().object);
+					bookModel2Mock.setup(model => model.getNotebook(TypeMoq.It.isValue(run.book2.notebook1))).returns((uri: string) => TypeMoq.Mock.ofType<BookTreeItem>().object);
-			bookModel2Mock.setup(model => model.getNotebook(TypeMoq.It.isValue(path.join(path.sep, 'temp', 'SubFolder2', 'content', 'sample', 'notebook2.ipynb')))).returns((uri: string) => TypeMoq.Mock.ofType<BookTreeItem>().object);
+					bookModel2Mock.setup(model => model.getNotebook(TypeMoq.It.isValue(run.book2.notebook2))).returns((uri: string) => TypeMoq.Mock.ofType<BookTreeItem>().object);
 					bookModel2Mock.setup(model => model.getNotebook(TypeMoq.It.isAnyString())).returns((uri: string) => undefined);
 
 					books = [bookModel1Mock.object, bookModel2Mock.object];
@@ -260,10 +330,10 @@ describe('BookTrustManagerTests', function () {
 				});
 
 				it('should trust notebooks in a trusted book in a folder', async () => {
-			bookTrustManager.setBookAsTrusted('/temp/SubFolder/');
+					bookTrustManager.setBookAsTrusted('/temp/SubFolder/', true);
 
-			let notebookUri1 = path.join(path.sep, 'temp', 'SubFolder', 'content', 'sample', 'notebook.ipynb');
+					let notebookUri1 = run.book1.notebook1;
-			let notebookUri2 = path.join(path.sep, 'temp', 'SubFolder', 'content', 'sample', 'notebook2.ipynb');
+					let notebookUri2 = run.book1.notebook2;
 
 					let isNotebook1Trusted = bookTrustManager.isNotebookTrustedByDefault(notebookUri1);
 					let isNotebook2Trusted = bookTrustManager.isNotebookTrustedByDefault(notebookUri2);
@@ -274,54 +344,64 @@ describe('BookTrustManagerTests', function () {
 				});
 
 				it('should NOT trust a notebook in an untrusted book in a folder', async () => {
-			let notebookUri = path.join(path.sep, 'temp', 'SubFolder2', 'content', 'sample', 'notebook.ipynb');
+					//Set book as not trusted before running test
+					bookTrustManager.setBookAsTrusted('/temp/SubFolder2/', false);
+
+					let notebookUri = run.book2.notebook1;
 					let isNotebookTrusted = bookTrustManager.isNotebookTrustedByDefault(notebookUri);
 
-			should(isNotebookTrusted).be.false('Notebook should be trusted');
+					should(isNotebookTrusted).be.false('Notebook not should be trusted');
 				});
 
 				it('should trust notebook after book has been added to a folder', async () => {
-			let notebookUri = path.join(path.sep, 'temp', 'SubFolder2', 'content', 'sample', 'notebook.ipynb');
+					let notebookUri = run.book2.notebook1;
 					let isNotebookTrustedBeforeChange = bookTrustManager.isNotebookTrustedByDefault(notebookUri);
 
 					should(isNotebookTrustedBeforeChange).be.false('Notebook should NOT be trusted');
 
-			bookTrustManager.setBookAsTrusted('/temp/SubFolder2/');
+					bookTrustManager.setBookAsTrusted('/temp/SubFolder2/', true);
 
 					let isNotebookTrustedAfterChange = bookTrustManager.isNotebookTrustedByDefault(notebookUri);
 
 					should(isNotebookTrustedAfterChange).be.true('Notebook should be trusted');
 				});
 
-		it('should NOT trust a notebook when untrusting a book in folder', async () => {
+				it('should NOT trust a notebook when removing all books from folders', async () => {
-			bookTrustManager.setBookAsTrusted('/temp/SubFolder/');
+					bookTrustManager.setBookAsTrusted('/temp/SubFolder/', true);
-			let notebookUri = path.join(path.sep, 'temp', 'SubFolder', 'content', 'sample', 'notebook.ipynb');
+					bookTrustManager.setBookAsTrusted('/temp/SubFolder2/', true);
+					let notebookUri = run.book1.notebook1;
 					let isNotebookTrusted = bookTrustManager.isNotebookTrustedByDefault(notebookUri);
+					let notebook2Uri = run.book2.notebook1;
+					let isNotebook2Trusted = bookTrustManager.isNotebookTrustedByDefault(notebook2Uri);
 
 					should(isNotebookTrusted).be.true('Notebook should be trusted');
+					should(isNotebook2Trusted).be.true('Notebook2 should be trusted');
 
 					trustedFolders = [];
 
 					let isNotebookTrustedAfterChange = bookTrustManager.isNotebookTrustedByDefault(notebookUri);
+					let isNotebook2TrustedAfterChange = bookTrustManager.isNotebookTrustedByDefault(notebook2Uri);
 
 					should(isNotebookTrustedAfterChange).be.false('Notebook should not be trusted after book removal');
+					should(isNotebook2TrustedAfterChange).be.false('Notebook2 should not be trusted after book removal');
 				});
 
 				it('should NOT trust an unknown book', async () => {
-			let notebookUri = path.join(path.sep, 'randomfolder', 'randomsubfolder', 'content', 'randomnotebook.ipynb');
+					let notebookUri = run.unknownBook.unknownNotebook;
 					let isNotebookTrusted = bookTrustManager.isNotebookTrustedByDefault(notebookUri);
 
 					should(isNotebookTrusted).be.false('Random notebooks should not be trusted');
 				});
 
 				it('should NOT trust notebook inside trusted subfolder when absent in table of contents ', async () => {
-			bookTrustManager.setBookAsTrusted('/temp/SubFolder/');
+					bookTrustManager.setBookAsTrusted('/temp/SubFolder/', true);
 
-			let notebookUri = path.join(path.sep, 'temp', 'SubFolder', 'content', 'sample', 'notInToc.ipynb');
+					let notebookUri = run.book1.notInTocNb;
 					let isNotebookTrusted = bookTrustManager.isNotebookTrustedByDefault(notebookUri);
 
 					should(isNotebookTrusted).be.false('Notebook should NOT be trusted');
 				});
-
+			});
+		});
 	});
 });