diff --git a/src/sql/workbench/contrib/notebook/browser/notebookEditor.component.ts b/src/sql/workbench/contrib/notebook/browser/notebookEditor.component.ts index 8cf8c90512..15a5327705 100644 --- a/src/sql/workbench/contrib/notebook/browser/notebookEditor.component.ts +++ b/src/sql/workbench/contrib/notebook/browser/notebookEditor.component.ts @@ -46,7 +46,7 @@ export class NotebookEditorComponent extends AngularDisposable { public views: NotebookViewsExtension; public activeView: INotebookView; public viewMode: ViewMode; - public ViewMode = ViewMode; + public ViewMode = ViewMode; //For use of the enum in the template constructor( @Inject(ILogService) private readonly logService: ILogService, diff --git a/src/sql/workbench/contrib/notebook/test/browser/notebookViewModel.test.ts b/src/sql/workbench/contrib/notebook/test/browser/notebookViewModel.test.ts index 9eb229415b..1f40f6eb92 100644 --- a/src/sql/workbench/contrib/notebook/test/browser/notebookViewModel.test.ts +++ b/src/sql/workbench/contrib/notebook/test/browser/notebookViewModel.test.ts @@ -208,7 +208,7 @@ suite('NotebookViewModel', function (): void { viewModel.initialize(); let cell = viewModel.cells[0]; - let cellMeta = notebookViews.getCellMetadata(cell); + let cellMeta = notebookViews.getExtensionCellMetadata(cell); assert(!isUndefinedOrNull(cellMeta.views.find(v => v.guid === viewModel.guid))); assert.deepStrictEqual(viewModel.getCellMetadata(cell), cellMeta.views.find(v => v.guid === viewModel.guid)); diff --git a/src/sql/workbench/contrib/notebook/test/browser/notebookViewsExtension.test.ts b/src/sql/workbench/contrib/notebook/test/browser/notebookViewsExtension.test.ts index 38d3864833..d9f6ac696f 100644 --- a/src/sql/workbench/contrib/notebook/test/browser/notebookViewsExtension.test.ts +++ b/src/sql/workbench/contrib/notebook/test/browser/notebookViewsExtension.test.ts @@ -76,6 +76,19 @@ suite('NotebookViews', function (): void { notebookViews = await initializeExtension(); }); + test('should not modify the notebook document until a view is created', async () => { + //Create some content + notebookViews.notebook.addCell(CellTypes.Code, 0); + const cell = notebookViews.notebook.cells[0]; + + assert.strictEqual(notebookViews.getExtensionMetadata(), undefined); + assert.strictEqual(notebookViews.getExtensionCellMetadata(cell), undefined); + + //Check that the view is created + notebookViews.createNewView(defaultViewName); + assert.notStrictEqual(notebookViews.getExtensionMetadata(), undefined); + }); + test('create new view', async function (): Promise { assert.strictEqual(notebookViews.getViews().length, 0, 'notebook should not initially generate any views'); diff --git a/src/sql/workbench/services/notebook/browser/models/notebookExtension.ts b/src/sql/workbench/services/notebook/browser/models/notebookExtension.ts index 5c30ae475a..c8ef461edf 100644 --- a/src/sql/workbench/services/notebook/browser/models/notebookExtension.ts +++ b/src/sql/workbench/services/notebook/browser/models/notebookExtension.ts @@ -9,27 +9,36 @@ import { deepClone } from 'vs/base/common/objects'; export class NotebookExtension { readonly version = 1; - readonly extensionName = 'azuredatastudio'; readonly extensionNamespace = 'extensions'; - public getNotebookMetadata(notebook: INotebookModel): TNotebookMeta { + private _extensionName: string; + + public constructor(extensionName: string) { + this._extensionName = extensionName; + } + + public get extensionName(): string { + return this._extensionName; + } + + public getExtensionMetadata(notebook: INotebookModel): TNotebookMeta { const metadata = notebook.getMetaValue(this.extensionNamespace) || {}; return metadata[this.extensionName] as TNotebookMeta; } - public setNotebookMetadata(notebook: INotebookModel, metadata: TNotebookMeta) { + public setExtensionMetadata(notebook: INotebookModel, metadata: TNotebookMeta) { const meta = {}; meta[this.extensionName] = metadata; notebook.setMetaValue(this.extensionNamespace, meta); notebook.serializationStateChanged(NotebookChangeType.MetadataChanged); } - public getCellMetadata(cell: ICellModel): TCellMeta { + public getExtensionCellMetadata(cell: ICellModel): TCellMeta { const namespaceMeta = cell.metadata[this.extensionNamespace] || {}; return namespaceMeta[this.extensionName] as TCellMeta; } - public setCellMetadata(cell: ICellModel, metadata: TCellMeta) { + public setExtensionCellMetadata(cell: ICellModel, metadata: TCellMeta) { const meta = {}; meta[this.extensionName] = metadata; cell.metadata[this.extensionNamespace] = meta; diff --git a/src/sql/workbench/services/notebook/browser/notebookViews/notebookViewModel.ts b/src/sql/workbench/services/notebook/browser/notebookViews/notebookViewModel.ts index b7cb649afe..fe191e89d1 100644 --- a/src/sql/workbench/services/notebook/browser/notebookViews/notebookViewModel.ts +++ b/src/sql/workbench/services/notebook/browser/notebookViews/notebookViewModel.ts @@ -51,11 +51,11 @@ export class NotebookViewModel implements INotebookView { } protected initializeCell(cell: ICellModel, idx: number) { - let meta = this._notebookViews.getCellMetadata(cell); + let meta = this._notebookViews.getExtensionCellMetadata(cell); if (!meta) { this._notebookViews.initializeCell(cell); - meta = this._notebookViews.getCellMetadata(cell); + meta = this._notebookViews.getExtensionCellMetadata(cell); } // Ensure that we are not duplicting view entries in cell metadata @@ -91,7 +91,7 @@ export class NotebookViewModel implements INotebookView { } public getCellMetadata(cell: ICellModel): INotebookViewCell { - const meta = this._notebookViews.getCellMetadata(cell); + const meta = this._notebookViews.getExtensionCellMetadata(cell); return meta?.views?.find(view => view.guid === this.guid); } diff --git a/src/sql/workbench/services/notebook/browser/notebookViews/notebookViewsExtension.ts b/src/sql/workbench/services/notebook/browser/notebookViews/notebookViewsExtension.ts index 53aed35b8b..b188626d3e 100644 --- a/src/sql/workbench/services/notebook/browser/notebookViews/notebookViewsExtension.ts +++ b/src/sql/workbench/services/notebook/browser/notebookViews/notebookViewsExtension.ts @@ -13,30 +13,40 @@ import { INotebookView, INotebookViewCell, INotebookViewCellMetadata, INotebookV export class NotebookViewsExtension extends NotebookExtension implements INotebookViews { static readonly defaultViewName = localize('notebookView.untitledView', "Untitled View"); + static readonly extension = 'notebookviews'; readonly maxNameIterationAttempts = 100; - readonly extension = 'azuredatastudio'; override readonly version = 1; - protected _metadata: INotebookViewMetadata; + protected _metadata: INotebookViewMetadata | undefined; + private _initialized: boolean = false; private _onViewDeleted = new Emitter(); private _onActiveViewChanged = new Emitter(); constructor(protected _notebook: INotebookModel) { - super(); - this.loadOrInitialize(); + super(NotebookViewsExtension.extension); + this.load(); } - public loadOrInitialize() { - this._metadata = this.getNotebookMetadata(this._notebook); + public load(): void { + this._metadata = this.getExtensionMetadata(); + + if (this._metadata) { + this._metadata.views = this._metadata.views.map(view => NotebookViewModel.load(view.guid, this)); + this._initialized = true; + } + } + + public initialize() { + this._metadata = this.getExtensionMetadata(); if (!this._metadata) { this.initializeNotebook(); this.initializeCells(); this.commit(); - } else { - this._metadata.views = this._metadata.views.map(view => NotebookViewModel.load(view.guid, this)); } + + this._initialized = true; } protected initializeNotebook() { @@ -59,12 +69,17 @@ export class NotebookViewsExtension extends NotebookExtension view.guid === guid); - if (viewToRemove !== -1) { - let removedView = this._metadata.views.splice(viewToRemove, 1); + let viewToRemove = this._metadata?.views.findIndex(view => view.guid === guid); + if (viewToRemove >= 0) { + let removedView = this._metadata?.views.splice(viewToRemove, 1); // Remove view data for each cell if (removedView.length === 1) { this._notebook?.cells.forEach((cell) => { - let meta = this.getCellMetadata(cell); + let meta = this.getExtensionCellMetadata(cell); meta.views = meta.views.filter(x => x.guid !== removedView[0].guid); - this.setCellMetadata(cell, meta); + this.setExtensionCellMetadata(cell, meta); }); } } - if (guid === this._metadata.activeView) { + if (guid === this._metadata?.activeView) { this._metadata.activeView = undefined; } @@ -113,13 +128,13 @@ export class NotebookViewsExtension extends NotebookExtension view.guid === currentView.guid); if (viewToUpdate >= 0) { cellMetadata.views[viewToUpdate] = override ? cellData : { ...cellMetadata.views[viewToUpdate], ...cellData }; - this.setCellMetadata(cell, cellMetadata); + this.setExtensionCellMetadata(cell, cellMetadata); } } } @@ -129,7 +144,7 @@ export class NotebookViewsExtension extends NotebookExtension this.getCellMetadata(cell)); + return this._notebook.cells.map(cell => this.getExtensionCellMetadata(cell)); + } + + public override getExtensionMetadata(): INotebookViewMetadata { + return super.getExtensionMetadata(this._notebook); } public getActiveView(): INotebookView { - return this.getViews().find(view => view.guid === this._metadata.activeView); + return this.getViews().find(view => view.guid === this._metadata?.activeView); } public setActiveView(view: INotebookView) { - this._metadata.activeView = view.guid; - this._onActiveViewChanged.fire(); + if (this._metadata) { + this._metadata.activeView = view.guid; + this._onActiveViewChanged.fire(); + } } public commit() { this._metadata = Object.assign({}, this._metadata); - this.setNotebookMetadata(this._notebook, this._metadata); + this.setExtensionMetadata(this._notebook, this._metadata); } public viewNameIsTaken(name: string): boolean { @@ -165,4 +186,8 @@ export class NotebookViewsExtension extends NotebookExtension { return this._onActiveViewChanged.event; } + + public get initialized(): boolean { + return this._initialized; + } }