-
Notifications
You must be signed in to change notification settings - Fork 381
feat(db): implement options pattern for database connection strings #211
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
base: main
Are you sure you want to change the base?
feat(db): implement options pattern for database connection strings #211
Conversation
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 PR implements the options pattern for database connection strings across all modules, replacing direct configuration access with strongly-typed options classes that include validation.
- Introduces persistence options classes for each module (Reports, Passes, Offers, Contracts) with required connection string properties
- Updates database connection factories and modules to use IOptions pattern instead of direct IConfiguration access
- Adds validation on startup to ensure connection strings are properly configured
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
ReportsModule.cs | Updates AddReports method to accept IConfiguration parameter for options configuration |
ReportsPersistenceOptions.cs | New options class for Reports module with required connection string property |
DatabaseConnectionFactory.cs | Refactored to use IOptions instead of IConfiguration |
DataAccessModule.cs | New module file implementing options pattern with validation for Reports |
Program.cs | Updates Reports module registration to pass configuration |
PassesPersistenceOptions.cs | New options class for Passes module with required connection string property |
Passes/DatabaseModule.cs | Refactored to use options pattern with validation instead of direct configuration access |
OffersPersistenceOptions.cs | New options class for Offers module with required connection string property |
Offers/DatabaseModule.cs | Refactored to use options pattern with validation instead of direct configuration access |
Contracts/DatabaseModule.cs | Refactored to use options pattern with validation instead of direct configuration access |
ContractsPersistenceOptions.cs | New options class for Contracts module with required connection string property |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
internal sealed class ReportsPersistenceOptions | ||
{ | ||
public const string SectionName = "ConnectionStrings"; | ||
|
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 section name "ConnectionStrings" is duplicated across all persistence options classes. Consider creating a shared constant or base class to avoid code duplication and ensure consistency.
internal sealed class ReportsPersistenceOptions | |
{ | |
public const string SectionName = "ConnectionStrings"; | |
internal static class PersistenceOptionsConstants | |
{ | |
public const string SectionName = "ConnectionStrings"; | |
} | |
internal sealed class ReportsPersistenceOptions | |
{ | |
public const string SectionName = PersistenceOptionsConstants.SectionName; |
Copilot uses AI. Check for mistakes.
internal sealed class PassesPersistenceOptions | ||
{ | ||
public const string SectionName = "ConnectionStrings"; | ||
|
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 section name "ConnectionStrings" is duplicated across all persistence options classes. Consider creating a shared constant or base class to avoid code duplication and ensure consistency.
internal sealed class PassesPersistenceOptions | |
{ | |
public const string SectionName = "ConnectionStrings"; | |
internal static class PersistenceOptionsConstants | |
{ | |
public const string SectionName = "ConnectionStrings"; | |
} | |
internal sealed class PassesPersistenceOptions | |
{ | |
public const string SectionName = PersistenceOptionsConstants.SectionName; |
Copilot uses AI. Check for mistakes.
internal sealed class OffersPersistenceOptions | ||
{ | ||
public const string SectionName = "ConnectionStrings"; | ||
|
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 section name "ConnectionStrings" is duplicated across all persistence options classes. Consider creating a shared constant or base class to avoid code duplication and ensure consistency.
internal sealed class OffersPersistenceOptions | |
{ | |
public const string SectionName = "ConnectionStrings"; | |
internal static class PersistenceOptionSectionNames | |
{ | |
public const string ConnectionStrings = "ConnectionStrings"; | |
} | |
internal sealed class OffersPersistenceOptions | |
{ | |
// Use PersistenceOptionSectionNames.ConnectionStrings where needed |
Copilot uses AI. Check for mistakes.
internal sealed class ContractsPersistenceOptions | ||
{ | ||
public const string SectionName = "ConnectionStrings"; | ||
|
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 section name "ConnectionStrings" is duplicated across all persistence options classes. Consider creating a shared constant or base class to avoid code duplication and ensure consistency.
internal sealed class ContractsPersistenceOptions | |
{ | |
public const string SectionName = "ConnectionStrings"; | |
internal static class PersistenceOptionsConstants | |
{ | |
public const string SectionName = "ConnectionStrings"; | |
} | |
internal sealed class ContractsPersistenceOptions | |
{ | |
// Use PersistenceOptionsConstants.SectionName where needed |
Copilot uses AI. Check for mistakes.
• Introduce IOptions to fetch configuration data instead of leveraging IConfiguration abstraction. This makes options easier to maintain and validate. ⚙️