Add Enrich EF API

spelling.dic

@@ -50,3 +50,5 @@ uninstrumented
 upsert
 uris
 urls
+Npgsql
+Postgre

src/Components/Aspire.Npgsql.EntityFrameworkCore.PostgreSQL/AspireEFPostgreSqlExtensions.cs

@@ -66,15 +66,124 @@ public static partial class AspireEFPostgreSqlExtensions
 
         configureSettings?.Invoke(settings);
 
-        if (settings.DbContextPooling)
+        builder.Services.AddDbContextPool<TContext>(ConfigureDbContext);
+
+        ConfigureInstrumentation<TContext>(builder, settings);
+
+        void ConfigureDbContext(DbContextOptionsBuilder dbContextOptionsBuilder)
+        {
+            // delay validating the ConnectionString until the DbContext is requested. This ensures an exception doesn't happen until a Logger is established.
+            if (string.IsNullOrEmpty(settings.ConnectionString))
+            {
+                throw new InvalidOperationException($"ConnectionString is missing. It should be provided in 'ConnectionStrings:{connectionName}' or under the 'ConnectionString' key in '{DefaultConfigSectionName}' or '{typeSpecificSectionName}' configuration section.");
+            }
+
+            // We don't register a logger factory, because there is no need to: https://learn.microsoft.com/dotnet/api/microsoft.entityframeworkcore.dbcontextoptionsbuilder.useloggerfactory?view=efcore-7.0#remarks
+            dbContextOptionsBuilder.UseNpgsql(settings.ConnectionString, builder =>
+            {
+                // Resiliency:
+                // 1. Connection resiliency automatically retries failed database commands: https://www.npgsql.org/efcore/misc/other.html#execution-strategy
+                if (settings.Retry)
+                {
+                    builder.EnableRetryOnFailure();
+                }
+                // 2. "Scale proportionally: You want to ensure that you don't scale out a resource to a point where it will exhaust other associated resources."
+                // The pooling is enabled by default, the min pool size is 0 by default: https://www.npgsql.org/doc/connection-string-parameters.html#pooling
+                // There is nothing for us to set here.
+                // 3. "Timeout: Places limit on the duration for which a caller can wait for a response."
+                // The timeouts have default values, except of Internal Command Timeout, which we should ignore:
+                // https://www.npgsql.org/doc/connection-string-parameters.html#timeouts-and-keepalive
+                // There is nothing for us to set here.
+            });
+            configureDbContextOptions?.Invoke(dbContextOptionsBuilder);
+        }
+    }
+
+    /// <summary>
+    /// Configures health check, logging and telemetry for the <see cref="DbContext" />.
+    /// </summary>
+    /// <exception cref="ArgumentNullException">Thrown if mandatory <paramref name="builder"/> is null.</exception>
+    /// <exception cref="InvalidOperationException">Thrown when mandatory <see cref="DbContext"/> is not registered in DI.</exception>
+    public static void EnrichNpgsqlDbContext<[DynamicallyAccessedMembers(RequiredByEF)] TContext>(
+            this IHostApplicationBuilder builder,
+            Action<NpgsqlEntityFrameworkCorePostgreSQLSettings>? configureSettings = null) where TContext : DbContext
+    {
+        ArgumentNullException.ThrowIfNull(builder);
+
+        NpgsqlEntityFrameworkCorePostgreSQLSettings settings = new();
+        var typeSpecificSectionName = $"{DefaultConfigSectionName}:{typeof(TContext).Name}";
+        var typeSpecificConfigurationSection = builder.Configuration.GetSection(typeSpecificSectionName);
+        if (typeSpecificConfigurationSection.Exists()) // https://github.com/dotnet/runtime/issues/91380
         {
-            builder.Services.AddDbContextPool<TContext>(ConfigureDbContext);
+            typeSpecificConfigurationSection.Bind(settings);
         }
         else
         {
-            builder.Services.AddDbContext<TContext>(ConfigureDbContext);
+            builder.Configuration.GetSection(DefaultConfigSectionName).Bind(settings);
         }
 
+        configureSettings?.Invoke(settings);
+
+        ConfigureRetry();
+
+        ConfigureInstrumentation<TContext>(builder, settings);
+
+        void ConfigureRetry()
+        {
+            if (!settings.Retry)
+            {
+                return;
+            }
+
+            // Resolving DbContext<TContextService> will resolve DbContextOptions<TContextImplementation>.
+            // We need to replace the DbContextOptions service descriptor to inject more logic. This won't be necessary once
+            // Aspire targets .NET 9 as EF will respect the calls to services.ConfigureDbContext<TContext>(). c.f. https://github.com/dotnet/efcore/pull/32518
+
+            var olDbContextOptionsDescriptor = builder.Services.FirstOrDefault(sd => sd.ServiceType == typeof(DbContextOptions<TContext>));
+
+            if (olDbContextOptionsDescriptor is null)
+            {
+                throw new InvalidOperationException($"DbContext<{nameof(TContext)}> was not registered");
+            }
+
+            builder.Services.Remove(olDbContextOptionsDescriptor);
+
+            var dbContextOptionsDescriptor = new ServiceDescriptor(
+                olDbContextOptionsDescriptor.ServiceType,
+                olDbContextOptionsDescriptor.ServiceKey,
+                factory: (sp, key) =>
+                {
+                    DbContextOptionsBuilder? optionsBuilder = null;
+
+                    if (olDbContextOptionsDescriptor.ImplementationFactory != null)
+                    {
+                        var instance = olDbContextOptionsDescriptor.ImplementationFactory?.Invoke(sp) as DbContextOptions<TContext>;
+
+                        if (instance == null)
+                        {
+                            throw new InvalidOperationException($"DbContextOptions<{nameof(TContext)}> couldn't be resolved");
+                        }
+
+                        optionsBuilder = new DbContextOptionsBuilder<TContext>(instance);
+                    }
+                    else
+                    {
+                        optionsBuilder ??= new DbContextOptionsBuilder<TContext>();
+                    }
+
+                    optionsBuilder.UseNpgsql(options => options.EnableRetryOnFailure());
+
+                    return optionsBuilder.Options;
+                },
+                olDbContextOptionsDescriptor.Lifetime
+                );
+
+            builder.Services.Add(dbContextOptionsDescriptor);
+        }
+    }
+
+    private static void ConfigureInstrumentation<[DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors | DynamicallyAccessedMemberTypes.PublicProperties)] TContext>(IHostApplicationBuilder builder, NpgsqlEntityFrameworkCorePostgreSQLSettings settings) where TContext : DbContext
+    {
         if (settings.HealthChecks)
         {
             // calling MapHealthChecks is the responsibility of the app, not Component
@@ -113,34 +222,5 @@ public static partial class AspireEFPostgreSqlExtensions
                     NpgsqlCommon.AddNpgsqlMetrics(meterProviderBuilder);
                 });
         }
-
-        void ConfigureDbContext(DbContextOptionsBuilder dbContextOptionsBuilder)
-        {
-            // delay validating the ConnectionString until the DbContext is requested. This ensures an exception doesn't happen until a Logger is established.
-            if (string.IsNullOrEmpty(settings.ConnectionString))
-            {
-                throw new InvalidOperationException($"ConnectionString is missing. It should be provided in 'ConnectionStrings:{connectionName}' or under the 'ConnectionString' key in '{DefaultConfigSectionName}' or '{typeSpecificSectionName}' configuration section.");
-            }
-
-            // We don't register a logger factory, because there is no need to: https://learn.microsoft.com/dotnet/api/microsoft.entityframeworkcore.dbcontextoptionsbuilder.useloggerfactory?view=efcore-7.0#remarks
-            dbContextOptionsBuilder.UseNpgsql(settings.ConnectionString, builder =>
-            {
-                // Resiliency:
-                // 1. Connection resiliency automatically retries failed database commands: https://www.npgsql.org/efcore/misc/other.html#execution-strategy
-                if (settings.MaxRetryCount > 0)
-                {
-                    builder.EnableRetryOnFailure(settings.MaxRetryCount);
-                }
-                // 2. "Scale proportionally: You want to ensure that you don't scale out a resource to a point where it will exhaust other associated resources."
-                // The pooling is enabled by default, the min pool size is 0 by default: https://www.npgsql.org/doc/connection-string-parameters.html#pooling
-                // There is nothing for us to set here.
-                // 3. "Timeout: Places limit on the duration for which a caller can wait for a response."
-                // The timeouts have default values, except of Internal Command Timeout, which we should ignore:
-                // https://www.npgsql.org/doc/connection-string-parameters.html#timeouts-and-keepalive
-                // There is nothing for us to set here.
-            });
-
-            configureDbContextOptions?.Invoke(dbContextOptionsBuilder);
-        }
     }
 }

src/Components/Aspire.Npgsql.EntityFrameworkCore.PostgreSQL/ConfigurationSchema.json

@@ -58,25 +58,21 @@
                       "type": "string",
                       "description": "Gets or sets the connection string of the PostgreSQL database to connect to."
                     },
-                    "DbContextPooling": {
-                      "type": "boolean",
-                      "description": "Gets or sets a boolean value that indicates whether the DbContext will be pooled or explicitly created every time it's requested.",
-                      "default": true
-                    },
                     "HealthChecks": {
                       "type": "boolean",
                       "description": "Gets or sets a boolean value that indicates whether the database health check is enabled or not.",
                       "default": true
                     },
-                    "MaxRetryCount": {
-                      "type": "integer",
-                      "description": "Gets or sets the maximum number of retry attempts. Default value is 6, set it to 0 to disable the retry mechanism."
-                    },
                     "Metrics": {
                       "type": "boolean",
                       "description": "Gets or sets a boolean value that indicates whether the OpenTelemetry metrics are enabled or not.",
                       "default": true
                     },
+                    "Retry": {
+                      "type": "boolean",
+                      "description": "Gets or sets whether retries should be enabled.",
+                      "default": true
+                    },
                     "Tracing": {
                       "type": "boolean",
                       "description": "Gets or sets a boolean value that indicates whether the OpenTelemetry tracing is enabled or not.",

src/Components/Aspire.Npgsql.EntityFrameworkCore.PostgreSQL/NpgsqlEntityFrameworkCorePostgreSQLSettings.cs

@@ -14,19 +14,12 @@ public sealed class NpgsqlEntityFrameworkCorePostgreSQLSettings
     public string? ConnectionString { get; set; }
 
     /// <summary>
-    /// Gets or sets a boolean value that indicates whether the DbContext will be pooled or explicitly created every time it's requested.
+    /// Gets or sets whether retries should be enabled.
     /// </summary>
     /// <value>
     /// The default value is <see langword="true"/>.
     /// </value>
-    /// <remarks>Should be set to false in multi-tenant scenarios.</remarks>
-    public bool DbContextPooling { get; set; } = true;
-
-    /// <summary>
-    /// <para>Gets or sets the maximum number of retry attempts.</para>
-    /// <para>Default value is 6, set it to 0 to disable the retry mechanism.</para>
-    /// </summary>
-    public int MaxRetryCount { get; set; } = 6;
+    public bool Retry { get; set; } = true;
 
     /// <summary>
     /// Gets or sets a boolean value that indicates whether the database health check is enabled or not.

tests/Aspire.Npgsql.EntityFrameworkCore.PostgreSQL.Tests/AspireEFPostgreSqlExtensionsTests.cs

@@ -18,6 +18,13 @@ public class AspireEFPostgreSqlExtensionsTests
 {
     private const string ConnectionString = "Host=localhost;Database=test;Username=postgres";
 
+    internal static void ConfigureDbContextOptionsBuilderForTesting(DbContextOptionsBuilder builder)
+    {
+        // Don't cache the service provider in testing.
+        // Works around https://github.com/npgsql/efcore.pg/issues/2891, which is errantly caches connection strings across DI containers.
+        builder.EnableServiceProviderCaching(false);
+    }
+
     [Fact]
     public void ReadsFromConnectionStringsCorrectly()
     {
@@ -26,7 +33,7 @@ public void ReadsFromConnectionStringsCorrectly()
             new KeyValuePair<string, string?>("ConnectionStrings:npgsql", ConnectionString)
         ]);
 
-        builder.AddNpgsqlDbContext<TestDbContext>("npgsql");
+        builder.AddNpgsqlDbContext<TestDbContext>("npgsql", configureDbContextOptions: ConfigureDbContextOptionsBuilderForTesting);
 
         var host = builder.Build();
         var context = host.Services.GetRequiredService<TestDbContext>();
@@ -42,7 +49,9 @@ public void ConnectionStringCanBeSetInCode()
             new KeyValuePair<string, string?>("ConnectionStrings:npgsql", "unused")
         ]);
 
-        builder.AddNpgsqlDbContext<TestDbContext>("npgsql", settings => settings.ConnectionString = ConnectionString);
+        builder.AddNpgsqlDbContext<TestDbContext>("npgsql",
+            settings => settings.ConnectionString = ConnectionString,
+            configureDbContextOptions: ConfigureDbContextOptionsBuilderForTesting);
 
         var host = builder.Build();
         var context = host.Services.GetRequiredService<TestDbContext>();
@@ -62,7 +71,7 @@ public void ConnectionNameWinsOverConfigSection()
             new KeyValuePair<string, string?>("ConnectionStrings:npgsql", ConnectionString)
         ]);
 
-        builder.AddNpgsqlDbContext<TestDbContext>("npgsql");
+        builder.AddNpgsqlDbContext<TestDbContext>("npgsql", configureDbContextOptions: ConfigureDbContextOptionsBuilderForTesting);
 
         var host = builder.Build();
         var context = host.Services.GetRequiredService<TestDbContext>();
@@ -74,16 +83,17 @@ public void ConnectionNameWinsOverConfigSection()
     }
 
     [Fact]
-    public void CanConfigureDbContextOptions()
+    public void AddNpgsqlCanConfigureDbContextOptions()
     {
         var builder = Host.CreateEmptyApplicationBuilder(null);
         builder.Configuration.AddInMemoryCollection([
             new KeyValuePair<string, string?>("ConnectionStrings:npgsql", ConnectionString),
-            new KeyValuePair<string, string?>("Aspire:Npgsql:EntityFrameworkCore:PostgreSQL:MaxRetryCount", "304")
+            new KeyValuePair<string, string?>("Aspire:Npgsql:EntityFrameworkCore:PostgreSQL:Retry", "true")
         ]);
 
         builder.AddNpgsqlDbContext<TestDbContext>("npgsql", configureDbContextOptions: optionsBuilder =>
         {
+            ConfigureDbContextOptionsBuilderForTesting(optionsBuilder);
             optionsBuilder.UseNpgsql(npgsqlBuilder =>
             {
                 npgsqlBuilder.CommandTimeout(123);
@@ -109,8 +119,56 @@ public void CanConfigureDbContextOptions()
         Assert.NotNull(extension.ExecutionStrategyFactory);
         var executionStrategy = extension.ExecutionStrategyFactory(new ExecutionStrategyDependencies(new CurrentDbContext(context), context.Options, null!));
         var retryStrategy = Assert.IsType<NpgsqlRetryingExecutionStrategy>(executionStrategy);
-        Assert.Equal(304, retryStrategy.MaxRetryCount);
+        Assert.Equal(new WorkaroundToReadProtectedField(context).MaxRetryCount, retryStrategy.MaxRetryCount);
+
+#pragma warning restore EF1001 // Internal EF Core API usage.
+    }
+
+    [Fact]
+    public void AddNpgsqlCanConfigureDbContextOptionsWithoutRetry()
+    {
+        var builder = Host.CreateEmptyApplicationBuilder(null);
+        builder.Configuration.AddInMemoryCollection([
+            new KeyValuePair<string, string?>("ConnectionStrings:npgsql", ConnectionString),
+            new KeyValuePair<string, string?>("Aspire:Npgsql:EntityFrameworkCore:PostgreSQL:Retry", "false")
+        ]);
+
+        builder.AddNpgsqlDbContext<TestDbContext>("npgsql", configureDbContextOptions: optionsBuilder =>
+        {
+            ConfigureDbContextOptionsBuilderForTesting(optionsBuilder);
+            optionsBuilder.UseNpgsql(npgsqlBuilder =>
+            {
+                npgsqlBuilder.CommandTimeout(123);
+            });
+        });
+
+        var host = builder.Build();
+        var context = host.Services.GetRequiredService<TestDbContext>();
+
+#pragma warning disable EF1001 // Internal EF Core API usage.
+
+        var extension = context.Options.FindExtension<NpgsqlOptionsExtension>();
+        Assert.NotNull(extension);
+
+        // ensure the command timeout was respected
+        Assert.Equal(123, extension.CommandTimeout);
+
+        // ensure the connection string from config was respected
+        var actualConnectionString = context.Database.GetDbConnection().ConnectionString;
+        Assert.Equal(ConnectionString, actualConnectionString);
+
+        // ensure no retry strategy was registered
+        Assert.Null(extension.ExecutionStrategyFactory);
 
 #pragma warning restore EF1001 // Internal EF Core API usage.
     }
+
+    public class WorkaroundToReadProtectedField : NpgsqlRetryingExecutionStrategy
+    {
+        public WorkaroundToReadProtectedField(DbContext context) : base(context)
+        {
+        }
+
+        public int RetryCount => base.MaxRetryCount;
+    }
 }

tests/Aspire.Npgsql.EntityFrameworkCore.PostgreSQL.Tests/ConfigurationTests.cs

@@ -1,7 +1,6 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using Aspire.Components.Common.Tests;
 using Microsoft.EntityFrameworkCore;
 using Npgsql.EntityFrameworkCore.PostgreSQL;
 using Xunit;
@@ -14,10 +13,6 @@ public class ConfigurationTests
     public void ConnectionStringIsNullByDefault()
         => Assert.Null(new NpgsqlEntityFrameworkCorePostgreSQLSettings().ConnectionString);
 
-    [Fact]
-    public void DbContextPoolingIsEnabledByDefault()
-        => Assert.True(new NpgsqlEntityFrameworkCorePostgreSQLSettings().DbContextPooling);
-
     [Fact]
     public void HealthCheckIsEnabledByDefault()
         => Assert.True(new NpgsqlEntityFrameworkCorePostgreSQLSettings().HealthChecks);
@@ -30,16 +25,6 @@ public void TracingIsEnabledByDefault()
     public void MetricsAreEnabledByDefault()
         => Assert.True(new NpgsqlEntityFrameworkCorePostgreSQLSettings().Metrics);
 
-    [Fact]
-    public void MaxRetryCountIsSameAsInTheDefaultNpgsqlPolicy()
-    {
-        DbContextOptionsBuilder<TestDbContext> dbContextOptionsBuilder = new();
-        dbContextOptionsBuilder.UseNpgsql("fakeConnectionString");
-        TestDbContext dbContext = new(dbContextOptionsBuilder.Options);
-
-        Assert.Equal(new WorkaroundToReadProtectedField(dbContext).RetryCount, new NpgsqlEntityFrameworkCorePostgreSQLSettings().MaxRetryCount);
-    }
-
     public class WorkaroundToReadProtectedField : NpgsqlRetryingExecutionStrategy
     {
         public WorkaroundToReadProtectedField(DbContext context) : base(context)