Fixing bug where templates may get mapped before loaded from their files (#10111)

* Fixing potential bad order of template loading and map construction

* Fixing bug where templates get mapped before loaded from file

* Parallelizing loading templates from file

extensions/sql-database-projects/src/common/constants.ts

@@ -38,6 +38,7 @@ export function projectAlreadyExists(name: string, path: string) { return locali
 
 // Project script types
 
+export const folderFriendlyName = localize('folderFriendlyName', "Folder");
 export const scriptFriendlyName = localize('scriptFriendlyName', "Script");
 export const tableFriendlyName = localize('tableFriendlyName', "Table");
 export const viewFriendlyName = localize('viewFriendlyName', "View");

extensions/sql-database-projects/src/controllers/mainController.ts

@@ -4,7 +4,6 @@
  *--------------------------------------------------------------------------------------------*/
 
 import * as vscode from 'vscode';
-import * as templateMap from '../templates/templateMap';
 import * as templates from '../templates/templates';
 import * as constants from '../common/constants';
 import * as path from 'path';
@@ -46,10 +45,10 @@ export default class MainController implements vscode.Disposable {
 		vscode.commands.registerCommand('sqlDatabaseProjects.open', async () => { await this.openProjectFromFile(); });
 		vscode.commands.registerCommand('sqlDatabaseProjects.close', (node: BaseProjectTreeItem) => { this.projectsController.closeProject(node); });
 
-		vscode.commands.registerCommand('sqlDatabaseProjects.newScript', async (node: BaseProjectTreeItem) => { await this.projectsController.addItemPrompt(node, templateMap.script); });
-		vscode.commands.registerCommand('sqlDatabaseProjects.newTable', async (node: BaseProjectTreeItem) => { await this.projectsController.addItemPrompt(node, templateMap.table); });
-		vscode.commands.registerCommand('sqlDatabaseProjects.newView', async (node: BaseProjectTreeItem) => { await this.projectsController.addItemPrompt(node, templateMap.view); });
-		vscode.commands.registerCommand('sqlDatabaseProjects.newStoredProcedure', async (node: BaseProjectTreeItem) => { await this.projectsController.addItemPrompt(node, templateMap.storedProcedure); });
+		vscode.commands.registerCommand('sqlDatabaseProjects.newScript', async (node: BaseProjectTreeItem) => { await this.projectsController.addItemPrompt(node, templates.script); });
+		vscode.commands.registerCommand('sqlDatabaseProjects.newTable', async (node: BaseProjectTreeItem) => { await this.projectsController.addItemPrompt(node, templates.table); });
+		vscode.commands.registerCommand('sqlDatabaseProjects.newView', async (node: BaseProjectTreeItem) => { await this.projectsController.addItemPrompt(node, templates.view); });
+		vscode.commands.registerCommand('sqlDatabaseProjects.newStoredProcedure', async (node: BaseProjectTreeItem) => { await this.projectsController.addItemPrompt(node, templates.storedProcedure); });
 		vscode.commands.registerCommand('sqlDatabaseProjects.newItem', async (node: BaseProjectTreeItem) => { await this.projectsController.addItemPrompt(node); });
 		vscode.commands.registerCommand('sqlDatabaseProjects.newFolder', async (node: BaseProjectTreeItem) => { await this.projectsController.addFolderPrompt(node); });
 

extensions/sql-database-projects/src/controllers/projectController.ts

@@ -7,7 +7,6 @@ import * as vscode from 'vscode';
 import * as path from 'path';
 import * as constants from '../common/constants';
 import * as dataSources from '../models/dataSources/dataSources';
-import * as templateMap from '../templates/templateMap';
 import * as utils from '../common/utils';
 import * as UUID from 'vscode-languageclient/lib/utils/uuid';
 import * as templates from '../templates/templates';
@@ -115,7 +114,7 @@ export class ProjectsController {
 
 	public async addFolderPrompt(treeNode: BaseProjectTreeItem) {
 		const project = this.getProjectContextFromTreeNode(treeNode);
-		const newFolderName = await this.promptForNewObjectName(new templateMap.ProjectScriptType(templateMap.folder, 'Folder', ''), project);
+		const newFolderName = await this.promptForNewObjectName(new templates.ProjectScriptType(templates.folder, constants.folderFriendlyName, ''), project);
 
 		if (!newFolderName) {
 			return; // user cancelled
@@ -134,7 +133,7 @@ export class ProjectsController {
 		if (!itemTypeName) {
 			let itemFriendlyNames: string[] = [];
 
-			for (const itemType of templateMap.projectScriptTypes) {
+			for (const itemType of templates.projectScriptTypes()) {
 				itemFriendlyNames.push(itemType.friendlyName);
 			}
 
@@ -147,7 +146,7 @@ export class ProjectsController {
 			}
 		}
 
-		const itemType = templateMap.projectScriptTypeMap[itemTypeName.toLocaleLowerCase()];
+		const itemType = templates.projectScriptTypeMap()[itemTypeName.toLocaleLowerCase()];
 		const itemObjectName = await this.promptForNewObjectName(itemType, project);
 
 		if (!itemObjectName) {
@@ -198,7 +197,7 @@ export class ProjectsController {
 		}
 	}
 
-	private async promptForNewObjectName(itemType: templateMap.ProjectScriptType, _project: Project): Promise<string | undefined> {
+	private async promptForNewObjectName(itemType: templates.ProjectScriptType, _project: Project): Promise<string | undefined> {
 		// TODO: ask project for suggested name that doesn't conflict
 		const suggestedName = itemType.friendlyName.replace(new RegExp('\s', 'g'), '') + '1';
 

extensions/sql-database-projects/src/templates/templateMap.ts

@@ -1,43 +0,0 @@
-/*---------------------------------------------------------------------------------------------
- *  Copyright (c) Microsoft Corporation. All rights reserved.
- *  Licensed under the Source EULA. See License.txt in the project root for license information.
- *--------------------------------------------------------------------------------------------*/
-
-import * as constants from '../common/constants';
-import * as templates from './templates';
-
-export class ProjectScriptType {
-	type: string;
-	friendlyName: string;
-	templateScript: string;
-
-	constructor(type: string, friendlyName: string, templateScript: string) {
-		this.type = type;
-		this.friendlyName = friendlyName;
-		this.templateScript = templateScript;
-	}
-}
-
-export const script: string = 'script';
-export const table: string = 'table';
-export const view: string = 'view';
-export const storedProcedure: string = 'storedProcedure';
-export const folder: string = 'folder';
-
-export const projectScriptTypes: ProjectScriptType[] = [
-	new ProjectScriptType(script, constants.scriptFriendlyName, templates.newSqlScriptTemplate),
-	new ProjectScriptType(table, constants.tableFriendlyName, templates.newSqlTableTemplate),
-	new ProjectScriptType(view, constants.viewFriendlyName, templates.newSqlViewTemplate),
-	new ProjectScriptType(storedProcedure, constants.storedProcedureFriendlyName, templates.newSqlStoredProcedureTemplate),
-];
-
-export const projectScriptTypeMap: Record<string, ProjectScriptType> = {};
-
-for (const scriptType of projectScriptTypes) {
-	if (Object.keys(projectScriptTypeMap).find(s => s === scriptType.type.toLocaleLowerCase() || s === scriptType.friendlyName.toLocaleLowerCase())) {
-		throw new Error(`Script type map already contains ${scriptType.type} or its friendlyName.`);
-	}
-
-	projectScriptTypeMap[scriptType.type.toLocaleLowerCase()] = scriptType;
-	projectScriptTypeMap[scriptType.friendlyName.toLocaleLowerCase()] = scriptType;
-}

extensions/sql-database-projects/src/templates/templates.ts

@@ -4,27 +4,85 @@
  *--------------------------------------------------------------------------------------------*/
 
 import * as path from 'path';
+import * as constants from '../common/constants';
 import { promises as fs } from 'fs';
 
-// Project templates
 export let newSqlProjectTemplate: string;
 
-// Script templates
+// Object types
 
-export let newSqlScriptTemplate: string;
-export let newSqlTableTemplate: string;
-export let newSqlViewTemplate: string;
-export let newSqlStoredProcedureTemplate: string;
+export const script: string = 'script';
+export const table: string = 'table';
+export const view: string = 'view';
+export const storedProcedure: string = 'storedProcedure';
+export const folder: string = 'folder';
+
+// Object maps
+
+let scriptTypeMap: Record<string, ProjectScriptType> = {};
+
+export function projectScriptTypeMap(): Record<string, ProjectScriptType> {
+	if (Object.keys(scriptTypeMap).length === 0) {
+		throw new Error('Templates must be loaded from file before attempting to use.');
+	}
+
+	return scriptTypeMap;
+}
+
+let scriptTypes: ProjectScriptType[] = [];
+
+export function projectScriptTypes(): ProjectScriptType[] {
+	if (scriptTypes.length === 0) {
+		throw new Error('Templates must be loaded from file before attempting to use.');
+	}
+
+	return scriptTypes;
+}
 
 export async function loadTemplates(templateFolderPath: string) {
-	newSqlProjectTemplate = await loadTemplate(templateFolderPath, 'newSqlProjectTemplate.xml');
+	await Promise.all([
+		Promise.resolve(newSqlProjectTemplate = await loadTemplate(templateFolderPath, 'newSqlProjectTemplate.xml')),
+		loadObjectTypeInfo(script, constants.scriptFriendlyName, templateFolderPath, 'newTsqlScriptTemplate.sql'),
+		loadObjectTypeInfo(table, constants.tableFriendlyName, templateFolderPath, 'newTsqlTableTemplate.sql'),
+		loadObjectTypeInfo(view, constants.viewFriendlyName, templateFolderPath, 'newTsqlViewTemplate.sql'),
+		loadObjectTypeInfo(storedProcedure, constants.storedProcedureFriendlyName, templateFolderPath, 'newTsqlStoredProcedureTemplate.sql')
+	]);
 
-	newSqlScriptTemplate = await loadTemplate(templateFolderPath, 'newTsqlScriptTemplate.sql');
-	newSqlTableTemplate = await loadTemplate(templateFolderPath, 'newTsqlTableTemplate.sql');
-	newSqlViewTemplate = await loadTemplate(templateFolderPath, 'newTsqlViewTemplate.sql');
-	newSqlStoredProcedureTemplate = await loadTemplate(templateFolderPath, 'newTsqlStoredProcedureTemplate.sql');
+	for (const scriptType of scriptTypes) {
+		if (Object.keys(projectScriptTypeMap).find(s => s === scriptType.type.toLocaleLowerCase() || s === scriptType.friendlyName.toLocaleLowerCase())) {
+			throw new Error(`Script type map already contains ${scriptType.type} or its friendlyName.`);
+		}
+
+		scriptTypeMap[scriptType.type.toLocaleLowerCase()] = scriptType;
+		scriptTypeMap[scriptType.friendlyName.toLocaleLowerCase()] = scriptType;
+	}
+}
+
+async function loadObjectTypeInfo(key: string, friendlyName: string, templateFolderPath: string, fileName: string) {
+	const template = await loadTemplate(templateFolderPath, fileName);
+	scriptTypes.push(new ProjectScriptType(key, friendlyName, template));
 }
 
 async function loadTemplate(templateFolderPath: string, fileName: string): Promise<string> {
 	return (await fs.readFile(path.join(templateFolderPath, fileName))).toString();
 }
+
+export class ProjectScriptType {
+	type: string;
+	friendlyName: string;
+	templateScript: string;
+
+	constructor(type: string, friendlyName: string, templateScript: string) {
+		this.type = type;
+		this.friendlyName = friendlyName;
+		this.templateScript = templateScript;
+	}
+}
+
+/**
+ * For testing purposes only
+ */
+export function reset() {
+	scriptTypeMap = {};
+	scriptTypes = [];
+}

extensions/sql-database-projects/src/test/templates.test.ts

@@ -0,0 +1,39 @@
+/*---------------------------------------------------------------------------------------------
+ *  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 path from 'path';
+import * as templates from '../templates/templates';
+import { shouldThrowSpecificError } from './testUtils';
+
+describe('Templates: loading templates from disk', function (): void {
+	beforeEach(() => {
+		templates.reset();
+	});
+
+	it('Should throw error when attempting to use templates before loaded from file', async function (): Promise<void> {
+		shouldThrowSpecificError(() => templates.projectScriptTypeMap(), 'Templates must be loaded from file before attempting to use.');
+		shouldThrowSpecificError(() => templates.projectScriptTypes(), 'Templates must be loaded from file before attempting to use.');
+	});
+
+	it('Should load all templates from files', async function (): Promise<void> {
+		await templates.loadTemplates(path.join(__dirname, '..', '..', 'resources', 'templates'));
+
+		// check expected counts
+
+		const numScriptObjectTypes = 4;
+
+		should(templates.projectScriptTypes().length).equal(numScriptObjectTypes);
+		should(Object.keys(templates.projectScriptTypes()).length).equal(numScriptObjectTypes);
+
+		// check everything has a value
+
+		should(templates.newSqlProjectTemplate).not.equal(undefined);
+
+		for (const obj of templates.projectScriptTypes()) {
+			should(obj.templateScript).not.equal(undefined);
+		}
+	});
+});

extensions/sql-database-projects/src/test/testUtils.ts

@@ -8,6 +8,23 @@ import * as os from 'os';
 import * as constants from '../common/constants';
 
 import { promises as fs } from 'fs';
+import should = require('should');
+import { AssertionError } from 'assert';
+
+export function shouldThrowSpecificError(block: Function, expectedMessage: string) {
+	let succeeded = false;
+	try {
+		block();
+		succeeded = true;
+	}
+	catch (err) {
+		should(err.message).equal(expectedMessage);
+	}
+
+	if (succeeded) {
+		throw new AssertionError({ message: 'Operation succeeded, but expected failure with exception: "' + expectedMessage + '"' });
+	}
+}
 
 export async function createTestSqlProj(contents: string, folderPath?: string): Promise<string> {
 	return await createTestFile(contents, 'TestProject.sqlproj', folderPath);