Fixes to database collation migrations

Fixes #23794
dotnet · Jan 4, 2021 · a2eb1bb · a2eb1bb
1 parent 6ef52c7
commit a2eb1bb
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 2 deletions.
diff --git a/src/EFCore.Design/Migrations/Design/CSharpMigrationOperationGenerator.cs b/src/EFCore.Design/Migrations/Design/CSharpMigrationOperationGenerator.cs
@@ -724,20 +724,31 @@ protected virtual void Generate([NotNull] AlterDatabaseOperation operation, [Not
 
             using (builder.Indent())
             {
+                var needComma = false;
+
                 if (operation.Collation != null)
                 {
                     builder
                         .AppendLine()
                         .Append("collation: ")
                         .Append(Code.Literal(operation.Collation));
+
+                    needComma = true;
                 }
 
                 if (operation.OldDatabase.Collation != null)
                 {
+                    if (needComma)
+                    {
+                        builder.Append(",");
+                    }
+
                     builder
-                        .AppendLine(",")
+                        .AppendLine()
                         .Append("oldCollation: ")
                         .Append(Code.Literal(operation.OldDatabase.Collation));
+
+                    needComma = true;
                 }
 
                 builder.Append(")");

diff --git a/src/EFCore.Relational/Migrations/MigrationBuilder.cs b/src/EFCore.Relational/Migrations/MigrationBuilder.cs
@@ -477,7 +477,14 @@ public virtual AlterOperationBuilder<AlterDatabaseOperation> AlterDatabase(
             [CanBeNull] string collation = null,
             [CanBeNull] string oldCollation = null)
         {
-            var operation = new AlterDatabaseOperation { Collation = collation };
+            var operation = new AlterDatabaseOperation
+            {
+                Collation = collation,
+                OldDatabase =
+                {
+                    Collation = oldCollation
+                }
+            };
             Operations.Add(operation);
 
             return new AlterOperationBuilder<AlterDatabaseOperation>(operation);

diff --git a/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationOperationGeneratorTest.cs b/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationOperationGeneratorTest.cs
@@ -804,11 +804,46 @@ public void AlterDatabaseOperation()
                 + "    .OldAnnotation(\"bar\", \"foo\");",
                 o =>
                 {
+                    Assert.Equal("Some collation", o.Collation);
+                    Assert.Equal("Some other collation", o.OldDatabase.Collation);
                     Assert.Equal("bar", o["foo"]);
                     Assert.Equal("foo", o.OldDatabase["bar"]);
                 });
         }
 
+        [ConditionalFact]
+        public void AlterDatabaseOperation_with_default_old_collation()
+        {
+            Test(
+                new AlterDatabaseOperation { Collation = "Some collation" },
+                "mb.AlterDatabase("
+                + _eol
+                + "    collation: \"Some collation\");",
+                o =>
+                {
+                    Assert.Equal("Some collation", o.Collation);
+                    Assert.Null(o.OldDatabase.Collation);
+                });
+        }
+
+        [ConditionalFact]
+        public void AlterDatabaseOperation_with_default_new_collation()
+        {
+            Test(
+                new AlterDatabaseOperation
+                {
+                    OldDatabase = { Collation = "Some collation" }
+                },
+                "mb.AlterDatabase("
+                + _eol
+                + "    oldCollation: \"Some collation\");",
+                o =>
+                {
+                    Assert.Null(o.Collation);
+                    Assert.Equal("Some collation", o.OldDatabase.Collation);
+                });
+        }
+
         [ConditionalFact]
         public void AlterSequenceOperation_required_args()
         {