Backend work for Notebook Parameterization and Tests (#12914)

* Backend work for  Parameterization + Tests

* minor comments

* fix test

* address comments

src/sql/platform/notebooks/common/outputRegistry.ts

@@ -10,6 +10,8 @@ export const Extensions = {
 };
 
 export const HideInputTag = 'hide_input';
+export const ParametersTag = 'parameters';
+export const InjectedParametersTag = 'injected-parameters';
 
 export interface ICellComponentRegistry {
 	registerComponent(component: any): void;

src/sql/workbench/contrib/notebook/browser/cellToolbarActions.ts

@@ -145,6 +145,9 @@ export class CellToggleMoreActions {
 			instantiationService.createInstance(CollapseCellAction, 'collapseCell', localize('collapseCell', "Collapse Cell"), true),
 			instantiationService.createInstance(CollapseCellAction, 'expandCell', localize('expandCell', "Expand Cell"), false),
 			new Separator(),
+			instantiationService.createInstance(ParametersCellAction, 'makeParameterCell', localize('makeParameterCell', "Make parameter cell"), true),
+			instantiationService.createInstance(ParametersCellAction, 'removeParameterCell', localize('RemoveParameterCell', "Remove parameter cell"), false),
+			new Separator(),
 			instantiationService.createInstance(ClearCellOutputAction, 'clear', localize('clear', "Clear Result")),
 		);
 	}
@@ -364,3 +367,40 @@ export class ToggleMoreActions extends Action {
 		return Promise.resolve(true);
 	}
 }
+
+export class ParametersCellAction extends CellActionBase {
+	constructor(id: string,
+		label: string,
+		private parametersCell: boolean,
+		@INotificationService notificationService: INotificationService
+	) {
+		super(id, label, undefined, notificationService);
+	}
+
+	public canRun(context: CellContext): boolean {
+		return context.cell?.cellType === CellTypes.Code;
+	}
+
+	async doRun(context: CellContext): Promise<void> {
+		try {
+			let cell = context.cell || context.model.activeCell;
+			if (cell) {
+				if (this.parametersCell) {
+					if (!cell.isParameter) {
+						cell.isParameter = true;
+					}
+				} else {
+					if (cell.isParameter) {
+						cell.isParameter = false;
+					}
+				}
+			}
+		} catch (error) {
+			let message = getErrorMessage(error);
+			this.notificationService.notify({
+				severity: Severity.Error,
+				message: message
+			});
+		}
+	}
+}

src/sql/workbench/contrib/notebook/test/browser/cellToolbarActions.test.ts

@@ -125,7 +125,7 @@ suite('CellToolbarActions', function (): void {
 			cellModelMock.setup(x => x.cellType).returns(() => 'code');
 			const action = new CellToggleMoreActions(instantiationService);
 			action.onInit(testContainer, contextMock.object);
-			assert.equal(action['_moreActions']['viewItems'][0]['_action']['_actions'].length, 15, 'Unexpected number of valid elements');
+			assert.equal(action['_moreActions']['viewItems'][0]['_action']['_actions'].length, 18, 'Unexpected number of valid elements');
 		});
 
 		test('CellToggleMoreActions with Markdown CellType', function (): void {

src/sql/workbench/contrib/notebook/test/electron-browser/cell.test.ts

@@ -340,7 +340,7 @@ suite('Cell Model', function (): void {
 		assert(!isCollapsed);
 	});
 
-	test('Should not allow markdown cells to be collapsible', async function (): Promise<void> {
+	test('Should not allow markdown cells to be collapsible or parameters', async function (): Promise<void> {
 		let mdCellData: nb.ICellContents = {
 			cell_type: CellTypes.Markdown,
 			source: 'some *markdown*',
@@ -349,11 +349,16 @@ suite('Cell Model', function (): void {
 		};
 		let cell = factory.createCell(mdCellData, undefined);
 		assert(cell.isCollapsed === false);
+		assert(cell.isParameter === false);
+
 		cell.isCollapsed = true;
-		// The typescript compiler will complain if we don't ignore the error from the following line,
+		cell.isParameter = true;
-		// claiming that cell.isCollapsed will return true. It doesn't.
+		// The typescript compiler will complain if we don't ignore the error from the following lines,
+		// claiming that cell.isCollapsed and cell.isParameter will return true. It doesn't.
 		// @ts-ignore
 		assert(cell.isCollapsed === false);
+		// @ts-ignore
+		assert(cell.isParameter === false);
 
 		let codeCellData: nb.ICellContents = {
 			cell_type: CellTypes.Code,
@@ -364,8 +369,159 @@ suite('Cell Model', function (): void {
 		};
 		cell = factory.createCell(codeCellData, undefined);
 		assert(cell.isCollapsed === false);
+		assert(cell.isParameter === false);
+
 		cell.isCollapsed = true;
 		assert(cell.isCollapsed === true);
+		cell.isParameter = true;
+		assert(cell.isParameter === true);
+	});
+
+	test('Should parse metadata\'s parameters tag correctly', async function (): Promise<void> {
+		// Setup Notebook Model and Contents
+		let notebookModel = new NotebookModelStub({
+			name: '',
+			version: '',
+			mimetype: ''
+		});
+		let contents: nb.ICellContents = {
+			cell_type: CellTypes.Code,
+			source: ''
+		};
+		let model = factory.createCell(contents, { notebook: notebookModel, isTrusted: false });
+
+		assert(!model.isParameter);
+		model.isParameter = true;
+		assert(model.isParameter);
+		model.isParameter = false;
+		assert(!model.isParameter);
+
+		// Should not have parameters cell
+		let modelJson = model.toJSON();
+		assert(!isUndefinedOrNull(modelJson.metadata.tags));
+		assert(!modelJson.metadata.tags.some(x => x === 'parameter'));
+
+		// Add parameters tag
+		contents.metadata = {
+			tags: ['parameters']
+		};
+		model = factory.createCell(contents, { notebook: notebookModel, isTrusted: false });
+
+		assert(model.isParameter);
+		model.isParameter = false;
+		assert(!model.isParameter);
+		model.isParameter = true;
+		assert(model.isParameter);
+
+		// Should find parameters tag in metadata
+		modelJson = model.toJSON();
+		assert(!isUndefinedOrNull(modelJson.metadata.tags));
+		assert(modelJson.metadata.tags.some(x => x === 'parameters'));
+
+		contents.metadata = {
+			tags: ['not_a_real_tag']
+		};
+		model = factory.createCell(contents, { notebook: notebookModel, isTrusted: false });
+		modelJson = model.toJSON();
+		assert(!isUndefinedOrNull(modelJson.metadata.tags));
+		assert(!modelJson.metadata.tags.some(x => x === 'parameters'));
+
+		contents.metadata = {
+			tags: ['not_a_real_tag', 'parameters']
+		};
+		model = factory.createCell(contents, { notebook: notebookModel, isTrusted: false });
+		modelJson = model.toJSON();
+		assert(!isUndefinedOrNull(modelJson.metadata.tags));
+		assert(modelJson.metadata.tags.some(x => x === 'parameters'));
+	});
+
+	test('Should emit event setting cell as Parameter', async function (): Promise<void> {
+		let notebookModel = new NotebookModelStub({
+			name: '',
+			version: '',
+			mimetype: ''
+		});
+		let contents: nb.ICellContents = {
+			cell_type: CellTypes.Code,
+			source: ''
+		};
+
+		let model = factory.createCell(contents, { notebook: notebookModel, isTrusted: false });
+		assert(!model.isParameter);
+
+		let createParameterPromise = () => {
+			return new Promise((resolve, reject) => {
+				setTimeout(() => reject(), 2000);
+				model.onParameterStateChanged(isParameter => {
+					resolve(isParameter);
+				});
+			});
+		};
+
+		assert(!model.isParameter);
+		let parameterPromise = createParameterPromise();
+		model.isParameter = true;
+		let isParameter = await parameterPromise;
+		assert(isParameter);
+
+		parameterPromise = createParameterPromise();
+		model.isParameter = false;
+		isParameter = await parameterPromise;
+		assert(!isParameter);
+	});
+
+	test('Should parse metadata\'s injected parameter tag correctly', async function (): Promise<void> {
+		let notebookModel = new NotebookModelStub({
+			name: '',
+			version: '',
+			mimetype: ''
+		});
+		let contents: nb.ICellContents = {
+			cell_type: CellTypes.Code,
+			source: ''
+		};
+		let model = factory.createCell(contents, { notebook: notebookModel, isTrusted: false });
+
+		assert(!model.isInjectedParameter);
+		model.isInjectedParameter = true;
+		assert(model.isInjectedParameter);
+		model.isInjectedParameter = false;
+		assert(!model.isInjectedParameter);
+
+		let modelJson = model.toJSON();
+		assert(!isUndefinedOrNull(modelJson.metadata.tags));
+		assert(!modelJson.metadata.tags.some(x => x === 'injected-parameters'));
+
+		contents.metadata = {
+			tags: ['injected-parameters']
+		};
+		model = factory.createCell(contents, { notebook: notebookModel, isTrusted: false });
+
+		assert(model.isInjectedParameter);
+		model.isInjectedParameter = false;
+		assert(!model.isInjectedParameter);
+		model.isInjectedParameter = true;
+		assert(model.isInjectedParameter);
+
+		modelJson = model.toJSON();
+		assert(!isUndefinedOrNull(modelJson.metadata.tags));
+		assert(modelJson.metadata.tags.some(x => x === 'injected-parameters'));
+
+		contents.metadata = {
+			tags: ['not_a_real_tag']
+		};
+		model = factory.createCell(contents, { notebook: notebookModel, isTrusted: false });
+		modelJson = model.toJSON();
+		assert(!isUndefinedOrNull(modelJson.metadata.tags));
+		assert(!modelJson.metadata.tags.some(x => x === 'injected-parameters'));
+
+		contents.metadata = {
+			tags: ['not_a_real_tag', 'injected-parameters']
+		};
+		model = factory.createCell(contents, { notebook: notebookModel, isTrusted: false });
+		modelJson = model.toJSON();
+		assert(!isUndefinedOrNull(modelJson.metadata.tags));
+		assert(modelJson.metadata.tags.some(x => x === 'injected-parameters'));
 	});
 
 	suite('Model Future handling', function (): void {

src/sql/workbench/services/notebook/browser/models/cell.ts

@@ -22,7 +22,7 @@ import { optional } from 'vs/platform/instantiation/common/instantiation';
 import { getErrorMessage, onUnexpectedError } from 'vs/base/common/errors';
 import { generateUuid } from 'vs/base/common/uuid';
 import { IModelContentChangedEvent } from 'vs/editor/common/model/textModelEvents';
-import { HideInputTag } from 'sql/platform/notebooks/common/outputRegistry';
+import { HideInputTag, ParametersTag, InjectedParametersTag } from 'sql/platform/notebooks/common/outputRegistry';
 import { FutureInternal, notebookConstants } from 'sql/workbench/services/notebook/browser/interfaces';
 import { ICommandService } from 'vs/platform/commands/common/commands';
 import { tryMatchCellMagic, extractCellMagicCommandPlusArgs } from 'sql/workbench/services/notebook/browser/utils';
@@ -68,6 +68,9 @@ export class CellModel extends Disposable implements ICellModel {
 	private _cellSourceChanged: boolean = false;
 	private _gridDataConversionComplete: Promise<void>[] = [];
 	private _defaultToWYSIWYG: boolean;
+	private _isParameter: boolean;
+	private _onParameterStateChanged = new Emitter<boolean>();
+	private _isInjectedParameter: boolean;
 
 	constructor(cellData: nb.ICellContents,
 		private _options: ICellModelOptions,
@@ -340,6 +343,78 @@ export class CellModel extends Disposable implements ICellModel {
 		return this._onCellMarkdownChanged.event;
 	}
 
+	public get onParameterStateChanged(): Event<boolean> {
+		return this._onParameterStateChanged.event;
+	}
+
+	public get isParameter() {
+		return this._isParameter;
+	}
+
+	public set isParameter(value: boolean) {
+		if (this.cellType !== CellTypes.Code) {
+			return;
+		}
+		let stateChanged = this._isParameter !== value;
+		this._isParameter = value;
+
+		let tagIndex = -1;
+		if (this._metadata.tags) {
+			tagIndex = this._metadata.tags.findIndex(tag => tag === ParametersTag);
+		}
+
+		if (this._isParameter) {
+			if (tagIndex === -1) {
+				if (!this._metadata.tags) {
+					this._metadata.tags = [];
+				}
+				this._metadata.tags.push(ParametersTag);
+			}
+		} else {
+			if (tagIndex > -1) {
+				this._metadata.tags.splice(tagIndex, 1);
+			}
+		}
+
+		if (stateChanged) {
+			this._onParameterStateChanged.fire(this._isParameter);
+			this.sendChangeToNotebook(NotebookChangeType.CellInputVisibilityChanged);
+		}
+	}
+
+	/**
+	Injected Parameters will be used for future scenarios
+	when we need to hide this cell for Parameterization
+	*/
+	public get isInjectedParameter() {
+		return this._isInjectedParameter;
+	}
+
+	public set isInjectedParameter(value: boolean) {
+		if (this.cellType !== CellTypes.Code) {
+			return;
+		}
+		this._isInjectedParameter = value;
+
+		let tagIndex = -1;
+		if (this._metadata.tags) {
+			tagIndex = this._metadata.tags.findIndex(tag => tag === InjectedParametersTag);
+		}
+
+		if (this._isInjectedParameter) {
+			if (tagIndex === -1) {
+				if (!this._metadata.tags) {
+					this._metadata.tags = [];
+				}
+				this._metadata.tags.push(InjectedParametersTag);
+			}
+		} else {
+			if (tagIndex > -1) {
+				this._metadata.tags.splice(tagIndex, 1);
+			}
+		}
+	}
+
 	private notifyExecutionComplete(): void {
 		if (this._notebookService) {
 			this._notebookService.serializeNotebookStateChange(this.notebookModel.notebookUri, NotebookChangeType.CellExecuted, this)
@@ -727,10 +802,14 @@ export class CellModel extends Disposable implements ICellModel {
 		this._source = this.getMultilineSource(cell.source);
 		this._metadata = cell.metadata || {};
 
-		if (this._metadata.tags && this._metadata.tags.some(x => x === HideInputTag) && this._cellType === CellTypes.Code) {
+		if (this._metadata.tags && this._metadata.tags.some(x => x === HideInputTag || x === ParametersTag || x === InjectedParametersTag) && this._cellType === CellTypes.Code) {
 			this._isCollapsed = true;
+			this._isParameter = true;
+			this._isInjectedParameter = true;
 		} else {
 			this._isCollapsed = false;
+			this._isParameter = false;
+			this._isInjectedParameter = false;
 		}
 
 		this._cellGuid = cell.metadata && cell.metadata.azdata_cell_guid ? cell.metadata.azdata_cell_guid : generateUuid();

src/sql/workbench/services/notebook/browser/models/modelInterfaces.ts

@@ -477,7 +477,10 @@ export interface ICellModel {
 	stdInVisible: boolean;
 	readonly onLoaded: Event<string>;
 	isCollapsed: boolean;
+	isParameter: boolean;
+	isInjectedParameter: boolean;
 	readonly onCollapseStateChanged: Event<boolean>;
+	readonly onParameterStateChanged: Event<boolean>;
 	readonly onCellModeChanged: Event<boolean>;
 	modelContentChangedEvent: IModelContentChangedEvent;
 	isEditMode: boolean;