Handle exclude folder for sdk style sql projects (#17826)

* handle exclude Folder for sdk style projects

* update comment

* fix tests

* cleanup

* handle nested folders

* cleanup

* addressing comments

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

@@ -578,3 +578,28 @@ export async function getFoldersInFolder(folderPath: string, ignoreBinObj?: bool
 		return await glob(folderFilter, { onlyDirectories: true });
 	}
 }
+
+/**
+ * Gets the folders between the startFolder to the file
+ * @param startFolder
+ * @param endFile
+ * @returns array of folders between startFolder and endFile
+ */
+export function getFoldersToFile(startFolder: string, endFile: string): string[] {
+	let folders: string[] = [];
+
+	const endFolderPath = path.dirname(endFile);
+
+	const relativePath = convertSlashesForSqlProj(endFolderPath.substring(startFolder.length));
+	const pathSegments = trimChars(relativePath, ' \\').split(constants.SqlProjPathSeparator);
+	let folderPath = convertSlashesForSqlProj(startFolder) + constants.SqlProjPathSeparator;
+
+	for (let segment of pathSegments) {
+		if (segment) {
+			folderPath += segment + constants.SqlProjPathSeparator;
+			folders.push(getPlatformSafeFileEntryPath(folderPath));
+		}
+	}
+
+	return folders;
+}

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

@@ -660,7 +660,8 @@ export class ProjectsController {
 		if (root && fileOrFolder) {
 			// use relative path and not tree paths for files and folder
 			const allFileEntries = project.files.concat(project.preDeployScripts).concat(project.postDeployScripts).concat(project.noneDeployScripts);
-			return allFileEntries.find(x => utils.getPlatformSafeFileEntryPath(x.relativePath) === utils.getPlatformSafeFileEntryPath(utils.trimUri(root.fileSystemUri, fileOrFolder.fileSystemUri)));
+			const trimmedUri = utils.trimChars(utils.getPlatformSafeFileEntryPath(utils.trimUri(root.fileSystemUri, fileOrFolder.fileSystemUri)), '/');
+			return allFileEntries.find(x => utils.trimChars(utils.getPlatformSafeFileEntryPath(x.relativePath), '/') === trimmedUri);
 		}
 		return project.files.find(x => utils.getPlatformSafeFileEntryPath(x.relativePath) === utils.getPlatformSafeFileEntryPath(utils.trimUri(context.root.projectUri, context.projectUri)));
 	}

extensions/sql-database-projects/src/models/project.ts

@@ -237,11 +237,17 @@ export class Project implements ISqlProject {
 
 	private async readFolders(): Promise<FileProjectEntry[]> {
 		const folderEntries: FileProjectEntry[] = [];
+
 		// glob style getting folders for sdk style projects
+		const foldersSet = new Set<string>();
 		if (this._isSdkStyleProject) {
-			const folders = await utils.getFoldersInFolder(this.projectFolderPath, true);
+			this.files.forEach(file => {
-			folders.forEach(f => {
+				// if file is in the project's folder, add the folders from the project file to this file to the list of folders. This is so that only non-empty folders in the project folder will be added by default.
-				folderEntries.push(this.createFileProjectEntry(utils.trimUri(Uri.file(this.projectFilePath), Uri.file(f)), EntryType.Folder));
+				// Empty folders won't be shown unless specified in the sqlproj (same as how it's handled for csproj in VS)
+				if (!file.relativePath.startsWith('..') && path.dirname(file.fsUri.fsPath) !== this.projectFolderPath) {
+					const foldersToFile = utils.getFoldersToFile(this.projectFolderPath, file.fsUri.fsPath);
+					foldersToFile.forEach(f => foldersSet.add(utils.convertSlashesForSqlProj(utils.trimUri(Uri.file(this.projectFilePath), Uri.file(f)))));
+				}
 			});
 		}
 
@@ -251,10 +257,13 @@ export class Project implements ISqlProject {
 			try {
 				const folderElements = itemGroup.getElementsByTagName(constants.Folder);
 				for (let f = 0; f < folderElements.length; f++) {
-					const relativePath = folderElements[f].getAttribute(constants.Include)!;
+					let relativePath = folderElements[f].getAttribute(constants.Include)!;
+
 					// don't add Properties folder since it isn't supported for now and don't add if the folder was already added
-					if (relativePath !== constants.Properties && !folderEntries.find(f => f.relativePath === utils.trimChars(relativePath, '\\'))) {
+					if (utils.trimChars(relativePath, '\\') !== constants.Properties) {
-						folderEntries.push(this.createFileProjectEntry(relativePath, EntryType.Folder));
+						// make sure folder relative path ends with \\ because sometimes SSDT adds folders without trailing \\
+						relativePath = relativePath.endsWith(constants.SqlProjPathSeparator) ? relativePath : relativePath + constants.SqlProjPathSeparator;
+						foldersSet.add(relativePath);
 					}
 				}
 			} catch (e) {
@@ -263,6 +272,10 @@ export class Project implements ISqlProject {
 			}
 		}
 
+		foldersSet.forEach(f => {
+			folderEntries.push(this.createFileProjectEntry(f, EntryType.Folder));
+		});
+
 		return folderEntries;
 	}
 
@@ -973,15 +986,53 @@ export class Project implements ISqlProject {
 		this.findOrCreateItemGroup(constants.Folder).appendChild(newFolderNode);
 	}
 
-	private removeFolderFromProjFile(path: string): void {
+	private async removeFolderFromProjFile(folderPath: string): Promise<void> {
 		const folderNodes = this.projFileXmlDoc!.documentElement.getElementsByTagName(constants.Folder);
-		const deleted = this.removeNode(path, folderNodes);
+		let deleted = this.removeNode(folderPath, folderNodes);
+
+		// if it wasn't deleted, try deleting the folder path without trailing backslash
+		// since sometimes SSDT adds folders without a trailing \
+		if (!deleted) {
+			deleted = this.removeNode(utils.trimChars(folderPath, '\\'), folderNodes);
+		}
+
+		// TODO: consider removing this check when working on migration scenario. If a user converts to an SDK-style project and adding this
+		// exclude XML doesn't hurt for non-SDK-style projects, then it might be better to just it anyway so that they don't have to exclude the folder
+		// again when they convert to an SDK-style project
+		if (this.isSdkStyleProject) {
+			// update sqlproj if a node was deleted and load files and folders again
+			if (deleted) {
+				await this.writeToSqlProjAndUpdateFilesFolders();
+			}
+			// get latest folders to see if it still exists
+			const currentFolders = await this.readFolders();
+
+			// add exclude entry if it's still in the current folders
+			if (currentFolders.find(f => f.relativePath === utils.convertSlashesForSqlProj(folderPath))) {
+				const removeFileNode = this.projFileXmlDoc!.createElement(constants.Build);
+				removeFileNode.setAttribute(constants.Remove, utils.convertSlashesForSqlProj(folderPath + '**'));
+				this.findOrCreateItemGroup(constants.Build).appendChild(removeFileNode);
+
+				// write changes and update files so everything is up to date for the next removal
+				await this.writeToSqlProjAndUpdateFilesFolders();
+			}
+
+			deleted = true;
+		}
 
 		if (!deleted) {
-			throw new Error(constants.unableToFindObject(path, constants.folderObject));
+			throw new Error(constants.unableToFindObject(folderPath, constants.folderObject));
 		}
 	}
 
+	private async writeToSqlProjAndUpdateFilesFolders(): Promise<void> {
+		await this.serializeToProjFile(this.projFileXmlDoc);
+		const projFileText = await fs.readFile(this._projectFilePath);
+		this.projFileXmlDoc = new xmldom.DOMParser().parseFromString(projFileText.toString());
+		this._files = await this.readFilesInProject();
+		this.files.push(...(await this.readFolders()));
+	}
+
 	private removeSqlCmdVariableFromProjFile(variableName: string): void {
 		const sqlCmdVariableNodes = this.projFileXmlDoc!.documentElement.getElementsByTagName(constants.SqlCmdVariable);
 		const deleted = this.removeNode(variableName, sqlCmdVariableNodes);
@@ -1256,14 +1307,20 @@ export class Project implements ISqlProject {
 			entries = [entries];
 		}
 
+		// remove any folders first, otherwise unnecessary Build remove entries might get added for sdk style
+		// projects to exclude both the folder and the files in the folder
+		const folderEntries = entries.filter(e => e.type === EntryType.Folder);
+		for (const folder of folderEntries) {
+			await this.removeFolderFromProjFile((<FileProjectEntry>folder).relativePath);
+		}
+
+		entries = entries.filter(e => e.type !== EntryType.Folder);
+
 		for (const entry of entries) {
 			switch (entry.type) {
 				case EntryType.File:
 					await this.removeFileFromProjFile((<FileProjectEntry>entry).relativePath);
 					break;
-				case EntryType.Folder:
-					this.removeFolderFromProjFile((<FileProjectEntry>entry).relativePath);
-					break;
 				case EntryType.DatabaseReference:
 					this.removeDatabaseReferenceFromProjFile(<IDatabaseReferenceProjectEntry>entry);
 					break;

extensions/sql-database-projects/src/models/tree/projectTreeItem.ts

@@ -115,7 +115,8 @@ export class ProjectRootTreeItem extends BaseProjectTreeItem {
 
 		for (const part of relativePathParts) {
 			if (current.fileChildren[part] === undefined) {
-				current.fileChildren[part] = new fileTree.FolderNode(vscode.Uri.file(path.join(path.dirname(this.project.projectFilePath), part)), current);
+				const parentPath = current instanceof ProjectRootTreeItem ? path.dirname(current.fileSystemUri.fsPath) : current.fileSystemUri.fsPath;
+				current.fileChildren[part] = new fileTree.FolderNode(vscode.Uri.file(path.join(parentPath, part)), current);
 			}
 
 			if (current.fileChildren[part] instanceof fileTree.FileNode) {

extensions/sql-database-projects/src/test/baselines/openSdkStyleSqlProjectWithFilesSpecifiedBaseline.xml

@@ -13,6 +13,7 @@
   <ItemGroup>
     <Folder Include="Properties" />
     <Folder Include="folder1" />
+    <Folder Include="folder2\" />
   </ItemGroup>
   <ItemGroup>
     <Build Include="folder1\file2.sql" />

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

@@ -37,7 +37,7 @@ describe('Project: sqlproj content operations', function (): void {
 		should(project.files.filter(f => f.type === EntryType.File).length).equal(6);
 		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.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
 		should(project.files.find(f => f.type === EntryType.File && f.relativePath === '..\\Test\\Test.sql')).not.equal(undefined); // mixed ItemGroup file
 		should(project.files.find(f => f.type === EntryType.File && f.relativePath === 'MyExternalStreamingJob.sql')).not.equal(undefined); // entry with custom attribute
@@ -843,8 +843,8 @@ describe('Project: sdk style project content operations', function (): void {
 		const project: Project = await Project.openProject(projFilePath);
 
 		// Files and folders
-		should(project.files.filter(f => f.type === EntryType.Folder).length).equal(2);
+		should(project.files.filter(f => f.type === EntryType.Folder).length).equal(3);
-		should(project.files.filter(f => f.type === EntryType.File).length).equal(15);
+		should(project.files.filter(f => f.type === EntryType.File).length).equal(17);
 
 		// SqlCmdVariables
 		should(Object.keys(project.sqlCmdVariables).length).equal(2);
@@ -880,7 +880,7 @@ describe('Project: sdk style project content operations', function (): void {
 		// these are also listed in the sqlproj, but there shouldn't be duplicate entries for them
 		should(project.files.filter(f => f.relativePath === 'folder1\\file2.sql').length).equal(1);
 		should(project.files.filter(f => f.relativePath === 'file1.sql').length).equal(1);
-		should(project.files.filter(f => f.relativePath === 'folder1').length).equal(1);
+		should(project.files.filter(f => f.relativePath === 'folder1\\').length).equal(1);
 	});
 
 	it('Should handle globbing patterns listed in sqlproj', async function (): Promise<void> {
@@ -1013,7 +1013,7 @@ describe('Project: sdk style project content operations', function (): void {
 		projFilePath = await testUtils.createTestSqlProjFile(baselines.openSdkStyleSqlProjectBaseline);
 		const project = await Project.openProject(projFilePath);
 
-		const folderPath = 'Stored Procedures';
+		const folderPath = 'Stored Procedures\\';
 		const scriptPath = path.join(folderPath, 'Fake Stored Proc.sql');
 		const scriptContents = 'SELECT \'This is not actually a stored procedure.\'';
 
@@ -1023,7 +1023,7 @@ describe('Project: sdk style project content operations', function (): void {
 		const outsideFolderScriptPath = path.join('..', 'Other Fake Stored Proc.sql');
 		const outsideFolderScriptContents = 'SELECT \'This is also not actually a stored procedure.\'';
 
-		const otherFolderPath = 'OtherFolder';
+		const otherFolderPath = 'OtherFolder\\';
 
 		await project.addScriptItem(scriptPath, scriptContents);
 		await project.addScriptItem(scriptPathTagged, scriptContentsTagged, templates.externalStreamingJob);
@@ -1037,7 +1037,9 @@ describe('Project: sdk style project content operations', function (): void {
 		should(newProject.files.find(f => f.type === EntryType.File && f.relativePath === convertSlashesForSqlProj(scriptPathTagged))).not.equal(undefined);
 		should(newProject.files.find(f => f.type === EntryType.File && f.relativePath === convertSlashesForSqlProj(scriptPathTagged))?.sqlObjectType).equal(constants.ExternalStreamingJob);
 		should(newProject.files.find(f => f.type === EntryType.File && f.relativePath === convertSlashesForSqlProj(outsideFolderScriptPath))).not.equal(undefined);
-		should(newProject.files.find(f => f.type === EntryType.Folder && f.relativePath === convertSlashesForSqlProj(otherFolderPath))).not.equal(undefined);
+
+		// TODO: uncomment after add empty folder is updated
+		// should(newProject.files.find(f => f.type === EntryType.Folder && f.relativePath === convertSlashesForSqlProj(otherFolderPath))).not.equal(undefined);
 
 		// only the external streaming job and file outside of the project folder should have been added to the sqlproj
 		const projFileText = (await fs.readFile(projFilePath)).toString();
@@ -1048,6 +1050,92 @@ describe('Project: sdk style project content operations', function (): void {
 		should(projFileText.includes('<Folder Include="OtherFolder" />')).equal(false, projFileText);
 	});
 
+	it('Should handle excluding glob included folders', async function (): Promise<void> {
+		const testFolderPath = await testUtils.generateTestFolderPath();
+		projFilePath = await testUtils.createTestSqlProjFile(baselines.openSdkStyleSqlProjectBaseline, testFolderPath);
+		await testUtils.createDummyFileStructureWithPrePostDeployScripts(false, undefined, path.dirname(projFilePath));
+
+		const project: Project = await Project.openProject(projFilePath);
+
+		should(project.files.filter(f => f.type === EntryType.File).length).equal(17);
+		should(project.files.filter(f => f.type === EntryType.Folder).length).equal(3);
+
+		// try to exclude a glob included folder
+		await project.exclude(project.files.find(f => f.relativePath === 'folder1\\')!);
+
+		// verify folder and contents are excluded
+		should(project.files.filter(f => f.type === EntryType.Folder).length).equal(1);
+		should(project.files.filter(f => f.type === EntryType.File).length).equal(9);
+		should(project.files.find(f => f.relativePath === 'folder1\\')).equal(undefined);
+
+		// verify sqlproj has glob exclude for folder, but not for files and inner folder
+		const projFileText = (await fs.readFile(projFilePath)).toString();
+		should(projFileText.includes('<Build Remove="folder1\\**" />')).equal(true, projFileText);
+		should(projFileText.includes('<Build Remove="folder1\\file1.sql" />')).equal(false, projFileText);
+		should(projFileText.includes('<Build Remove="folder1\\nestedFolder\\**" />')).equal(false, projFileText);
+	});
+
+
+	it('Should handle excluding nested glob included folders', async function (): Promise<void> {
+		const testFolderPath = await testUtils.generateTestFolderPath();
+		projFilePath = await testUtils.createTestSqlProjFile(baselines.openSdkStyleSqlProjectBaseline, testFolderPath);
+		await testUtils.createDummyFileStructureWithPrePostDeployScripts(false, undefined, path.dirname(projFilePath));
+
+		const project: Project = await Project.openProject(projFilePath);
+
+		should(project.files.filter(f => f.type === EntryType.File).length).equal(17);
+		should(project.files.filter(f => f.type === EntryType.Folder).length).equal(3);
+
+		// try to exclude a glob included folder
+		await project.exclude(project.files.find(f => f.relativePath === 'folder1\\nestedFolder\\')!);
+
+		// verify folder and contents are excluded
+		should(project.files.filter(f => f.type === EntryType.Folder).length).equal(2);
+		should(project.files.filter(f => f.type === EntryType.File).length).equal(15);
+		should(project.files.find(f => f.relativePath === 'folder1\\nestedFolder\\')).equal(undefined);
+
+		// verify sqlproj has glob exclude for folder, but not for files
+		const projFileText = (await fs.readFile(projFilePath)).toString();
+		should(projFileText.includes('<Build Remove="folder1\\nestedFolder\\**" />')).equal(true, projFileText);
+		should(projFileText.includes('<Build Remove="folder1\\nestedFolder\\otherFile1.sql" />')).equal(false, projFileText);
+	});
+
+	it('Should handle excluding explicitly included folders', async function (): Promise<void> {
+		const testFolderPath = await testUtils.generateTestFolderPath();
+		projFilePath = await testUtils.createTestSqlProjFile(baselines.openSdkStyleSqlProjectWithFilesSpecifiedBaseline, testFolderPath);
+		await testUtils.createDummyFileStructure(false, undefined, path.dirname(projFilePath));
+
+		const project: Project = await Project.openProject(projFilePath);
+
+		should(project.files.filter(f => f.type === EntryType.File).length).equal(11);
+		should(project.files.filter(f => f.type === EntryType.Folder).length).equal(2);
+		should(project.files.find(f => f.relativePath === 'folder1\\')!).not.equal(undefined);
+		should(project.files.find(f => f.relativePath === 'folder2\\')!).not.equal(undefined);
+
+		// try to exclude an explicitly included folder without trailing \ in sqlproj
+		await project.exclude(project.files.find(f => f.relativePath === 'folder1\\')!);
+
+		// verify folder and contents are excluded
+		should(project.files.filter(f => f.type === EntryType.Folder).length).equal(1);
+		should(project.files.filter(f => f.type === EntryType.File).length).equal(6);
+		should(project.files.find(f => f.relativePath === 'folder1\\')).equal(undefined);
+
+		// try to exclude an explicitly included folder with trailing \ in sqlproj
+		await project.exclude(project.files.find(f => f.relativePath === 'folder2\\')!);
+
+		// verify folder and contents are excluded
+		should(project.files.filter(f => f.type === EntryType.Folder).length).equal(0);
+		should(project.files.filter(f => f.type === EntryType.File).length).equal(1);
+		should(project.files.find(f => f.relativePath === 'folder2\\')).equal(undefined);
+
+		// make sure both folders are removed from sqlproj and remove entry is added
+		const projFileText = (await fs.readFile(projFilePath)).toString();
+		should(projFileText.includes('<Folder Include="folder1" />')).equal(false, projFileText);
+		should(projFileText.includes('<Folder Include="folder2\" />')).equal(false, projFileText);
+
+		should(projFileText.includes('<Build Remove="folder1\\**" />')).equal(true, projFileText);
+		should(projFileText.includes('<Build Remove="folder2\\**" />')).equal(true, projFileText);
+	});
 });
 
 describe('Project: add SQLCMD Variables', function (): void {

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

@@ -125,6 +125,9 @@ export async function createDummyFileStructure(createList?: boolean, list?: Uri[
  * 			-file4.sql
  * 			-file5.sql
  * 			-Script.PostDeployment2.sql
+ * 			- nestedFolder
+ * 				-otherFile1.sql
+ * 				-otherFile2.sql
  *	 	- folder2
  * 			-file1.sql
  * 			-file2.sql
@@ -159,6 +162,14 @@ export async function createDummyFileStructureWithPrePostDeployScripts(createLis
 	const postdeployscript2 = path.join(testFolderPath, 'folder1', 'Script.PostDeployment2.sql');
 	await fs.writeFile(postdeployscript2, '');
 
+
+	// add nested files
+	await fs.mkdir(path.join(testFolderPath, 'folder1', 'nestedFolder'));
+	const otherfile1 = path.join(testFolderPath, 'folder1', 'nestedFolder', 'otherFile1.sql');
+	await fs.writeFile(otherfile1, '');
+	const otherfile2 = path.join(testFolderPath, 'folder1', 'nestedFolder', 'otherFile2.sql');
+	await fs.writeFile(otherfile2, '');
+
 	if (createList) {
 		list?.push(Uri.file(postdeployscript1));
 		list?.push(Uri.file(postdeployscript2));