Skip to content

Commit dc36fd2

Browse files
authored
Fixes to trusting native notebooks (#13292)
1 parent 8ec2285 commit dc36fd2

File tree

4 files changed

+445
-4
lines changed

4 files changed

+445
-4
lines changed

src/client/datascience/notebook/kernelProvider.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,9 @@ import { KernelSelectionProvider } from '../jupyter/kernels/kernelSelections';
1717
import { KernelSelector } from '../jupyter/kernels/kernelSelector';
1818
import { KernelSwitcher } from '../jupyter/kernels/kernelSwitcher';
1919
import { KernelSelection } from '../jupyter/kernels/types';
20+
import { INotebookStorageProvider } from '../notebookStorage/notebookStorageProvider';
2021
import { INotebook, INotebookProvider } from '../types';
21-
import { getNotebookMetadata, updateKernelInNotebookMetadata } from './helpers/helpers';
22+
import { getNotebookMetadata, isJupyterNotebook, updateKernelInNotebookMetadata } from './helpers/helpers';
2223
import { NotebookKernel } from './notebookKernel';
2324
import { INotebookContentProvider, INotebookExecutionService } from './types';
2425
@injectable()
@@ -33,6 +34,7 @@ export class VSCodeKernelPickerProvider implements NotebookKernelProvider {
3334
@inject(KernelSelectionProvider) private readonly kernelSelectionProvider: KernelSelectionProvider,
3435
@inject(KernelSelector) private readonly kernelSelector: KernelSelector,
3536
@inject(IVSCodeNotebook) private readonly notebook: IVSCodeNotebook,
37+
@inject(INotebookStorageProvider) private readonly storageProvider: INotebookStorageProvider,
3638
@inject(INotebookProvider) private readonly notebookProvider: INotebookProvider,
3739
@inject(KernelSwitcher) private readonly kernelSwitcher: KernelSwitcher,
3840
@inject(INotebookContentProvider) private readonly notebookContentProvider: INotebookContentProvider,
@@ -119,13 +121,21 @@ export class VSCodeKernelPickerProvider implements NotebookKernelProvider {
119121
}
120122

121123
const document = newKernelInfo.document;
124+
if (!isJupyterNotebook(document)) {
125+
return;
126+
}
122127
const selection = await newKernelInfo.kernel.validate(document.uri);
123128
const editor = this.notebook.notebookEditors.find((item) => item.document === document);
124129
if (!selection || !editor || editor.kernel !== newKernelInfo.kernel) {
125130
// Possibly closed or different kernel picked.
126131
return;
127132
}
128-
133+
const model = await this.storageProvider.getOrCreateModel(document.uri);
134+
if (!model || !model.isTrusted) {
135+
// If a model is not trusted, we cannot change the kernel (this results in changes to notebook metadata).
136+
// This is because we store selected kernel in the notebook metadata.
137+
return;
138+
}
129139
// Change kernel and update metadata.
130140
const notebook = await this.notebookProvider.getOrCreateNotebook({
131141
resource: document.uri,

src/client/datascience/notebookStorage/vscNotebookModel.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,9 @@ export class VSCodeNotebookModel extends BaseNotebookModel {
4141
return this.document?.isDirty === true;
4242
}
4343
public get cells(): ICell[] {
44-
return this.document
44+
// When a notebook is not trusted, return original cells.
45+
// This is because the VSCode NotebookDocument object will not have any output in the cells.
46+
return this.document && this.isTrusted
4547
? this.document.cells.map((cell) => createCellFromVSCNotebookCell(cell, this))
4648
: this._cells;
4749
}
@@ -82,7 +84,7 @@ export class VSCodeNotebookModel extends BaseNotebookModel {
8284
}
8385
protected generateNotebookJson() {
8486
const json = super.generateNotebookJson();
85-
if (this.document) {
87+
if (this.document && this.isTrusted) {
8688
// The metadata will be in the notebook document.
8789
const metadata = getNotebookMetadata(this.document);
8890
if (metadata) {

src/test/datascience/notebook/contentProvider.ds.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { CellErrorOutput, Uri } from 'vscode';
1313
import { CellDisplayOutput } from '../../../../types/vscode-proposed';
1414
import { IVSCodeNotebook } from '../../../client/common/application/types';
1515
import { IDisposable } from '../../../client/common/types';
16+
import { INotebookStorageProvider } from '../../../client/datascience/notebookStorage/notebookStorageProvider';
1617
import { INotebookEditorProvider } from '../../../client/datascience/types';
1718
import { IExtensionTestApi } from '../../common';
1819
import { EXTENSION_ROOT_DIR_FOR_TESTS, initialize } from '../../initialize';
@@ -49,7 +50,28 @@ suite('DataScience - VSCode Notebook - (Open)', function () {
4950
testIPynb = Uri.file(await createTemporaryNotebook(templateIPynb, disposables));
5051
});
5152
teardown(async () => closeNotebooksAndCleanUpAfterTests(disposables));
53+
test('Verify Notebook Json', async () => {
54+
const storageProvider = api.serviceContainer.get<INotebookStorageProvider>(INotebookStorageProvider);
55+
const file = path.join(
56+
EXTENSION_ROOT_DIR_FOR_TESTS,
57+
'src',
58+
'test',
59+
'datascience',
60+
'notebook',
61+
'testJsonContents.ipynb'
62+
);
63+
const model = await storageProvider.getOrCreateModel(Uri.file(file));
64+
disposables.push(model);
65+
model.trust();
66+
const jsonStr = fs.readFileSync(file, { encoding: 'utf8' });
67+
68+
// JSON should be identical, before and after trusting a notebook.
69+
assert.deepEqual(JSON.parse(jsonStr), JSON.parse(model.getContent()));
5270

71+
model.trust();
72+
73+
assert.deepEqual(JSON.parse(jsonStr), JSON.parse(model.getContent()));
74+
});
5375
test('Verify cells (content, metadata & output)', async () => {
5476
const vscodeNotebook = api.serviceContainer.get<IVSCodeNotebook>(IVSCodeNotebook);
5577
const editorProvider = api.serviceContainer.get<INotebookEditorProvider>(INotebookEditorProvider);

0 commit comments

Comments
 (0)