Core: harden enabled gateway startup validation (#66)#125
Merged
Conversation
📝 WalkthroughWalkthrough
ChangesGateway configuration validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant AddPayBridge
participant GatewayValidation
participant PaymentConfigurationException
AddPayBridge->>GatewayValidation: validate gateway configuration
GatewayValidation->>GatewayValidation: collect invalid values and missing keys
GatewayValidation->>PaymentConfigurationException: throw aggregated errors
Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens PayBridge SDK startup validation for gateway configuration to ensure misconfigured EnabledGateways/DefaultGateway fail deterministically during service registration (per Issue #66).
Changes:
- Validates
EnabledGatewaysas a strict set (rejects undefined enum values, rejectsAutomatic, rejects duplicates) and aggregates all enabled-gateway validation issues into a single startup exception. - Adds explicit startup validation for undefined
DefaultGatewayenum values. - Adds unit tests for duplicate/undefined enabled gateways and placeholder credential scenarios.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| PayBridge.SDK/Externsions/IServiceCollectionExtensions.cs | Adds stricter startup validation for enabled gateways and undefined default gateway values, including aggregated enabled-gateway errors. |
| PayBridge.SDK.Test/Unit/IServiceCollectionExtensionsTests.cs | Adds new unit tests for duplicate enabled gateways, undefined enabled gateway enum values, and placeholder credentials. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
209
to
213
| if (!IsGatewayConfigured(config, gateway)) | ||
| { | ||
| throw new PaymentConfigurationException( | ||
| errors.Add( | ||
| $"Enabled gateway '{gateway}' is missing required configuration values."); | ||
| } |
Comment on lines
+227
to
+231
| if (!Enum.IsDefined(typeof(PaymentGatewayType), config.DefaultGateway)) | ||
| { | ||
| throw new PaymentConfigurationException( | ||
| $"Default gateway value '{(int)config.DefaultGateway}' is not a defined PaymentGatewayType."); | ||
| } |
Comment on lines
+216
to
+222
| if (errors.Count > 0) | ||
| { | ||
| throw new PaymentConfigurationException( | ||
| "Invalid PaymentGatewayConfig:EnabledGateways configuration:" + | ||
| Environment.NewLine + | ||
| string.Join(Environment.NewLine, errors.Select(error => $" - {error}"))); | ||
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
PayBridge.SDK/Externsions/IServiceCollectionExtensions.cs (1)
360-418: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding a
defaultcase for future-proofing.If a new
PaymentGatewayTypeis added in the future but thisswitchstatement is not updated,GetMissingGatewayConfigurationKeyswill silently fall through and return an empty list. This would result in a confusing validation error message like:is missing required configuration values: .Adding a
defaultcase that throws an exception will help catch missed gateway configurations immediately during development.♻️ Proposed refactor
case PaymentGatewayType.PeachPayments: AddIfMissing("PaymentGatewayConfig:PeachPayments:EntityId", config.PeachPayments.EntityId); AddIfMissing("PaymentGatewayConfig:PeachPayments:AccessToken", config.PeachPayments.AccessToken); break; + default: + throw new NotImplementedException($"Missing configuration keys check is not implemented for gateway '{gateway}'."); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@PayBridge.SDK/Externsions/IServiceCollectionExtensions.cs` around lines 360 - 418, Add a default branch to the PaymentGatewayType switch in GetMissingGatewayConfigurationKeys that throws an exception for unsupported or newly added gateway values. Preserve all existing gateway-specific AddIfMissing handling while ensuring unhandled enum values fail immediately instead of returning an empty configuration-key list.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@PayBridge.SDK/Externsions/IServiceCollectionExtensions.cs`:
- Around line 360-418: Add a default branch to the PaymentGatewayType switch in
GetMissingGatewayConfigurationKeys that throws an exception for unsupported or
newly added gateway values. Preserve all existing gateway-specific AddIfMissing
handling while ensuring unhandled enum values fail immediately instead of
returning an empty configuration-key list.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 92758b8a-d251-45eb-a8ce-526c5a123ebe
📒 Files selected for processing (2)
PayBridge.SDK.Test/Unit/IServiceCollectionExtensionsTests.csPayBridge.SDK/Externsions/IServiceCollectionExtensions.cs
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.
Summary
EnabledGatewaysas a strict set: reject undefined enum values, rejectAutomatic, and reject duplicatesDefaultGatewayvalues with a clear startup errorValidation
Addresses #66
Summary by CodeRabbit