From 84b3e876d79b348b4da17f7d86ff52731829180d Mon Sep 17 00:00:00 2001 From: Chris LaFreniere <40371649+chlafreniere@users.noreply.github.com> Date: Mon, 26 Aug 2019 10:17:58 -0700 Subject: [PATCH] Notebook Performance Improvements to Cell Editing/Output Changes/Execution Count Changes (#6867) * edit perf * Save multiline source in notebooks * More merges * Single, multi line works needs work * Works with single + multi and recomputes active * Actual perf improvements this time * code cleanup * Calculating output position on the fly * Hmm can we use brackets to make this simpler? * monday progress * output working. lots of improvements. * First tests working * 10 tests now, fixed bugs * Cleanup, add output test * More fixes * Need to still fix execution count bug * Tests pass, added comments * Cleanup * PR comments round 1 * Deal with merge issues from master, layering * Deleting duplicate file * More PR Comments * PR Comments --- .../notebook/language-configuration.json | 19 + extensions/notebook/package.json | 3 +- src/sql/azdata.d.ts | 1 + .../mainThreadNotebookDocumentsAndEditors.ts | 7 +- .../browser/cellViews/code.component.ts | 1 + .../browser/cellViews/codeCell.component.ts | 2 +- .../notebook/browser/models/notebookInput.ts | 88 ++- .../notebook/browser/notebook.component.ts | 25 +- .../parts/notebook/common/models/cell.ts | 48 +- .../parts/notebook/common/models/contracts.ts | 3 +- .../notebook/common/models/modelInterfaces.ts | 30 +- .../notebook/common/models/notebookModel.ts | 20 +- .../common/models/notebookTextFileModel.ts | 258 +++++++ .../cellViews/textCell.component.ts | 2 +- .../test/common/notebookEditorModel.test.ts | 644 ++++++++++++++++++ .../parts/notebook/test/node/cell.test.ts | 62 +- .../parts/notebook/test/node/common.ts | 9 +- .../notebook/common/notebookService.ts | 2 +- .../notebook/common/notebookServiceImpl.ts | 6 +- 19 files changed, 1157 insertions(+), 73 deletions(-) create mode 100644 extensions/notebook/language-configuration.json create mode 100644 src/sql/workbench/parts/notebook/common/models/notebookTextFileModel.ts create mode 100644 src/sql/workbench/parts/notebook/test/common/notebookEditorModel.test.ts diff --git a/extensions/notebook/language-configuration.json b/extensions/notebook/language-configuration.json new file mode 100644 index 000000000000..9a73ac64aae3 --- /dev/null +++ b/extensions/notebook/language-configuration.json @@ -0,0 +1,19 @@ +{ + "comments": { + "lineComment": "//", + "blockComment": [ "/*", "*/" ] + }, + "brackets": [ + ["{", "}"], + ["[", "]"] + ], + "autoClosingPairs": [ + { "open": "{", "close": "}", "notIn": ["string"] }, + { "open": "[", "close": "]", "notIn": ["string"] }, + { "open": "(", "close": ")", "notIn": ["string"] }, + { "open": "'", "close": "'", "notIn": ["string"] }, + { "open": "/*", "close": "*/", "notIn": ["string"] }, + { "open": "\"", "close": "\"", "notIn": ["string", "comment"] }, + { "open": "`", "close": "`", "notIn": ["string", "comment"] } + ] +} \ No newline at end of file diff --git a/extensions/notebook/package.json b/extensions/notebook/package.json index 2736f246d722..3392993f4b6b 100644 --- a/extensions/notebook/package.json +++ b/extensions/notebook/package.json @@ -149,7 +149,8 @@ ], "aliases": [ "Notebook" - ] + ], + "configuration": "./language-configuration.json" } ], "menus": { diff --git a/src/sql/azdata.d.ts b/src/sql/azdata.d.ts index 5d0fc587064b..3f9887d4f240 100644 --- a/src/sql/azdata.d.ts +++ b/src/sql/azdata.d.ts @@ -4448,6 +4448,7 @@ declare module 'azdata' { source: string | string[]; metadata?: { language?: string; + azdata_cell_guid?: string; }; execution_count?: number; outputs?: ICellOutput[]; diff --git a/src/sql/workbench/api/browser/mainThreadNotebookDocumentsAndEditors.ts b/src/sql/workbench/api/browser/mainThreadNotebookDocumentsAndEditors.ts index bf6ff8b27108..f14e84749b11 100644 --- a/src/sql/workbench/api/browser/mainThreadNotebookDocumentsAndEditors.ts +++ b/src/sql/workbench/api/browser/mainThreadNotebookDocumentsAndEditors.ts @@ -626,6 +626,7 @@ export class MainThreadNotebookDocumentsAndEditors extends Disposable implements case NotebookChangeType.CellOutputUpdated: case NotebookChangeType.CellSourceUpdated: case NotebookChangeType.DirtyStateChanged: + case NotebookChangeType.CellOutputCleared: return NotebookChangeKind.ContentUpdated; case NotebookChangeType.KernelChanged: case NotebookChangeType.TrustChanged: @@ -654,7 +655,8 @@ export class MainThreadNotebookDocumentsAndEditors extends Disposable implements cell_type: cell.cellType, execution_count: cell.executionCount, metadata: { - language: cell.language + language: cell.language, + azdata_cell_guid: cell.cellGuid }, source: undefined, outputs: [...cell.outputs] @@ -669,7 +671,8 @@ export class MainThreadNotebookDocumentsAndEditors extends Disposable implements cell_type: cells.cellType, execution_count: undefined, metadata: { - language: cells.language + language: cells.language, + azdata_cell_guid: cells.cellGuid }, source: undefined } diff --git a/src/sql/workbench/parts/notebook/browser/cellViews/code.component.ts b/src/sql/workbench/parts/notebook/browser/cellViews/code.component.ts index b900b02dd671..6700b0e9babc 100644 --- a/src/sql/workbench/parts/notebook/browser/cellViews/code.component.ts +++ b/src/sql/workbench/parts/notebook/browser/cellViews/code.component.ts @@ -221,6 +221,7 @@ export class CodeComponent extends AngularDisposable implements OnInit, OnChange this._register(this._editorInput); this._register(this._editorModel.onDidChangeContent(e => { this._editor.setHeightToScrollHeight(); + this.cellModel.modelContentChangedEvent = e; this.cellModel.source = this._editorModel.getValue(); this.onContentChanged.emit(); this.checkForLanguageMagics(); diff --git a/src/sql/workbench/parts/notebook/browser/cellViews/codeCell.component.ts b/src/sql/workbench/parts/notebook/browser/cellViews/codeCell.component.ts index 31294a452960..8ad8d88ff93d 100644 --- a/src/sql/workbench/parts/notebook/browser/cellViews/codeCell.component.ts +++ b/src/sql/workbench/parts/notebook/browser/cellViews/codeCell.component.ts @@ -30,7 +30,7 @@ export class CodeCellComponent extends CellView implements OnInit, OnChanges { @HostListener('document:keydown.escape', ['$event']) handleKeyboardEvent() { this.cellModel.active = false; - this._model.activeCell = undefined; + this._model.updateActiveCell(undefined); } private _model: NotebookModel; diff --git a/src/sql/workbench/parts/notebook/browser/models/notebookInput.ts b/src/sql/workbench/parts/notebook/browser/models/notebookInput.ts index f09cbe45a3a8..2ce5cc110c77 100644 --- a/src/sql/workbench/parts/notebook/browser/models/notebookInput.ts +++ b/src/sql/workbench/parts/notebook/browser/models/notebookInput.ts @@ -16,7 +16,6 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti import { ITextModelService } from 'vs/editor/common/services/resolverService'; import { INotebookModel, IContentManager, NotebookContentChange } from 'sql/workbench/parts/notebook/common/models/modelInterfaces'; import { TextFileEditorModel } from 'vs/workbench/services/textfile/common/textFileEditorModel'; -import { Range } from 'vs/editor/common/core/range'; import { UntitledEditorModel } from 'vs/workbench/common/editor/untitledEditorModel'; import { Schemas } from 'vs/base/common/network'; import { ITextFileService, ISaveOptions, StateChange } from 'vs/workbench/services/textfile/common/textfiles'; @@ -26,12 +25,17 @@ import { UntitledEditorInput } from 'vs/workbench/common/editor/untitledEditorIn import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions'; import { IDisposable } from 'vs/base/common/lifecycle'; import { NotebookChangeType } from 'sql/workbench/parts/notebook/common/models/contracts'; +import { Deferred } from 'sql/base/common/promise'; +import { NotebookTextFileModel } from 'sql/workbench/parts/notebook/common/models/notebookTextFileModel'; export type ModeViewSaveHandler = (handle: number) => Thenable; export class NotebookEditorModel extends EditorModel { - private dirty: boolean; + private _dirty: boolean; + private _changeEventsHookedUp: boolean = false; + private _notebookTextFileModel: NotebookTextFileModel = new NotebookTextFileModel(); private readonly _onDidChangeDirty: Emitter = this._register(new Emitter()); + private _lastEditFullReplacement: boolean; constructor(public readonly notebookUri: URI, private textEditorModel: TextFileEditorModel | UntitledEditorModel, @INotebookService private notebookService: INotebookService, @@ -41,9 +45,17 @@ export class NotebookEditorModel extends EditorModel { this._register(this.notebookService.onNotebookEditorAdd(notebook => { if (notebook.id === this.notebookUri.toString()) { // Hook to content change events - notebook.modelReady.then(() => { - this._register(notebook.model.kernelChanged(e => this.updateModel())); - this._register(notebook.model.contentChanged(e => this.updateModel(e))); + notebook.modelReady.then((model) => { + if (!this._changeEventsHookedUp) { + this._changeEventsHookedUp = true; + this._register(model.kernelChanged(e => this.updateModel(undefined, NotebookChangeType.KernelChanged))); + this._register(model.contentChanged(e => this.updateModel(e, e.changeType))); + this._register(notebook.model.onActiveCellChanged((cell) => { + if (cell) { + this._notebookTextFileModel.activeCellGuid = cell.cellGuid; + } + })); + } }, err => undefined); } })); @@ -58,7 +70,7 @@ export class NotebookEditorModel extends EditorModel { } })); } - this.dirty = this.textEditorModel.isDirty(); + this._dirty = this.textEditorModel.isDirty(); } public get contentString(): string { @@ -66,15 +78,19 @@ export class NotebookEditorModel extends EditorModel { return model.getValue(); } + public get lastEditFullReplacement(): boolean { + return this._lastEditFullReplacement; + } + isDirty(): boolean { return this.textEditorModel.isDirty(); } public setDirty(dirty: boolean): void { - if (this.dirty === dirty) { + if (this._dirty === dirty) { return; } - this.dirty = dirty; + this._dirty = dirty; this._onDidChangeDirty.fire(); } @@ -92,7 +108,8 @@ export class NotebookEditorModel extends EditorModel { } } - public updateModel(contentChange?: NotebookContentChange): void { + public updateModel(contentChange?: NotebookContentChange, type?: NotebookChangeType): void { + this._lastEditFullReplacement = false; if (contentChange && contentChange.changeType === NotebookChangeType.Saved) { // We send the saved events out, so ignore. Otherwise we double-count this as a change // and cause the text to be reapplied @@ -104,22 +121,42 @@ export class NotebookEditorModel extends EditorModel { // Request serialization so trusted state is preserved but don't update the model this.sendNotebookSerializationStateChange(); } else { - // For all other changes, update the backing model with the latest contents let notebookModel = this.getNotebookModel(); + let editAppliedSuccessfully = false; if (notebookModel && this.textEditorModel && this.textEditorModel.textEditorModel) { - let content = JSON.stringify(notebookModel.toJSON(), undefined, ' '); - let model = this.textEditorModel.textEditorModel; - let endLine = model.getLineCount(); - let endCol = model.getLineMaxColumn(endLine); - - this.textEditorModel.textEditorModel.applyEdits([{ - range: new Range(1, 1, endLine, endCol), - text: content - }]); + if (contentChange && contentChange.cells && contentChange.cells[0]) { + if (type === NotebookChangeType.CellSourceUpdated) { + if (this._notebookTextFileModel.transformAndApplyEditForSourceUpdate(contentChange, this.textEditorModel)) { + editAppliedSuccessfully = true; + } + } else if (type === NotebookChangeType.CellOutputUpdated) { + if (this._notebookTextFileModel.transformAndApplyEditForOutputUpdate(contentChange, this.textEditorModel)) { + editAppliedSuccessfully = true; + } + } else if (type === NotebookChangeType.CellOutputCleared) { + if (this._notebookTextFileModel.transformAndApplyEditForClearOutput(contentChange, this.textEditorModel)) { + editAppliedSuccessfully = true; + } + } else if (type === NotebookChangeType.CellExecuted) { + if (this._notebookTextFileModel.transformAndApplyEditForCellUpdated(contentChange, this.textEditorModel)) { + editAppliedSuccessfully = true; + } + } + } + // If edit was already applied, skip replacing entire text model + if (editAppliedSuccessfully) { + return; + } + this.replaceEntireTextEditorModel(notebookModel, type); + this._lastEditFullReplacement = true; } } } + public replaceEntireTextEditorModel(notebookModel: INotebookModel, type: NotebookChangeType) { + this._notebookTextFileModel.replaceEntireTextEditorModel(notebookModel, type, this.textEditorModel); + } + private sendNotebookSerializationStateChange(): void { let notebookModel = this.getNotebookModel(); if (notebookModel) { @@ -142,6 +179,10 @@ export class NotebookEditorModel extends EditorModel { get onDidChangeDirty(): Event { return this._onDidChangeDirty.event; } + + get editorModel() { + return this.textEditorModel; + } } export class NotebookInput extends EditorInput { @@ -161,6 +202,8 @@ export class NotebookInput extends EditorInput { private _providersLoaded: Promise; private _dirtyListener: IDisposable; private _notebookEditorOpenedTimestamp: number; + private _modelResolveInProgress: boolean = false; + private _modelResolved: Deferred = new Deferred(); constructor(private _title: string, private resource: URI, @@ -283,6 +326,12 @@ export class NotebookInput extends EditorInput { } async resolve(): Promise { + if (!this._modelResolveInProgress) { + this._modelResolveInProgress = true; + } else { + await this._modelResolved; + return this._model; + } if (this._model) { return Promise.resolve(this._model); } else { @@ -296,6 +345,7 @@ export class NotebookInput extends EditorInput { } this._model = this.instantiationService.createInstance(NotebookEditorModel, this.resource, textOrUntitledEditorModel); this.hookDirtyListener(this._model.onDidChangeDirty, () => this._onDidChangeDirty.fire()); + this._modelResolved.resolve(); return this._model; } } diff --git a/src/sql/workbench/parts/notebook/browser/notebook.component.ts b/src/sql/workbench/parts/notebook/browser/notebook.component.ts index 73bf44d52cb6..6c9cef3b502d 100644 --- a/src/sql/workbench/parts/notebook/browser/notebook.component.ts +++ b/src/sql/workbench/parts/notebook/browser/notebook.component.ts @@ -176,14 +176,8 @@ export class NotebookComponent extends AngularDisposable implements OnInit, OnDe if (event) { event.stopPropagation(); } - if (cell !== this.model.activeCell) { - if (this.model.activeCell) { - this.model.activeCell.active = false; - } - this._model.activeCell = cell; - this._model.activeCell.active = true; - this.detectChanges(); - } + this.model.updateActiveCell(cell); + this.detectChanges(); } //Saves scrollTop value on scroll change @@ -192,10 +186,7 @@ export class NotebookComponent extends AngularDisposable implements OnInit, OnDe } public unselectActiveCell() { - if (this.model && this.model.activeCell) { - this.model.activeCell.active = false; - this.model.activeCell = undefined; - } + this.model.updateActiveCell(undefined); this.detectChanges(); } @@ -311,10 +302,10 @@ export class NotebookComponent extends AngularDisposable implements OnInit, OnDe editorLoadedTimestamp: this._notebookParams.input.editorOpenedTimestamp }, this.profile, this.logService, this.notificationService, this.telemetryService); let trusted = await this.notebookService.isNotebookTrustCached(this._notebookParams.notebookUri, this.isDirty()); - model.onError((errInfo: INotification) => this.handleModelError(errInfo)); - model.contentChanged((change) => this.handleContentChanged(change)); - model.onProviderIdChange((provider) => this.handleProviderIdChanged(provider)); - model.kernelChanged((kernelArgs) => this.handleKernelChanged(kernelArgs)); + this._register(model.onError((errInfo: INotification) => this.handleModelError(errInfo))); + this._register(model.contentChanged((change) => this.handleContentChanged())); + this._register(model.onProviderIdChange((provider) => this.handleProviderIdChanged(provider))); + this._register(model.kernelChanged((kernelArgs) => this.handleKernelChanged(kernelArgs))); this._model = this._register(model); await this._model.loadContents(trusted); this.setLoading(false); @@ -382,7 +373,7 @@ export class NotebookComponent extends AngularDisposable implements OnInit, OnDe this.notificationService.notify(notification); } - private handleContentChanged(change: NotebookContentChange) { + private handleContentChanged() { // Note: for now we just need to set dirty state and refresh the UI. this.detectChanges(); } diff --git a/src/sql/workbench/parts/notebook/common/models/cell.ts b/src/sql/workbench/parts/notebook/common/models/cell.ts index 443d3ff258b6..53f92a8fe465 100644 --- a/src/sql/workbench/parts/notebook/common/models/cell.ts +++ b/src/sql/workbench/parts/notebook/common/models/cell.ts @@ -20,12 +20,15 @@ import { Schemas } from 'vs/base/common/network'; import { INotebookService } from 'sql/workbench/services/notebook/common/notebookService'; import { optional } from 'vs/platform/instantiation/common/instantiation'; import { getErrorMessage } from 'vs/base/common/errors'; +import { generateUuid } from 'vs/base/common/uuid'; +import { IModelContentChangedEvent } from 'vs/editor/common/model/textModelEvents'; let modelId = 0; export class CellModel implements ICellModel { private _cellType: nb.CellType; private _source: string | string[]; private _language: string; + private _cellGuid: string; private _future: FutureInternal; private _outputs: nb.ICellOutput[] = []; private _isEditMode: boolean; @@ -43,7 +46,8 @@ export class CellModel implements ICellModel { private _onCellLoaded = new Emitter(); private _loaded: boolean; private _stdInVisible: boolean; - private _metadata: { language?: string; }; + private _metadata: { language?: string, cellGuid?: string; }; + private _modelContentChangedEvent: IModelContentChangedEvent; constructor(cellData: nb.ICellContents, private _options: ICellModelOptions, @@ -64,6 +68,8 @@ export class CellModel implements ICellModel { } else { this._isTrusted = false; } + // if the fromJson() method was already called and _cellGuid was previously set, don't generate another UUID unnecessarily + this._cellGuid = this._cellGuid || generateUuid(); this.createUri(); } @@ -165,6 +171,15 @@ export class CellModel implements ICellModel { this._source = newSource; this.sendChangeToNotebook(NotebookChangeType.CellSourceUpdated); } + this._modelContentChangedEvent = undefined; + } + + public get modelContentChangedEvent(): IModelContentChangedEvent { + return this._modelContentChangedEvent; + } + + public set modelContentChangedEvent(e: IModelContentChangedEvent) { + this._modelContentChangedEvent = e; } public get language(): string { @@ -177,6 +192,10 @@ export class CellModel implements ICellModel { return this.options.notebook.language; } + public get cellGuid(): string { + return this._cellGuid; + } + public setOverrideLanguage(newLanguage: string) { this._language = newLanguage; } @@ -214,7 +233,7 @@ export class CellModel implements ICellModel { private notifyExecutionComplete(): void { if (this._notebookService) { - this._notebookService.serializeNotebookStateChange(this.notebookModel.notebookUri, NotebookChangeType.CellExecuted); + this._notebookService.serializeNotebookStateChange(this.notebookModel.notebookUri, NotebookChangeType.CellExecuted, this); } } @@ -232,11 +251,8 @@ export class CellModel implements ICellModel { public async runCell(notificationService?: INotificationService, connectionManagementService?: IConnectionManagementService): Promise { try { if (!this.active && this !== this.notebookModel.activeCell) { - if (this.notebookModel.activeCell) { - this.notebookModel.activeCell.active = false; - } + this.notebookModel.updateActiveCell(this); this.active = true; - this.notebookModel.activeCell = this; } if (connectionManagementService) { @@ -266,7 +282,7 @@ export class CellModel implements ICellModel { } } let content = this.source; - if (content) { + if ((Array.isArray(content) && content.length > 0) || (!Array.isArray(content) && content)) { // requestExecute expects a string for the code parameter content = Array.isArray(content) ? content.join('') : content; let future = await kernel.requestExecute({ @@ -369,7 +385,11 @@ export class CellModel implements ICellModel { shouldScroll: !!shouldScroll }; this._onOutputsChanged.fire(outputEvent); - this.sendChangeToNotebook(NotebookChangeType.CellOutputUpdated); + if (this.outputs.length !== 0) { + this.sendChangeToNotebook(NotebookChangeType.CellOutputUpdated); + } else { + this.sendChangeToNotebook(NotebookChangeType.CellOutputCleared); + } } private sendChangeToNotebook(change: NotebookChangeType): void { @@ -521,9 +541,10 @@ export class CellModel implements ICellModel { source: this._source, metadata: this._metadata || {} }; + cellJson.metadata.azdata_cell_guid = this._cellGuid; if (this._cellType === CellTypes.Code) { - cellJson.metadata.language = this._language, - cellJson.outputs = this._outputs; + cellJson.metadata.language = this._language; + cellJson.outputs = this._outputs; cellJson.execution_count = this.executionCount ? this.executionCount : 0; } return cellJson as nb.ICellContents; @@ -537,6 +558,7 @@ export class CellModel implements ICellModel { this.executionCount = cell.execution_count; this._source = this.getMultilineSource(cell.source); this._metadata = cell.metadata; + this._cellGuid = cell.metadata && cell.metadata.azdata_cell_guid ? cell.metadata.azdata_cell_guid : generateUuid(); this.setLanguageFromContents(cell); if (cell.outputs) { for (let output of cell.outputs) { @@ -600,8 +622,10 @@ export class CellModel implements ICellModel { if (typeof source === 'string') { let sourceMultiline = source.split('\n'); // If source is one line (i.e. no '\n'), return it immediately - if (sourceMultiline.length <= 1) { - return source; + if (sourceMultiline.length === 1) { + return [source]; + } else if (sourceMultiline.length === 0) { + return []; } // Otherwise, add back all of the newlines here // Note: for Windows machines that require '/r/n', diff --git a/src/sql/workbench/parts/notebook/common/models/contracts.ts b/src/sql/workbench/parts/notebook/common/models/contracts.ts index bbf8ca536d61..3e1c62d31940 100644 --- a/src/sql/workbench/parts/notebook/common/models/contracts.ts +++ b/src/sql/workbench/parts/notebook/common/models/contracts.ts @@ -44,5 +44,6 @@ export enum NotebookChangeType { KernelChanged, TrustChanged, Saved, - CellExecuted + CellExecuted, + CellOutputCleared } diff --git a/src/sql/workbench/parts/notebook/common/models/modelInterfaces.ts b/src/sql/workbench/parts/notebook/common/models/modelInterfaces.ts index 2cf3bb5b341f..72abd9c247dd 100644 --- a/src/sql/workbench/parts/notebook/common/models/modelInterfaces.ts +++ b/src/sql/workbench/parts/notebook/common/models/modelInterfaces.ts @@ -22,6 +22,7 @@ import { ICapabilitiesService } from 'sql/platform/capabilities/common/capabilit import { localize } from 'vs/nls'; import { NotebookModel } from 'sql/workbench/parts/notebook/common/models/notebookModel'; import { mssqlProviderName } from 'sql/platform/connection/common/constants'; +import { IModelContentChangedEvent } from 'vs/editor/common/model/textModelEvents'; export interface IClientSessionOptions { notebookUri: URI; @@ -338,6 +339,11 @@ export interface INotebookModel { */ readonly onProviderIdChange: Event; + /** + * Event fired on active cell change + */ + readonly onActiveCellChanged: Event; + /** * The trusted mode of the Notebook */ @@ -377,7 +383,7 @@ export interface INotebookModel { /** * Serialize notebook cell content to JSON */ - toJSON(): nb.INotebookContents; + toJSON(type?: NotebookChangeType): nb.INotebookContents; /** * Notifies the notebook of a change in the cell @@ -403,9 +409,15 @@ export interface INotebookModel { /** Event fired once we get call back from ConfigureConnection method in sqlops extension */ readonly onValidConnectionSelected: Event; - serializationStateChanged(changeType: NotebookChangeType): void; + serializationStateChanged(changeType: NotebookChangeType, cell?: ICellModel): void; standardKernels: IStandardKernelWithProvider[]; + + /** + * Updates the model's view of an active cell to the new active cell + * @param cell New active cell + */ + updateActiveCell(cell: ICellModel); } export interface NotebookContentChange { @@ -426,6 +438,11 @@ export interface NotebookContentChange { * Optional value indicating if the notebook is in a dirty or clean state after this change */ isDirty?: boolean; + + /** + * Text content changed event for cell edits + */ + modelContentChangedEvent?: IModelContentChangedEvent; } export interface ICellModelOptions { @@ -449,6 +466,7 @@ export interface ICellModel { cellUri: URI; id: string; readonly language: string; + readonly cellGuid: string; source: string | string[]; cellType: CellType; trustedMode: boolean; @@ -470,6 +488,7 @@ export interface ICellModel { loaded: boolean; stdInVisible: boolean; readonly onLoaded: Event; + modelContentChangedEvent: IModelContentChangedEvent; } export interface FutureInternal extends nb.IFuture { @@ -533,3 +552,10 @@ export namespace notebookConstants { display_name: sqlKernel }); } + +export interface INotebookContentsEditable { + cells: nb.ICellContents[]; + metadata: nb.INotebookMetadata; + nbformat: number; + nbformat_minor: number; +} diff --git a/src/sql/workbench/parts/notebook/common/models/notebookModel.ts b/src/sql/workbench/parts/notebook/common/models/notebookModel.ts index df331e792925..a24ee081b52e 100644 --- a/src/sql/workbench/parts/notebook/common/models/notebookModel.ts +++ b/src/sql/workbench/parts/notebook/common/models/notebookModel.ts @@ -9,7 +9,7 @@ import { localize } from 'vs/nls'; import { Event, Emitter } from 'vs/base/common/event'; import { Disposable, DisposableStore } from 'vs/base/common/lifecycle'; -import { IClientSession, INotebookModel, IDefaultConnection, INotebookModelOptions, ICellModel, NotebookContentChange, notebookConstants } from 'sql/workbench/parts/notebook/common/models/modelInterfaces'; +import { IClientSession, INotebookModel, IDefaultConnection, INotebookModelOptions, ICellModel, NotebookContentChange, notebookConstants, INotebookContentsEditable } from 'sql/workbench/parts/notebook/common/models/modelInterfaces'; import { NotebookChangeType, CellType, CellTypes } from 'sql/workbench/parts/notebook/common/models/contracts'; import { nbversion } from 'sql/workbench/parts/notebook/common/models/notebookConstants'; import * as notebookUtils from 'sql/workbench/parts/notebook/common/models/notebookUtils'; @@ -56,6 +56,7 @@ export class NotebookModel extends Disposable implements INotebookModel { private _onProviderIdChanged = new Emitter(); private _activeContexts: IDefaultConnection; private _trustedMode: boolean; + private _onActiveCellChanged = new Emitter(); private _cells: ICellModel[]; private _defaultLanguageInfo: nb.ILanguageInfo; @@ -268,6 +269,10 @@ export class NotebookModel extends Disposable implements INotebookModel { return this._onValidConnectionSelected.event; } + public get onActiveCellChanged(): Event { + return this._onActiveCellChanged.event; + } + public get standardKernels(): notebookUtils.IStandardKernelWithProvider[] { return this._standardKernels; } @@ -359,12 +364,15 @@ export class NotebookModel extends Disposable implements INotebookModel { return cell; } - private updateActiveCell(cell: ICellModel) { + public updateActiveCell(cell: ICellModel) { if (this._activeCell) { this._activeCell.active = false; } this._activeCell = cell; - this._activeCell.active = true; + if (cell) { + this._activeCell.active = true; + } + this._onActiveCellChanged.fire(cell); } private createCell(cellType: CellType): ICellModel { @@ -999,8 +1007,8 @@ export class NotebookModel extends Disposable implements INotebookModel { switch (change) { case NotebookChangeType.CellOutputUpdated: case NotebookChangeType.CellSourceUpdated: - changeInfo.changeType = NotebookChangeType.DirtyStateChanged; changeInfo.isDirty = true; + changeInfo.modelContentChangedEvent = cell.modelContentChangedEvent; break; default: // Do nothing for now @@ -1008,10 +1016,10 @@ export class NotebookModel extends Disposable implements INotebookModel { this._contentChangedEmitter.fire(changeInfo); } - serializationStateChanged(changeType: NotebookChangeType): void { + serializationStateChanged(changeType: NotebookChangeType, cell?: ICellModel): void { let changeInfo: NotebookContentChange = { changeType: changeType, - cells: undefined + cells: [cell] }; this._contentChangedEmitter.fire(changeInfo); diff --git a/src/sql/workbench/parts/notebook/common/models/notebookTextFileModel.ts b/src/sql/workbench/parts/notebook/common/models/notebookTextFileModel.ts new file mode 100644 index 000000000000..005aead4a2a0 --- /dev/null +++ b/src/sql/workbench/parts/notebook/common/models/notebookTextFileModel.ts @@ -0,0 +1,258 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the Source EULA. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { Range, IRange } from 'vs/editor/common/core/range'; +import { UntitledEditorModel } from 'vs/workbench/common/editor/untitledEditorModel'; +import { TextFileEditorModel } from 'vs/workbench/services/textfile/common/textFileEditorModel'; +import { FindMatch } from 'vs/editor/common/model'; +import { NotebookContentChange, INotebookModel } from 'sql/workbench/parts/notebook/common/models/modelInterfaces'; +import { NotebookChangeType } from 'sql/workbench/parts/notebook/common/models/contracts'; + +export class NotebookTextFileModel { + // save active cell's line/column in editor model for the beginning of the source property + private _sourceBeginRange: Range; + // save active cell's line/column in editor model for the beginning of the output property + private _outputBeginRange: Range; + // save active cell guid + private _activeCellGuid: string; + + constructor() { + } + + public get activeCellGuid(): string { + return this._activeCellGuid; + } + + public set activeCellGuid(guid: string) { + if (this._activeCellGuid !== guid) { + this._sourceBeginRange = undefined; + this._outputBeginRange = undefined; + this._activeCellGuid = guid; + } + } + + public transformAndApplyEditForSourceUpdate(contentChange: NotebookContentChange, textEditorModel: TextFileEditorModel | UntitledEditorModel): boolean { + let cellGuidRange = this.getCellNodeByGuid(textEditorModel, contentChange.cells[0].cellGuid); + + // convert the range to leverage offsets in the json + if (contentChange && contentChange.modelContentChangedEvent && areRangePropertiesPopulated(cellGuidRange)) { + contentChange.modelContentChangedEvent.changes.forEach(change => { + let convertedRange: IRange = { + startLineNumber: change.range.startLineNumber + cellGuidRange.startLineNumber - 1, + endLineNumber: change.range.endLineNumber + cellGuidRange.startLineNumber - 1, + startColumn: change.range.startColumn + cellGuidRange.startColumn, + endColumn: change.range.endColumn + cellGuidRange.startColumn + }; + // Need to subtract one because we're going from 1-based to 0-based + let startSpaces: string = ' '.repeat(cellGuidRange.startColumn - 1); + // The text here transforms a string from 'This is a string\n this is another string' to: + // This is a string + // this is another string + textEditorModel.textEditorModel.applyEdits([{ + range: new Range(convertedRange.startLineNumber, convertedRange.startColumn, convertedRange.endLineNumber, convertedRange.endColumn), + text: change.text.split('\n').join('\\n\",\n'.concat(startSpaces).concat('\"')) + }]); + }); + } else { + return false; + } + return true; + } + + public transformAndApplyEditForOutputUpdate(contentChange: NotebookContentChange, textEditorModel: TextFileEditorModel | UntitledEditorModel): boolean { + if (Array.isArray(contentChange.cells[0].outputs) && contentChange.cells[0].outputs.length > 0) { + let newOutput = JSON.stringify(contentChange.cells[0].outputs[contentChange.cells[0].outputs.length - 1], undefined, ' '); + if (contentChange.cells[0].outputs.length > 1) { + newOutput = ', '.concat(newOutput); + } else { + newOutput = '\n'.concat(newOutput).concat('\n'); + } + let range = this.getEndOfOutputs(textEditorModel, contentChange.cells[0].cellGuid); + if (range) { + textEditorModel.textEditorModel.applyEdits([{ + range: new Range(range.startLineNumber, range.startColumn, range.startLineNumber, range.startColumn), + text: newOutput + }]); + } + } else { + return false; + } + return true; + } + + public transformAndApplyEditForCellUpdated(contentChange: NotebookContentChange, textEditorModel: TextFileEditorModel | UntitledEditorModel): boolean { + let executionCountMatch = this.getExecutionCountRange(textEditorModel, contentChange.cells[0].cellGuid); + if (executionCountMatch && executionCountMatch.range) { + // Execution count can be between 0 and n characters long + let beginExecutionCountColumn = executionCountMatch.range.endColumn; + let endExecutionCountColumn = beginExecutionCountColumn + 1; + let lineContent = textEditorModel.textEditorModel.getLineContent(executionCountMatch.range.endLineNumber); + while (lineContent[endExecutionCountColumn - 1]) { + endExecutionCountColumn++; + } + if (contentChange.cells[0].executionCount) { + textEditorModel.textEditorModel.applyEdits([{ + range: new Range(executionCountMatch.range.startLineNumber, beginExecutionCountColumn, executionCountMatch.range.endLineNumber, endExecutionCountColumn), + text: contentChange.cells[0].executionCount.toString() + }]); + } else { + // This is a special case when cells are canceled; there will be no execution count included + return true; + } + } else { + return false; + } + return true; + } + + public transformAndApplyEditForClearOutput(contentChange: NotebookContentChange, textEditorModel: TextFileEditorModel | UntitledEditorModel): boolean { + if (!textEditorModel || !contentChange || !contentChange.cells || !contentChange.cells[0] || !contentChange.cells[0].cellGuid) { + return false; + } + if (!this.getOutputNodeByGuid(textEditorModel, contentChange.cells[0].cellGuid)) { + this.updateOutputBeginRange(textEditorModel, contentChange.cells[0].cellGuid); + } + let outputEndRange = this.getEndOfOutputs(textEditorModel, contentChange.cells[0].cellGuid); + let outputStartRange = this.getOutputNodeByGuid(textEditorModel, contentChange.cells[0].cellGuid); + if (outputStartRange && outputEndRange) { + textEditorModel.textEditorModel.applyEdits([{ + range: new Range(outputStartRange.startLineNumber, outputStartRange.endColumn, outputEndRange.endLineNumber, outputEndRange.endColumn), + text: '' + }]); + return true; + } + return false; + } + + public replaceEntireTextEditorModel(notebookModel: INotebookModel, type: NotebookChangeType, textEditorModel: TextFileEditorModel | UntitledEditorModel) { + let content = JSON.stringify(notebookModel.toJSON(type), undefined, ' '); + let model = textEditorModel.textEditorModel; + let endLine = model.getLineCount(); + let endCol = model.getLineMaxColumn(endLine); + textEditorModel.textEditorModel.applyEdits([{ + range: new Range(1, 1, endLine, endCol), + text: content + }]); + } + + // Find the beginning of a cell's source in the text editor model + private updateSourceBeginRange(textEditorModel: TextFileEditorModel | UntitledEditorModel, cellGuid: string): void { + if (!cellGuid) { + return; + } + this._sourceBeginRange = undefined; + + let cellGuidMatches = findOrSetCellGuidMatch(textEditorModel, cellGuid); + if (cellGuidMatches && cellGuidMatches.length > 0) { + let sourceBefore = textEditorModel.textEditorModel.findPreviousMatch('"source": [', { lineNumber: cellGuidMatches[0].range.startLineNumber, column: cellGuidMatches[0].range.startColumn }, false, true, undefined, true); + if (!sourceBefore || !sourceBefore.range) { + return; + } + let firstQuoteOfSource = textEditorModel.textEditorModel.findNextMatch('"', { lineNumber: sourceBefore.range.startLineNumber, column: sourceBefore.range.endColumn }, false, true, undefined, true); + this._sourceBeginRange = firstQuoteOfSource.range; + } else { + return; + } + } + + // Find the beginning of a cell's outputs in the text editor model + private updateOutputBeginRange(textEditorModel: TextFileEditorModel | UntitledEditorModel, cellGuid: string): void { + if (!cellGuid) { + return undefined; + } + this._outputBeginRange = undefined; + + let cellGuidMatches = findOrSetCellGuidMatch(textEditorModel, cellGuid); + if (cellGuidMatches && cellGuidMatches.length > 0) { + let outputsBegin = textEditorModel.textEditorModel.findNextMatch('"outputs": [', { lineNumber: cellGuidMatches[0].range.endLineNumber, column: cellGuidMatches[0].range.endColumn }, false, true, undefined, true); + if (!outputsBegin || !outputsBegin.range) { + return undefined; + } + this._outputBeginRange = outputsBegin.range; + } else { + return undefined; + } + } + + // Find the end of a cell's outputs in the text editor model + // This will be used as a starting point for any future outputs + private getEndOfOutputs(textEditorModel: TextFileEditorModel | UntitledEditorModel, cellGuid: string) { + let outputsBegin; + if (this._activeCellGuid === cellGuid) { + outputsBegin = this._outputBeginRange; + } + if (!outputsBegin || !textEditorModel.textEditorModel.getLineContent(outputsBegin.startLineNumber).trim().includes('output')) { + this.updateOutputBeginRange(textEditorModel, cellGuid); + outputsBegin = this._outputBeginRange; + if (!outputsBegin) { + return undefined; + } + } + let outputsEnd = textEditorModel.textEditorModel.matchBracket({ column: outputsBegin.endColumn - 1, lineNumber: outputsBegin.endLineNumber }); + if (!outputsEnd || outputsEnd.length < 2) { + return undefined; + } + // single line output [i.e. no outputs exist for a cell] + if (outputsBegin.endLineNumber === outputsEnd[1].startLineNumber) { + // Adding 1 to startColumn to replace text starting one character after '[' + return { + startColumn: outputsEnd[0].startColumn + 1, + startLineNumber: outputsEnd[0].startLineNumber, + endColumn: outputsEnd[0].endColumn, + endLineNumber: outputsEnd[0].endLineNumber + }; + } else { + // Last 2 lines in multi-line output will look like the following: + // " }" + // " ]," + if (textEditorModel.textEditorModel.getLineContent(outputsEnd[1].endLineNumber - 1).trim() === '}') { + return { + startColumn: textEditorModel.textEditorModel.getLineFirstNonWhitespaceColumn(outputsEnd[1].endLineNumber - 1) + 1, + startLineNumber: outputsEnd[1].endLineNumber - 1, + endColumn: outputsEnd[1].endColumn - 1, + endLineNumber: outputsEnd[1].endLineNumber + }; + } + return undefined; + } + } + + // Determine what text needs to be replaced when execution counts are updated + private getExecutionCountRange(textEditorModel: TextFileEditorModel | UntitledEditorModel, cellGuid: string) { + let endOutputRange = this.getEndOfOutputs(textEditorModel, cellGuid); + if (endOutputRange && endOutputRange.endLineNumber) { + return textEditorModel.textEditorModel.findNextMatch('"execution_count": ', { lineNumber: endOutputRange.endLineNumber, column: endOutputRange.endColumn }, false, true, undefined, true); + } + return undefined; + } + + // Find a cell's location, given its cellGuid + // If it doesn't exist (e.g. it's not the active cell), attempt to find it + private getCellNodeByGuid(textEditorModel: TextFileEditorModel | UntitledEditorModel, guid: string) { + if (this._activeCellGuid !== guid || !this._sourceBeginRange) { + this.updateSourceBeginRange(textEditorModel, guid); + } + return this._sourceBeginRange; + } + + private getOutputNodeByGuid(textEditorModel: TextFileEditorModel | UntitledEditorModel, guid: string) { + if (this._activeCellGuid !== guid) { + this.updateOutputBeginRange(textEditorModel, guid); + } + return this._outputBeginRange; + } + +} + +function areRangePropertiesPopulated(range: Range) { + return range && range.startLineNumber && range.startColumn && range.endLineNumber && range.endColumn; +} + +function findOrSetCellGuidMatch(textEditorModel: TextFileEditorModel | UntitledEditorModel, cellGuid: string): FindMatch[] { + if (!textEditorModel || !cellGuid) { + return undefined; + } + return textEditorModel.textEditorModel.findMatches(cellGuid, false, false, true, undefined, true); +} diff --git a/src/sql/workbench/parts/notebook/electron-browser/cellViews/textCell.component.ts b/src/sql/workbench/parts/notebook/electron-browser/cellViews/textCell.component.ts index 0ed0a4b11be5..d4d8b655d7a6 100644 --- a/src/sql/workbench/parts/notebook/electron-browser/cellViews/textCell.component.ts +++ b/src/sql/workbench/parts/notebook/electron-browser/cellViews/textCell.component.ts @@ -65,7 +65,7 @@ export class TextCellComponent extends CellView implements OnInit, OnChanges { this.toggleEditMode(false); } this.cellModel.active = false; - this._model.activeCell = undefined; + this._model.updateActiveCell(undefined); } private _content: string | string[]; diff --git a/src/sql/workbench/parts/notebook/test/common/notebookEditorModel.test.ts b/src/sql/workbench/parts/notebook/test/common/notebookEditorModel.test.ts new file mode 100644 index 000000000000..1cdd1379890e --- /dev/null +++ b/src/sql/workbench/parts/notebook/test/common/notebookEditorModel.test.ts @@ -0,0 +1,644 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the Source EULA. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as should from 'should'; +import * as TypeMoq from 'typemoq'; + +import { TestCapabilitiesService } from 'sql/platform/capabilities/test/common/testCapabilitiesService'; +import { ConnectionManagementService } from 'sql/platform/connection/common/connectionManagementService'; +import { CellModel } from 'sql/workbench/parts/notebook/common/models/cell'; +import { CellTypes, NotebookChangeType } from 'sql/workbench/parts/notebook/common/models/contracts'; +import { ModelFactory } from 'sql/workbench/parts/notebook/common/models/modelFactory'; +import { INotebookModelOptions, NotebookContentChange } from 'sql/workbench/parts/notebook/common/models/modelInterfaces'; +import { NotebookEditorModel } from 'sql/workbench/parts/notebook/browser/models/notebookInput'; +import { NotebookModel } from 'sql/workbench/parts/notebook/common/models/notebookModel'; +import { NotebookService } from 'sql/workbench/services/notebook/common/notebookServiceImpl'; +import { URI } from 'vs/base/common/uri'; +import { toResource } from 'vs/base/test/common/utils'; +import { IModelService } from 'vs/editor/common/services/modelService'; +import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; +import { MockContextKeyService } from 'vs/platform/keybinding/test/common/mockKeybindingService'; +import { NullLogService } from 'vs/platform/log/common/log'; +import { TestNotificationService } from 'vs/platform/notification/test/common/testNotificationService'; +import { Memento } from 'vs/workbench/common/memento'; +import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; +import { TextFileEditorModel } from 'vs/workbench/services/textfile/common/textFileEditorModel'; +import { TextFileEditorModelManager } from 'vs/workbench/services/textfile/common/textFileEditorModelManager'; +import { ITextFileService } from 'vs/workbench/services/textfile/common/textfiles'; +import { TestEnvironmentService, TestLifecycleService, TestStorageService, TestTextFileService, workbenchInstantiationService } from 'vs/workbench/test/workbenchTestServices'; +import { Range } from 'vs/editor/common/core/range'; +import { nb } from 'azdata'; +import { Emitter } from 'vs/base/common/event'; +import { INotebookEditor, INotebookManager } from 'sql/workbench/services/notebook/common/notebookService'; + + +class ServiceAccessor { + constructor( + @IEditorService public editorService: IEditorService, + @ITextFileService public textFileService: TestTextFileService, + @IModelService public modelService: IModelService + ) { + } +} + +class NotebookManagerStub implements INotebookManager { + providerId: string; + contentManager: nb.ContentManager; + sessionManager: nb.SessionManager; + serverManager: nb.ServerManager; +} + +let defaultUri = URI.file('/some/path.ipynb'); + +// Note: these tests are intentionally written to be extremely brittle and break on any changes to notebook/cell serialization changes. +// If any of these tests fail, it is likely that notebook editor rehydration will fail with cryptic JSON messages. +suite('Notebook Editor Model', function (): void { + let notebookManagers = [new NotebookManagerStub()]; + let notebookModel: NotebookModel; + const instantiationService: IInstantiationService = workbenchInstantiationService(); + let accessor: ServiceAccessor; + let defaultModelOptions: INotebookModelOptions; + const logService = new NullLogService(); + const notificationService = TypeMoq.Mock.ofType(TestNotificationService, TypeMoq.MockBehavior.Loose); + let memento = TypeMoq.Mock.ofType(Memento, TypeMoq.MockBehavior.Loose, ''); + memento.setup(x => x.getMemento(TypeMoq.It.isAny())).returns(() => void 0); + const queryConnectionService = TypeMoq.Mock.ofType(ConnectionManagementService, TypeMoq.MockBehavior.Loose, memento.object, undefined, new TestStorageService()); + queryConnectionService.callBase = true; + const capabilitiesService = TypeMoq.Mock.ofType(TestCapabilitiesService); + let mockModelFactory = TypeMoq.Mock.ofType(ModelFactory); + mockModelFactory.callBase = true; + mockModelFactory.setup(f => f.createCell(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => { + return new CellModel({ + cell_type: CellTypes.Code, + source: '', + outputs: [ + { + output_type: 'display_data', + data: { + 'text/html': [ + '
', + '
' + ] + } + } + ] + }, undefined, undefined); + }); + + let mockNotebookService: TypeMoq.Mock; + mockNotebookService = TypeMoq.Mock.ofType(NotebookService, undefined, new TestLifecycleService(), undefined, undefined, undefined, instantiationService, new MockContextKeyService(), + undefined, undefined, undefined, undefined, undefined, undefined, TestEnvironmentService); + + mockNotebookService.setup(s => s.findNotebookEditor(TypeMoq.It.isAny())).returns(() => { + return { + cells: undefined, + id: '0', + notebookParams: undefined, + modelReady: undefined, + model: notebookModel, + isDirty: undefined, + isActive: undefined, + isVisible: undefined, + runAllCells: undefined, + runCell: undefined, + clearAllOutputs: undefined, + clearOutput: undefined, + executeEdits: undefined, + getSections: undefined, + navigateToSection: undefined + }; + }); + + let mockOnNotebookEditorAddEvent = new Emitter(); + mockNotebookService.setup(s => s.onNotebookEditorAdd).returns(() => mockOnNotebookEditorAddEvent.event); + + setup(() => { + accessor = instantiationService.createInstance(ServiceAccessor); + + defaultModelOptions = { + notebookUri: defaultUri, + factory: new ModelFactory(instantiationService), + notebookManagers, + contentManager: undefined, + notificationService: notificationService.object, + connectionService: queryConnectionService.object, + providerId: 'SQL', + cellMagicMapper: undefined, + defaultKernel: undefined, + layoutChanged: undefined, + capabilitiesService: capabilitiesService.object + }; + }); + + teardown(() => { + if (accessor && accessor.textFileService && accessor.textFileService.models) { + (accessor.textFileService.models).clear(); + } + }); + + test('should replace entire text model if NotebookChangeType is undefined', async function (): Promise { + await createNewNotebookModel(); + let notebookEditorModel = await createTextEditorModel(this); + notebookEditorModel.replaceEntireTextEditorModel(notebookModel, undefined); + + should(notebookEditorModel.editorModel.textEditorModel.getLineCount()).equal(6); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(5)).equal(' "cells": []'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(2)).equal(' "metadata": {},'); + }); + + test('should replace entire text model for add cell (0 -> 1 cells)', async function (): Promise { + await createNewNotebookModel(); + let notebookEditorModel = await createTextEditorModel(this); + + notebookEditorModel.replaceEntireTextEditorModel(notebookModel, undefined); + + let newCell = notebookModel.addCell(CellTypes.Code); + + let contentChange: NotebookContentChange = { + changeType: NotebookChangeType.CellsModified, + cells: [newCell], + cellIndex: 0 + }; + notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); + should(notebookEditorModel.lastEditFullReplacement).equal(true); + + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(8)).equal(' "source": ['); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(12)).equal(' "azdata_cell_guid": "' + newCell.cellGuid + '"'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(14)).equal(' "outputs": ['); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(25)).equal(' "execution_count": 0'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(26)).equal(' }'); + + should(notebookEditorModel.lastEditFullReplacement).equal(true); + }); + + test('should not replace entire text model for execution count change', async function (): Promise { + await createNewNotebookModel(); + let notebookEditorModel = await createTextEditorModel(this); + notebookEditorModel.replaceEntireTextEditorModel(notebookModel, undefined); + + let newCell = notebookModel.addCell(CellTypes.Code); + + let contentChange: NotebookContentChange = { + changeType: NotebookChangeType.CellsModified, + cells: [newCell], + cellIndex: 0 + }; + notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); + should(notebookEditorModel.lastEditFullReplacement).equal(true); + + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(25)).equal(' "execution_count": 0'); + + newCell.executionCount = 1; + contentChange = { + changeType: NotebookChangeType.CellExecuted, + cells: [newCell], + cellIndex: 0 + }; + + notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellExecuted); + + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(8)).equal(' "source": ['); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(12)).equal(' "azdata_cell_guid": "' + newCell.cellGuid + '"'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(14)).equal(' "outputs": ['); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(25)).equal(' "execution_count": 1'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(26)).equal(' }'); + + should(notebookEditorModel.lastEditFullReplacement).equal(false); + + newCell.executionCount = 10; + contentChange = { + changeType: NotebookChangeType.CellExecuted, + cells: [newCell], + cellIndex: 0 + }; + + notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellExecuted); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(25)).equal(' "execution_count": 10'); + should(notebookEditorModel.lastEditFullReplacement).equal(false); + + newCell.executionCount = 15; + contentChange = { + changeType: NotebookChangeType.CellExecuted, + cells: [newCell], + cellIndex: 0 + }; + + notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellExecuted); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(25)).equal(' "execution_count": 15'); + should(notebookEditorModel.lastEditFullReplacement).equal(false); + + newCell.executionCount = 105; + contentChange = { + changeType: NotebookChangeType.CellExecuted, + cells: [newCell], + cellIndex: 0 + }; + + notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellExecuted); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(25)).equal(' "execution_count": 105'); + should(notebookEditorModel.lastEditFullReplacement).equal(false); + }); + + test('should not replace entire text model for clear output', async function (): Promise { + await createNewNotebookModel(); + let notebookEditorModel = await createTextEditorModel(this); + notebookEditorModel.replaceEntireTextEditorModel(notebookModel, undefined); + + let newCell = notebookModel.addCell(CellTypes.Code); + + let contentChange: NotebookContentChange = { + changeType: NotebookChangeType.CellsModified, + cells: [newCell], + cellIndex: 0 + }; + notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); + should(notebookEditorModel.lastEditFullReplacement).equal(true); + + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(14)).equal(' "outputs": ['); + + contentChange = { + changeType: NotebookChangeType.CellOutputCleared, + cells: [newCell], + cellIndex: 0 + }; + + notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellOutputCleared); + + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(8)).equal(' "source": ['); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(12)).equal(' "azdata_cell_guid": "' + newCell.cellGuid + '"'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(14)).equal(' "outputs": [],'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(15)).equal(' "execution_count": 0'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(16)).equal(' }'); + + should(notebookEditorModel.lastEditFullReplacement).equal(false); + }); + + test('should not replace entire text model for multiline source change', async function (): Promise { + await createNewNotebookModel(); + let notebookEditorModel = await createTextEditorModel(this); + notebookEditorModel.replaceEntireTextEditorModel(notebookModel, undefined); + + let newCell = notebookModel.addCell(CellTypes.Code); + + let contentChange: NotebookContentChange = { + changeType: NotebookChangeType.CellsModified, + cells: [newCell], + cellIndex: 0 + }; + notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); + should(notebookEditorModel.lastEditFullReplacement).equal(true); + + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(14)).equal(' "outputs": ['); + + contentChange = { + changeType: NotebookChangeType.CellSourceUpdated, + cells: [newCell], + cellIndex: 0, + modelContentChangedEvent: { + changes: [{ range: new Range(1, 1, 1, 1), rangeLength: 0, rangeOffset: 0, text: 'This is a test\nLine 2 test\nLine 3 test' }], + eol: '\n', + isFlush: false, + isRedoing: false, + isUndoing: false, + versionId: 2 + } + }; + + notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellSourceUpdated); + + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(8)).equal(' "source": ['); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(9)).equal(' "This is a test\\n",'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(10)).equal(' "Line 2 test\\n",'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(11)).equal(' "Line 3 test"'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(12)).equal(' ],'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(14)).equal(' "azdata_cell_guid": "' + newCell.cellGuid + '"'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(16)).equal(' "outputs": ['); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(27)).equal(' "execution_count": 0'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(28)).equal(' }'); + + should(notebookEditorModel.lastEditFullReplacement).equal(false); + }); + + test('should not replace entire text model for single line source change', async function (): Promise { + await createNewNotebookModel(); + let notebookEditorModel = await createTextEditorModel(this); + notebookEditorModel.replaceEntireTextEditorModel(notebookModel, undefined); + + let newCell = notebookModel.addCell(CellTypes.Code); + + let contentChange: NotebookContentChange = { + changeType: NotebookChangeType.CellsModified, + cells: [newCell], + cellIndex: 0 + }; + notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); + should(notebookEditorModel.lastEditFullReplacement).equal(true); + + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(14)).equal(' "outputs": ['); + + contentChange = { + changeType: NotebookChangeType.CellSourceUpdated, + cells: [newCell], + cellIndex: 0, + modelContentChangedEvent: { + changes: [{ range: new Range(1, 1, 1, 1), rangeLength: 0, rangeOffset: 0, text: 'This is a test' }], + eol: '\n', + isFlush: false, + isRedoing: false, + isUndoing: false, + versionId: 2 + } + }; + + notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellSourceUpdated); + + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(8)).equal(' "source": ['); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(9)).equal(' "This is a test"'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(10)).equal(' ],'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(12)).equal(' "azdata_cell_guid": "' + newCell.cellGuid + '"'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(14)).equal(' "outputs": ['); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(25)).equal(' "execution_count": 0'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(26)).equal(' }'); + + should(notebookEditorModel.lastEditFullReplacement).equal(false); + }); + + test('should not replace entire text model for single line source change then delete', async function (): Promise { + await createNewNotebookModel(); + let notebookEditorModel = await createTextEditorModel(this); + notebookEditorModel.replaceEntireTextEditorModel(notebookModel, undefined); + + let newCell = notebookModel.addCell(CellTypes.Code); + + let contentChange: NotebookContentChange = { + changeType: NotebookChangeType.CellsModified, + cells: [newCell], + cellIndex: 0 + }; + notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); + should(notebookEditorModel.lastEditFullReplacement).equal(true); + + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(8)).equal(' "source": ['); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(9)).equal(' ""'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(10)).equal(' ],'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(14)).equal(' "outputs": ['); + + contentChange = { + changeType: NotebookChangeType.CellSourceUpdated, + cells: [newCell], + cellIndex: 0, + modelContentChangedEvent: { + changes: [{ range: new Range(1, 1, 1, 1), rangeLength: 0, rangeOffset: 0, text: 'This is a test' }], + eol: '\n', + isFlush: false, + isRedoing: false, + isUndoing: false, + versionId: 2 + } + }; + + notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellSourceUpdated); + should(notebookEditorModel.lastEditFullReplacement).equal(false); + + contentChange = { + changeType: NotebookChangeType.CellSourceUpdated, + cells: [newCell], + cellIndex: 0, + modelContentChangedEvent: { + changes: [{ range: new Range(1, 1, 1, 15), rangeLength: 14, rangeOffset: 0, text: '' }], + eol: '\n', + isFlush: false, + isRedoing: false, + isUndoing: false, + versionId: 3 + } + }; + + notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellSourceUpdated); + + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(8)).equal(' "source": ['); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(9)).equal(' ""'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(10)).equal(' ],'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(12)).equal(' "azdata_cell_guid": "' + newCell.cellGuid + '"'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(14)).equal(' "outputs": ['); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(25)).equal(' "execution_count": 0'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(26)).equal(' }'); + + should(notebookEditorModel.lastEditFullReplacement).equal(false); + }); + + test('should not replace entire text model for multiline source delete', async function (): Promise { + await createNewNotebookModel(); + let notebookEditorModel = await createTextEditorModel(this); + notebookEditorModel.replaceEntireTextEditorModel(notebookModel, undefined); + + let newCell = notebookModel.addCell(CellTypes.Code); + + let contentChange: NotebookContentChange = { + changeType: NotebookChangeType.CellsModified, + cells: [newCell], + cellIndex: 0 + }; + notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); + + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(14)).equal(' "outputs": ['); + + contentChange = { + changeType: NotebookChangeType.CellSourceUpdated, + cells: [newCell], + cellIndex: 0, + modelContentChangedEvent: { + changes: [{ range: new Range(1, 1, 1, 1), rangeLength: 0, rangeOffset: 0, text: 'This is a test\nLine 2 test\nLine 3 test' }], + eol: '\n', + isFlush: false, + isRedoing: false, + isUndoing: false, + versionId: 2 + } + }; + + notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellSourceUpdated); + should(notebookEditorModel.lastEditFullReplacement).equal(false); + + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(8)).equal(' "source": ['); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(9)).equal(' "This is a test\\n",'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(10)).equal(' "Line 2 test\\n",'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(11)).equal(' "Line 3 test"'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(12)).equal(' ],'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(14)).equal(' "azdata_cell_guid": "' + newCell.cellGuid + '"'); + + contentChange = { + changeType: NotebookChangeType.CellSourceUpdated, + cells: [newCell], + cellIndex: 0, + modelContentChangedEvent: { + changes: [{ range: new Range(1, 2, 3, 11), rangeLength: 36, rangeOffset: 1, text: '' }], + eol: '\n', + isFlush: false, + isRedoing: false, + isUndoing: false, + versionId: 3 + } + }; + + notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellSourceUpdated); + should(notebookEditorModel.lastEditFullReplacement).equal(false); + + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(8)).equal(' "source": ['); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(9)).equal(' "Tt"'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(10)).equal(' ],'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(12)).equal(' "azdata_cell_guid": "' + newCell.cellGuid + '"'); + }); + + test('should not replace entire text model and affect only edited cell', async function (): Promise { + await createNewNotebookModel(); + let notebookEditorModel = await createTextEditorModel(this); + notebookEditorModel.replaceEntireTextEditorModel(notebookModel, undefined); + + let newCell; + let contentChange: NotebookContentChange; + for (let i = 0; i < 10; i++) { + let cell; + if (i === 7) { + newCell = notebookModel.addCell(CellTypes.Code); + cell = newCell; + } else { + cell = notebookModel.addCell(CellTypes.Code); + } + + contentChange = { + changeType: NotebookChangeType.CellsModified, + cells: [cell], + cellIndex: 0 + }; + notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); + should(notebookEditorModel.lastEditFullReplacement).equal(true); + } + + contentChange = { + changeType: NotebookChangeType.CellSourceUpdated, + cells: [newCell], + modelContentChangedEvent: { + changes: [{ range: new Range(1, 1, 1, 1), rangeLength: 0, rangeOffset: 0, text: 'This is a test' }], + eol: '\n', + isFlush: false, + isRedoing: false, + isUndoing: false, + versionId: 2 + } + }; + + notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellSourceUpdated); + should(notebookEditorModel.lastEditFullReplacement).equal(false); + + for (let i = 0; i < 10; i++) { + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(8 + i * 21)).equal(' "source": ['); + if (i === 7) { + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(9 + i * 21)).equal(' "This is a test"'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(12 + i * 21)).equal(' "azdata_cell_guid": "' + newCell.cellGuid + '"'); + } else { + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(9 + i * 21)).equal(' ""'); + } + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(10 + i * 21)).equal(' ],'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(14 + i * 21)).equal(' "outputs": ['); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(25 + i * 21)).equal(' "execution_count": 0'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(26 + i * 21)).startWith(' }'); + } + }); + + test('should not replace entire text model for output changes', async function (): Promise { + await createNewNotebookModel(); + let notebookEditorModel = await createTextEditorModel(this); + notebookEditorModel.replaceEntireTextEditorModel(notebookModel, undefined); + + let newCell = notebookModel.addCell(CellTypes.Code); + + let contentChange: NotebookContentChange = { + changeType: NotebookChangeType.CellsModified, + cells: [newCell], + cellIndex: 0 + }; + notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); + should(notebookEditorModel.lastEditFullReplacement).equal(true); + + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(14)).equal(' "outputs": ['); + + newCell['_outputs'] = newCell.outputs.concat(newCell.outputs); + + contentChange = { + changeType: NotebookChangeType.CellOutputUpdated, + cells: [newCell] + }; + + notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellOutputUpdated); + + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(8)).equal(' "source": ['); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(12)).equal(' "azdata_cell_guid": "' + newCell.cellGuid + '"'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(14)).equal(' "outputs": ['); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(23)).equal(' }, {'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(31)).equal('}'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(32)).equal(' ],'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(33)).equal(' "execution_count": 0'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(34)).equal(' }'); + + should(notebookEditorModel.lastEditFullReplacement).equal(false); + }); + + test('should not replace entire text model for output changes (1st update)', async function (): Promise { + await createNewNotebookModel(); + let notebookEditorModel = await createTextEditorModel(this); + notebookEditorModel.replaceEntireTextEditorModel(notebookModel, undefined); + + let newCell = notebookModel.addCell(CellTypes.Code); + let previousOutputs = newCell.outputs; + // clear outputs + newCell['_outputs'] = []; + + let contentChange: NotebookContentChange = { + changeType: NotebookChangeType.CellsModified, + cells: [newCell], + cellIndex: 0 + }; + + notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellsModified); + should(notebookEditorModel.lastEditFullReplacement).equal(true); + + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(14)).equal(' "outputs": [],'); + + // add output + newCell['_outputs'] = previousOutputs; + + contentChange = { + changeType: NotebookChangeType.CellOutputUpdated, + cells: [newCell] + }; + + notebookEditorModel.updateModel(contentChange, NotebookChangeType.CellOutputUpdated); + + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(8)).equal(' "source": ['); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(12)).equal(' "azdata_cell_guid": "' + newCell.cellGuid + '"'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(14)).equal(' "outputs": ['); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(23)).equal('}'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(25)).equal(' "execution_count": 0'); + should(notebookEditorModel.editorModel.textEditorModel.getLineContent(26)).equal(' }'); + + should(notebookEditorModel.lastEditFullReplacement).equal(false); + }); + + async function createNewNotebookModel() { + let options: INotebookModelOptions = Object.assign({}, defaultModelOptions, >{ + factory: mockModelFactory.object + }); + notebookModel = new NotebookModel(options, undefined, logService, undefined, undefined); + await notebookModel.loadContents(); + } + + async function createTextEditorModel(self: Mocha.ITestCallbackContext): Promise { + let textFileEditorModel = instantiationService.createInstance(TextFileEditorModel, toResource.call(self, defaultUri.toString()), 'utf8', undefined); + (accessor.textFileService.models).add(textFileEditorModel.getResource(), textFileEditorModel); + await textFileEditorModel.load(); + return new NotebookEditorModel(defaultUri, textFileEditorModel, mockNotebookService.object, accessor.textFileService); + } +}); diff --git a/src/sql/workbench/parts/notebook/test/node/cell.test.ts b/src/sql/workbench/parts/notebook/test/node/cell.test.ts index 6d949607fb5e..4437dc5b7dc8 100644 --- a/src/sql/workbench/parts/notebook/test/node/cell.test.ts +++ b/src/sql/workbench/parts/notebook/test/node/cell.test.ts @@ -37,7 +37,7 @@ suite('Cell Model', function (): void { cell.setOverrideLanguage('sql'); should(cell.language).equal('sql'); cell.source = 'abcd'; - should(cell.source).equal('abcd'); + should(JSON.stringify(cell.source)).equal(JSON.stringify(['abcd'])); }); test('Should match ICell values if defined', async function (): Promise { @@ -55,7 +55,7 @@ suite('Cell Model', function (): void { }; let cell = factory.createCell(cellData, undefined); should(cell.cellType).equal(cellData.cell_type); - should(cell.source).equal(cellData.source); + should(JSON.stringify(cell.source)).equal(JSON.stringify([cellData.source])); should(cell.outputs).have.length(1); should(cell.outputs[0].output_type).equal('stream'); should((cell.outputs[0]).text).equal('Some output'); @@ -163,8 +163,8 @@ suite('Cell Model', function (): void { mimetype: '' }); let cell = factory.createCell(cellData, { notebook: notebookModel, isTrusted: false }); - should(Array.isArray(cell.source)).equal(false); - should(cell.source).equal('print(1)'); + should(Array.isArray(cell.source)).equal(true); + should(JSON.stringify(cell.source)).equal(JSON.stringify(['print(1)'])); }); test('Should allow source of type string with newline and split it', async function (): Promise { @@ -241,8 +241,8 @@ suite('Cell Model', function (): void { mimetype: '' }); let cell = factory.createCell(cellData, { notebook: notebookModel, isTrusted: false }); - should(Array.isArray(cell.source)).equal(false); - should(cell.source).equal(''); + should(Array.isArray(cell.source)).equal(true); + should(JSON.stringify(cell.source)).equal(JSON.stringify([''])); }); suite('Model Future handling', function (): void { @@ -422,6 +422,56 @@ suite('Cell Model', function (): void { oldFuture.verify(f => f.dispose(), TypeMoq.Times.once()); }); + + test('should include cellGuid', async () => { + let notebookModel = new NotebookModelStub({ + name: '', + version: '', + mimetype: '' + }); + + let cell = factory.createCell(undefined, { notebook: notebookModel, isTrusted: false }); + should(cell.cellGuid).not.be.undefined(); + should(cell.cellGuid.length).equal(36); + let cellJson = cell.toJSON(); + should(cellJson.metadata.azdata_cell_guid).not.be.undefined(); + }); + + test('should include azdata_cell_guid in metadata', async () => { + let notebookModel = new NotebookModelStub({ + name: '', + version: '', + mimetype: '' + }); + + let cell = factory.createCell(undefined, { notebook: notebookModel, isTrusted: false }); + let cellJson = cell.toJSON(); + should(cellJson.metadata.azdata_cell_guid).not.be.undefined(); + }); + + // This is critical for the notebook editor model to parse changes correctly + // If this test fails, please ensure that the notebookEditorModel tests still pass + test('should stringify in the correct order', async () => { + let notebookModel = new NotebookModelStub({ + name: '', + version: '', + mimetype: '' + }); + + let cell = factory.createCell(undefined, { notebook: notebookModel, isTrusted: false }); + let content = JSON.stringify(cell.toJSON(), undefined, ' '); + let contentSplit = content.split('\n'); + should(contentSplit.length).equal(9); + should(contentSplit[0].trim().startsWith('{')).equal(true); + should(contentSplit[1].trim().startsWith('"cell_type": "code",')).equal(true); + should(contentSplit[2].trim().startsWith('"source": ""')).equal(true); + should(contentSplit[3].trim().startsWith('"metadata": {')).equal(true); + should(contentSplit[4].trim().startsWith('"azdata_cell_guid": "')).equal(true); + should(contentSplit[5].trim().startsWith('}')).equal(true); + should(contentSplit[6].trim().startsWith('"outputs": []')).equal(true); + should(contentSplit[7].trim().startsWith('"execution_count": 0')).equal(true); + should(contentSplit[8].trim().startsWith('}')).equal(true); + }); }); }); diff --git a/src/sql/workbench/parts/notebook/test/node/common.ts b/src/sql/workbench/parts/notebook/test/node/common.ts index f6bbf8e7ea18..8016e39ae1a2 100644 --- a/src/sql/workbench/parts/notebook/test/node/common.ts +++ b/src/sql/workbench/parts/notebook/test/node/common.ts @@ -106,6 +106,13 @@ export class NotebookModelStub implements INotebookModel { serializationStateChanged(changeType: NotebookChangeType): void { throw new Error('Method not implemented.'); } + get onActiveCellChanged(): Event { + throw new Error('Method not implemented.'); + } + updateActiveCell(cell: ICellModel) { + throw new Error('Method not implemented.'); + } + } export class NotebookManagerStub implements INotebookManager { @@ -131,4 +138,4 @@ export class ServerManagerStub implements nb.ServerManager { this.calledEnd = true; return this.result; } -} \ No newline at end of file +} diff --git a/src/sql/workbench/services/notebook/common/notebookService.ts b/src/sql/workbench/services/notebook/common/notebookService.ts index de863215fc58..aae942515196 100644 --- a/src/sql/workbench/services/notebook/common/notebookService.ts +++ b/src/sql/workbench/services/notebook/common/notebookService.ts @@ -99,7 +99,7 @@ export interface INotebookService { * sent to listeners that can act on the point-in-time notebook state * @param notebookUri the URI identifying a notebook */ - serializeNotebookStateChange(notebookUri: URI, changeType: NotebookChangeType): void; + serializeNotebookStateChange(notebookUri: URI, changeType: NotebookChangeType, cell?: ICellModel): void; /** * diff --git a/src/sql/workbench/services/notebook/common/notebookServiceImpl.ts b/src/sql/workbench/services/notebook/common/notebookServiceImpl.ts index 763238038262..4d46a104937a 100644 --- a/src/sql/workbench/services/notebook/common/notebookServiceImpl.ts +++ b/src/sql/workbench/services/notebook/common/notebookServiceImpl.ts @@ -30,7 +30,7 @@ import { NotebookEditor } from 'sql/workbench/parts/notebook/browser/notebookEdi import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { registerNotebookThemes } from 'sql/workbench/parts/notebook/browser/notebookStyles'; import { IQueryManagementService } from 'sql/platform/query/common/queryManagement'; -import { notebookConstants } from 'sql/workbench/parts/notebook/common/models/modelInterfaces'; +import { notebookConstants, ICellModel } from 'sql/workbench/parts/notebook/common/models/modelInterfaces'; import { ILifecycleService } from 'vs/platform/lifecycle/common/lifecycle'; import { SqlNotebookProvider } from 'sql/workbench/services/notebook/common/sql/sqlNotebookProvider'; import { IEditorGroupsService } from 'vs/workbench/services/editor/common/editorGroupsService'; @@ -578,7 +578,7 @@ export class NotebookService extends Disposable implements INotebookService { } } - serializeNotebookStateChange(notebookUri: URI, changeType: NotebookChangeType): void { + serializeNotebookStateChange(notebookUri: URI, changeType: NotebookChangeType, cell?: ICellModel): void { if (notebookUri.scheme !== Schemas.untitled) { // Conditions for saving: // 1. Not untitled. They're always trusted as we open them @@ -598,7 +598,7 @@ export class NotebookService extends Disposable implements INotebookService { let editor = this.findNotebookEditor(notebookUri); if (editor && editor.model) { - editor.model.serializationStateChanged(changeType); + editor.model.serializationStateChanged(changeType, cell); // TODO add history notification if a non-untitled notebook has a state change } }