Optimizations of the validation process#4
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This pull request focuses on eliminating duplicate message validation logic across all major connectors (Twilio SMS, Twilio WhatsApp, SendGrid Email) and centralizing validation in the base connector class. It also updates the MediaContent type to use a more appropriate content type and adjusts related tests to use actual framework classes instead of test helpers.
- Removed duplicate validation calls from concrete connector implementations, relying on centralized validation in
ChannelConnectorBase - Changed
MediaContent.ContentTypefromBinarytoMediafor better semantic accuracy - Replaced custom test helper classes with actual framework classes (
Message,Endpoint,TextContent, etc.) across all test files
Reviewed Changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| TwilioWhatsAppConnector.cs | Removed duplicate validation logic from SendMessageCoreAsync method |
| TwilioSmsConnector.cs | Removed duplicate validation logic from SendMessageCoreAsync method |
| SendGridEmailConnector.cs | Removed duplicate validation logic from SendMessageCoreAsync method |
| MediaContent.cs | Changed ContentType from Binary to Media |
| Test files | Replaced test helper classes with actual framework classes and updated assertions |
| ChannelSchemaExtensions.cs | Removed obsolete ValidateMessageProperties method |
| ChannelConnectorBase.cs | Updated ValidateMessageCoreAsync to use centralized schema validation |
| Documentation files | Added comprehensive action plan and architecture documentation |
…hatsAppConnectorExtendedMockTests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request focuses on eliminating duplicate message validation logic across all major connectors (Twilio SMS, Twilio WhatsApp, SendGrid Email) and centralizing validation in the base connector class. It also updates the
MediaContenttype to use a more appropriate content type and adjusts related tests. These changes improve performance, maintainability, and consistency in message validation, while cleaning up redundant code in connector implementations.Validation Centralization and Connector Refactoring
SendMessageCoreAsyncinTwilioSmsConnector,TwilioWhatsAppConnector, andSendGridEmailConnector, ensuring validation is only performed once in the base class (ChannelConnectorBase.SendMessageAsync). [1] [2] [3]ChannelConnectorBase.ValidateMessageCoreAsyncto delegate all validation to the schema, covering message ID, endpoints, content type, and properties.ValidateMessagePropertiesextension method fromChannelSchemaExtensions, further enforcing centralized validation.VALIDATION_DUPLICATION_FIX_ACTION_PLAN.mdoutlining the rationale, steps, testing, risks, and success criteria for the validation refactor.Media Content Type Update
ContentTypeofMediaContentfromBinarytoMediafor better semantic accuracy.MessageContentType.Mediainstead ofBinaryfor media content. [1] [2] [3] [4] [5] [6] [7]Minor Enhancements
Endpoint.Create(IEndpoint? endpoint)to simplify endpoint conversions.