Skip to content

Commit 2505b6a

Browse files
DonJayamanneDonJayamanne
authored
Split to INotebookModel and INotebookStorage (#11703)
1 parent 408d0ec commit 2505b6a

15 files changed

Lines changed: 490 additions & 421 deletions

‎src/client/datascience/interactive-common/interactiveWindowTypes.ts‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,10 @@ export interface INotebookModelChange {
340340
source: 'undo' | 'user' | 'redo';
341341
}
342342

343+
export interface INotebookModelSaved extends INotebookModelChange {
344+
kind: 'save';
345+
}
346+
343347
export interface INotebookModelRemoveAllChange extends INotebookModelChange {
344348
kind: 'remove_all';
345349
oldCells: ICell[];
@@ -453,13 +457,8 @@ export interface INotebookModelVersionChange extends INotebookModelChange {
453457
kernelSpec: IJupyterKernelSpec | LiveKernelModel | undefined;
454458
}
455459

456-
export interface INotebookModelFileChange extends INotebookModelChange {
457-
kind: 'file';
458-
newFile: Uri;
459-
oldFile: Uri;
460-
}
461-
462460
export type NotebookModelChange =
461+
| INotebookModelSaved
463462
| INotebookModelModifyChange
464463
| INotebookModelRemoveAllChange
465464
| INotebookModelClearChange
@@ -469,7 +468,6 @@ export type NotebookModelChange =
469468
| INotebookModelAddChange
470469
| INotebookModelEditChange
471470
| INotebookModelVersionChange
472-
| INotebookModelFileChange
473471
| INotebookModelChangeTypeChange;
474472

475473
export interface IRunByLine {

‎src/client/datascience/interactive-ipynb/nativeEditor.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,7 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
219219

220220
public dispose(): Promise<void> {
221221
super.dispose();
222+
this.model?.dispose(); // NOSONAR
222223
return this.close();
223224
}
224225

@@ -303,7 +304,7 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
303304
public async getNotebookMetadata(): Promise<nbformat.INotebookMetadata | undefined> {
304305
await this.loadedPromise.promise;
305306
if (this.model) {
306-
return (await this.model.getJson()).metadata;
307+
return this.model.metadata;
307308
}
308309
}
309310

@@ -640,15 +641,15 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
640641
}
641642

642643
@captureTelemetry(Telemetry.ConvertToPythonFile, undefined, false)
643-
private async export(cells: ICell[]): Promise<void> {
644+
private async export(): Promise<void> {
644645
const status = this.setStatus(localize.DataScience.convertingToPythonFile(), false);
645646
// First generate a temporary notebook with these cells.
646647
let tempFile: TemporaryFile | undefined;
647648
try {
648649
tempFile = await this.fileSystem.createTemporaryFile('.ipynb');
649650

650651
// Translate the cells into a notebook
651-
const content = this.model ? await this.model.getContent(cells) : '';
652+
const content = this.model ? this.model.getContent() : '';
652653
await this.fileSystem.writeFile(tempFile.filePath, content, 'utf-8');
653654

654655
// Import this file and show it

‎src/client/datascience/interactive-ipynb/nativeEditorOldWebView.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import '../../common/extensions';
55

66
import { inject, injectable, multiInject, named } from 'inversify';
77
import * as path from 'path';
8-
import { Memento, Uri, WebviewPanel } from 'vscode';
8+
import { CancellationTokenSource, Memento, Uri, WebviewPanel } from 'vscode';
99

1010
import {
1111
IApplicationShell,
@@ -49,8 +49,8 @@ import {
4949
IThemeFinder
5050
} from '../types';
5151
import { NativeEditor } from './nativeEditor';
52-
import { NativeEditorStorage } from './nativeEditorStorage';
5352
import { NativeEditorSynchronizer } from './nativeEditorSynchronizer';
53+
import { INotebookStorageProvider } from './notebookStorageProvider';
5454

5555
enum AskForSaveResult {
5656
Yes,
@@ -97,7 +97,8 @@ export class NativeEditorOldWebView extends NativeEditor {
9797
@inject(IAsyncDisposableRegistry) asyncRegistry: IAsyncDisposableRegistry,
9898
@inject(KernelSwitcher) switcher: KernelSwitcher,
9999
@inject(INotebookProvider) notebookProvider: INotebookProvider,
100-
@inject(UseCustomEditorApi) useCustomEditorApi: boolean
100+
@inject(UseCustomEditorApi) useCustomEditorApi: boolean,
101+
@inject(INotebookStorageProvider) private readonly storage: INotebookStorageProvider
101102
) {
102103
super(
103104
listeners,
@@ -200,8 +201,7 @@ export class NativeEditorOldWebView extends NativeEditor {
200201
const filesConfig = this.workspaceService.getConfiguration('files', this.file);
201202
const autoSave = filesConfig.get('autoSave', 'off');
202203
if (autoSave === 'off') {
203-
const model = this.model as NativeEditorStorage;
204-
await model.storeContentsInHotExitFile();
204+
await this.storage.backup(this.model, new CancellationTokenSource().token);
205205
}
206206
this.commandManager.executeCommand(Commands.OpenNotebookNonCustomEditor, this.model.file).then(noop, noop);
207207
}

‎src/client/datascience/interactive-ipynb/nativeEditorProvider.ts‎

Lines changed: 32 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@ import { IServiceContainer } from '../../ioc/types';
2828
import { captureTelemetry, sendTelemetryEvent } from '../../telemetry';
2929
import { Telemetry } from '../constants';
3030
import { NotebookModelChange } from '../interactive-common/interactiveWindowTypes';
31-
import { INotebookEditor, INotebookEditorProvider, INotebookModel, INotebookStorage } from '../types';
31+
import { INotebookEditor, INotebookEditorProvider, INotebookModel } from '../types';
32+
import { INotebookStorageProvider } from './notebookStorageProvider';
3233

3334
// Class that is registered as the custom editor provider for notebooks. VS code will call into this class when
3435
// opening an ipynb file. This class then creates a backing storage, model, and opens a view for the file.
@@ -69,9 +70,8 @@ export class NativeEditorProvider
6970
protected customDocuments = new Map<string, CustomDocument>();
7071
private readonly _onDidCloseNotebookEditor = new EventEmitter<INotebookEditor>();
7172
private openedEditors: Set<INotebookEditor> = new Set<INotebookEditor>();
72-
private storageAndModels = new Map<string, Promise<{ model: INotebookModel; storage: INotebookStorage }>>();
7373
private executedEditors: Set<string> = new Set<string>();
74-
private models = new WeakSet<INotebookModel>();
74+
private models = new Set<INotebookModel>();
7575
private notebookCount: number = 0;
7676
private openedNotebookCount: number = 0;
7777
private _id = uuid();
@@ -81,7 +81,8 @@ export class NativeEditorProvider
8181
@inject(IDisposableRegistry) protected readonly disposables: IDisposableRegistry,
8282
@inject(IWorkspaceService) protected readonly workspace: IWorkspaceService,
8383
@inject(IConfigurationService) protected readonly configuration: IConfigurationService,
84-
@inject(ICustomEditorService) private customEditorService: ICustomEditorService
84+
@inject(ICustomEditorService) private customEditorService: ICustomEditorService,
85+
@inject(INotebookStorageProvider) private readonly storage: INotebookStorageProvider
8586
) {
8687
traceInfo(`id is ${this._id}`);
8788
asyncRegistry.push(this);
@@ -100,19 +101,13 @@ export class NativeEditorProvider
100101
});
101102
}
102103

103-
public save(document: CustomDocument, cancellation: CancellationToken): Promise<void> {
104-
return this.loadStorage(document.uri).then(async (s) => {
105-
if (s) {
106-
await s.save(cancellation);
107-
}
108-
});
104+
public async save(document: CustomDocument, cancellation: CancellationToken): Promise<void> {
105+
const model = await this.loadModel(document.uri);
106+
await this.storage.save(model, cancellation);
109107
}
110-
public saveAs(document: CustomDocument, targetResource: Uri): Promise<void> {
111-
return this.loadStorage(document.uri).then(async (s) => {
112-
if (s) {
113-
await s.saveAs(targetResource);
114-
}
115-
});
108+
public async saveAs(document: CustomDocument, targetResource: Uri): Promise<void> {
109+
const model = await this.loadModel(document.uri);
110+
await this.storage.saveAs(model, targetResource);
116111
}
117112
public applyEdits(document: CustomDocument, edits: readonly NotebookModelChange[]): Promise<void> {
118113
return this.loadModel(document.uri).then((s) => {
@@ -132,11 +127,8 @@ export class NativeEditorProvider
132127
noop();
133128
}
134129
public async backup(document: CustomDocument, cancellation: CancellationToken): Promise<void> {
135-
return this.loadStorage(document.uri).then(async (s) => {
136-
if (s) {
137-
await s.backup(cancellation);
138-
}
139-
});
130+
const model = await this.loadModel(document.uri);
131+
await this.storage.backup(model, cancellation);
140132
}
141133

142134
public async resolveCustomEditor(document: CustomDocument, panel: WebviewPanel) {
@@ -146,12 +138,7 @@ export class NativeEditorProvider
146138

147139
public async resolveCustomDocument(document: CustomDocument): Promise<void> {
148140
this.customDocuments.set(document.uri.fsPath, document);
149-
await this.loadStorage(document.uri);
150-
}
151-
152-
public async resolveNativeEditorStorage(document: CustomDocument): Promise<INotebookStorage> {
153-
this.customDocuments.set(document.uri.fsPath, document);
154-
return this.loadStorage(document.uri);
141+
await this.loadModel(document.uri);
155142
}
156143

157144
public async dispose(): Promise<void> {
@@ -204,11 +191,18 @@ export class NativeEditorProvider
204191
this.notebookCount += 1;
205192

206193
// Set these contents into the storage before the file opens
207-
await this.loadStorage(uri, contents);
194+
await this.loadModel(uri, contents);
208195

209196
return this.open(uri);
210197
}
211198

199+
public loadModel(file: Uri, contents?: string) {
200+
return this.storage.load(file, contents).then((m) => {
201+
this.trackModel(m);
202+
return m;
203+
});
204+
}
205+
212206
protected async createNotebookEditor(resource: Uri, panel?: WebviewPanel) {
213207
try {
214208
// Get the model
@@ -242,13 +236,15 @@ export class NativeEditorProvider
242236

243237
private closedEditor(editor: INotebookEditor): void {
244238
this.openedEditors.delete(editor);
245-
// If last editor, dispose of the storage
246-
const key = editor.file.toString();
247-
if (![...this.openedEditors].find((e) => e.file.toString() === key)) {
248-
this.storageAndModels.delete(key);
249-
}
250239
this._onDidCloseNotebookEditor.fire(editor);
251240
}
241+
private trackModel(model: INotebookModel) {
242+
if (!this.models.has(model)) {
243+
this.models.add(model);
244+
this.disposables.push(model.onDidDispose(() => this.models.delete(model)));
245+
this.disposables.push(model.onDidEdit(this.modelEdited.bind(this, model)));
246+
}
247+
}
252248

253249
private onChangedViewState(): void {
254250
this._onDidChangeActiveNotebookEditor.fire(this.activeEditor);
@@ -260,15 +256,6 @@ export class NativeEditorProvider
260256
}
261257
}
262258

263-
private async modelChanged(change: NotebookModelChange) {
264-
if (change.source === 'user' && change.kind === 'file') {
265-
// Update our storage
266-
const promise = this.storageAndModels.get(change.oldFile.toString());
267-
this.storageAndModels.delete(change.oldFile.toString());
268-
this.storageAndModels.set(change.newFile.toString(), promise!);
269-
}
270-
}
271-
272259
private async modelEdited(model: INotebookModel, change: NotebookModelChange) {
273260
// Find the document associated with this edit.
274261
const document = this.customDocuments.get(model.file.fsPath);
@@ -277,43 +264,15 @@ export class NativeEditorProvider
277264
}
278265
}
279266

280-
private async loadModel(file: Uri, contents?: string): Promise<INotebookModel> {
281-
const modelAndStorage = await this.loadModelAndStorage(file, contents);
282-
return modelAndStorage.model;
283-
}
284-
285-
private async loadStorage(file: Uri, contents?: string): Promise<INotebookStorage> {
286-
const modelAndStorage = await this.loadModelAndStorage(file, contents);
287-
return modelAndStorage.storage;
288-
}
289-
290-
private loadModelAndStorage(file: Uri, contents?: string) {
291-
const key = file.toString();
292-
let modelPromise = this.storageAndModels.get(key);
293-
if (!modelPromise) {
294-
const storage = this.serviceContainer.get<INotebookStorage>(INotebookStorage);
295-
modelPromise = storage.load(file, contents).then((m) => {
296-
if (!this.models.has(m)) {
297-
this.models.add(m);
298-
this.disposables.push(m.changed(this.modelChanged.bind(this)));
299-
this.disposables.push(storage.onDidEdit(this.modelEdited.bind(this, m)));
300-
}
301-
return { model: m, storage };
302-
});
303-
this.storageAndModels.set(key, modelPromise);
304-
}
305-
return modelPromise;
306-
}
307-
308267
private async getNextNewNotebookUri(): Promise<Uri> {
268+
// tslint:disable-next-line: no-suspicious-comment
269+
// TODO: This will not work, if we close an untitled document.
309270
// See if we have any untitled storage already
310-
const untitledStorage = [...this.storageAndModels.keys()].filter((k) => Uri.parse(k).scheme === 'untitled');
311-
271+
const untitledStorage = Array.from(this.models.values()).filter((model) => model?.file?.scheme === 'untitled');
312272
// Just use the length (don't bother trying to fill in holes). We never remove storage objects from
313273
// our map, so we'll keep creating new untitled notebooks.
314274
const fileName = `${localize.DataScience.untitledNotebookFileName()}-${untitledStorage.length + 1}.ipynb`;
315275
const fileUri = Uri.file(fileName);
316-
317276
// Turn this back into an untitled
318277
return fileUri.with({ scheme: 'untitled', path: fileName });
319278
}

‎src/client/datascience/interactive-ipynb/nativeEditorProviderOld.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import { IServiceContainer } from '../../ioc/types';
1818
import { Commands } from '../constants';
1919
import { IDataScienceErrorHandler, INotebookEditor } from '../types';
2020
import { NativeEditorProvider } from './nativeEditorProvider';
21+
import { INotebookStorageProvider } from './notebookStorageProvider';
2122

2223
@injectable()
2324
export class NativeEditorProviderOld extends NativeEditorProvider {
@@ -42,9 +43,10 @@ export class NativeEditorProviderOld extends NativeEditorProvider {
4243
@inject(IFileSystem) private fileSystem: IFileSystem,
4344
@inject(IDocumentManager) private documentManager: IDocumentManager,
4445
@inject(ICommandManager) private readonly cmdManager: ICommandManager,
45-
@inject(IDataScienceErrorHandler) private dataScienceErrorHandler: IDataScienceErrorHandler
46+
@inject(IDataScienceErrorHandler) private dataScienceErrorHandler: IDataScienceErrorHandler,
47+
@inject(INotebookStorageProvider) storage: INotebookStorageProvider
4648
) {
47-
super(serviceContainer, asyncRegistry, disposables, workspace, configuration, customEditorService);
49+
super(serviceContainer, asyncRegistry, disposables, workspace, configuration, customEditorService, storage);
4850

4951
// No live share sync required as open document from vscode will give us our contents.
5052

0 commit comments

Comments
 (0)