Fix for Sqlproj tree bug (microsoft#10605)

Fixes bug where files located in a subfolder defined before the folders themselves in .sqlproj would result in those files not appearing in the ADS treeview
winzig · Jun 2, 2020 · 0ac6a07 · 0ac6a07
1 parent 8fc8a79
commit 0ac6a07
Show file tree

Hide file tree

Showing 4 changed files with 106 additions and 12 deletions.
diff --git a/extensions/sql-database-projects/src/models/project.ts b/extensions/sql-database-projects/src/models/project.ts
@@ -141,7 +141,7 @@ export class Project {
 		return fileEntry;
 	}
 
-	private createProjectEntry(relativePath: string, entryType: EntryType): ProjectEntry {
+	public createProjectEntry(relativePath: string, entryType: EntryType): ProjectEntry {
 		return new ProjectEntry(Uri.file(path.join(this.projectFolderPath, relativePath)), relativePath, entryType);
 	}
 

diff --git a/extensions/sql-database-projects/src/models/tree/fileFolderTreeItem.ts b/extensions/sql-database-projects/src/models/tree/fileFolderTreeItem.ts
@@ -22,7 +22,7 @@ export class FolderNode extends BaseProjectTreeItem {
 	}
 
 	public get children(): BaseProjectTreeItem[] {
-		return Object.values(this.fileChildren).sort();
+		return Object.values(this.fileChildren).sort(sortFileFolderNodes);
 	}
 
 	public get treeItem(): vscode.TreeItem {
@@ -64,6 +64,23 @@ export class FileNode extends BaseProjectTreeItem {
 	}
 }
 
+/**
+ * Compares two folder/file tree nodes so that folders come before files, then alphabetically
+ * @param a a folder or file tree node
+ * @param b another folder or file tree node
+ */
+export function sortFileFolderNodes(a: (FolderNode | FileNode), b: (FolderNode | FileNode)): number {
+	if (a instanceof FolderNode && !(b instanceof FolderNode)) {
+		return -1;
+	}
+	else if (!(a instanceof FolderNode) && b instanceof FolderNode) {
+		return 1;
+	}
+	else {
+		return a.uri.fsPath.localeCompare(b.uri.fsPath);
+	}
+}
+
 /**
  * Converts a full filesystem URI to a project-relative URI that's compatible with the project tree
  */
@@ -75,8 +92,7 @@ function fsPathToProjectUri(fileSystemUri: vscode.Uri, projectNode: ProjectRootT
 		localUri = fileSystemUri.fsPath.substring(projBaseDir.length);
 	}
 	else {
-		vscode.window.showErrorMessage('Project pointing to file outside of directory');
-		throw new Error('Project pointing to file outside of directory');
+		throw new Error(`Project (${projBaseDir}) pointing to file outside of directory (${fileSystemUri.fsPath})`);
 	}
 
 	return vscode.Uri.file(path.join(projectNode.uri.path, localUri));

diff --git a/extensions/sql-database-projects/src/models/tree/projectTreeItem.ts b/extensions/sql-database-projects/src/models/tree/projectTreeItem.ts
@@ -32,14 +32,7 @@ export class ProjectRootTreeItem extends BaseProjectTreeItem {
 		const output: BaseProjectTreeItem[] = [];
 		output.push(this.dataSourceNode);
 
-		// sort children so that folders come first, then alphabetical
-		const sortedChildren = Object.values(this.fileChildren).sort((a: (fileTree.FolderNode | fileTree.FileNode), b: (fileTree.FolderNode | fileTree.FileNode)) => {
-			if (a instanceof fileTree.FolderNode && !(b instanceof fileTree.FolderNode)) { return -1; }
-			else if (!(a instanceof fileTree.FolderNode) && b instanceof fileTree.FolderNode) { return 1; }
-			else { return a.uri.fsPath.localeCompare(b.uri.fsPath); }
-		});
-
-		return output.concat(sortedChildren);
+		return output.concat(Object.values(this.fileChildren).sort(fileTree.sortFileFolderNodes));
 	}
 
 	public get treeItem(): vscode.TreeItem {
@@ -53,6 +46,10 @@ export class ProjectRootTreeItem extends BaseProjectTreeItem {
 		for (const entry of this.project.files) {
 			const parentNode = this.getEntryParentNode(entry);
 
+			if (Object.keys(parentNode.fileChildren).includes(path.basename(entry.fsUri.path))) {
+				continue; // ignore duplicate entries
+			}
+
 			let newNode: fileTree.FolderNode | fileTree.FileNode;
 
 			switch (entry.type) {

diff --git a/extensions/sql-database-projects/src/test/projectTree.test.ts b/extensions/sql-database-projects/src/test/projectTree.test.ts
@@ -0,0 +1,81 @@
+/*---------------------------------------------------------------------------------------------
+ *  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 vscode from 'vscode';
+import * as os from 'os';
+import * as path from 'path';
+
+import { Project, EntryType } from '../models/project';
+import { FolderNode, FileNode, sortFileFolderNodes } from '../models/tree/fileFolderTreeItem';
+import { ProjectRootTreeItem } from '../models/tree/projectTreeItem';
+
+describe('Project Tree tests', function (): void {
+	it('Should correctly order tree nodes by type, then by name', async function (): Promise<void> {
+		const root = os.platform() === 'win32' ? 'Z:\\' : '/';
+
+		const parent = new ProjectRootTreeItem(new Project(vscode.Uri.file(`${root}Fake.sqlproj`).fsPath));
+
+		let inputNodes: (FileNode | FolderNode)[] = [
+			new FileNode(vscode.Uri.file(`${root}C`), parent),
+			new FileNode(vscode.Uri.file(`${root}D`), parent),
+			new FolderNode(vscode.Uri.file(`${root}Z`), parent),
+			new FolderNode(vscode.Uri.file(`${root}X`), parent),
+			new FileNode(vscode.Uri.file(`${root}B`), parent),
+			new FileNode(vscode.Uri.file(`${root}A`), parent),
+			new FolderNode(vscode.Uri.file(`${root}W`), parent),
+			new FolderNode(vscode.Uri.file(`${root}Y`), parent)
+		];
+
+		inputNodes = inputNodes.sort(sortFileFolderNodes);
+
+		const expectedNodes: (FileNode | FolderNode)[] = [
+			new FolderNode(vscode.Uri.file(`${root}W`), parent),
+			new FolderNode(vscode.Uri.file(`${root}X`), parent),
+			new FolderNode(vscode.Uri.file(`${root}Y`), parent),
+			new FolderNode(vscode.Uri.file(`${root}Z`), parent),
+			new FileNode(vscode.Uri.file(`${root}A`), parent),
+			new FileNode(vscode.Uri.file(`${root}B`), parent),
+			new FileNode(vscode.Uri.file(`${root}C`), parent),
+			new FileNode(vscode.Uri.file(`${root}D`), parent)
+		];
+
+		should(inputNodes.map(n => n.uri.path)).deepEqual(expectedNodes.map(n => n.uri.path));
+	});
+
+	it('Should build tree from Project file correctly', async function (): Promise<void> {
+		const root = os.platform() === 'win32' ? 'Z:\\' : '/';
+		const proj = new Project(vscode.Uri.file(`${root}TestProj.sqlproj`).fsPath);
+
+		// nested entries before explicit top-level folder entry
+		// also, ordering of files/folders at all levels
+		proj.files.push(proj.createProjectEntry(path.join('someFolder', 'bNestedTest.sql'), EntryType.File));
+		proj.files.push(proj.createProjectEntry(path.join('someFolder', 'bNestedFolder'), EntryType.Folder));
+		proj.files.push(proj.createProjectEntry(path.join('someFolder', 'aNestedTest.sql'), EntryType.File));
+		proj.files.push(proj.createProjectEntry(path.join('someFolder', 'aNestedFolder'), EntryType.Folder));
+		proj.files.push(proj.createProjectEntry('someFolder', EntryType.Folder));
+
+		// duplicate files
+		proj.files.push(proj.createProjectEntry('duplicate.sql', EntryType.File));
+		proj.files.push(proj.createProjectEntry('duplicate.sql', EntryType.File));
+
+		// duplicate folders
+		proj.files.push(proj.createProjectEntry('duplicateFolder', EntryType.Folder));
+		proj.files.push(proj.createProjectEntry('duplicateFolder', EntryType.Folder));
+
+		const tree = new ProjectRootTreeItem(proj);
+		should(tree.children.map(x => x.uri.path)).deepEqual([
+			'/TestProj.sqlproj/Data Sources',
+			'/TestProj.sqlproj/duplicateFolder',
+			'/TestProj.sqlproj/someFolder',
+			'/TestProj.sqlproj/duplicate.sql']);
+
+		should(tree.children.find(x => x.uri.path === '/TestProj.sqlproj/someFolder')?.children.map(y => y.uri.path)).deepEqual([
+			'/TestProj.sqlproj/someFolder/aNestedFolder',
+			'/TestProj.sqlproj/someFolder/bNestedFolder',
+			'/TestProj.sqlproj/someFolder/aNestedTest.sql',
+			'/TestProj.sqlproj/someFolder/bNestedTest.sql']);
+	});
+});