From a4c630d5d3c669fa6d0125a51bd8505605fa0726 Mon Sep 17 00:00:00 2001 From: Kevin Cunnane Date: Fri, 26 May 2017 13:11:26 -0700 Subject: [PATCH] Avoid throwing exception in LangageService for OE connections (#361) * Ensure connection open for OE queries and fix connection disposal - Dispose connection in Metadata service, to ensure we cleanly dispose and don't rely on garbage colleciton - Fixed issue where if the connection was closed, expanding databases in the Server would fail. This is because SMO doesn't always reopen the connection, certainly not for Server level queries. The solution is to always check if open and reopen. - Added unit tests for this, which required mocking the relevant IsOpen / OpenConnection methods. Refactored SMO wrapper calls into a dedicated class file to handle this * Avoid throwing exception in LangageService for OE connections - Handle non-File URIs by skipping language service binding in those cases. Added test for this - VSCode passes untitled:, git: and other paths, often without a '/'. Updated logic to handle this. * Handle windows file paths --- .../LanguageServices/AutoCompleteHelper.cs | 5 ++ .../Workspace/Contracts/ScriptFile.cs | 2 +- .../Workspace/Workspace.cs | 76 ++++++++++++++----- .../Workspace/WorkspaceTests.cs | 32 ++++++++ 4 files changed, 96 insertions(+), 19 deletions(-) diff --git a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/AutoCompleteHelper.cs b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/AutoCompleteHelper.cs index a76c3548..b702e4cd 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/AutoCompleteHelper.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/LanguageServices/AutoCompleteHelper.cs @@ -507,6 +507,11 @@ namespace Microsoft.SqlTools.ServiceLayer.LanguageServices if (scriptInfo.IsConnected) { var scriptFile = AutoCompleteHelper.WorkspaceServiceInstance.Workspace.GetFile(info.OwnerUri); + if (scriptFile == null) + { + return; + } + LanguageService.Instance.ParseAndBind(scriptFile, info); if (Monitor.TryEnter(scriptInfo.BuildingMetadataLock, LanguageService.OnConnectionWaitTimeout)) diff --git a/src/Microsoft.SqlTools.ServiceLayer/Workspace/Contracts/ScriptFile.cs b/src/Microsoft.SqlTools.ServiceLayer/Workspace/Contracts/ScriptFile.cs index 275d9a51..fc7dc1b3 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/Workspace/Contracts/ScriptFile.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/Workspace/Contracts/ScriptFile.cs @@ -121,7 +121,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace.Contracts FilePath = filePath; ClientFilePath = clientFilePath; IsAnalysisEnabled = true; - IsInMemory = Workspace.IsPathInMemory(filePath); + IsInMemory = Workspace.IsPathInMemoryOrNonFileUri(filePath); SetFileContents(textReader.ReadToEnd()); } diff --git a/src/Microsoft.SqlTools.ServiceLayer/Workspace/Workspace.cs b/src/Microsoft.SqlTools.ServiceLayer/Workspace/Workspace.cs index 6853401d..139e2be0 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/Workspace/Workspace.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/Workspace/Workspace.cs @@ -23,6 +23,13 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace { #region Private Fields + private static readonly HashSet fileUriSchemes = new HashSet(StringComparer.OrdinalIgnoreCase) + { + "file", + "untitled", + "tsqloutput" + }; + private Dictionary workspaceFiles = new Dictionary(); #endregion @@ -81,7 +88,11 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace public virtual ScriptFile GetFile(string filePath) { Validate.IsNotNullOrWhitespaceString("filePath", filePath); - + if (IsNonFileUri(filePath)) + { + return null; + } + // Resolve the full file path string resolvedFilePath = this.ResolveFilePath(filePath); string keyName = resolvedFilePath.ToLower(); @@ -108,7 +119,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace private string ResolveFilePath(string filePath) { - if (!IsPathInMemory(filePath)) + if (!IsPathInMemoryOrNonFileUri(filePath)) { if (filePath.StartsWith(@"file://")) { @@ -141,20 +152,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace return filePath; } - - internal static bool IsPathInMemory(string filePath) - { - // When viewing SqlTools files in the Git diff viewer, VS Code - // sends the contents of the file at HEAD with a URI that starts - // with 'inmemory'. Untitled files which have been marked of - // type SqlTools have a path starting with 'untitled'. - return - filePath.StartsWith("inmemory:") || - filePath.StartsWith("tsqloutput:") || - filePath.StartsWith("git:") || - filePath.StartsWith("untitled:"); - } - + /// /// Unescapes any escaped [, ] or space characters. Typically use this before calling a /// .NET API that doesn't understand PowerShell escaped chars. @@ -181,6 +179,10 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace public ScriptFile GetFileBuffer(string filePath, string initialBuffer) { Validate.IsNotNullOrWhitespaceString("filePath", filePath); + if (IsNonFileUri(filePath)) + { + return null; + } // Resolve the full file path string resolvedFilePath = this.ResolveFilePath(filePath); @@ -222,7 +224,7 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace internal string GetBaseFilePath(string filePath) { - if (IsPathInMemory(filePath)) + if (IsPathInMemoryOrNonFileUri(filePath)) { // If the file is in memory, use the workspace path return this.WorkspacePath; @@ -257,7 +259,45 @@ namespace Microsoft.SqlTools.ServiceLayer.Workspace relativePath)); return combinedPath; - } + } + internal static bool IsPathInMemoryOrNonFileUri(string path) + { + string scheme = GetScheme(path); + if (!string.IsNullOrEmpty(scheme)) + { + return !scheme.Equals("file"); + } + return false; + } + + public static string GetScheme(string uri) + { + string windowsFilePattern = @"^(?:[\w]\:|\\)"; + if (Regex.IsMatch(uri, windowsFilePattern)) + { + // Handle windows paths, these conflict with other "URI" handling + return null; + } + + // Match anything that starts with xyz:, as VSCode send URIs in the format untitled:, git: etc. + string pattern = "^([a-z][a-z0-9+.-]*):"; + Match match = Regex.Match(uri, pattern); + if (match != null && match.Success) + { + return match.Groups[1].Value; + } + return null; + } + + private bool IsNonFileUri(string path) + { + string scheme = GetScheme(path); + if (!string.IsNullOrEmpty(scheme)) + { + return !fileUriSchemes.Contains(scheme); ; + } + return false; + } #endregion diff --git a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Workspace/WorkspaceTests.cs b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Workspace/WorkspaceTests.cs index 39e24373..67093634 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Workspace/WorkspaceTests.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.UnitTests/Workspace/WorkspaceTests.cs @@ -196,5 +196,37 @@ namespace Microsoft.SqlTools.ServiceLayer.UnitTests.Workspace // verify the file is being tracked by workspace Assert.True(workspaceService.Workspace.ContainsFile(TestObjects.ScriptUri)); } + + [Fact] + public void DontBindToObjectExplorerConnectEvents() + { + // when I ask for a non-file object in the workspace, it should return null + var workspace = new ServiceLayer.Workspace.Workspace(); + ScriptFile file = workspace.GetFile("objectexplorer://server;database=database;user=user"); + Assert.Null(file); + + // when I ask for a file, it should return the file + string tempFile = Path.GetTempFileName(); + string fileContents = "hello world"; + File.WriteAllText(tempFile, fileContents); + + file = workspace.GetFile(tempFile); + Assert.Equal(fileContents, file.Contents); + + if (tempFile.StartsWith("/")) + { + tempFile = tempFile.Substring(1); + } + file = workspace.GetFile("file://" + tempFile); + Assert.Equal(fileContents, file.Contents); + + file = workspace.GetFileBuffer("untitled://"+ tempFile, fileContents); + Assert.Equal(fileContents, file.Contents); + + // For windows files, just check scheme is null since it's hard to mock file contents in these + Assert.Null(ServiceLayer.Workspace.Workspace.GetScheme(@"C:\myfile.sql")); + Assert.Null(ServiceLayer.Workspace.Workspace.GetScheme(@"\\myfile.sql")); + } + } }