Add folder for SDK style sql projects (#17918)

* update add folder for sdk style sql projects

* fix tests

* add test for add folder to sdk style project

* handle nested folders

* update helper function name

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

@@ -603,3 +603,26 @@ export function getFoldersToFile(startFolder: string, endFile: string): string[]
 
 	return folders;
 }
+
+/**
+ * Gets the folders between the startFolder and endFolder
+ * @param startFolder
+ * @param endFolder
+ * @returns array of folders between startFolder and endFolder
+ */
+export function getFoldersAlongPath(startFolder: string, endFolder: string): string[] {
+	let folders: string[] = [];
+
+	const relativePath = convertSlashesForSqlProj(endFolder.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/models/project.ts

@@ -238,8 +238,13 @@ 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>();
+
+		// get any folders listed in the project file
+		const sqlprojFolders = await this.foldersListedInSqlproj();
+		sqlprojFolders.forEach(f => foldersSet.add(f));
+
+		// glob style getting folders for sdk style projects
 		if (this._isSdkStyleProject) {
 			this.files.forEach(file => {
 				// 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.
@@ -249,8 +254,29 @@ export class Project implements ISqlProject {
 					foldersToFile.forEach(f => foldersSet.add(utils.convertSlashesForSqlProj(utils.trimUri(Uri.file(this.projectFilePath), Uri.file(f)))));
 				}
 			});
+
+			// add any intermediate folders of the folders that are listed in the sqlproj
+			// If there are nested empty folders, there will only be a Folder entry for the inner most folder, so we need to add entries for the intermediate folders
+			sqlprojFolders.forEach(folder => {
+				const fullPath = path.join(utils.getPlatformSafeFileEntryPath(this.projectFolderPath), utils.getPlatformSafeFileEntryPath(folder));
+				const intermediateFolders = utils.getFoldersAlongPath(this.projectFolderPath, utils.getPlatformSafeFileEntryPath(fullPath));
+				intermediateFolders.forEach(f => foldersSet.add(utils.convertSlashesForSqlProj(utils.trimUri(Uri.file(this.projectFilePath), Uri.file(f)))));
+			});
 		}
 
+		foldersSet.forEach(f => {
+			folderEntries.push(this.createFileProjectEntry(f, EntryType.Folder));
+		});
+
+		return folderEntries;
+	}
+
+	/**
+	 * @returns Array of folders specified in the sqlproj
+	 */
+	private async foldersListedInSqlproj(): Promise<string[]> {
+		const folders: string[] = [];
+
 		// get any folders listed in the project file
 		for (let ig = 0; ig < this.projFileXmlDoc!.documentElement.getElementsByTagName(constants.ItemGroup).length; ig++) {
 			const itemGroup = this.projFileXmlDoc!.documentElement.getElementsByTagName(constants.ItemGroup)[ig];
@@ -262,8 +288,7 @@ export class Project implements ISqlProject {
 					// don't add Properties folder since it isn't supported for now and don't add if the folder was already added
 					if (utils.trimChars(relativePath, '\\') !== constants.Properties) {
 						// make sure folder relative path ends with \\ because sometimes SSDT adds folders without trailing \\
-						relativePath = relativePath.endsWith(constants.SqlProjPathSeparator) ? relativePath : relativePath + constants.SqlProjPathSeparator;
+						folders.push(utils.ensureTrailingSlash(relativePath));
-						foldersSet.add(relativePath);
 					}
 				}
 			} catch (e) {
@@ -272,11 +297,7 @@ export class Project implements ISqlProject {
 			}
 		}
 
-		foldersSet.forEach(f => {
+		return folders;
-			folderEntries.push(this.createFileProjectEntry(f, EntryType.Folder));
-		});
-
-		return folderEntries;
 	}
 
 	private readPreDeployScripts(): FileProjectEntry[] {
@@ -875,7 +896,7 @@ export class Project implements ISqlProject {
 		return outputItemGroup;
 	}
 
-	private async addFileToProjFile(path: string, xmlTag: string, attributes?: Map<string, string>): Promise<void> {
+	private async addFileToProjFile(filePath: string, xmlTag: string, attributes?: Map<string, string>): Promise<void> {
 		let itemGroup;
 
 		if (xmlTag === constants.PreDeploy || xmlTag === constants.PostDeploy) {
@@ -888,11 +909,25 @@ export class Project implements ISqlProject {
 			}
 		}
 		else {
+			if (this.isSdkStyleProject) {
+				// if there's a folder entry for the folder containing this file, remove it from the sqlproj because the folder will now be
+				// included by the glob that includes this file (same as how csproj does it)
+				const folders = await this.foldersListedInSqlproj();
+				folders.forEach(folder => {
+					const trimmedUri = utils.trimUri(Uri.file(utils.getPlatformSafeFileEntryPath(folder)), Uri.file(utils.getPlatformSafeFileEntryPath(filePath)));
+					const basename = path.basename(utils.getPlatformSafeFileEntryPath(filePath));
+					if (trimmedUri === basename) {
+						// remove folder entry from sqlproj
+						this.removeFolderNode(folder);
+					}
+				});
+			}
+
 			const currentFiles = await this.readFilesInProject();
 
 			// don't need to add an entry if it's already included by a glob pattern
 			// unless it has an attribute that needs to be added, like external streaming job which needs it so it can be determined if validation can run on it
-			if (attributes?.size === 0 && currentFiles.find(f => f.relativePath === utils.convertSlashesForSqlProj(path))) {
+			if (attributes?.size === 0 && currentFiles.find(f => f.relativePath === utils.convertSlashesForSqlProj(filePath))) {
 				return;
 			}
 
@@ -901,7 +936,7 @@ export class Project implements ISqlProject {
 
 		const newFileNode = this.projFileXmlDoc!.createElement(xmlTag);
 
-		newFileNode.setAttribute(constants.Include, utils.convertSlashesForSqlProj(path));
+		newFileNode.setAttribute(constants.Include, utils.convertSlashesForSqlProj(filePath));
 
 		if (attributes) {
 			for (const key of attributes.keys()) {
@@ -979,22 +1014,29 @@ export class Project implements ISqlProject {
 		return false;
 	}
 
-	private addFolderToProjFile(path: string): void {
+	private async addFolderToProjFile(folderPath: string): Promise<void> {
+		if (this.isSdkStyleProject) {
+			// if there's a folder entry for the folder containing this folder, remove it from the sqlproj because the folder will now be
+			// included by the glob that includes this folder (same as how csproj does it)
+			const folders = await this.foldersListedInSqlproj();
+			folders.forEach(folder => {
+				const trimmedUri = utils.trimChars(utils.trimUri(Uri.file(utils.getPlatformSafeFileEntryPath(folder)), Uri.file(utils.getPlatformSafeFileEntryPath(folderPath))), '/');
+				const basename = path.basename(utils.getPlatformSafeFileEntryPath(folderPath));
+				if (trimmedUri === basename) {
+					// remove folder entry from sqlproj
+					this.removeFolderNode(folder);
+				}
+			});
+		}
+
 		const newFolderNode = this.projFileXmlDoc!.createElement(constants.Folder);
-		newFolderNode.setAttribute(constants.Include, utils.convertSlashesForSqlProj(path));
+		newFolderNode.setAttribute(constants.Include, utils.convertSlashesForSqlProj(folderPath));
 
 		this.findOrCreateItemGroup(constants.Folder).appendChild(newFolderNode);
 	}
 
 	private async removeFolderFromProjFile(folderPath: string): Promise<void> {
-		const folderNodes = this.projFileXmlDoc!.documentElement.getElementsByTagName(constants.Folder);
+		let deleted = this.removeFolderNode(folderPath);
-		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
@@ -1025,6 +1067,19 @@ export class Project implements ISqlProject {
 		}
 	}
 
+	private removeFolderNode(folderPath: string): boolean {
+		const folderNodes = this.projFileXmlDoc!.documentElement.getElementsByTagName(constants.Folder);
+		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);
+		}
+
+		return deleted;
+	}
+
 	private async writeToSqlProjAndUpdateFilesFolders(): Promise<void> {
 		await this.serializeToProjFile(this.projFileXmlDoc);
 		const projFileText = await fs.readFile(this._projectFilePath);
@@ -1289,7 +1344,7 @@ export class Project implements ISqlProject {
 				await this.addFileToProjFile((<FileProjectEntry>entry).relativePath, xmlTag ? xmlTag : constants.Build, attributes);
 				break;
 			case EntryType.Folder:
-				this.addFolderToProjFile((<FileProjectEntry>entry).relativePath);
+				await this.addFolderToProjFile((<FileProjectEntry>entry).relativePath);
 				break;
 			case EntryType.DatabaseReference:
 				await this.addDatabaseReferenceToProjFile(<IDatabaseReferenceProjectEntry>entry);
@@ -1444,9 +1499,18 @@ export class Project implements ISqlProject {
 		// If folder doesn't exist, create it
 		await fs.mkdir(absoluteFolderPath, { recursive: true });
 
-		// don't need to add the folder to the sqlproj if this is an sdk style project because globbing will get the folders
+		// for SDK style projects, only add this folder to the sqlproj if needed
+		// intermediate folders don't need to be added in the sqlproj
 		if (this.isSdkStyleProject) {
-			return this.createFileProjectEntry(relativeFolderPath, EntryType.Folder);
+			let folderEntry = this.files.find(f => utils.ensureTrailingSlash(f.relativePath.toUpperCase()) === utils.ensureTrailingSlash((relativeFolderPath.toUpperCase())));
+
+			if (!folderEntry) {
+				folderEntry = this.createFileProjectEntry(utils.ensureTrailingSlash(relativeFolderPath), EntryType.Folder);
+				this.files.push(folderEntry);
+				await this.addToProjFile(folderEntry);
+			}
+
+			return folderEntry;
 		}
 
 		// Add project file entries for all folders in the path.

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

@@ -1038,8 +1038,7 @@ describe('Project: sdk style project content operations', function (): void {
 		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);
 
-		// TODO: uncomment after add empty folder is updated
+		should(newProject.files.find(f => f.type === EntryType.Folder && f.relativePath === convertSlashesForSqlProj(otherFolderPath))).not.equal(undefined);
-		// 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();
@@ -1131,11 +1130,106 @@ describe('Project: sdk style project content operations', function (): void {
 		// 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('<Folder Include="folder2\\" />')).equal(false, projFileText);
 
 		should(projFileText.includes('<Build Remove="folder1\\**" />')).equal(true, projFileText);
 		should(projFileText.includes('<Build Remove="folder2\\**" />')).equal(true, projFileText);
 	});
+
+	it('Should handle adding empty folders and removing the Folder entry when the folder is no longer empty', 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 add a new folder
+		await project.addFolderItem('folder3\\');
+
+		// try to add a new folder without trailing backslash
+		await project.addFolderItem('folder4');
+
+		// verify folders were added
+		should(project.files.filter(f => f.type === EntryType.Folder).length).equal(4);
+		should(project.files.filter(f => f.type === EntryType.File).length).equal(11);
+		should(project.files.find(f => f.relativePath === 'folder3\\')).not.equal(undefined);
+		should(project.files.find(f => f.relativePath === 'folder4\\')).not.equal(undefined);
+
+		// verify folders were added and the entries have a backslash in the sqlproj
+		let projFileText = (await fs.readFile(projFilePath)).toString();
+		should(projFileText.includes('<Folder Include="folder3\\" />')).equal(true, projFileText);
+		should(projFileText.includes('<Folder Include="folder4\\" />')).equal(true, projFileText);
+
+		// add file to folder3
+		await project.addScriptItem(path.join('folder3', 'test.sql'), 'fake contents');
+		should(project.files.filter(f => f.type === EntryType.Folder).length).equal(4);
+		should(project.files.filter(f => f.type === EntryType.File).length).equal(12);
+		should(project.files.find(f => f.relativePath === 'folder3\\test.sql')).not.equal(undefined);
+
+		// verify folder3 entry is no longer in sqlproj
+		projFileText = (await fs.readFile(projFilePath)).toString();
+		should(projFileText.includes('<Folder Include="folder3\\" />')).equal(false, projFileText);
+		should(projFileText.includes('<Folder Include="folder4\\" />')).equal(true, projFileText);
+	});
+
+	it('Should handle adding nested empty 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));
+
+		let 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 add a new folder
+		await project.addFolderItem('folder3\\');
+
+		// try to add a nested folder
+		await project.addFolderItem('folder3\\innerFolder\\');
+
+		// verify folders were added
+		should(project.files.filter(f => f.type === EntryType.Folder).length).equal(4);
+		should(project.files.filter(f => f.type === EntryType.File).length).equal(11);
+		should(project.files.find(f => f.relativePath === 'folder3\\')).not.equal(undefined);
+		should(project.files.find(f => f.relativePath === 'folder3\\innerFolder\\')).not.equal(undefined);
+
+		// verify there's only one folder entry for the two folders that were added
+		let projFileText = (await fs.readFile(projFilePath)).toString();
+		should(projFileText.includes('<Folder Include="folder3\\" />')).equal(false, projFileText);
+		should(projFileText.includes('<Folder Include="folder3\\innerFolder\\" />')).equal(true, projFileText);
+
+		// load the project again and make sure both new folders get loaded
+		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(4);
+		should(project.files.find(f => f.relativePath === 'folder3\\')!).not.equal(undefined, 'folder3\\ should be loaded');
+		should(project.files.find(f => f.relativePath === 'folder3\\innerFolder\\')!).not.equal(undefined, 'folder3\\innerFolder\\ should be loaded');
+
+		// add file to folder3
+		await project.addScriptItem(path.join('folder3', 'test.sql'), 'fake contents');
+		should(project.files.filter(f => f.type === EntryType.Folder).length).equal(4);
+		should(project.files.filter(f => f.type === EntryType.File).length).equal(12);
+		should(project.files.find(f => f.relativePath === 'folder3\\test.sql')).not.equal(undefined, 'folder3\\test.sql should be in the project files');
+
+		// verify folder entry for innerFolder entry is still there
+		projFileText = (await fs.readFile(projFilePath)).toString();
+		should(projFileText.includes('<Folder Include="folder3\\innerFolder\\" />')).equal(true, projFileText);
+
+		// load the project again and make sure the folders still get loaded
+		project = await Project.openProject(projFilePath);
+		should(project.files.filter(f => f.type === EntryType.File).length).equal(12);
+		should(project.files.filter(f => f.type === EntryType.Folder).length).equal(4);
+		should(project.files.find(f => f.relativePath === 'folder3\\')!).not.equal(undefined, 'folder3\\ should be loaded');
+		should(project.files.find(f => f.relativePath === 'folder3\\innerFolder\\')!).not.equal(undefined, 'folder3\\innerFolder\\ should be loaded');
+	});
 });
 
 describe('Project: add SQLCMD Variables', function (): void {