Skip to content

Conversation

@kamilbaczek
Copy link
Collaborator

No description provided.

…uraiton-chapter2

# Conflicts:
#	Chapter-1-initial-architecture/Src/Fitnet/Reports/DataAccess/DatabaseConnectionFactory.cs
@kamilbaczek kamilbaczek requested a review from Copilot September 4, 2025 19:51

This comment was marked as outdated.

@kamilbaczek kamilbaczek marked this pull request as ready for review September 8, 2025 19:32
@kamilbaczek kamilbaczek requested a review from Copilot September 8, 2025 19:45
Copy link

Copilot AI left a 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 multiple modules in the Fitnet application. It replaces direct configuration access with strongly-typed options classes that provide better type safety and validation.

  • Introduces PersistenceOptions classes for each module with required connection string properties
  • Updates database modules to use IOptions<T> pattern with validation
  • Adds framework references to enable options validation features

Reviewed Changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated no comments.

Show a summary per file
File Description
ReportsModule.cs (both chapters) Updated to pass configuration to data access modules
*PersistenceOptions.cs files New options classes defining connection string configurations
DatabaseModule.cs / DatabaseAccessModule.cs files Refactored to use options pattern instead of direct configuration access
DatabaseConnectionFactory.cs Updated to use options instead of IConfiguration
*.csproj files Added framework references for ASP.NET Core features
Program.cs Added comment explaining module registration pattern
Comments suppressed due to low confidence (1)

Chapter-2-modules-separation/Src/Reports/Fitnet.Reports/DataAccess/DatabaseConnectionFactory.cs:24

  • The DatabaseConnectionFactory is registered as a singleton but maintains mutable state (_connection) without thread synchronization. This could lead to race conditions when multiple threads access Create() simultaneously. Consider adding thread safety mechanisms or changing the registration lifetime.
internal sealed class DatabaseConnectionFactory(IOptions<ReportsPersistenceOptions> persistenceOptions) : IDatabaseConnectionFactory
{
    private NpgsqlConnection? _connection;

    public IDbConnection Create()
    {
        if (_connection is { State: ConnectionState.Open })
        {
            return _connection;
        }

        var connectionString = persistenceOptions.Value.Primary;
        _connection = new NpgsqlConnection(connectionString);
        _connection.Open();

        return _connection;
    }

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@kamilbaczek kamilbaczek enabled auto-merge (squash) September 10, 2025 18:19
@kamilbaczek kamilbaczek merged commit f0e5e34 into main Sep 10, 2025
1 check passed
@kamilbaczek kamilbaczek deleted the feature/introduce-options-over-using-iconfiguraiton-chapter2 branch September 10, 2025 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants