From a2eb1bb96de5de5c1a0b0813e5124f94570a694b Mon Sep 17 00:00:00 2001
From: Shay Rojansky <roji@roji.org>
Date: Mon, 4 Jan 2021 16:11:54 +0100
Subject: [PATCH] Fixes to database collation migrations

Fixes #23794
---
 .../CSharpMigrationOperationGenerator.cs      | 13 ++++++-
 .../Migrations/MigrationBuilder.cs            |  9 ++++-
 .../CSharpMigrationOperationGeneratorTest.cs  | 35 +++++++++++++++++++
 3 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/src/EFCore.Design/Migrations/Design/CSharpMigrationOperationGenerator.cs b/src/EFCore.Design/Migrations/Design/CSharpMigrationOperationGenerator.cs
index 960b708372a..4ee0d5e56a3 100644
--- 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
index 91f91b7f6d6..ab7ed79c89f 100644
--- 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
index 34da79fd593..da2c578965c 100644
--- 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()
         {