From 68edbaf50e38879499cf4ad9fe2cd388618749de Mon Sep 17 00:00:00 2001 From: Gil LaHaye Date: Wed, 6 Dec 2023 20:19:08 -0800 Subject: [PATCH 1/4] Use chatId from URL rather than payload for chats --- integration-tests/ChatTests.cs | 50 ++++++++++++++++++++++++++ webapi/Controllers/ChatController.cs | 31 ++++++++++++---- webapi/Extensions/ServiceExtensions.cs | 5 --- webapi/Program.cs | 1 - webapi/Utilities/AskConverter.cs | 41 --------------------- 5 files changed, 75 insertions(+), 53 deletions(-) create mode 100644 integration-tests/ChatTests.cs delete mode 100644 webapi/Utilities/AskConverter.cs diff --git a/integration-tests/ChatTests.cs b/integration-tests/ChatTests.cs new file mode 100644 index 000000000..6a1bf8238 --- /dev/null +++ b/integration-tests/ChatTests.cs @@ -0,0 +1,50 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System.Collections.Generic; +using System.Net.Http; +using System.Net.Http.Json; +using System.Text.Json; +using CopilotChat.WebApi.Models.Request; +using CopilotChat.WebApi.Models.Response; +using Xunit; +using static CopilotChat.WebApi.Models.Storage.CopilotChatMessage; + +namespace ChatCopilotIntegrationTests; + +public class ChatTests : ChatCopilotIntegrationTest +{ + [Fact] + public async void ChatWithBot() + { + await this.SetUpAuth(); + + // Create chat session + var createChatParams = new CreateChatParameters() { Title = nameof(ChatWithBot) }; + HttpResponseMessage response = await this._httpClient.PostAsJsonAsync("chats", createChatParams); + response.EnsureSuccessStatusCode(); + + var contentStream = await response.Content.ReadAsStreamAsync(); + var createChatReponse = await JsonSerializer.DeserializeAsync(contentStream, new JsonSerializerOptions { PropertyNameCaseInsensitive = true }); + Assert.NotNull(createChatReponse); + + // Ask something to the bot + var ask = new Ask + { + Input = "Who is Satya Nadella?", + Variables = new KeyValuePair[] { new("MessageType", ChatMessageType.Message.ToString()) } + }; + response = await this._httpClient.PostAsJsonAsync($"chats/{createChatReponse.ChatSession.Id}/messages", ask); + response.EnsureSuccessStatusCode(); + + contentStream = await response.Content.ReadAsStreamAsync(); + var askResult = await JsonSerializer.DeserializeAsync(contentStream, new JsonSerializerOptions { PropertyNameCaseInsensitive = true }); + Assert.NotNull(askResult); + Assert.False(string.IsNullOrEmpty(askResult.Value)); + + + // Clean up + response = await this._httpClient.DeleteAsync($"chats/{createChatReponse.ChatSession.Id}"); + response.EnsureSuccessStatusCode(); + } +} + diff --git a/webapi/Controllers/ChatController.cs b/webapi/Controllers/ChatController.cs index 2519303b0..1cd4a9a0c 100644 --- a/webapi/Controllers/ChatController.cs +++ b/webapi/Controllers/ChatController.cs @@ -99,7 +99,6 @@ public async Task ChatAsync( [FromServices] IKernel kernel, [FromServices] IHubContext messageRelayHubContext, [FromServices] CopilotChatPlanner planner, - [FromServices] AskConverter askConverter, [FromServices] ChatSessionRepository chatSessionRepository, [FromServices] ChatParticipantRepository chatParticipantRepository, [FromServices] IAuthInfo authInfo, @@ -108,7 +107,7 @@ public async Task ChatAsync( { this._logger.LogDebug("Chat message received."); - return await this.HandleRequest(ChatFunctionName, kernel, messageRelayHubContext, planner, askConverter, chatSessionRepository, chatParticipantRepository, authInfo, ask, chatId.ToString()); + return await this.HandleRequest(ChatFunctionName, kernel, messageRelayHubContext, planner, chatSessionRepository, chatParticipantRepository, authInfo, ask, chatId.ToString()); } /// @@ -135,7 +134,6 @@ public async Task ProcessPlanAsync( [FromServices] IKernel kernel, [FromServices] IHubContext messageRelayHubContext, [FromServices] CopilotChatPlanner planner, - [FromServices] AskConverter askConverter, [FromServices] ChatSessionRepository chatSessionRepository, [FromServices] ChatParticipantRepository chatParticipantRepository, [FromServices] IAuthInfo authInfo, @@ -144,7 +142,7 @@ public async Task ProcessPlanAsync( { this._logger.LogDebug("plan request received."); - return await this.HandleRequest(ProcessPlanFunctionName, kernel, messageRelayHubContext, planner, askConverter, chatSessionRepository, chatParticipantRepository, authInfo, ask, chatId.ToString()); + return await this.HandleRequest(ProcessPlanFunctionName, kernel, messageRelayHubContext, planner, chatSessionRepository, chatParticipantRepository, authInfo, ask, chatId.ToString()); } /// @@ -166,7 +164,6 @@ private async Task HandleRequest( IKernel kernel, IHubContext messageRelayHubContext, CopilotChatPlanner planner, - AskConverter askConverter, ChatSessionRepository chatSessionRepository, ChatParticipantRepository chatParticipantRepository, IAuthInfo authInfo, @@ -174,7 +171,7 @@ private async Task HandleRequest( string chatId) { // Put ask's variables in the context we will use. - var contextVariables = askConverter.GetContextVariables(ask); + var contextVariables = GetContextVariables(ask, authInfo, chatId); // Verify that the chat exists and that the user has access to it. ChatSession? chat = null; @@ -415,6 +412,28 @@ await planner.Kernel.ImportOpenAIPluginFunctionsAsync( return; } + private static ContextVariables GetContextVariables(Ask ask, IAuthInfo authInfo, string chatId) + { + const string UserIdKey = "userId"; + const string UserNameKey = "userName"; + const string ChatIdKey = "chatId"; + + var contextVariables = new ContextVariables(ask.Input); + foreach (var variable in ask.Variables) + { + if (variable.Key != UserIdKey && variable.Key != UserNameKey) + { + contextVariables.Set(variable.Key, variable.Value); + } + } + + contextVariables.Set(UserIdKey, authInfo.UserId); + contextVariables.Set(UserNameKey, authInfo.Name); + contextVariables.Set(ChatIdKey, chatId); + + return contextVariables; + } + /// /// Dispose of the object. /// diff --git a/webapi/Extensions/ServiceExtensions.cs b/webapi/Extensions/ServiceExtensions.cs index 120821e80..a5e790aa2 100644 --- a/webapi/Extensions/ServiceExtensions.cs +++ b/webapi/Extensions/ServiceExtensions.cs @@ -81,11 +81,6 @@ internal static void AddOptions(this IServiceCollection services, ICon .PostConfigure(TrimStringProperties); } - internal static IServiceCollection AddUtilities(this IServiceCollection services) - { - return services.AddScoped(); - } - internal static IServiceCollection AddPlugins(this IServiceCollection services, IConfiguration configuration) { var plugins = configuration.GetSection("Plugins").Get>() ?? new List(); diff --git a/webapi/Program.cs b/webapi/Program.cs index eaec533c5..051e0ddcb 100644 --- a/webapi/Program.cs +++ b/webapi/Program.cs @@ -45,7 +45,6 @@ public static async Task Main(string[] args) .AddOptions(builder.Configuration) .AddPersistentChatStore() .AddPlugins(builder.Configuration) - .AddUtilities() .AddChatCopilotAuthentication(builder.Configuration) .AddChatCopilotAuthorization(); diff --git a/webapi/Utilities/AskConverter.cs b/webapi/Utilities/AskConverter.cs deleted file mode 100644 index ad911b8f3..000000000 --- a/webapi/Utilities/AskConverter.cs +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. - -using CopilotChat.WebApi.Auth; -using CopilotChat.WebApi.Models.Request; -using Microsoft.SemanticKernel.Orchestration; - -namespace CopilotChat.WebApi.Utilities; - -/// -/// Converts variables to , inserting some system variables along the way. -/// -public class AskConverter -{ - private readonly IAuthInfo _authInfo; - - public AskConverter(IAuthInfo authInfo) - { - this._authInfo = authInfo; - } - - /// - /// Converts variables to , inserting some system variables along the way. - /// - public ContextVariables GetContextVariables(Ask ask) - { - const string userIdKey = "userId"; - const string userNameKey = "userName"; - var contextVariables = new ContextVariables(ask.Input); - foreach (var variable in ask.Variables) - { - if (variable.Key != userIdKey && variable.Key != userNameKey) - { - contextVariables.Set(variable.Key, variable.Value); - } - } - - contextVariables.Set(userIdKey, this._authInfo.UserId); - contextVariables.Set(userNameKey, this._authInfo.Name); - return contextVariables; - } -} From a4afdcb6b9ae281869906c68a3c7992a9e378082 Mon Sep 17 00:00:00 2001 From: Gil LaHaye Date: Wed, 6 Dec 2023 20:29:46 -0800 Subject: [PATCH 2/4] Fix typo --- integration-tests/ChatTests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/integration-tests/ChatTests.cs b/integration-tests/ChatTests.cs index 6a1bf8238..4b1470024 100644 --- a/integration-tests/ChatTests.cs +++ b/integration-tests/ChatTests.cs @@ -24,8 +24,8 @@ public async void ChatWithBot() response.EnsureSuccessStatusCode(); var contentStream = await response.Content.ReadAsStreamAsync(); - var createChatReponse = await JsonSerializer.DeserializeAsync(contentStream, new JsonSerializerOptions { PropertyNameCaseInsensitive = true }); - Assert.NotNull(createChatReponse); + var createChatResponse = await JsonSerializer.DeserializeAsync(contentStream, new JsonSerializerOptions { PropertyNameCaseInsensitive = true }); + Assert.NotNull(createChatResponse); // Ask something to the bot var ask = new Ask @@ -33,7 +33,7 @@ public async void ChatWithBot() Input = "Who is Satya Nadella?", Variables = new KeyValuePair[] { new("MessageType", ChatMessageType.Message.ToString()) } }; - response = await this._httpClient.PostAsJsonAsync($"chats/{createChatReponse.ChatSession.Id}/messages", ask); + response = await this._httpClient.PostAsJsonAsync($"chats/{createChatResponse.ChatSession.Id}/messages", ask); response.EnsureSuccessStatusCode(); contentStream = await response.Content.ReadAsStreamAsync(); @@ -43,7 +43,7 @@ public async void ChatWithBot() // Clean up - response = await this._httpClient.DeleteAsync($"chats/{createChatReponse.ChatSession.Id}"); + response = await this._httpClient.DeleteAsync($"chats/{createChatResponse.ChatSession.Id}"); response.EnsureSuccessStatusCode(); } } From b6dc461592db0d098bf7d0363ff1ab8124966017 Mon Sep 17 00:00:00 2001 From: Gil LaHaye Date: Thu, 7 Dec 2023 10:41:32 -0800 Subject: [PATCH 3/4] Change test name --- integration-tests/ChatTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/integration-tests/ChatTests.cs b/integration-tests/ChatTests.cs index 4b1470024..6e05d5b7b 100644 --- a/integration-tests/ChatTests.cs +++ b/integration-tests/ChatTests.cs @@ -14,12 +14,12 @@ namespace ChatCopilotIntegrationTests; public class ChatTests : ChatCopilotIntegrationTest { [Fact] - public async void ChatWithBot() + public async void ChatMessagePostSucceedsWithValidInput() { await this.SetUpAuth(); // Create chat session - var createChatParams = new CreateChatParameters() { Title = nameof(ChatWithBot) }; + var createChatParams = new CreateChatParameters() { Title = nameof(ChatMessagePostSucceedsWithValidInput) }; HttpResponseMessage response = await this._httpClient.PostAsJsonAsync("chats", createChatParams); response.EnsureSuccessStatusCode(); From 017c1eaff2f0f347aaeba873f250c6c46494abdb Mon Sep 17 00:00:00 2001 From: Gil LaHaye Date: Fri, 8 Dec 2023 17:56:09 -0800 Subject: [PATCH 4/4] Address review comment --- webapi/Controllers/ChatController.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/webapi/Controllers/ChatController.cs b/webapi/Controllers/ChatController.cs index 1cd4a9a0c..22de83e09 100644 --- a/webapi/Controllers/ChatController.cs +++ b/webapi/Controllers/ChatController.cs @@ -421,10 +421,7 @@ private static ContextVariables GetContextVariables(Ask ask, IAuthInfo authInfo, var contextVariables = new ContextVariables(ask.Input); foreach (var variable in ask.Variables) { - if (variable.Key != UserIdKey && variable.Key != UserNameKey) - { - contextVariables.Set(variable.Key, variable.Value); - } + contextVariables.Set(variable.Key, variable.Value); } contextVariables.Set(UserIdKey, authInfo.UserId);