diff --git a/extensions/sql-database-projects/src/common/constants.ts b/extensions/sql-database-projects/src/common/constants.ts index 541b5eb49f..0f29f51a53 100644 --- a/extensions/sql-database-projects/src/common/constants.ts +++ b/extensions/sql-database-projects/src/common/constants.ts @@ -106,6 +106,7 @@ export function deleteReferenceConfirmation(toDelete: string) { return localize( export function selectTargetPlatform(currentTargetPlatform: string) { return localize('selectTargetPlatform', "Current target platform: {0}. Select new target platform", currentTargetPlatform); } export function currentTargetPlatform(projectName: string, currentTargetPlatform: string) { return localize('currentTargetPlatform', "Target platform of the project {0} is now {1}", projectName, currentTargetPlatform); } export function projectUpdatedToSdkStyle(projectName: string) { return localize('projectUpdatedToSdkStyle', "The project {0} has been updated to be an SDK-style project. Click 'Learn More' for details on the Microsoft.Build.Sql SDK and ways to simplify the project file.", projectName); } +export function updatedToSdkStyleError(projectName: string) { return localize('updatedToSdkStyleError', "Converting the project {0} to SDK-style was unsuccessful. Changes to the .sqlproj have been rolled back.", projectName); } // Publish dialog strings diff --git a/extensions/sql-database-projects/src/controllers/projectController.ts b/extensions/sql-database-projects/src/controllers/projectController.ts index 32ccfbd8ed..ff54b95317 100644 --- a/extensions/sql-database-projects/src/controllers/projectController.ts +++ b/extensions/sql-database-projects/src/controllers/projectController.ts @@ -887,15 +887,19 @@ export class ProjectsController { */ public async convertToSdkStyleProject(context: dataworkspace.WorkspaceTreeItem): Promise { const project = this.getProjectFromContext(context); + const updateResult = await project.convertProjectToSdkStyle(); - await project.convertProjectToSdkStyle(); - void this.reloadProject(context); + if (!updateResult) { + void vscode.window.showErrorMessage(constants.updatedToSdkStyleError(project.projectFileName)); + } else { + void this.reloadProject(context); - // show message that project file can be simplified - const result = await vscode.window.showInformationMessage(constants.projectUpdatedToSdkStyle(project.projectFileName), constants.learnMore); + // show message that project file can be simplified + const result = await vscode.window.showInformationMessage(constants.projectUpdatedToSdkStyle(project.projectFileName), constants.learnMore); - if (result === constants.learnMore) { - void vscode.env.openExternal(vscode.Uri.parse(constants.sdkLearnMoreUrl!)); + if (result === constants.learnMore) { + void vscode.env.openExternal(vscode.Uri.parse(constants.sdkLearnMoreUrl!)); + } } } diff --git a/extensions/sql-database-projects/src/models/project.ts b/extensions/sql-database-projects/src/models/project.ts index e01a78bbff..704e88ffe6 100644 --- a/extensions/sql-database-projects/src/models/project.ts +++ b/extensions/sql-database-projects/src/models/project.ts @@ -617,43 +617,90 @@ export class Project implements ISqlProject { await this.createCleanFileNode(beforeBuildNode); } - public async convertProjectToSdkStyle(): Promise { + public async convertProjectToSdkStyle(): Promise { // don't do anything if the project is already SDK style or it's an SSDT project that hasn't been updated to build in ADS if (this.isSdkStyleProject || !this._importedTargets.includes(constants.NetCoreTargets)) { - return; + return false; } // make backup copy of project await fs.copyFile(this._projectFilePath, this._projectFilePath + '_backup'); - // remove SSDT and ADS SqlTasks imports - const importsToRemove = []; - for (let i = 0; i < this.projFileXmlDoc!.documentElement.getElementsByTagName(constants.Import).length; i++) { - const importTarget = this.projFileXmlDoc!.documentElement.getElementsByTagName(constants.Import)[i]; - const projectAttributeVal = importTarget.getAttribute(constants.Project); + try { + // remove Build includes and folder includes + const beforeFiles = this.files.filter(f => f.type === EntryType.File); + const beforeFolders = this.files.filter(f => f.type === EntryType.Folder); - if (projectAttributeVal === constants.NetCoreTargets || projectAttributeVal === constants.SqlDbTargets || projectAttributeVal === constants.MsBuildtargets) { - importsToRemove.push(importTarget); + // remove Build includes + for (const file of beforeFiles) { + // only remove build includes in the same folder as the project + if (!file.relativePath.includes('..')) { + await this.exclude(file); + } } + + // remove Folder includes + for (const folder of beforeFolders) { + await this.exclude(folder); + } + + // remove "Properties" folder if it's there. This isn't tracked in the project's folders here because ADS doesn't support it. + // It's a reserved folder only used for the UI in SSDT + try { + await this.removeFolderFromProjFile('Properties'); + } catch { } + + // remove SSDT and ADS SqlTasks imports + const importsToRemove = []; + for (let i = 0; i < this.projFileXmlDoc!.documentElement.getElementsByTagName(constants.Import).length; i++) { + const importTarget = this.projFileXmlDoc!.documentElement.getElementsByTagName(constants.Import)[i]; + const projectAttributeVal = importTarget.getAttribute(constants.Project); + + if (projectAttributeVal === constants.NetCoreTargets || projectAttributeVal === constants.SqlDbTargets || projectAttributeVal === constants.MsBuildtargets) { + importsToRemove.push(importTarget); + } + } + + const parent = importsToRemove[0]?.parentNode; + importsToRemove.forEach(i => { parent?.removeChild(i); }); + + // add SDK node + const sdkNode = this.projFileXmlDoc!.createElement(constants.Sdk); + sdkNode.setAttribute(constants.Name, constants.sqlProjectSdk); + sdkNode.setAttribute(constants.Version, constants.sqlProjectSdkVersion); + + const projectNode = this.projFileXmlDoc!.documentElement; + projectNode.insertBefore(sdkNode, projectNode.firstChild); + + // TODO: also update system dacpac path, but might as well wait for them to get included in the SDK since the path will probably change again + + await this.serializeToProjFile(this.projFileXmlDoc!); + await this.readProjFile(); + + // Make sure the same files are included as before and there aren't extra files included by the default **/*.sql glob + for (const file of this.files.filter(f => f.type === EntryType.File)) { + if (!beforeFiles.find(f => f.pathForSqlProj() === file.pathForSqlProj())) { + await this.exclude(file); + } + } + + // add back any folders that were previously specified in the sqlproj, but aren't included by the **/*.sql glob because they're empty + const folders = this.files.filter(f => f.type === EntryType.Folder); + for (const folder of beforeFolders) { + if (!folders.find(f => f.relativePath === folder.relativePath)) { + await this.addFolderItem(folder.relativePath); + } + } + } catch (e) { + console.error(e); + + // if there was an uncaught error during conversion, rollback project update + await fs.copyFile(this._projectFilePath + '_backup', this._projectFilePath); + await this.readProjFile(); + return false; } - const parent = importsToRemove[0]?.parentNode; - importsToRemove.forEach(i => { parent?.removeChild(i); }); - - // add SDK node - const sdkNode = this.projFileXmlDoc!.createElement(constants.Sdk); - sdkNode.setAttribute(constants.Name, constants.sqlProjectSdk); - sdkNode.setAttribute(constants.Version, constants.sqlProjectSdkVersion); - - const projectNode = this.projFileXmlDoc!.documentElement; - projectNode.insertBefore(sdkNode, projectNode.firstChild); - - // TODO: also update system dacpac path, but might as well wait for them to get included in the SDK since the path will probably change again - - // TODO: remove Build includes and folder includes. Make sure the same files and folders are being included and there aren't extra files included by the default **/*.sql glob - - await this.serializeToProjFile(this.projFileXmlDoc!); - await this.readProjFile(); + return true; } private async createCleanFileNode(parentNode: Element): Promise { diff --git a/extensions/sql-database-projects/src/test/baselines/baselines.ts b/extensions/sql-database-projects/src/test/baselines/baselines.ts index 7f3eb63deb..73cf5e1786 100644 --- a/extensions/sql-database-projects/src/test/baselines/baselines.ts +++ b/extensions/sql-database-projects/src/test/baselines/baselines.ts @@ -9,6 +9,7 @@ import { promises as fs } from 'fs'; // Project baselines export let newProjectFileBaseline: string; export let newProjectFileWithScriptBaseline: string; +export let newProjectFileNoPropertiesFolderBaseline: string; export let openProjectFileBaseline: string; export let openDataSourcesBaseline: string; export let SSDTProjectFileBaseline: string; @@ -41,6 +42,7 @@ const baselineFolderPath = __dirname; export async function loadBaselines() { newProjectFileBaseline = await loadBaseline(baselineFolderPath, 'newSqlProjectBaseline.xml'); newProjectFileWithScriptBaseline = await loadBaseline(baselineFolderPath, 'newSqlProjectWithScriptBaseline.xml'); + newProjectFileNoPropertiesFolderBaseline = await loadBaseline(baselineFolderPath, 'newSqlProjectNoPropertiesFolderBaseline.xml'); openProjectFileBaseline = await loadBaseline(baselineFolderPath, 'openSqlProjectBaseline.xml'); openDataSourcesBaseline = await loadBaseline(baselineFolderPath, 'openDataSourcesBaseline.json'); SSDTProjectFileBaseline = await loadBaseline(baselineFolderPath, 'SSDTProjectBaseline.xml'); diff --git a/extensions/sql-database-projects/src/test/baselines/newSqlProjectNoPropertiesFolderBaseline.xml b/extensions/sql-database-projects/src/test/baselines/newSqlProjectNoPropertiesFolderBaseline.xml new file mode 100644 index 0000000000..cc10a3d8c8 --- /dev/null +++ b/extensions/sql-database-projects/src/test/baselines/newSqlProjectNoPropertiesFolderBaseline.xml @@ -0,0 +1,64 @@ + + + + Debug + AnyCPU + TestProjectName + 2.0 + 4.1 + {BA5EBA11-C0DE-5EA7-ACED-BABB1E70A575} + Microsoft.Data.Tools.Schema.Sql.Sql150DatabaseSchemaProvider + Database + + + TestProjectName + TestProjectName + 1033, CI + BySchemaAndSchemaType + True + v4.5 + CS + Properties + False + True + True + + + bin\Release\ + $(MSBuildProjectName).sql + False + pdbonly + true + false + true + prompt + 4 + + + bin\Debug\ + $(MSBuildProjectName).sql + false + true + full + false + true + true + prompt + 4 + + + 11.0 + + True + 11.0 + + + + + + + + + + + diff --git a/extensions/sql-database-projects/src/test/project.test.ts b/extensions/sql-database-projects/src/test/project.test.ts index c643d85e2c..bc49c13c1c 100644 --- a/extensions/sql-database-projects/src/test/project.test.ts +++ b/extensions/sql-database-projects/src/test/project.test.ts @@ -1657,16 +1657,149 @@ describe('Project: legacy to SDK-style updates', function (): void { }); it('Should update legacy style project to SDK-style', async function (): Promise { - const projFilePath = await testUtils.createTestSqlProjFile(baselines.openProjectFileBaseline); + const projFilePath = await testUtils.createTestSqlProjFile(baselines.newProjectFileBaseline); + const list: Uri[] = []; + await testUtils.createDummyFileStructure(true, list, path.dirname(projFilePath)); const project = await Project.openProject(projFilePath); + await project.addToProject(list); + const beforeFileCount = project.files.filter(f => f.type === EntryType.File).length; + const beforeFolderCount = project.files.filter(f => f.type === EntryType.Folder).length; + should(beforeFolderCount).equal(2, 'There should be 2 folders in the project'); + should(beforeFileCount).equal(11, 'There should be 11 files in the project'); should(project.importedTargets.length).equal(3, 'SSDT and ADS imports should be in the project'); should(project.isSdkStyleProject).equal(false); + let projFileText = (await fs.readFile(projFilePath)).toString(); + should(projFileText.includes(' f.type === EntryType.File).length).equal(beforeFileCount, 'Same number of files should be included after Build Includes are removed'); + should(project.files.filter(f => f.type === EntryType.Folder).length).equal(beforeFolderCount, 'Same number of folders should be included after Folder Includes are removed'); + }); + + it('Should not fail if legacy style project does not have Properties folder in sqlproj', async function (): Promise { + const projFilePath = await testUtils.createTestSqlProjFile(baselines.newProjectFileNoPropertiesFolderBaseline); + const list: Uri[] = []; + await testUtils.createDummyFileStructure(true, list, path.dirname(projFilePath)); + const project = await Project.openProject(projFilePath); + await project.addToProject(list); + + const beforeFolderCount = project.files.filter(f => f.type === EntryType.Folder).length; + + await project.convertProjectToSdkStyle(); + + should(project.isSdkStyleProject).equal(true); + const projFileText = (await fs.readFile(projFilePath)).toString(); + should(projFileText.includes(' f.type === EntryType.Folder).length).equal(beforeFolderCount, 'Same number of folders should be included after Folder Includes are removed'); + }); + + it('Should exclude sql files that were not in previously included in legacy style sqlproj', async function (): Promise { + const projFilePath = await testUtils.createTestSqlProjFile(baselines.newProjectFileBaseline); + const list: Uri[] = []; + await testUtils.createDummyFileStructure(true, list, path.dirname(projFilePath)); + const project = await Project.openProject(projFilePath); + + // don't add file1.sql, folder1\file1.sql and folder2\file1.sql + await project.addToProject(list.filter(f => !f.fsPath.includes('file1.sql'))); + + const beforeFileCount = project.files.filter(f => f.type === EntryType.File).length; + const beforeFolderCount = project.files.filter(f => f.type === EntryType.Folder).length; + should(beforeFileCount).equal(8, 'There should be 8 files in the project before converting'); + + await project.convertProjectToSdkStyle(); + + should(project.isSdkStyleProject).equal(true); + const projFileText = (await fs.readFile(projFilePath)).toString(); + should(projFileText.includes(' f.type === EntryType.File).length).equal(beforeFileCount, 'Same number of files should be included after Build Includes are removed'); + should(project.files.filter(f => f.type === EntryType.Folder).length).equal(beforeFolderCount, 'Same number of folders should be included after Folder Includes are removed'); + }); + + it('Should keep Build Includes for files outside of project folder', async function (): Promise { + const testFolderPath = await testUtils.generateTestFolderPath(); + const mainProjectPath = path.join(testFolderPath, 'project'); + const otherFolderPath = path.join(testFolderPath, 'other'); + projFilePath = await testUtils.createTestSqlProjFile(baselines.newProjectFileBaseline, mainProjectPath); + let list: Uri[] = []; + await testUtils.createDummyFileStructure(true, list, path.dirname(projFilePath)); + + // create files outside of project folder that are included in the project file + await fs.mkdir(otherFolderPath); + const otherFiles = await testUtils.createOtherDummyFiles(otherFolderPath); + list = list.concat(otherFiles); + + const project = await Project.openProject(projFilePath); + + // add all the files, except the pre and post deploy scripts + await project.addToProject(list.filter(f => !f.fsPath.includes('Script.'))); + + const beforeFileCount = project.files.filter(f => f.type === EntryType.File).length; + const beforeFolderCount = project.files.filter(f => f.type === EntryType.Folder).length; + should(beforeFileCount).equal(19, 'There should be 19 files in the project'); + + await project.convertProjectToSdkStyle(); + + should(project.isSdkStyleProject).equal(true); + const projFileText = (await fs.readFile(projFilePath)).toString(); + should(projFileText.match(/ f.type === EntryType.File).length).equal(beforeFileCount, 'Same number of files should be included after Build Includes are removed'); + should(project.files.filter(f => f.type === EntryType.Folder).length).equal(beforeFolderCount, 'Same number of folders should be included after Folder Includes are removed'); + }); + + it('Should list previously included empty folders in sqlproj after converting to SDK-style', async function (): Promise { + const projFilePath = await testUtils.createTestSqlProjFile(baselines.newProjectFileBaseline); + const list: Uri[] = []; + const folderPath = path.dirname(projFilePath); + await testUtils.createDummyFileStructure(true, list, folderPath); + const project = await Project.openProject(projFilePath); + await project.addToProject(list); + + await project.addFolderItem('folder3'); + await project.addFolderItem('folder3\\nestedFolder'); + await project.addFolderItem('folder4'); + + const beforeFolderCount = project.files.filter(f => f.type === EntryType.Folder).length; + should(beforeFolderCount).equal(5); + + await project.convertProjectToSdkStyle(); + + should(project.isSdkStyleProject).equal(true); + const projFileText = (await fs.readFile(projFilePath)).toString(); + should(projFileText.includes('')).equal(false, 'There should not be a folder include for folder3\\nestedFolder because it gets included by the nestedFolder entry'); + should(projFileText.includes('')).equal(true, 'There should be a folder include for folder3\\nestedFolder'); + should(projFileText.includes('')).equal(true, 'There should be a folder include for folder4'); + should(project.files.filter(f => f.type === EntryType.Folder).length).equal(beforeFolderCount, 'Same number of folders should be included after Folder Includes are removed'); + }); + + it('Should rollback changes if there was an error during conversion to SDK-style', async function (): Promise { + const folderPath = await testUtils.generateTestFolderPath(); + const sqlProjPath = await testUtils.createTestSqlProjFile(baselines.newProjectFileBaseline, folderPath); + const project = await Project.openProject(Uri.file(sqlProjPath).fsPath); + should(project.isSdkStyleProject).equal(false); + + // add an empty folder so that addFolderItem() will get called during the conversion. Empty folders aren't included by glob, so they need to be added to the sqlproj + // to show up in the project tree + await project.addFolderItem('folder1'); + + sinon.stub(Project.prototype, 'addFolderItem').throwsException('error'); + const result = await project.convertProjectToSdkStyle(); + + should(result).equal(false); + should(project.isSdkStyleProject).equal(false); + should(project.importedTargets.length).equal(3, 'SSDT and ADS imports should still be there'); }); it('Should not update project and no backup file should be created when project is already SDK-style', async function (): Promise { diff --git a/extensions/sql-database-projects/src/test/testUtils.ts b/extensions/sql-database-projects/src/test/testUtils.ts index a298a42b1a..d4ac849726 100644 --- a/extensions/sql-database-projects/src/test/testUtils.ts +++ b/extensions/sql-database-projects/src/test/testUtils.ts @@ -203,9 +203,11 @@ export async function createListOfFiles(filePath?: string): Promise { * - Script.PostDeployment2.sql * */ -export async function createOtherDummyFiles(testFolderPath: string): Promise { +export async function createOtherDummyFiles(testFolderPath: string): Promise { + const filesList: Uri[] = []; let filePath = path.join(testFolderPath, 'file1.sql'); await fs.writeFile(filePath, ''); + filesList.push(Uri.file(filePath)); for (let dirCount = 1; dirCount <= 2; dirCount++) { let dirName = path.join(testFolderPath, `folder${dirCount}`); @@ -214,23 +216,31 @@ export async function createOtherDummyFiles(testFolderPath: string): Promise