From 16c27baf105e9064896251ce9d5b18b142c3419e Mon Sep 17 00:00:00 2001 From: Anthony Dresser Date: Mon, 15 Jun 2020 14:23:31 -0700 Subject: [PATCH] maintain scheme in insights (#10904) * maintain scheme in insights * fix compiles * fix tests * handle undefined * fix windows tests * fix imports * handle remote resources --- .../insights/insightsWidget.component.ts | 4 +-- .../browser/insightsDialogController.ts | 6 ++-- .../services/insights/common/insightsUtils.ts | 29 ++++++++++++++----- .../electron-browser/insightsUtils.test.ts | 11 +++---- 4 files changed, 32 insertions(+), 18 deletions(-) diff --git a/src/sql/workbench/contrib/dashboard/browser/widgets/insights/insightsWidget.component.ts b/src/sql/workbench/contrib/dashboard/browser/widgets/insights/insightsWidget.component.ts index 922f460b88..e3998dfd7f 100644 --- a/src/sql/workbench/contrib/dashboard/browser/widgets/insights/insightsWidget.component.ts +++ b/src/sql/workbench/contrib/dashboard/browser/widgets/insights/insightsWidget.component.ts @@ -313,9 +313,9 @@ export class InsightsWidget extends DashboardWidget implements IDashboardWidget, if (types.isStringArray(this.insightConfig.query)) { this.insightConfig.query = this.insightConfig.query.join(' '); } else if (this.insightConfig.queryFile) { - const filePath = await this.instantiationService.invokeFunction(resolveQueryFilePath, this.insightConfig.queryFile); + const fileUri: URI = await this.instantiationService.invokeFunction(resolveQueryFilePath, this.insightConfig.queryFile); - this.insightConfig.query = (await this.fileService.readFile(URI.file(filePath))).value.toString(); + this.insightConfig.query = (await this.fileService.readFile(fileUri)).value.toString(); } } } diff --git a/src/sql/workbench/services/insights/browser/insightsDialogController.ts b/src/sql/workbench/services/insights/browser/insightsDialogController.ts index 40079bc5ac..a894519842 100644 --- a/src/sql/workbench/services/insights/browser/insightsDialogController.ts +++ b/src/sql/workbench/services/insights/browser/insightsDialogController.ts @@ -58,9 +58,9 @@ export class InsightsDialogController { this._errorMessageService.showDialog(Severity.Error, nls.localize("insightsError", "Insights error"), e); }).then(() => undefined); } else if (types.isString(input.queryFile)) { - let filePath: string; + let fileUri: URI; try { - filePath = await this._instantiationService.invokeFunction(resolveQueryFilePath, input.queryFile); + fileUri = await this._instantiationService.invokeFunction(resolveQueryFilePath, input.queryFile); } catch (e) { this._notificationService.notify({ @@ -71,7 +71,7 @@ export class InsightsDialogController { } try { - let buffer = await this.fileService.readFile(URI.file(filePath)); + let buffer = await this.fileService.readFile(fileUri); this.createQuery(buffer.value.toString(), connectionProfile).catch(e => { this._errorMessageService.showDialog(Severity.Error, nls.localize("insightsError", "Insights error"), e); }); diff --git a/src/sql/workbench/services/insights/common/insightsUtils.ts b/src/sql/workbench/services/insights/common/insightsUtils.ts index 98efe95e59..8ddacaa0b6 100644 --- a/src/sql/workbench/services/insights/common/insightsUtils.ts +++ b/src/sql/workbench/services/insights/common/insightsUtils.ts @@ -10,6 +10,7 @@ import { IConfigurationResolverService } from 'vs/workbench/services/configurati import { ServicesAccessor } from 'vs/platform/instantiation/common/instantiation'; import { IFileService } from 'vs/platform/files/common/files'; import { URI } from 'vs/base/common/uri'; +import { Schemas } from 'vs/base/common/network'; /** * Resolves the given file path using the VS ConfigurationResolver service, replacing macros such as @@ -19,9 +20,11 @@ import { URI } from 'vs/base/common/uri'; * @param workspaceContextService The workspace context to use for resolving workspace vars * @param configurationResolverService The resolver service to use to resolve the vars */ -export async function resolveQueryFilePath(services: ServicesAccessor, filePath: string): Promise { +export async function resolveQueryFilePath(services: ServicesAccessor, filePath: undefined): Promise; +export async function resolveQueryFilePath(services: ServicesAccessor, filePath: string): Promise; +export async function resolveQueryFilePath(services: ServicesAccessor, filePath?: string): Promise { if (!filePath) { - return filePath; + return undefined; } const workspaceContextService = services.get(IWorkspaceContextService); @@ -31,15 +34,25 @@ export async function resolveQueryFilePath(services: ServicesAccessor, filePath: let workspaceFolders: IWorkspaceFolder[] = workspaceContextService.getWorkspace().folders; // Resolve the path using each folder in our workspace, or undefined if there aren't any // (so that non-folder vars such as environment vars still resolve) - let resolvedFilePaths = (workspaceFolders.length > 0 ? workspaceFolders : [undefined]) - .map(f => configurationResolverService.resolve(f, filePath)); + const isRemote = fileService.canHandleResource(URI.from({ scheme: Schemas.vscodeRemote })); + let resolvedFileUris = (workspaceFolders.length > 0 ? workspaceFolders : [undefined]) + .map(f => { + const uri = URI.file(configurationResolverService.resolve(f, filePath)); + if (f) { + return uri.with({ scheme: f.uri.scheme }); // ensure we maintain the correct scheme + } else if (isRemote) { + return uri.with({ scheme: Schemas.vscodeRemote }); + } else { + return uri; + } + }); // Just need a single query file so use the first we find that exists - for (const path of resolvedFilePaths) { - if (await fileService.exists(URI.file(path))) { - return path; + for (const uri of resolvedFileUris) { + if (await fileService.exists(uri)) { + return uri; } } - throw Error(localize('insightsDidNotFindResolvedFile', "Could not find query file at any of the following paths :\n {0}", resolvedFilePaths.join('\n'))); + throw Error(localize('insightsDidNotFindResolvedFile', "Could not find query file at any of the following paths :\n {0}", resolvedFileUris.join('\n'))); } diff --git a/src/sql/workbench/services/insights/test/electron-browser/insightsUtils.test.ts b/src/sql/workbench/services/insights/test/electron-browser/insightsUtils.test.ts index 6980bdfc47..25305cbbb1 100644 --- a/src/sql/workbench/services/insights/test/electron-browser/insightsUtils.test.ts +++ b/src/sql/workbench/services/insights/test/electron-browser/insightsUtils.test.ts @@ -3,7 +3,7 @@ * Licensed under the Source EULA. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { equal, fail } from 'assert'; +import { ok, fail } from 'assert'; import * as os from 'os'; import { resolveQueryFilePath } from 'sql/workbench/services/insights/common/insightsUtils'; @@ -23,6 +23,7 @@ import { getRandomTestPath } from 'vs/base/test/node/testUtils'; import { IProcessEnvironment } from 'vs/base/common/platform'; import { NativeWorkbenchEnvironmentService } from 'vs/workbench/services/environment/electron-browser/environmentService'; import { TestWindowConfiguration } from 'vs/workbench/test/electron-browser/workbenchTestServices'; +import { isEqual } from 'vs/base/common/resources'; class MockWorkbenchEnvironmentService extends NativeWorkbenchEnvironmentService { @@ -71,7 +72,7 @@ suite('Insights Utils tests', function () { instantiationService.set(IFileService, fileService); const resolvedPath = await instantiationService.invokeFunction(resolveQueryFilePath, queryFilePath); - equal(resolvedPath, queryFilePath); + ok(isEqual(resolvedPath, URI.file(queryFilePath))); }); test('resolveQueryFilePath resolves path correctly with workspaceRoot var and non-empty workspace containing file', async () => { @@ -101,7 +102,7 @@ suite('Insights Utils tests', function () { instantiationService.set(IFileService, fileService); const resolvedPath = await instantiationService.invokeFunction(resolveQueryFilePath, path.join('${workspaceRoot}', 'test.sql')); - equal(resolvedPath, queryFilePath); + ok(isEqual(resolvedPath, URI.file(queryFilePath))); }); test('resolveQueryFilePath throws with workspaceRoot var and non-empty workspace not containing file', async () => { @@ -198,7 +199,7 @@ suite('Insights Utils tests', function () { instantiationService.set(IFileService, fileService); const resolvedPath = await instantiationService.invokeFunction(resolveQueryFilePath, path.join('${env:TEST_PATH}', 'test.sql')); - equal(resolvedPath, queryFilePath); + ok(isEqual(resolvedPath, URI.file(queryFilePath))); }); test('resolveQueryFilePath resolves path correctly with env var and non-empty workspace', async () => { @@ -227,7 +228,7 @@ suite('Insights Utils tests', function () { instantiationService.set(IFileService, fileService); const resolvedPath = await instantiationService.invokeFunction(resolveQueryFilePath, path.join('${env:TEST_PATH}', 'test.sql')); - equal(resolvedPath, queryFilePath); + ok(isEqual(resolvedPath, URI.file(queryFilePath))); }); test('resolveQueryFilePath throws if invalid param var specified', async () => {