Stop validating non-null/empty connection strings

Instead, let ADO.NET validate. This allows the connection string to be set as late as possible, for example in the ConnectionOpening interceptor. Fixes #23085
dotnet · Apr 23, 2022 · 7c1ad4d · 7c1ad4d
1 parent 1b9becc
commit 7c1ad4d
Show file tree

Hide file tree

Showing 8 changed files with 14 additions and 52 deletions.
diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs
diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx
@@ -719,9 +719,6 @@
   <data name="NoActiveTransaction" xml:space="preserve">
     <value>The connection does not have any active transactions.</value>
   </data>
-  <data name="NoConnectionOrConnectionString" xml:space="preserve">
-    <value>A relational store has been configured without specifying either the DbConnection or connection string to use.</value>
-  </data>
   <data name="NoDbCommand" xml:space="preserve">
     <value>Cannot create a DbCommand for a non-relational query.</value>
   </data>
@@ -887,4 +884,4 @@
   <data name="VisitChildrenMustBeOverridden" xml:space="preserve">
     <value>'VisitChildren' must be overridden in the class deriving from 'SqlExpression'.</value>
   </data>
-</root>
+</root>
diff --git a/src/EFCore.Relational/Storage/RelationalConnection.cs b/src/EFCore.Relational/Storage/RelationalConnection.cs
@@ -125,15 +125,7 @@ public virtual string? ConnectionString
     /// <returns>The connection string.</returns>
     /// <exception cref="InvalidOperationException">when connection string cannot be obtained.</exception>
     protected virtual string GetValidatedConnectionString()
-    {
-        var connectionString = ConnectionString;
-        if (connectionString == null)
-        {
-            throw new InvalidOperationException(RelationalStrings.NoConnectionOrConnectionString);
-        }
-
-        return connectionString;
-    }
+        => ConnectionString!;
 
     /// <summary>
     ///     Gets or sets the underlying <see cref="System.Data.Common.DbConnection" /> used to connect to the database.
@@ -149,16 +141,7 @@ protected virtual string GetValidatedConnectionString()
     [AllowNull]
     public virtual DbConnection DbConnection
     {
-        get
-        {
-            if (_connection == null
-                && _connectionString == null)
-            {
-                throw new InvalidOperationException(RelationalStrings.NoConnectionOrConnectionString);
-            }
-
-            return _connection ??= CreateDbConnection();
-        }
+        get => _connection ??= CreateDbConnection();
         set
         {
             if (!ReferenceEquals(_connection, value))

diff --git a/test/EFCore.Relational.Specification.Tests/TwoDatabasesTestBase.cs b/test/EFCore.Relational.Specification.Tests/TwoDatabasesTestBase.cs
@@ -73,8 +73,10 @@ public virtual void Can_query_from_one_connection_and_save_changes_to_another()
         Assert.Equal(new[] { "Modified One", "Modified Two" }, context2.Foos.Select(e => e.Bar).ToList());
     }
 
-    [ConditionalFact]
-    public virtual void Can_set_connection_string_in_interceptor()
+    [ConditionalTheory]
+    [InlineData(false)]
+    [InlineData(true)]
+    public virtual void Can_set_connection_string_in_interceptor(bool withConnectionString)
     {
         using var context1 = CreateBackingContext("TwoDatabasesIntercept");
 
@@ -83,10 +85,10 @@ public virtual void Can_set_connection_string_in_interceptor()
         context1.Database.EnsureCreatedResiliently();
 
         using (var context = new TwoDatabasesContext(
-                   CreateTestOptions(new DbContextOptionsBuilder(), withConnectionString: true)
+                   CreateTestOptions(new DbContextOptionsBuilder(), withConnectionString)
                        .AddInterceptors(
                            new ConnectionStringConnectionInterceptor(
-                               connectionString1, DummyConnectionString))
+                               connectionString1, withConnectionString ? DummyConnectionString : ""))
                        .Options))
         {
             var data = context.Foos.ToList();

diff --git a/test/EFCore.Relational.Tests/RelationalConnectionTest.cs b/test/EFCore.Relational.Tests/RelationalConnectionTest.cs
@@ -911,19 +911,6 @@ public override void PopulateDebugInfo(IDictionary<string, string> debugInfo)
         }
     }
 
-    [ConditionalFact]
-    public void Throws_if_no_connection_or_connection_string_is_specified_only_when_accessed()
-    {
-        var connection = new FakeRelationalConnection(CreateOptions(new FakeRelationalOptionsExtension()));
-
-        Assert.Equal(
-            RelationalStrings.NoConnectionOrConnectionString,
-            Assert.Throws<InvalidOperationException>(
-                () => connection.DbConnection).Message);
-
-        Assert.Null(connection.ConnectionString);
-    }
-
     [ConditionalFact]
     public void Puts_connection_string_on_connection_if_both_are_specified()
     {

diff --git a/test/EFCore.SqlServer.FunctionalTests/ConnectionSpecificationTest.cs b/test/EFCore.SqlServer.FunctionalTests/ConnectionSpecificationTest.cs
@@ -51,10 +51,7 @@ public void Throws_if_context_used_with_no_connection_or_connection_string()
         {
             using var context = new NoneInOnConfiguringContext();
 
-            Assert.Equal(
-                RelationalStrings.NoConnectionOrConnectionString,
-                Assert.Throws<InvalidOperationException>(
-                    () => context.Customers.Any()).Message);
+            Assert.Throws<InvalidOperationException>(() => context.Customers.Any());
         }
     }
 

diff --git a/test/EFCore.SqlServer.FunctionalTests/TwoDatabasesSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/TwoDatabasesSqlServerTest.cs
@@ -23,5 +23,6 @@ protected override DbContextOptionsBuilder CreateTestOptions(
     protected override TwoDatabasesWithDataContext CreateBackingContext(string databaseName)
         => new(Fixture.CreateOptions(SqlServerTestStore.Create(databaseName)));
 
-    protected override string DummyConnectionString { get; } = "Database=DoesNotExist";
+    protected override string DummyConnectionString
+        => "Database=DoesNotExist";
 }
diff --git a/test/EFCore.Sqlite.FunctionalTests/TwoDatabasesSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/TwoDatabasesSqliteTest.cs
@@ -23,7 +23,8 @@ protected override DbContextOptionsBuilder CreateTestOptions(
     protected override TwoDatabasesWithDataContext CreateBackingContext(string databaseName)
         => new(Fixture.CreateOptions(SqliteTestStore.Create(databaseName)));
 
-    protected override string DummyConnectionString { get; } = "DataSource=DummyDatabase";
+    protected override string DummyConnectionString
+        => "DataSource=DummyDatabase";
 
     public class TwoDatabasesFixture : ServiceProviderFixtureBase
     {