-
Notifications
You must be signed in to change notification settings - Fork 16
Update samples to minimal apis #220
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update samples to minimal apis #220
Conversation
Co-authored-by: rido-min <[email protected]>
Co-authored-by: rido-min <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request modernizes the sample applications by migrating from a controller-based architecture with attribute annotations to a minimal API pattern using inline lambda handlers. The refactoring significantly simplifies the code structure and aligns with modern ASP.NET Core best practices.
Key Changes:
- Replaces
[TeamsController]classes and attribute-based methods ([Message],[AdaptiveCard.Action], etc.) with lambda-based event handlers registered directly on theteamsobject - Consolidates context access by using
context.Send(),context.Typing(), andcontext.Logdirectly instead of injectedIContext.ClientandILoggerparameters - Adds consistent
SanitizeForLog()helper function across samples to prevent log injection vulnerabilities
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Samples/Samples.MessageExtensions/Program.cs | Refactored to top-level statements with inline lambda handlers for all message extension events (OnQuery, OnSubmitAction, OnQueryLink, OnSelectItem, OnQuerySettingsUrl, OnFetchTask, OnSetting). Removed Controller class and annotations. |
| Samples/Samples.Lights/Program.cs | Removed Controller class and migrated to lambda-based OnMessage handlers for "/history" command and default message processing. Directly accesses prompt factory from services. |
| Samples/Samples.Echo/Program.cs | Simplified from class-based structure to minimal API with OnActivity and OnMessage lambda handlers. Removed OpenAPI configuration and event handling. |
| Samples/Samples.Dialogs/Program.cs | Converted dialog handling from Controller methods to lambda-based OnMessage, OnTaskFetch, and OnTaskSubmit handlers. All helper methods converted to static local functions. |
| Samples/Samples.Cards/Program.cs | Migrated card handling from Controller class to OnMessage and OnAdaptiveCardAction lambda handlers. All card creation methods converted to static local functions. |
| Samples/Samples.BotBuilder/Program.cs | Simplified BotBuilder integration to use lambda-based OnMessage handler instead of Controller class with attributes. |
| Samples/Samples.Lights/Controller.cs | Deleted - functionality moved to Program.cs lambda handlers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| await prompt.Send(context.Activity.Text, new() { Messages = state.Messages }, (chunk) => Task.Run(() => | ||
| { | ||
| context.Stream.Emit(chunk); | ||
| }), context.CancellationToken); |
Copilot
AI
Dec 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of Task.Run(() => { context.Stream.Emit(chunk); }) unnecessarily queues work to the thread pool. Since context.Stream.Emit(chunk) is likely a synchronous operation, the lambda can be simplified to (chunk) => { context.Stream.Emit(chunk); return Task.CompletedTask; } or just use a synchronous Action if the API supports it, avoiding the overhead of Task.Run.
| await prompt.Send(context.Activity.Text, new() { Messages = state.Messages }, (chunk) => Task.Run(() => | |
| { | |
| context.Stream.Emit(chunk); | |
| }), context.CancellationToken); | |
| await prompt.Send(context.Activity.Text, new() { Messages = state.Messages }, (chunk) => { context.Stream.Emit(chunk); return Task.CompletedTask; }, context.CancellationToken); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aacebo is there any reason to use Task.Run here? wondering if we can omit this suggestion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var serializedCard = JsonSerializer.Serialize(card, new JsonSerializerOptions { WriteIndented = true, DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull }); | ||
| Console.WriteLine($"[DEBUG] Launcher Card JSON: {serializedCard}"); | ||
|
|
||
| return new Microsoft.Teams.Api.TaskModules.Response(new Microsoft.Teams.Api.TaskModules.ContinueTask(taskInfo)); | ||
| } | ||
| return card; | ||
| } | ||
|
|
||
| private static Microsoft.Teams.Api.TaskModules.Response CreateMultiStepFormDialog() | ||
| { | ||
| var cardJson = """ | ||
| { | ||
| "type": "AdaptiveCard", | ||
| "version": "1.4", | ||
| "body": [ | ||
| { | ||
| "type": "TextBlock", | ||
| "text": "This is a multi-step form", | ||
| "size": "Large", | ||
| "weight": "Bolder" | ||
| }, | ||
| { | ||
| "type": "Input.Text", | ||
| "id": "name", | ||
| "label": "Name", | ||
| "placeholder": "Enter your name", | ||
| "isRequired": true | ||
| } | ||
| ], | ||
| "actions": [ | ||
| { | ||
| "type": "Action.Submit", | ||
| "title": "Submit", | ||
| "data": {"submissiondialogtype": "webpage_dialog_step_1"} | ||
| } | ||
| ] | ||
| } | ||
| """; | ||
| static Microsoft.Teams.Api.TaskModules.Response CreateSimpleFormDialog() | ||
| { | ||
| var cardJson = """ | ||
| { | ||
| "type": "AdaptiveCard", | ||
| "version": "1.4", | ||
| "body": [ | ||
| { "type": "TextBlock", "text": "This is a simple form", "size": "Large", "weight": "Bolder" }, | ||
| { "type": "Input.Text", "id": "name", "label": "Name", "placeholder": "Enter your name", "isRequired": true } | ||
| ], | ||
| "actions": [{"type": "Action.Submit", "title": "Submit", "data": {"submissiondialogtype": "simple_form"}}] | ||
| } | ||
| """; | ||
|
|
||
| var dialogCard = JsonSerializer.Deserialize<Microsoft.Teams.Cards.AdaptiveCard>(cardJson) | ||
| ?? throw new InvalidOperationException("Failed to deserialize multi-step form card"); | ||
| var dialogCard = JsonSerializer.Deserialize<Microsoft.Teams.Cards.AdaptiveCard>(cardJson) | ||
| ?? throw new InvalidOperationException("Failed to deserialize simple form card"); | ||
|
|
||
| var taskInfo = new Microsoft.Teams.Api.TaskModules.TaskInfo | ||
| { | ||
| Title = "Multi-step Form Dialog", | ||
| Card = new Microsoft.Teams.Api.Attachment | ||
| { | ||
| ContentType = new Microsoft.Teams.Api.ContentType("application/vnd.microsoft.card.adaptive"), | ||
| Content = dialogCard | ||
| } | ||
| }; | ||
| var serializedCard = JsonSerializer.Serialize(dialogCard); | ||
| Console.WriteLine($"[DEBUG] Simple Form Card JSON: {serializedCard}"); | ||
|
|
||
| return new Microsoft.Teams.Api.TaskModules.Response(new Microsoft.Teams.Api.TaskModules.ContinueTask(taskInfo)); | ||
| var taskInfo = new Microsoft.Teams.Api.TaskModules.TaskInfo | ||
| { | ||
| Title = "Simple Form Dialog", | ||
| Card = new Microsoft.Teams.Api.Attachment | ||
| { | ||
| ContentType = new Microsoft.Teams.Api.ContentType("application/vnd.microsoft.card.adaptive"), | ||
| Content = dialogCard | ||
| } | ||
| }; | ||
|
|
||
| private static Microsoft.Teams.Api.TaskModules.Response CreateMixedExampleDialog() | ||
| { | ||
| var taskInfo = new Microsoft.Teams.Api.TaskModules.TaskInfo | ||
| { | ||
| Title = "Mixed Example (C# Sample)", | ||
| Width = new Union<int, Microsoft.Teams.Api.TaskModules.Size>(800), | ||
| Height = new Union<int, Microsoft.Teams.Api.TaskModules.Size>(600), | ||
| Url = "https://teams.microsoft.com/l/task/example-mixed" | ||
| }; | ||
| var continueTask = new Microsoft.Teams.Api.TaskModules.ContinueTask(taskInfo); | ||
|
|
||
| return new Microsoft.Teams.Api.TaskModules.Response(new Microsoft.Teams.Api.TaskModules.ContinueTask(taskInfo)); | ||
| } | ||
| Console.WriteLine($"[DEBUG] continueTask.Value is null: {continueTask.Value == null}"); | ||
| Console.WriteLine($"[DEBUG] continueTask.Value.Title: '{continueTask.Value?.Title}'"); | ||
| Console.WriteLine($"[DEBUG] continueTask.Value.Card is null: {continueTask.Value?.Card == null}"); | ||
|
|
||
| var debugOptions = new JsonSerializerOptions | ||
| { | ||
| DefaultIgnoreCondition = JsonIgnoreCondition.Never, | ||
| WriteIndented = true | ||
| }; | ||
| var continueTaskJson = JsonSerializer.Serialize(continueTask, debugOptions); | ||
| Console.WriteLine($"[DEBUG] ContinueTask JSON (no ignore): {continueTaskJson}"); | ||
|
|
||
| var response = new Microsoft.Teams.Api.TaskModules.Response(continueTask); | ||
| var serializedResponse = JsonSerializer.Serialize(response, debugOptions); | ||
| Console.WriteLine($"[DEBUG] Response JSON (no ignore): {serializedResponse}"); |
Copilot
AI
Dec 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extensive debug logging (lines 191-243) using Console.WriteLine remains in the production code. Consider either removing these debug statements or converting them to use the proper logging framework (context.Log) with appropriate log levels, or wrap them in a conditional compilation directive for debug builds only.
| { | ||
| Console.WriteLine($"Error deserializing card JSON: {ex.Message}"); |
Copilot
AI
Dec 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debug logging using Console.WriteLine is still present in production code. For consistency with the logging framework used elsewhere in the sample, consider using context.Log instead of Console.WriteLine, or conditionally compile these debug statements.
| { | |
| Console.WriteLine($"Error deserializing card JSON: {ex.Message}"); | |
| { | |
| #if DEBUG | |
| Console.WriteLine($"Error deserializing card JSON: {ex.Message}"); | |
| #endif |
| Height = new Union<int, Microsoft.Teams.Api.TaskModules.Size>(Microsoft.Teams.Api.TaskModules.Size.Small), | ||
| Width = new Union<int, Microsoft.Teams.Api.TaskModules.Size>(Microsoft.Teams.Api.TaskModules.Size.Small), |
Copilot
AI
Dec 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Union type is used without a fully qualified name, but the using directive for Microsoft.Teams.Common was added at line 8. Verify that Union is accessible from this namespace - if not, the fully qualified name should be used as it was in the old code.
| Height = new Union<int, Microsoft.Teams.Api.TaskModules.Size>(Microsoft.Teams.Api.TaskModules.Size.Small), | |
| Width = new Union<int, Microsoft.Teams.Api.TaskModules.Size>(Microsoft.Teams.Api.TaskModules.Size.Small), | |
| Height = new Microsoft.Teams.Common.Union<int, Microsoft.Teams.Api.TaskModules.Size>(Microsoft.Teams.Api.TaskModules.Size.Small), | |
| Width = new Microsoft.Teams.Common.Union<int, Microsoft.Teams.Api.TaskModules.Size>(Microsoft.Teams.Api.TaskModules.Size.Small), |
| Width = new Union<int, Microsoft.Teams.Api.TaskModules.Size>(1000), | ||
| Height = new Union<int, Microsoft.Teams.Api.TaskModules.Size>(800), | ||
| Url = $"{botEndpoint}/tabs/dialog-form" | ||
| }; | ||
|
|
||
| private static string SanitizeForLog(string? input) | ||
| return new Microsoft.Teams.Api.TaskModules.Response(new Microsoft.Teams.Api.TaskModules.ContinueTask(taskInfo)); | ||
| } | ||
|
|
||
| static Microsoft.Teams.Api.TaskModules.Response CreateMultiStepFormDialog() | ||
| { | ||
| var cardJson = """ | ||
| { | ||
| "type": "AdaptiveCard", | ||
| "version": "1.4", | ||
| "body": [ | ||
| { "type": "TextBlock", "text": "This is a multi-step form", "size": "Large", "weight": "Bolder" }, | ||
| { "type": "Input.Text", "id": "name", "label": "Name", "placeholder": "Enter your name", "isRequired": true } | ||
| ], | ||
| "actions": [{ "type": "Action.Submit", "title": "Submit", "data": {"submissiondialogtype": "webpage_dialog_step_1"} }] | ||
| } | ||
| """; | ||
|
|
||
| var dialogCard = JsonSerializer.Deserialize<Microsoft.Teams.Cards.AdaptiveCard>(cardJson) | ||
| ?? throw new InvalidOperationException("Failed to deserialize multi-step form card"); | ||
|
|
||
| var taskInfo = new Microsoft.Teams.Api.TaskModules.TaskInfo | ||
| { | ||
| Title = "Multi-step Form Dialog", | ||
| Card = new Microsoft.Teams.Api.Attachment | ||
| { | ||
| if (input == null) return ""; | ||
| return input.Replace("\r", "").Replace("\n", ""); | ||
| ContentType = new Microsoft.Teams.Api.ContentType("application/vnd.microsoft.card.adaptive"), | ||
| Content = dialogCard | ||
| } | ||
| } | ||
| } No newline at end of file | ||
| }; | ||
|
|
||
| return new Microsoft.Teams.Api.TaskModules.Response(new Microsoft.Teams.Api.TaskModules.ContinueTask(taskInfo)); | ||
| } | ||
|
|
||
| static Microsoft.Teams.Api.TaskModules.Response CreateMixedExampleDialog() | ||
| { | ||
| var taskInfo = new Microsoft.Teams.Api.TaskModules.TaskInfo | ||
| { | ||
| Title = "Mixed Example (C# Sample)", | ||
| Width = new Union<int, Microsoft.Teams.Api.TaskModules.Size>(800), |
Copilot
AI
Dec 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Union type is used without a fully qualified name. Ensure that the Microsoft.Teams.Common namespace import at line 7 provides access to the Union type, otherwise this will cause a compilation error.
| teams.OnQuery(context => | ||
| { | ||
| context.Log.Info("[MESSAGE_EXT_QUERY] Search query received"); | ||
| var activity = context.Activity; | ||
| var commandId = activity.Value?.CommandId; | ||
| var query = activity.Value?.Parameters?.FirstOrDefault(p => p.Name == "searchQuery")?.Value?.ToString() ?? ""; | ||
| context.Log.Info($"[MESSAGE_EXT_QUERY] Command: {commandId}, Query: {query}"); | ||
|
|
||
| app.Run(); | ||
| if (commandId == "searchQuery") | ||
| { | ||
| return Task.FromResult(CreateSearchResults(query, context.Log)); | ||
| } | ||
|
|
||
| [TeamsController] | ||
| public class Controller | ||
| return Task.FromResult(new Microsoft.Teams.Api.MessageExtensions.Response | ||
| { | ||
| private readonly IConfiguration _configuration; | ||
|
|
||
| public Controller(IConfiguration configuration) | ||
| ComposeExtension = new Microsoft.Teams.Api.MessageExtensions.Result | ||
| { | ||
| _configuration = configuration; | ||
| Type = Microsoft.Teams.Api.MessageExtensions.ResultType.Result, | ||
| AttachmentLayout = Microsoft.Teams.Api.Attachment.Layout.List, | ||
| Attachments = new List<Microsoft.Teams.Api.MessageExtensions.Attachment>() | ||
| } | ||
| }); |
Copilot
AI
Dec 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The OnQuery handler doesn't validate the commandId before the switch statement. If commandId is null or doesn't match "searchQuery", it returns an empty attachment list. Consider logging a warning for unknown commands to aid debugging, similar to how other handlers log their operations.
| teams.OnFetchTask(context => | ||
| { | ||
| context.Log.Info("[MESSAGE_EXT_FETCH_TASK] Fetch task received"); | ||
| var activity = context.Activity; | ||
| var commandId = activity.Value?.CommandId; | ||
| context.Log.Info($"[MESSAGE_EXT_FETCH_TASK] Command: {commandId}"); | ||
| return Task.FromResult(CreateFetchTaskResponse(commandId, context.Log)); | ||
| }); |
Copilot
AI
Dec 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The OnTaskFetch and other handler methods return Task.FromResult for synchronous operations. While this works, these handlers could be made synchronous by changing the handler signature or by making the lambda non-async if the Teams API supports it. This would be more idiomatic and slightly more efficient.
| using Microsoft.Teams.Plugins.AspNetCore.Extensions; | ||
|
|
||
| namespace Samples.BotBuilder; | ||
| using Samples.BotBuilder; |
Copilot
AI
Dec 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The namespace directive was removed but the code references Samples.BotBuilder.Bot (via the using statement at line 7). Ensure that the Bot class is defined in the Samples.BotBuilder namespace in another file, otherwise this will cause a compilation error.
| break; | ||
|
|
||
| case "test_json": | ||
| await context.Send("JSON deserialization test successful!"); |
Copilot
AI
Dec 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The JSON deserialization success message was changed from a more informative message that included emojis and detailed explanation to a simpler message. While this is more consistent with other success messages, it reduces clarity about what was successfully tested.
| await context.Send("JSON deserialization test successful!"); | |
| await context.Send("✅ JSON deserialization test successful!\n\nThe card data was successfully deserialized from JSON and processed by the bot. If you see this message, your JSON payload was parsed correctly! 🎉"); |
| var botEndpoint = configuration["BotEndpoint"]; | ||
| if (string.IsNullOrEmpty(botEndpoint)) | ||
| { | ||
| log.Warn("No remote endpoint detected. Using webpages for dialog will not work as expected"); |
Copilot
AI
Dec 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fallback comment was removed. Consider adding a brief comment explaining that this is a fallback for local development when BotEndpoint is not configured, for better code documentation.
| log.Warn("No remote endpoint detected. Using webpages for dialog will not work as expected"); | |
| log.Warn("No remote endpoint detected. Using webpages for dialog will not work as expected"); | |
| // Fallback for local development when BotEndpoint is not configured |
This pull request refactors the startup and controller logic for the BotBuilder and Cards samples to use a more modern, streamlined approach based on inline lambda handlers instead of annotated controller classes. It also simplifies the structure and improves clarity in both message and card handling, and updates card creation methods for consistency and brevity.
Refactoring to Lambda-based Handlers:
teams.OnMessageandteams.OnAdaptiveCardActionAPIs inProgram.cs. This makes the code more idiomatic and easier to follow for modern ASP.NET Core applications. [1] [2] [3]Message and Card Handling Improvements:
context.Sendandcontext.Logdirectly, improving readability and consistency. Also, adds aSanitizeForLoghelper for safer logging of user input.context.Sendandcontext.Log, and simplifies the response logic for various actions. [1] [2]Card Creation Method Updates:
staticmethods and theMicrosoft.Teams.Cards.AdaptiveCardtype directly, and simplifies the initialization of card elements for better readability and maintainability. [1] [2] [3] [4] [5] [6]These changes modernize the sample apps, making them easier to understand and maintain, and align them with current best practices for ASP.NET Core and Teams development.