Corrects workspace project tree refresh behavior for adding new projects to the workspace (#16650) (#16710)

* bugfix and updates

* PR feedback

* Deferred promise for project disk scan

* fix casing

* fixing race condition on extension activation, test failure

extensions/data-workspace/package.json

@@ -59,7 +59,7 @@
       {
         "command": "dataworkspace.refresh",
         "title": "%refresh-workspace-command%",
-        "category": "",
+        "category": "%data-workspace-view-container-name%",
         "icon": "$(refresh)"
       },
       {
@@ -78,12 +78,12 @@
         {
           "command": "dataworkspace.refresh",
           "when": "view == dataworkspace.views.main",
-          "group": "1_currentWorkspace"
+          "group": "navigation"
         },
         {
           "command": "dataworkspace.close",
           "when": "view == dataworkspace.views.main && workbenchState == workspace",
-          "group": "2_commands"
+          "group": "1_commands"
         },
         {
           "command": "projects.new",

extensions/data-workspace/src/common/dataWorkspaceExtension.ts

@@ -13,8 +13,8 @@ export class DataWorkspaceExtension implements IExtension {
 	constructor(private workspaceService: WorkspaceService) {
 	}
 
-	getProjectsInWorkspace(ext?: string): Promise<vscode.Uri[]> {
-		return this.workspaceService.getProjectsInWorkspace(ext);
+	getProjectsInWorkspace(ext?: string, refreshFromDisk?: boolean): Promise<vscode.Uri[]> {
+		return this.workspaceService.getProjectsInWorkspace(ext, refreshFromDisk);
 	}
 
 	addProjectsToWorkspace(projectFiles: vscode.Uri[]): Promise<void> {

extensions/data-workspace/src/common/interfaces.ts

@@ -50,8 +50,10 @@ export interface IWorkspaceService {
 
 	/**
 	 * Gets the project files in current workspace
+	 * @param ext project extension to filter on. If this is passed in, this will only return projects with this file extension
+	 * @param refreshFromDisk whether to rescan the folder for project files, or return the cached version. Defaults to false.
 	 */
-	getProjectsInWorkspace(): Promise<vscode.Uri[]>;
+	getProjectsInWorkspace(ext?: string, refreshFromDisk?: boolean): Promise<vscode.Uri[]>;
 
 	/**
 	 * Gets the project provider by project file

extensions/data-workspace/src/common/promise.ts

@@ -0,0 +1,25 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the Source EULA. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+/**
+ * Deferred promise
+ */
+export class Deferred<T = void> {
+	promise: Promise<T>;
+	resolve!: (value: T | PromiseLike<T>) => void;
+	reject!: (reason?: any) => void;
+	constructor() {
+		this.promise = new Promise<T>((resolve, reject) => {
+			this.resolve = resolve;
+			this.reject = reject;
+		});
+	}
+
+	then<TResult>(onfulfilled?: (value: T) => TResult | Thenable<TResult>, onrejected?: (reason: any) => TResult | Thenable<TResult>): Thenable<TResult>;
+	then<TResult>(onfulfilled?: (value: T) => TResult | Thenable<TResult>, onrejected?: (reason: any) => void): Thenable<TResult>;
+	then<TResult>(onfulfilled?: (value: T) => TResult | Thenable<TResult>, onrejected?: (reason: any) => TResult | Thenable<TResult>): Thenable<TResult> {
+		return this.promise.then(onfulfilled, onrejected);
+	}
+}

extensions/data-workspace/src/common/workspaceTreeDataProvider.ts

@@ -23,7 +23,8 @@ export class WorkspaceTreeDataProvider implements vscode.TreeDataProvider<Worksp
 	private _onDidChangeTreeData: vscode.EventEmitter<void | WorkspaceTreeItem | null | undefined> | undefined = new vscode.EventEmitter<WorkspaceTreeItem | undefined | void>();
 	readonly onDidChangeTreeData?: vscode.Event<void | WorkspaceTreeItem | null | undefined> | undefined = this._onDidChangeTreeData?.event;
 
-	refresh(): void {
+	async refresh(): Promise<void> {
+		await this._workspaceService.getProjectsInWorkspace(undefined, true);
 		this._onDidChangeTreeData?.fire();
 	}
 
@@ -38,7 +39,7 @@ export class WorkspaceTreeDataProvider implements vscode.TreeDataProvider<Worksp
 		}
 		else {
 			// if the element is undefined return the project tree items
-			const projects = await this._workspaceService.getProjectsInWorkspace();
+			const projects = await this._workspaceService.getProjectsInWorkspace(undefined, false);
 			await vscode.commands.executeCommand('setContext', 'isProjectsViewEmpty', projects.length === 0);
 			const unknownProjects: string[] = [];
 			const treeItems: WorkspaceTreeItem[] = [];

extensions/data-workspace/src/dataworkspace.d.ts

@@ -18,8 +18,9 @@ declare module 'dataworkspace' {
 		/**
 		 * Returns all the projects in the workspace
 		 * @param ext project extension to filter on. If this is passed in, this will only return projects with this file extension
+		 * @param refreshFromDisk whether to rescan the folder for project files, or return the cached version. Defaults to false.
 		 */
-		getProjectsInWorkspace(ext?: string): Promise<vscode.Uri[]>;
+		getProjectsInWorkspace(ext?: string, refreshFromDisk?: boolean): Promise<vscode.Uri[]>;
 
 		/**
 		 * Add projects to the workspace

extensions/data-workspace/src/main.ts

@@ -22,8 +22,8 @@ export async function activate(context: vscode.ExtensionContext): Promise<IExten
 	const workspaceService = new WorkspaceService();
 
 	const workspaceTreeDataProvider = new WorkspaceTreeDataProvider(workspaceService);
-	context.subscriptions.push(vscode.workspace.onDidChangeWorkspaceFolders(() => {
-		workspaceTreeDataProvider.refresh();
+	context.subscriptions.push(vscode.workspace.onDidChangeWorkspaceFolders(async () => {
+		await workspaceTreeDataProvider.refresh();
 	}));
 	const dataWorkspaceExtension = new DataWorkspaceExtension(workspaceService);
 	context.subscriptions.push(vscode.window.registerTreeDataProvider('dataworkspace.views.main', workspaceTreeDataProvider));
@@ -54,8 +54,8 @@ export async function activate(context: vscode.ExtensionContext): Promise<IExten
 		}
 	}));
 
-	context.subscriptions.push(vscode.commands.registerCommand('dataworkspace.refresh', () => {
-		workspaceTreeDataProvider.refresh();
+	context.subscriptions.push(vscode.commands.registerCommand('dataworkspace.refresh', async () => {
+		await workspaceTreeDataProvider.refresh();
 	}));
 
 	context.subscriptions.push(vscode.commands.registerCommand('dataworkspace.close', () => {

extensions/data-workspace/src/services/workspaceService.ts

@@ -13,13 +13,18 @@ import { IWorkspaceService } from '../common/interfaces';
 import { ProjectProviderRegistry } from '../common/projectProviderRegistry';
 import Logger from '../common/logger';
 import { TelemetryReporter, TelemetryViews, TelemetryActions } from '../common/telemetry';
+import { Deferred } from '../common/promise';
 import { getAzdataApi } from '../common/utils';
 
 export class WorkspaceService implements IWorkspaceService {
 	private _onDidWorkspaceProjectsChange: vscode.EventEmitter<void> = new vscode.EventEmitter<void>();
 	readonly onDidWorkspaceProjectsChange: vscode.Event<void> = this._onDidWorkspaceProjectsChange?.event;
 
-	constructor() { }
+	private openedProjects: vscode.Uri[] = [];
+
+	constructor() {
+		this.getProjectsInWorkspace(undefined, true);
+	}
 
 	get isProjectProviderAvailable(): boolean {
 		for (const extension of vscode.extensions.all) {
@@ -49,46 +54,53 @@ export class WorkspaceService implements IWorkspaceService {
 		}
 	}
 
-	async addProjectsToWorkspace(projectFiles: vscode.Uri[]): Promise<void> {
-		if (!projectFiles || projectFiles.length === 0) {
-			return;
-		}
+	public async addProjectsToWorkspace(projectFiles: vscode.Uri[]): Promise<void> {
+		// 1. Include new workspace folders if any of the new projects' locations aren't already included
 
-		const currentProjects: vscode.Uri[] = await this.getProjectsInWorkspace();
 		const newWorkspaceFolders: string[] = [];
-		let newProjectFileAdded = false;
+
 		for (const projectFile of projectFiles) {
-			if (currentProjects.findIndex((p: vscode.Uri) => p.fsPath === projectFile.fsPath) === -1) {
-				currentProjects.push(projectFile);
-				newProjectFileAdded = true;
+			const relativePath = vscode.workspace.asRelativePath(projectFile, false);
 
-				TelemetryReporter.createActionEvent(TelemetryViews.WorkspaceTreePane, TelemetryActions.ProjectAddedToWorkspace)
-					.withAdditionalProperties({
-						projectType: path.extname(projectFile.fsPath)
-					}).send();
-
-				// if the relativePath and the original path is the same, that means the project file is not under
-				// any workspace folders, we should add the parent folder of the project file to the workspace
-				const relativePath = vscode.workspace.asRelativePath(projectFile, false);
-				if (vscode.Uri.file(relativePath).fsPath === projectFile.fsPath) {
-					newWorkspaceFolders.push(path.dirname(projectFile.path));
-				}
-			} else {
-				vscode.window.showInformationMessage(constants.ProjectAlreadyOpened(projectFile.fsPath));
+			if (relativePath === undefined || vscode.Uri.file(relativePath).fsPath === projectFile.fsPath) {
+				newWorkspaceFolders.push(path.dirname(projectFile.path));
 			}
 		}
 
-		if (newProjectFileAdded) {
-			this._onDidWorkspaceProjectsChange.fire();
-		}
-
 		if (newWorkspaceFolders.length > 0) {
 			// Add to the end of the workspace folders to avoid a restart of the extension host if we can
 			vscode.workspace.updateWorkspaceFolders(vscode.workspace.workspaceFolders?.length || 0, undefined, ...(newWorkspaceFolders.map(folder => ({ uri: vscode.Uri.file(folder) }))));
 		}
+
+		// 2. Re-detect projects from the updated set of workspace folders
+
+		const previousProjects: string[] = await (await this.getProjectsInWorkspace(undefined, true)).map(p => p.path);
+		let newProjectAdded: boolean = false;
+		const projectsAlreadyOpen: string[] = [];
+
+		for (const projectFile of projectFiles) {
+			if (previousProjects.includes(projectFile.path)) {
+				projectsAlreadyOpen.push(projectFile.fsPath);
+				vscode.window.showInformationMessage(constants.ProjectAlreadyOpened(projectFile.fsPath));
+			}
+			else {
+				newProjectAdded = true;
+
+				TelemetryReporter.createActionEvent(TelemetryViews.WorkspaceTreePane, TelemetryActions.ProjectAddedToWorkspace)
+					.withAdditionalProperties({
+						projectType: path.extname(projectFile.fsPath)
+					}).send();
+			}
+		}
+
+		// 3. If any new projects are detected, fire event to refresh projects tree
+
+		if (newProjectAdded) {
+			this._onDidWorkspaceProjectsChange.fire();
+		}
 	}
 
-	async getAllProjectTypes(): Promise<dataworkspace.IProjectType[]> {
+	public async getAllProjectTypes(): Promise<dataworkspace.IProjectType[]> {
 		await this.ensureProviderExtensionLoaded();
 		const projectTypes: dataworkspace.IProjectType[] = [];
 		ProjectProviderRegistry.providers.forEach(provider => {
@@ -97,19 +109,46 @@ export class WorkspaceService implements IWorkspaceService {
 		return projectTypes;
 	}
 
-	async getProjectsInWorkspace(ext?: string): Promise<vscode.Uri[]> {
-		const projectPromises = vscode.workspace.workspaceFolders?.map(f => this.getAllProjectsInFolder(f.uri));
-		if (!projectPromises) {
-			return [];
+	private getProjectsPromise: Deferred<void> | undefined = undefined;
+
+	/**
+	 * Returns all the projects in the workspace
+	 * @param ext project extension to filter on. If this is passed in, this will only return projects with this file extension
+	 * @param refreshFromDisk whether to rescan the folder for project files, or return the cached version. Defaults to false.
+	 * @returns array of file URIs for projects
+	 */
+	public async getProjectsInWorkspace(ext?: string, refreshFromDisk: boolean = false): Promise<vscode.Uri[]> {
+
+		if (refreshFromDisk || this.openedProjects.length === 0) { // always check if nothing cached
+			await this.refreshProjectsFromDisk();
 		}
-		let projects = (await Promise.all(projectPromises)).reduce((prev, curr) => prev.concat(curr), []);
 
 		// filter by specified extension
 		if (ext) {
-			projects = projects.filter(p => p.fsPath.toLowerCase().endsWith(ext.toLowerCase()));
+			return this.openedProjects.filter(p => p.fsPath.toLowerCase().endsWith(ext.toLowerCase()));
+		} else {
+			return this.openedProjects;
+		}
+	}
+
+	private async refreshProjectsFromDisk(): Promise<void> {
+		// Only allow one disk scan to be happening at a time
+		if (this.getProjectsPromise) {
+			return this.getProjectsPromise.promise;
 		}
 
-		return projects;
+		this.getProjectsPromise = new Deferred();
+
+		try {
+			const projectPromises = vscode.workspace.workspaceFolders?.map(f => this.getAllProjectsInFolder(f.uri)) ?? [];
+			this.openedProjects = (await Promise.all(projectPromises)).reduce((prev, curr) => prev.concat(curr), []);
+			this.getProjectsPromise.resolve();
+		} catch (err) {
+			this.getProjectsPromise.reject(err);
+			throw err;
+		} finally {
+			this.getProjectsPromise = undefined;
+		}
 	}
 
 	/**

extensions/data-workspace/src/test/workspaceService.test.ts

@@ -44,19 +44,19 @@ suite('WorkspaceService', function (): void {
 
 	test('getProjectsInWorkspace', async () => {
 		// No workspace is loaded
-		let projects = await service.getProjectsInWorkspace();
+		let projects = await service.getProjectsInWorkspace(undefined, true);
 		should.strictEqual(projects.length, 0, 'no projects should be returned when no workspace is loaded');
 
 		// No projects are present in the workspace file
 		const workspaceFoldersStub = sinon.stub(vscode.workspace, 'workspaceFolders').value([]);
-		projects = await service.getProjectsInWorkspace();
+		projects = await service.getProjectsInWorkspace(undefined, true);
 		should.strictEqual(projects.length, 0, 'no projects should be returned when projects are present in the workspace file');
 		workspaceFoldersStub.restore();
 
 		// Projects are present
 		sinon.stub(vscode.workspace, 'workspaceFolders').value([{ uri: vscode.Uri.file('')}]);
 		sinon.stub(service, 'getAllProjectsInFolder').resolves([vscode.Uri.file('/test/folder/abc.sqlproj'), vscode.Uri.file('/test/folder/folder1/abc1.sqlproj'), vscode.Uri.file('/test/folder/folder2/abc2.sqlproj')]);
-		projects = await service.getProjectsInWorkspace();
+		projects = await service.getProjectsInWorkspace(undefined, true);
 		should.strictEqual(projects.length, 3, 'there should be 3 projects');
 		const project1 = vscode.Uri.file('/test/folder/abc.sqlproj');
 		const project2 = vscode.Uri.file('/test/folder/folder1/abc1.sqlproj');

extensions/data-workspace/src/test/workspaceTreeDataProvider.test.ts

@@ -20,12 +20,12 @@ suite('workspaceTreeDataProvider Tests', function (): void {
 		sinon.restore();
 	});
 
-	test('test refresh()', () => {
+	test('test refresh()', async () => {
 		const treeDataChangeHandler = sinon.stub();
 		treeProvider.onDidChangeTreeData!((e) => {
 			treeDataChangeHandler(e);
 		});
-		treeProvider.refresh();
+		await treeProvider.refresh();
 		should.strictEqual(treeDataChangeHandler.calledOnce, true);
 	});
 
@@ -100,16 +100,17 @@ suite('workspaceTreeDataProvider Tests', function (): void {
 			}],
 			getDashboardComponents: (projectFile: string): IDashboardTable[] => {
 				return [{
-				name: 'Deployments',
-				columns: [{ displayName: 'c1', width: 75, type: 'string' }],
-				data: [['d1']]
-			},
-			{
-				name: 'Builds',
-				columns: [{ displayName: 'c1', width: 75, type: 'string' }],
-				data: [['d1']]
-			}];
-		}};
+					name: 'Deployments',
+					columns: [{ displayName: 'c1', width: 75, type: 'string' }],
+					data: [['d1']]
+				},
+				{
+					name: 'Builds',
+					columns: [{ displayName: 'c1', width: 75, type: 'string' }],
+					data: [['d1']]
+				}];
+			}
+		};
 		const getProjectProviderStub = sinon.stub(workspaceService, 'getProjectProvider');
 		getProjectProviderStub.onFirstCall().resolves(undefined);
 		getProjectProviderStub.onSecondCall().resolves(projectProvider);

extensions/sql-database-projects/src/test/dialogs/addDatabaseReferenceDialog.test.ts

@@ -23,7 +23,7 @@ describe('Add Database Reference Dialog', () => {
 
 	beforeEach(function (): void {
 		const dataWorkspaceMock = TypeMoq.Mock.ofType<dataworkspace.IExtension>();
-		dataWorkspaceMock.setup(x => x.getProjectsInWorkspace(TypeMoq.It.isAny())).returns(() => Promise.resolve([]));
+		dataWorkspaceMock.setup(x => x.getProjectsInWorkspace(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve([]));
 		sinon.stub(vscode.extensions, 'getExtension').returns(<any>{ exports: dataWorkspaceMock.object });
 	});
 

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

@@ -597,7 +597,7 @@ describe('ProjectsController', function (): void {
 			const project2 = await Project.openProject(vscode.Uri.file(projPath2).fsPath);
 			const showErrorMessageSpy = sinon.spy(vscode.window, 'showErrorMessage');
 			const dataWorkspaceMock = TypeMoq.Mock.ofType<dataworkspace.IExtension>();
-			dataWorkspaceMock.setup(x => x.getProjectsInWorkspace(TypeMoq.It.isAny())).returns(() => Promise.resolve([vscode.Uri.file(project1.projectFilePath), vscode.Uri.file(project2.projectFilePath)]));
+			dataWorkspaceMock.setup(x => x.getProjectsInWorkspace(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve([vscode.Uri.file(project1.projectFilePath), vscode.Uri.file(project2.projectFilePath)]));
 			sinon.stub(vscode.extensions, 'getExtension').returns(<any>{ exports: dataWorkspaceMock.object });
 
 			// add project reference from project1 to project2