From c0a0f27e49eb850e1b02a4e0552666db11ea6242 Mon Sep 17 00:00:00 2001 From: Lewis Sanchez <87730006+lewis-sanchez@users.noreply.github.com> Date: Thu, 31 Aug 2023 16:28:05 -0700 Subject: [PATCH] Return context after it has been generated (#2194) * Emit generate scripts complete event to client * Rename Message to ErrorMessage * Sets owner URI for complete params obj * Setting complete flag explicitly * Making errorMessage prop nullable * Localizes error messages * Return context scripts and remove script tabs * Send event when script gen isn't needed * Change notification to request endpoint * test get context when context doesn't exist * Stop reading old context files --- .../Localization/sr.cs | 19 ++++ .../Localization/sr.resx | 9 ++ .../Localization/sr.strings | 7 ++ .../Localization/sr.xlf | 13 ++- ...rateServerContextualizationNotification.cs | 26 ------ .../GenerateServerContextualizationRequest.cs | 34 ++++++++ .../GetServerContextualizationRequest.cs | 4 +- .../Metadata/MetadataService.cs | 87 ++++++++++++------- .../Metadata/SmoScripterHelpers.cs | 6 +- .../Metadata/MetadataServiceTests.cs | 46 +++++++--- 10 files changed, 174 insertions(+), 77 deletions(-) delete mode 100644 src/Microsoft.SqlTools.ServiceLayer/Metadata/Contracts/GenerateServerContextualizationNotification.cs create mode 100644 src/Microsoft.SqlTools.ServiceLayer/Metadata/Contracts/GenerateServerContextualizationRequest.cs diff --git a/src/Microsoft.SqlTools.ServiceLayer/Localization/sr.cs b/src/Microsoft.SqlTools.ServiceLayer/Localization/sr.cs index 30a1a75989..a4ad4361e1 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/Localization/sr.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/Localization/sr.cs @@ -437,6 +437,14 @@ public static string SqlCmdUnsupportedToken } } + public static string FailedToGenerateServerContextualizationScripts + { + get + { + return Keys.GetString(Keys.FailedToGenerateServerContextualizationScripts); + } + } + public static string PeekDefinitionNoResultsError { get @@ -10862,6 +10870,11 @@ public static string ScriptTooLarge(int maxChars, int currentChars) return Keys.GetString(Keys.ScriptTooLarge, maxChars, currentChars); } + public static string WritingServerContextualizationToCacheError(string message) + { + return Keys.GetString(Keys.WritingServerContextualizationToCacheError, message); + } + public static string SerializationServiceUnsupportedFormat(string formatName) { return Keys.GetString(Keys.SerializationServiceUnsupportedFormat, formatName); @@ -11431,6 +11444,12 @@ public class Keys public const string ScriptTooLarge = "ScriptTooLarge"; + public const string WritingServerContextualizationToCacheError = "WritingServerContextualizationToCacheError"; + + + public const string FailedToGenerateServerContextualizationScripts = "FailedToGenerateServerContextualizationScripts"; + + public const string SerializationServiceUnsupportedFormat = "SerializationServiceUnsupportedFormat"; diff --git a/src/Microsoft.SqlTools.ServiceLayer/Localization/sr.resx b/src/Microsoft.SqlTools.ServiceLayer/Localization/sr.resx index 1c61994c77..2a43274e9c 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/Localization/sr.resx +++ b/src/Microsoft.SqlTools.ServiceLayer/Localization/sr.resx @@ -426,6 +426,15 @@ . Parameters: 0 - maxChars (int), 1 - currentChars (int) + + An error was encountered while writing server contextualization scripts to the cache. Error: {0} + . + Parameters: 0 - message (string) + + + Failed to generate server contextualization scripts + + Unsupported Save Format: {0} . diff --git a/src/Microsoft.SqlTools.ServiceLayer/Localization/sr.strings b/src/Microsoft.SqlTools.ServiceLayer/Localization/sr.strings index ac181ec3f3..10eea2edf8 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/Localization/sr.strings +++ b/src/Microsoft.SqlTools.ServiceLayer/Localization/sr.strings @@ -188,6 +188,13 @@ ParsingErrorHeader (int lineNumber, int columnNumber) = Line {0}, column {1} ScriptTooLarge (int maxChars, int currentChars) = The current script is too large for Parameterization for Always Encrypted, please disable Parameterization for Always Encrypted in Query Options (Query > Query Options > Execution > Advanced). Maximum allowable length: {0} characters, Current script length: {1} characters +############################################################################ +# Server Contextualization Service + +WritingServerContextualizationToCacheError (string message) = An error was encountered while writing server contextualization scripts to the cache. Error: {0} + +FailedToGenerateServerContextualizationScripts = Failed to generate server contextualization scripts + ############################################################################ # Serialization Service diff --git a/src/Microsoft.SqlTools.ServiceLayer/Localization/sr.xlf b/src/Microsoft.SqlTools.ServiceLayer/Localization/sr.xlf index fc751f35c9..75ce095fd6 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/Localization/sr.xlf +++ b/src/Microsoft.SqlTools.ServiceLayer/Localization/sr.xlf @@ -1,4 +1,4 @@ - + @@ -7248,6 +7248,17 @@ The Query Processor estimates that implementing the following index could improv Error parsing ScriptingListObjectsCompleteParams.ConnectionString property. + + An error was encountered while writing server contextualization scripts to the cache. Error: {0} + An error was encountered while writing server contextualization scripts to the cache. Error: {0} + . + Parameters: 0 - message (string) + + + Failed to generate server contextualization scripts + Failed to generate server contextualization scripts + + \ No newline at end of file diff --git a/src/Microsoft.SqlTools.ServiceLayer/Metadata/Contracts/GenerateServerContextualizationNotification.cs b/src/Microsoft.SqlTools.ServiceLayer/Metadata/Contracts/GenerateServerContextualizationNotification.cs deleted file mode 100644 index 8285d0051f..0000000000 --- a/src/Microsoft.SqlTools.ServiceLayer/Metadata/Contracts/GenerateServerContextualizationNotification.cs +++ /dev/null @@ -1,26 +0,0 @@ -// -// Copyright (c) Microsoft. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. -// - -using Microsoft.SqlTools.Hosting.Protocol.Contracts; - -namespace Microsoft.SqlTools.ServiceLayer.Metadata.Contracts -{ - public class GenerateServerContextualizationParams - { - /// - /// The URI of the connection to generate context for. - /// - public string OwnerUri { get; set; } - } - - /// - /// Event set after a connection to a server is completed. - /// - public class GenerateServerContextualizationNotification - { - public static readonly EventType Type = - EventType.Create("metadata/generateServerContext"); - } -} diff --git a/src/Microsoft.SqlTools.ServiceLayer/Metadata/Contracts/GenerateServerContextualizationRequest.cs b/src/Microsoft.SqlTools.ServiceLayer/Metadata/Contracts/GenerateServerContextualizationRequest.cs new file mode 100644 index 0000000000..9774c23041 --- /dev/null +++ b/src/Microsoft.SqlTools.ServiceLayer/Metadata/Contracts/GenerateServerContextualizationRequest.cs @@ -0,0 +1,34 @@ +// +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. +// + +using Microsoft.SqlTools.Hosting.Protocol.Contracts; + +namespace Microsoft.SqlTools.ServiceLayer.Metadata.Contracts +{ + public class GenerateServerContextualizationParams + { + /// + /// The URI of the connection to generate context for. + /// + public string OwnerUri { get; set; } + } + + public class GenerateServerContextualizationResult + { + /// + /// The generated server context. + /// + public string? Context { get; set; } + } + + /// + /// Generate server context request endpoint. + /// + public class GenerateServerContextualizationRequest + { + public static readonly RequestType Type = + RequestType.Create("metadata/generateServerContext"); + } +} diff --git a/src/Microsoft.SqlTools.ServiceLayer/Metadata/Contracts/GetServerContextualizationRequest.cs b/src/Microsoft.SqlTools.ServiceLayer/Metadata/Contracts/GetServerContextualizationRequest.cs index 7bb7692296..acf0464e7c 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/Metadata/Contracts/GetServerContextualizationRequest.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/Metadata/Contracts/GetServerContextualizationRequest.cs @@ -18,9 +18,9 @@ public class GetServerContextualizationParams public class GetServerContextualizationResult { /// - /// An array containing the generated server context. + /// The generated server context. /// - public string[] Context { get; set; } + public string? Context { get; set; } } public class GetServerContextualizationRequest diff --git a/src/Microsoft.SqlTools.ServiceLayer/Metadata/MetadataService.cs b/src/Microsoft.SqlTools.ServiceLayer/Metadata/MetadataService.cs index 4e6a0e4639..b46812e87f 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/Metadata/MetadataService.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/Metadata/MetadataService.cs @@ -59,7 +59,7 @@ public void InitializeService(ServiceHost serviceHost) serviceHost.SetRequestHandler(MetadataListRequest.Type, HandleMetadataListRequest, true); serviceHost.SetRequestHandler(TableMetadataRequest.Type, HandleGetTableRequest, true); serviceHost.SetRequestHandler(ViewMetadataRequest.Type, HandleGetViewRequest, true); - serviceHost.SetEventHandler(GenerateServerContextualizationNotification.Type, HandleGenerateServerContextualizationNotification, true); + serviceHost.SetRequestHandler(GenerateServerContextualizationRequest.Type, HandleGenerateServerContextualizationNotification, true); serviceHost.SetRequestHandler(GetServerContextualizationRequest.Type, HandleGetServerContextualizationRequest, true); } @@ -125,11 +125,11 @@ internal static async Task HandleGetViewRequest( /// Handles the event for generating server contextualization scripts. /// internal static Task HandleGenerateServerContextualizationNotification(GenerateServerContextualizationParams contextualizationParams, - EventContext eventContext) + RequestContext requestContext) { - _ = Task.Factory.StartNew(() => + _ = Task.Factory.StartNew(async () => { - GenerateServerContextualization(contextualizationParams); + await GenerateServerContextualization(contextualizationParams, requestContext); }, CancellationToken.None, TaskCreationOptions.None, @@ -143,7 +143,7 @@ internal static Task HandleGenerateServerContextualizationNotification(GenerateS /// database objects like tables and views. /// /// The contextualization parameters. - internal static void GenerateServerContextualization(GenerateServerContextualizationParams contextualizationParams) + internal static async Task GenerateServerContextualization(GenerateServerContextualizationParams contextualizationParams, RequestContext requestContext) { MetadataService.ConnectionServiceInstance.TryFindConnection(contextualizationParams.OwnerUri, out ConnectionInfo connectionInfo); @@ -153,26 +153,39 @@ internal static void GenerateServerContextualization(GenerateServerContextualiza { // If scripts have been generated within the last 30 days then there isn't a need to go through the process // of generating scripts again. - if (!MetadataScriptTempFileStream.IsScriptTempFileUpdateNeeded(connectionInfo.ConnectionDetails.ServerName)) - { - return; - } - - var scripts = SmoScripterHelpers.GenerateAllServerTableScripts(sqlConn); - if (scripts != null) + if (MetadataScriptTempFileStream.IsScriptTempFileUpdateNeeded(connectionInfo.ConnectionDetails.ServerName)) { - try + var scripts = SmoScripterHelpers.GenerateAllServerTableScripts(sqlConn)?.ToArray(); + if (scripts != null) { - MetadataScriptTempFileStream.Write(connectionInfo.ConnectionDetails.ServerName, scripts); + try + { + await requestContext.SendResult(new GenerateServerContextualizationResult() + { + Context = string.Join('\n', scripts) + }); + + MetadataScriptTempFileStream.Write(connectionInfo.ConnectionDetails.ServerName, scripts); + } + catch (Exception ex) + { + Logger.Error($"An error was encountered while writing server contextualization scripts to the cache. Error: {ex.Message}"); + await requestContext.SendError(ex); + } } - catch (Exception ex) + else { - Logger.Error($"An error was encountered while writing to the cache. Error: {ex.Message}"); + Logger.Error("Failed to generate server contextualization scripts"); + await requestContext.SendError(SR.FailedToGenerateServerContextualizationScripts); } } else { - Logger.Error("Failed to generate server scripts"); + var generateContextResult = new GenerateServerContextualizationResult() + { + Context = null + }; + await requestContext.SendResult(generateContextResult); } } } @@ -204,28 +217,38 @@ internal static Task HandleGetServerContextualizationRequest(GetServerContextual internal static async Task GetServerContextualization(GetServerContextualizationParams contextualizationParams, RequestContext requestContext) { MetadataService.ConnectionServiceInstance.TryFindConnection(contextualizationParams.OwnerUri, out ConnectionInfo connectionInfo); - - if (connectionInfo != null) + // When the filed context is too old don't read it + if (MetadataScriptTempFileStream.IsScriptTempFileUpdateNeeded(connectionInfo.ConnectionDetails.ServerName)) { - try + await requestContext.SendResult(new GetServerContextualizationResult { - var scripts = MetadataScriptTempFileStream.Read(connectionInfo.ConnectionDetails.ServerName); - await requestContext.SendResult(new GetServerContextualizationResult + Context = null + }); + } + else + { + if (connectionInfo != null) + { + try + { + var scripts = MetadataScriptTempFileStream.Read(connectionInfo.ConnectionDetails.ServerName).ToArray(); + await requestContext.SendResult(new GetServerContextualizationResult + { + Context = string.Join('\n', scripts) + }); + } + catch (Exception ex) { - Context = scripts.ToArray() - }); + Logger.Error("Failed to read scripts from the script cache"); + await requestContext.SendError(ex); + } } - catch (Exception ex) + else { - Logger.Error("Failed to read scripts from the script cache"); - await requestContext.SendError(ex); + Logger.Error("Failed to find connection info about the server."); + await requestContext.SendError("Failed to find connection info about the server."); } } - else - { - Logger.Error("Failed to find connection info about the server."); - await requestContext.SendError("Failed to find connection info about the server."); - } } /// diff --git a/src/Microsoft.SqlTools.ServiceLayer/Metadata/SmoScripterHelpers.cs b/src/Microsoft.SqlTools.ServiceLayer/Metadata/SmoScripterHelpers.cs index 414913a2cf..a657ea4adb 100644 --- a/src/Microsoft.SqlTools.ServiceLayer/Metadata/SmoScripterHelpers.cs +++ b/src/Microsoft.SqlTools.ServiceLayer/Metadata/SmoScripterHelpers.cs @@ -161,11 +161,11 @@ private static IEnumerable GenerateTableScripts(Server server) var scripts = new List(); foreach (var s in generatedScripts) { - // Needed to remove '\r' and '\n' characters from script, so that an entire create script + // Needed to remove '\r', '\n', and '\t' characters from script, so that an entire create script // can be written and read as a single line to and from a temp file. Since scripts aren't - // going to be read by people, and mainly sent to Copilot to generate accurate suggestions, + // going to be read by people, and sent to Copilot to generate accurate suggestions, // a lack of formatting is fine. - var script = s.Replace("\r", string.Empty).Replace("\n", string.Empty); + var script = s.Replace("\r", string.Empty).Replace("\n", string.Empty).Replace("\t", string.Empty); scripts.Add(script); } diff --git a/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/Metadata/MetadataServiceTests.cs b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/Metadata/MetadataServiceTests.cs index d3daf1854f..df02efb4ee 100644 --- a/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/Metadata/MetadataServiceTests.cs +++ b/test/Microsoft.SqlTools.ServiceLayer.IntegrationTests/Metadata/MetadataServiceTests.cs @@ -150,11 +150,22 @@ public async Task VerifyGenerateServerContextualizationNotification() OwnerUri = connectionResult.ConnectionInfo.OwnerUri }; - MetadataService.GenerateServerContextualization(generateServerContextualizationParams); + var actualGenerateServerContextResponse = new GenerateServerContextualizationResult(); + var mockGenerateRequestContext = new Mock>(); + mockGenerateRequestContext.Setup(x => x.SendResult(It.IsAny())) + .Callback(result => actualGenerateServerContextResponse = result) + .Returns(Task.CompletedTask); + await MetadataService.GenerateServerContextualization(generateServerContextualizationParams, mockGenerateRequestContext.Object); DeleteTestTable(sqlConn, this.testTableSchema, this.testTableName); DeleteTestTable(sqlConn, this.testTableSchema, this.testTableName2); + var firstCreateTableScript = $"CREATE TABLE [{this.testTableSchema}].[{this.testTableName}]([id] [int] NULL)"; + var secondCreateTableScript = $"CREATE TABLE [{this.testTableSchema}].[{this.testTableName2}]([id] [int] NULL)"; + + Assert.IsTrue(actualGenerateServerContextResponse.Context.Contains(firstCreateTableScript)); + Assert.IsTrue(actualGenerateServerContextResponse.Context.Contains(secondCreateTableScript)); + DeleteServerContextualizationTempFile(sqlConn.DataSource); } @@ -170,18 +181,8 @@ public async Task VerifyGetServerContextualizationRequest() CreateTestTable(sqlConn, this.testTableSchema, this.testTableName); CreateTestTable(sqlConn, this.testTableSchema, this.testTableName2); - var generateServerContextualizationParams = new GenerateServerContextualizationParams - { - OwnerUri = connectionResult.ConnectionInfo.OwnerUri - }; - - MetadataService.GenerateServerContextualization(generateServerContextualizationParams); - - DeleteTestTable(sqlConn, this.testTableSchema, this.testTableName); - DeleteTestTable(sqlConn, this.testTableSchema, this.testTableName2); - - var firstCreateTableScript = $"CREATE TABLE [{this.testTableSchema}].[{this.testTableName}](\t[id] [int] NULL)"; - var secondCreateTableScript = $"CREATE TABLE [{this.testTableSchema}].[{this.testTableName2}](\t[id] [int] NULL)"; + var firstCreateTableScript = $"CREATE TABLE [{this.testTableSchema}].[{this.testTableName}]([id] [int] NULL)"; + var secondCreateTableScript = $"CREATE TABLE [{this.testTableSchema}].[{this.testTableName2}]([id] [int] NULL)"; var mockGetServerContextualizationRequestContext = new Mock>(); var actualGetServerContextualizationResponse = new GetServerContextualizationResult(); @@ -193,6 +194,25 @@ public async Task VerifyGetServerContextualizationRequest() { OwnerUri = connectionResult.ConnectionInfo.OwnerUri }; + await MetadataService.GetServerContextualization(getServerContextualizationParams, mockGetServerContextualizationRequestContext.Object); + + Assert.IsNull(actualGetServerContextualizationResponse.Context); + Assert.IsNull(actualGetServerContextualizationResponse.Context); + + var generateServerContextualizationParams = new GenerateServerContextualizationParams + { + OwnerUri = connectionResult.ConnectionInfo.OwnerUri + }; + + var actualGenerateServerContextResponse = new GenerateServerContextualizationResult(); + var mockGenerateRequestContext = new Mock>(); + mockGenerateRequestContext.Setup(x => x.SendResult(It.IsAny())) + .Callback(actual => actualGenerateServerContextResponse = actual) + .Returns(Task.CompletedTask); + await MetadataService.GenerateServerContextualization(generateServerContextualizationParams, mockGenerateRequestContext.Object); + + DeleteTestTable(sqlConn, this.testTableSchema, this.testTableName); + DeleteTestTable(sqlConn, this.testTableSchema, this.testTableName2); await MetadataService.GetServerContextualization(getServerContextualizationParams, mockGetServerContextualizationRequestContext.Object);