diff --git a/extensions/sql-database-projects/src/common/constants.ts b/extensions/sql-database-projects/src/common/constants.ts index d282b11e06..24e93e2089 100644 --- a/extensions/sql-database-projects/src/common/constants.ts +++ b/extensions/sql-database-projects/src/common/constants.ts @@ -107,6 +107,8 @@ export function projectAlreadyOpened(path: string) { return localize('projectAlr export function projectAlreadyExists(name: string, path: string) { return localize('projectAlreadyExists', "A project named {0} already exists in {1}.", name, path); } export function noFileExist(fileName: string) { return localize('noFileExist', "File {0} doesn't exist", fileName); } export function cannotResolvePath(path: string) { return localize('cannotResolvePath', "Cannot resolve path {0}", path); } +export function fileAlreadyExists(filename: string) { return localize('fileAlreadyExists', "A file with the name '{0}' already exists on disk at this location. Please choose another name.", filename); } +export function folderAlreadyExists(filename: string) { return localize('folderAlreadyExists', "A folder with the name '{0}' already exists on disk at this location. Please choose another name.", filename); } export function invalidInput(input: string) { return localize('invalidInput', "Invalid input: {0}", input); } export function mssqlNotFound(mssqlConfigDir: string) { return localize('mssqlNotFound', "Could not get mssql extension's install location at {0}", mssqlConfigDir); } @@ -153,6 +155,7 @@ export const ArtifactReference = 'ArtifactReference'; export const SuppressMissingDependenciesErrors = 'SuppressMissingDependenciesErrors'; export const DatabaseVariableLiteralValue = 'DatabaseVariableLiteralValue'; export const DSP = 'DSP'; +export const Properties = 'Properties'; // SqlProj File targets export const NetCoreTargets = '$(NETCoreTargetsPath)\\Microsoft.Data.Tools.Schema.SqlTasks.targets'; diff --git a/extensions/sql-database-projects/src/controllers/projectController.ts b/extensions/sql-database-projects/src/controllers/projectController.ts index 793d1041c5..709cf2c1bd 100644 --- a/extensions/sql-database-projects/src/controllers/projectController.ts +++ b/extensions/sql-database-projects/src/controllers/projectController.ts @@ -18,7 +18,7 @@ import { IConnectionProfile, TaskExecutionMode } from 'azdata'; import { promises as fs } from 'fs'; import { ApiWrapper } from '../common/apiWrapper'; import { DeployDatabaseDialog } from '../dialogs/deployDatabaseDialog'; -import { Project, DatabaseReferenceLocation, SystemDatabase, TargetPlatform, ProjectEntry } from '../models/project'; +import { Project, DatabaseReferenceLocation, SystemDatabase, TargetPlatform, ProjectEntry, reservedProjectFolders } from '../models/project'; import { SqlDatabaseProjectTreeViewProvider } from './databaseProjectTreeViewProvider'; import { FolderNode, FileNode } from '../models/tree/fileFolderTreeItem'; import { IDeploymentProfile, IGenerateScriptProfile, PublishSettings } from '../models/IDeploymentProfile'; @@ -280,9 +280,26 @@ export class ProjectsController { const relativeFolderPath = path.join(this.getRelativePath(treeNode), newFolderName); - await project.addFolderItem(relativeFolderPath); + try { + // check if folder already exists or is a reserved folder + const absoluteFolderPath = path.join(project.projectFolderPath, relativeFolderPath); + const folderExists = await utils.exists(absoluteFolderPath); - this.refreshProjectsTree(); + if (folderExists || this.isReservedFolder(absoluteFolderPath, project.projectFolderPath)) { + throw new Error(constants.folderAlreadyExists(path.parse(absoluteFolderPath).name)); + } + + await project.addFolderItem(relativeFolderPath); + this.refreshProjectsTree(); + } catch (err) { + this.apiWrapper.showErrorMessage(utils.getErrorMessage(err)); + } + } + + public isReservedFolder(absoluteFolderPath: string, projectFolderPath: string): boolean { + const sameName = reservedProjectFolders.find(f => f === path.parse(absoluteFolderPath).name) !== undefined; + const sameLocation = path.parse(absoluteFolderPath).dir === projectFolderPath; + return sameName && sameLocation; } public async addItemPromptFromNode(treeNode: BaseProjectTreeItem, itemTypeName?: string) { @@ -315,16 +332,26 @@ export class ProjectsController { return; // user cancelled } - // TODO: file already exists? - const newFileText = this.macroExpansion(itemType.templateScript, { 'OBJECT_NAME': itemObjectName }); const relativeFilePath = path.join(relativePath, itemObjectName + constants.sqlFileExtension); - const newEntry = await project.addScriptItem(relativeFilePath, newFileText); + try { + // check if file already exists + const absoluteFilePath = path.join(project.projectFolderPath, relativeFilePath); + const fileExists = await utils.exists(absoluteFilePath); - this.apiWrapper.executeCommand('vscode.open', newEntry.fsUri); + if (fileExists) { + throw new Error(constants.fileAlreadyExists(path.parse(absoluteFilePath).name)); + } - this.refreshProjectsTree(); + const newEntry = await project.addScriptItem(relativeFilePath, newFileText); + + this.apiWrapper.executeCommand('vscode.open', newEntry.fsUri); + + this.refreshProjectsTree(); + } catch (err) { + this.apiWrapper.showErrorMessage(utils.getErrorMessage(err)); + } } public async exclude(context: FileNode | FolderNode): Promise { diff --git a/extensions/sql-database-projects/src/models/project.ts b/extensions/sql-database-projects/src/models/project.ts index b2fdf6cb96..1dc3afcc9f 100644 --- a/extensions/sql-database-projects/src/models/project.ts +++ b/extensions/sql-database-projects/src/models/project.ts @@ -48,18 +48,24 @@ export class Project { for (let ig = 0; ig < this.projFileXmlDoc.documentElement.getElementsByTagName(constants.ItemGroup).length; ig++) { const itemGroup = this.projFileXmlDoc.documentElement.getElementsByTagName(constants.ItemGroup)[ig]; - for (let b = 0; b < itemGroup.getElementsByTagName(constants.Build).length; b++) { - this.files.push(this.createProjectEntry(itemGroup.getElementsByTagName(constants.Build)[b].getAttribute(constants.Include), EntryType.File)); + const buildElements = itemGroup.getElementsByTagName(constants.Build); + for (let b = 0; b < buildElements.length; b++) { + this.files.push(this.createProjectEntry(buildElements[b].getAttribute(constants.Include), EntryType.File)); } - for (let f = 0; f < itemGroup.getElementsByTagName(constants.Folder).length; f++) { - this.files.push(this.createProjectEntry(itemGroup.getElementsByTagName(constants.Folder)[f].getAttribute(constants.Include), EntryType.Folder)); + const folderElements = itemGroup.getElementsByTagName(constants.Folder); + for (let f = 0; f < folderElements.length; f++) { + // don't add Properties folder since it isn't supported for now + if (folderElements[f].getAttribute(constants.Include) !== constants.Properties) { + this.files.push(this.createProjectEntry(folderElements[f].getAttribute(constants.Include), EntryType.Folder)); + } } } // find all import statements to include - for (let i = 0; i < this.projFileXmlDoc.documentElement.getElementsByTagName(constants.Import).length; i++) { - const importTarget = this.projFileXmlDoc.documentElement.getElementsByTagName(constants.Import)[i]; + const importElements = this.projFileXmlDoc.documentElement.getElementsByTagName(constants.Import); + for (let i = 0; i < importElements.length; i++) { + const importTarget = importElements[i]; this.importedTargets.push(importTarget.getAttribute(constants.Project)); } @@ -589,3 +595,5 @@ export enum SystemDatabase { master, msdb } + +export const reservedProjectFolders = ['Properties', 'Data Sources', 'Database References']; diff --git a/extensions/sql-database-projects/src/test/project.test.ts b/extensions/sql-database-projects/src/test/project.test.ts index 0bd7e81829..c429074919 100644 --- a/extensions/sql-database-projects/src/test/project.test.ts +++ b/extensions/sql-database-projects/src/test/project.test.ts @@ -33,7 +33,7 @@ describe('Project: sqlproj content operations', function (): void { // Files and folders should(project.files.filter(f => f.type === EntryType.File).length).equal(4); - should(project.files.filter(f => f.type === EntryType.Folder).length).equal(5); + should(project.files.filter(f => f.type === EntryType.Folder).length).equal(4); should(project.files.find(f => f.type === EntryType.Folder && f.relativePath === 'Views\\User')).not.equal(undefined); // mixed ItemGroup folder should(project.files.find(f => f.type === EntryType.File && f.relativePath === 'Views\\User\\Profile.sql')).not.equal(undefined); // mixed ItemGroup file @@ -81,7 +81,7 @@ describe('Project: sqlproj content operations', function (): void { await project.addToProject(list); should(project.files.filter(f => f.type === EntryType.File).length).equal(11); // txt file shouldn't be added to the project - should(project.files.filter(f => f.type === EntryType.Folder).length).equal(3); // 2folders + default Properties folder + should(project.files.filter(f => f.type === EntryType.Folder).length).equal(2); // 2 folders }); it('Should throw error while adding Folder and Build entries to sqlproj when a file/folder does not exist on disk', async function (): Promise { diff --git a/extensions/sql-database-projects/src/test/projectController.test.ts b/extensions/sql-database-projects/src/test/projectController.test.ts index 1ae0fbcea7..83c116ef3d 100644 --- a/extensions/sql-database-projects/src/test/projectController.test.ts +++ b/extensions/sql-database-projects/src/test/projectController.test.ts @@ -18,13 +18,14 @@ import { SqlDatabaseProjectTreeViewProvider } from '../controllers/databaseProje import { ProjectsController } from '../controllers/projectController'; import { promises as fs } from 'fs'; import { createContext, TestContext, mockDacFxResult } from './testContext'; -import { Project, SystemDatabase, ProjectEntry } from '../models/project'; +import { Project, SystemDatabase, ProjectEntry, reservedProjectFolders } from '../models/project'; import { DeployDatabaseDialog } from '../dialogs/deployDatabaseDialog'; import { ApiWrapper } from '../common/apiWrapper'; import { IDeploymentProfile, IGenerateScriptProfile } from '../models/IDeploymentProfile'; import { exists } from '../common/utils'; import { ProjectRootTreeItem } from '../models/tree/projectTreeItem'; import { FolderNode } from '../models/tree/fileFolderTreeItem'; +import { BaseProjectTreeItem } from '../models/tree/baseTreeItem'; let testContext: TestContext; @@ -77,7 +78,7 @@ describe('ProjectsController: project controller operations', function (): void const project = await projController.openProject(vscode.Uri.file(sqlProjPath)); - should(project.files.length).equal(9); // detailed sqlproj tests in their own test file + should(project.files.length).equal(8); // detailed sqlproj tests in their own test file should(project.dataSources.length).equal(2); // detailed datasources tests in their own test file }); @@ -110,6 +111,75 @@ describe('ProjectsController: project controller operations', function (): void } }); + it('Should show error if trying to add a file that already exists', async function (): Promise { + const tableName = 'table1'; + testContext.apiWrapper.reset(); + testContext.apiWrapper.setup(x => x.showInputBox(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve(tableName)); + testContext.apiWrapper.setup(x => x.showErrorMessage(TypeMoq.It.isAny())).returns((s) => { throw new Error(s); }); + + const projController = new ProjectsController(testContext.apiWrapper.object, new SqlDatabaseProjectTreeViewProvider()); + const project = await testUtils.createTestProject(baselines.newProjectFileBaseline); + + should(project.files.length).equal(0, 'There should be no files'); + await projController.addItemPrompt(project, '', templates.script); + should(project.files.length).equal(1, 'File should be successfully added'); + await testUtils.shouldThrowSpecificError(async () => await projController.addItemPrompt(project, '', templates.script), constants.fileAlreadyExists(tableName)); + }); + + it('Should show error if trying to add a folder that already exists', async function (): Promise { + const folderName = 'folder1'; + testContext.apiWrapper.reset(); + testContext.apiWrapper.setup(x => x.showInputBox(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve(folderName)); + testContext.apiWrapper.setup(x => x.showErrorMessage(TypeMoq.It.isAny())).returns((s) => { throw new Error(s); }); + + const projController = new ProjectsController(testContext.apiWrapper.object, new SqlDatabaseProjectTreeViewProvider()); + const project = await testUtils.createTestProject(baselines.newProjectFileBaseline); + const projectRoot = new ProjectRootTreeItem(project); + + should(project.files.length).equal(0, 'There should be no other folders'); + await projController.addFolderPrompt(projectRoot); + should(project.files.length).equal(1, 'Folder should be successfully added'); + projController.refreshProjectsTree(); + + await verifyFolderNotAdded(folderName, projController, project, projectRoot); + + // reserved folder names + for (let i in reservedProjectFolders) { + await verifyFolderNotAdded(reservedProjectFolders[i], projController, project, projectRoot); + } + }); + + it('Should be able to add folder with reserved name as long as not at project root', async function (): Promise { + const folderName = 'folder1'; + testContext.apiWrapper.reset(); + testContext.apiWrapper.setup(x => x.showInputBox(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve(folderName)); + testContext.apiWrapper.setup(x => x.showErrorMessage(TypeMoq.It.isAny())).returns((s) => { throw new Error(s); }); + + const projController = new ProjectsController(testContext.apiWrapper.object, new SqlDatabaseProjectTreeViewProvider()); + const project = await testUtils.createTestProject(baselines.openProjectFileBaseline); + const projectRoot = new ProjectRootTreeItem(project); + + // make sure it's ok to add these folders if they aren't where the reserved folders are at the root of the project + let node = projectRoot.children.find(c => c.friendlyName === 'Tables'); + for (let i in reservedProjectFolders) { + await verfiyFolderAdded(reservedProjectFolders[i], projController, project, node); + } + }); + + async function verfiyFolderAdded(folderName: string, projController: ProjectsController, project: Project, node: BaseProjectTreeItem): Promise { + const beforeFileCount = project.files.length; + testContext.apiWrapper.setup(x => x.showInputBox(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve(folderName)); + await projController.addFolderPrompt(node); + should(project.files.length).equal(beforeFileCount + 1, `File count should be increased by one after adding the folder ${folderName}`); + } + + async function verifyFolderNotAdded(folderName: string, projController: ProjectsController, project: Project, node: BaseProjectTreeItem): Promise { + const beforeFileCount = project.files.length; + testContext.apiWrapper.setup(x => x.showInputBox(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve(folderName)); + await testUtils.shouldThrowSpecificError(async () => await projController.addFolderPrompt(node), constants.folderAlreadyExists(folderName)); + should(project.files.length).equal(beforeFileCount, 'File count should be the same as before the folder was attempted to be added'); + } + it('Should delete nested ProjectEntry from node', async function (): Promise { let proj = await testUtils.createTestProject(templates.newSqlProjectTemplate); const setupResult = await setupDeleteExcludeTest(proj); @@ -123,8 +193,8 @@ describe('ProjectsController: project controller operations', function (): void await proj.readProjFile(); // reload edited sqlproj from disk // confirm result - should(proj.files.length).equal(2, 'number of file/folder entries'); // lowerEntry and the contained scripts should be deleted - should(proj.files[1].relativePath).equal('UpperFolder'); + should(proj.files.length).equal(1, 'number of file/folder entries'); // lowerEntry and the contained scripts should be deleted + should(proj.files[0].relativePath).equal('UpperFolder'); should(await exists(scriptEntry.fsUri.fsPath)).equal(false, 'script is supposed to be deleted'); }); @@ -142,8 +212,8 @@ describe('ProjectsController: project controller operations', function (): void await proj.readProjFile(); // reload edited sqlproj from disk // confirm result - should(proj.files.length).equal(2, 'number of file/folder entries'); // LowerFolder and the contained scripts should be deleted - should(proj.files[1].relativePath).equal('UpperFolder'); // UpperFolder should still be there + should(proj.files.length).equal(1, 'number of file/folder entries'); // LowerFolder and the contained scripts should be deleted + should(proj.files[0].relativePath).equal('UpperFolder'); // UpperFolder should still be there should(await exists(scriptEntry.fsUri.fsPath)).equal(true, 'script is supposed to still exist on disk'); }); @@ -456,7 +526,7 @@ async function setupDeleteExcludeTest(proj: Project): Promise<[ProjectEntry, Pro testContext.apiWrapper.setup(x => x.showWarningMessageOptions(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve(constants.yesString)); // confirm setup - should(proj.files.length).equal(5, 'number of file/folder entries'); + should(proj.files.length).equal(4, 'number of file/folder entries'); should(path.parse(scriptEntry.fsUri.fsPath).base).equal('someScript.sql'); should((await fs.readFile(scriptEntry.fsUri.fsPath)).toString()).equal('not a real script');