From 9f18766c91191e7377bb71381a3cb6f396b40bf2 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Tue, 5 Nov 2024 11:47:39 +0100 Subject: [PATCH] Invoke connection interceptor before tweaking connection string in Exists() Fixes #2880 --- .../Storage/Internal/NpgsqlDatabaseCreator.cs | 74 +++++++++---------- .../NpgsqlDatabaseCreatorTest.cs | 5 +- 2 files changed, 40 insertions(+), 39 deletions(-) diff --git a/src/EFCore.PG/Storage/Internal/NpgsqlDatabaseCreator.cs b/src/EFCore.PG/Storage/Internal/NpgsqlDatabaseCreator.cs index 884cb6431..db12255d0 100644 --- a/src/EFCore.PG/Storage/Internal/NpgsqlDatabaseCreator.cs +++ b/src/EFCore.PG/Storage/Internal/NpgsqlDatabaseCreator.cs @@ -10,27 +10,13 @@ namespace Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal; /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// -public class NpgsqlDatabaseCreator : RelationalDatabaseCreator -{ - private readonly INpgsqlRelationalConnection _connection; - private readonly IRawSqlCommandBuilder _rawSqlCommandBuilder; - - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - public NpgsqlDatabaseCreator( +public class NpgsqlDatabaseCreator( RelationalDatabaseCreatorDependencies dependencies, INpgsqlRelationalConnection connection, - IRawSqlCommandBuilder rawSqlCommandBuilder) - : base(dependencies) - { - _connection = connection; - _rawSqlCommandBuilder = rawSqlCommandBuilder; - } - + IRawSqlCommandBuilder rawSqlCommandBuilder, + IRelationalConnectionDiagnosticsLogger connectionLogger) + : RelationalDatabaseCreator(dependencies) +{ /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in @@ -55,7 +41,7 @@ public NpgsqlDatabaseCreator( /// public override void Create() { - using (var masterConnection = _connection.CreateAdminConnection()) + using (var masterConnection = connection.CreateAdminConnection()) { try { @@ -82,7 +68,7 @@ public override void Create() /// public override async Task CreateAsync(CancellationToken cancellationToken = default) { - var masterConnection = _connection.CreateAdminConnection(); + var masterConnection = connection.CreateAdminConnection(); await using (masterConnection.ConfigureAwait(false)) { try @@ -112,7 +98,7 @@ await Dependencies.MigrationCommandExecutor public override bool HasTables() => Dependencies.ExecutionStrategy .Execute( - _connection, + connection, connection => (bool)CreateHasTablesCommand() .ExecuteScalar( new RelationalCommandParameterObject( @@ -130,7 +116,7 @@ public override bool HasTables() /// public override Task HasTablesAsync(CancellationToken cancellationToken = default) => Dependencies.ExecutionStrategy.ExecuteAsync( - _connection, + connection, async (connection, ct) => (bool)(await CreateHasTablesCommand() .ExecuteScalarAsync( new RelationalCommandParameterObject( @@ -142,7 +128,7 @@ public override Task HasTablesAsync(CancellationToken cancellationToken = cancellationToken: ct).ConfigureAwait(false))!, cancellationToken); private IRelationalCommand CreateHasTablesCommand() - => _rawSqlCommandBuilder + => rawSqlCommandBuilder .Build( """ SELECT CASE WHEN COUNT(*) = 0 THEN FALSE ELSE TRUE END @@ -173,7 +159,7 @@ private IReadOnlyList CreateCreateOperations() [ new NpgsqlCreateDatabaseOperation { - Name = _connection.DbConnection.Database, + Name = connection.DbConnection.Database, Template = designTimeModel.GetDatabaseTemplate(), Collation = designTimeModel.GetCollation(), Tablespace = designTimeModel.GetTablespace() @@ -201,15 +187,29 @@ public override Task ExistsAsync(CancellationToken cancellationToken = def private async Task Exists(bool async, CancellationToken cancellationToken = default) { + var logger = connectionLogger; + var startTime = DateTimeOffset.UtcNow; + + var interceptionResult = async + ? await logger.ConnectionOpeningAsync(connection, startTime, cancellationToken).ConfigureAwait(false) + : logger.ConnectionOpening(connection, startTime); + + if (interceptionResult.IsSuppressed) + { + // If the connection attempt was suppressed by an interceptor, assume that the interceptor took care of all the opening + // details, and the database exists. + return true; + } + // When checking whether a database exists, pooling must be off, otherwise we may // attempt to reuse a pooled connection, which may be broken (this happened in the tests). // If Pooling is off, but Multiplexing is on - NpgsqlConnectionStringBuilder.Validate will throw, // so we turn off Multiplexing as well. - var unpooledCsb = new NpgsqlConnectionStringBuilder(_connection.ConnectionString) { Pooling = false, Multiplexing = false }; + var unpooledCsb = new NpgsqlConnectionStringBuilder(connection.ConnectionString) { Pooling = false, Multiplexing = false }; using var _ = new TransactionScope(TransactionScopeOption.Suppress, TransactionScopeAsyncFlowOption.Enabled); var unpooledRelationalConnection = - await _connection.CloneWith(unpooledCsb.ToString(), async, cancellationToken).ConfigureAwait(false); + await connection.CloneWith(unpooledCsb.ToString(), async, cancellationToken).ConfigureAwait(false); try { @@ -273,7 +273,7 @@ private static bool IsDoesNotExist(PostgresException exception) /// public override void Delete() { - switch (_connection.DataSource) + switch (connection.DataSource) { case NpgsqlDataSource dataSource: dataSource.Clear(); @@ -283,7 +283,7 @@ public override void Delete() break; } - using (var masterConnection = _connection.CreateAdminConnection()) + using (var masterConnection = connection.CreateAdminConnection()) { Dependencies.MigrationCommandExecutor .ExecuteNonQuery(CreateDropCommands(), masterConnection); @@ -298,7 +298,7 @@ public override void Delete() /// public override async Task DeleteAsync(CancellationToken cancellationToken = default) { - switch (_connection.DataSource) + switch (connection.DataSource) { case NpgsqlDataSource dataSource: // TODO: Do this asynchronously once https://github.com/npgsql/npgsql/issues/4499 is done @@ -309,7 +309,7 @@ public override async Task DeleteAsync(CancellationToken cancellationToken = def break; } - var masterConnection = _connection.CreateAdminConnection(); + var masterConnection = connection.CreateAdminConnection(); await using (masterConnection) { await Dependencies.MigrationCommandExecutor @@ -339,7 +339,7 @@ public override void CreateTables() try { - Dependencies.MigrationCommandExecutor.ExecuteNonQuery(commands, _connection); + Dependencies.MigrationCommandExecutor.ExecuteNonQuery(commands, connection); } catch (PostgresException e) when (e is { SqlState: "23505", ConstraintName: "pg_type_typname_nsp_index" }) { @@ -347,7 +347,7 @@ public override void CreateTables() // (happens in the tests). Simply ignore the error. } - if (reloadTypes && _connection.DbConnection is NpgsqlConnection npgsqlConnection) + if (reloadTypes && connection.DbConnection is NpgsqlConnection npgsqlConnection) { npgsqlConnection.Open(); try @@ -375,7 +375,7 @@ public override async Task CreateTablesAsync(CancellationToken cancellationToken try { - await Dependencies.MigrationCommandExecutor.ExecuteNonQueryAsync(commands, _connection, cancellationToken) + await Dependencies.MigrationCommandExecutor.ExecuteNonQueryAsync(commands, connection, cancellationToken) .ConfigureAwait(false); } catch (PostgresException e) when (e is { SqlState: "23505", ConstraintName: "pg_type_typname_nsp_index" }) @@ -389,7 +389,7 @@ await Dependencies.MigrationCommandExecutor.ExecuteNonQueryAsync(commands, _conn .OfType() .Any(o => o.GetPostgresExtensions().Any() || o.GetPostgresEnums().Any() || o.GetPostgresRanges().Any()); - if (reloadTypes && _connection.DbConnection is NpgsqlConnection npgsqlConnection) + if (reloadTypes && connection.DbConnection is NpgsqlConnection npgsqlConnection) { await npgsqlConnection.OpenAsync(cancellationToken).ConfigureAwait(false); try @@ -409,7 +409,7 @@ private IReadOnlyList CreateDropCommands() { // TODO Check DbConnection.Database always gives us what we want // Issue #775 - new NpgsqlDropDatabaseOperation { Name = _connection.DbConnection.Database } + new NpgsqlDropDatabaseOperation { Name = connection.DbConnection.Database } }; return Dependencies.MigrationsSqlGenerator.Generate(operations); @@ -422,5 +422,5 @@ private static void ClearAllPools() // Clear connection pool for the database connection since after the 'create database' call, a previously // invalid connection may now be valid. private void ClearPool() - => NpgsqlConnection.ClearPool((NpgsqlConnection)_connection.DbConnection); + => NpgsqlConnection.ClearPool((NpgsqlConnection)connection.DbConnection); } diff --git a/test/EFCore.PG.FunctionalTests/NpgsqlDatabaseCreatorTest.cs b/test/EFCore.PG.FunctionalTests/NpgsqlDatabaseCreatorTest.cs index edf3087bd..9954a7b15 100644 --- a/test/EFCore.PG.FunctionalTests/NpgsqlDatabaseCreatorTest.cs +++ b/test/EFCore.PG.FunctionalTests/NpgsqlDatabaseCreatorTest.cs @@ -632,8 +632,9 @@ public class Blog public class TestDatabaseCreator( RelationalDatabaseCreatorDependencies dependencies, INpgsqlRelationalConnection connection, - IRawSqlCommandBuilder rawSqlCommandBuilder) - : NpgsqlDatabaseCreator(dependencies, connection, rawSqlCommandBuilder) + IRawSqlCommandBuilder rawSqlCommandBuilder, + IRelationalConnectionDiagnosticsLogger connectionLogger) + : NpgsqlDatabaseCreator(dependencies, connection, rawSqlCommandBuilder, connectionLogger) { public bool HasTablesBase() => HasTables();