refactor NuGet migrations code (#4875)

NuGet · Nov 3, 2022 · 59738b8 · 59738b8
1 parent d99a5e5
commit 59738b8
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 40 deletions.
diff --git a/src/NuGet.Core/NuGet.Common/Migrations/MigrationRunner.cs b/src/NuGet.Core/NuGet.Common/Migrations/MigrationRunner.cs
@@ -2,27 +2,17 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
-using System.Collections.Generic;
-using System.Diagnostics;
-using System.Globalization;
 using System.IO;
 using System.Threading;
 
 namespace NuGet.Common.Migrations
 {
     public static class MigrationRunner
     {
-        private static readonly IReadOnlyList<Action> Migrations = new List<Action>()
-        {
-            Migration1.Run
-        };
         private const string MaxMigrationFilename = "1";
 
         public static void Run()
         {
-            // since migrations run once per machine, optimize for the scenario where the migration has already run
-            Debug.Assert(MaxMigrationFilename == Migrations.Count.ToString(CultureInfo.InvariantCulture));
-
             string migrationsDirectory = GetMigrationsDirectory();
             var expectedMigrationFilename = Path.Combine(migrationsDirectory, MaxMigrationFilename);
 
@@ -34,24 +24,21 @@ public static void Run()
                 {
                     if (WaitForMutex(mutex))
                     {
-                        if (!File.Exists(expectedMigrationFilename))
+                        try
                         {
                             // Only run migrations that have not already been run
-                            int highestMigrationRun = GetHighestMigrationRun(migrationsDirectory);
-                            for (int i = highestMigrationRun + 1; i < Migrations.Count; i++)
+                            if (!File.Exists(expectedMigrationFilename))
                             {
-                                try
-                                {
-                                    Migrations[i]();
-                                    // Create file for every migration run, so that if an older version of NuGet is run, it doesn't try to run
-                                    // migrations again.
-                                    string migrationFile = Path.Combine(migrationsDirectory, (i + 1).ToString(CultureInfo.InvariantCulture));
-                                    File.WriteAllText(migrationFile, string.Empty);
-                                }
-                                catch { }
+                                Migration1.Run();
+                                // Create file for the migration run, so that if an older version of NuGet is run, it doesn't try to run migrations again.
+                                File.WriteAllText(expectedMigrationFilename, string.Empty);
                             }
                         }
-                        mutex.ReleaseMutex();
+                        catch { }
+                        finally
+                        {
+                            mutex.ReleaseMutex();
+                        }
                     }
                 }
             }
@@ -90,19 +77,5 @@ internal static string GetMigrationsDirectory()
             Directory.CreateDirectory(migrationsDirectory);
             return migrationsDirectory;
         }
-
-        private static int GetHighestMigrationRun(string directory)
-        {
-            for (int i = Migrations.Count - 1; i >= 0; --i)
-            {
-                var filename = Path.Combine(directory, (i + 1).ToString(CultureInfo.InvariantCulture));
-                if (File.Exists(filename))
-                {
-                    return i;
-                }
-            }
-
-            return -1;
-        }
     }
 }
diff --git a/test/NuGet.Core.Tests/NuGet.Common.Test/MigrationRunnerTests.cs b/test/NuGet.Core.Tests/NuGet.Common.Test/MigrationRunnerTests.cs
@@ -5,7 +5,6 @@
 using System.Collections.Generic;
 using System.IO;
 using System.Threading;
-using System.Threading.Tasks;
 using NuGet.Common.Migrations;
 using Xunit;
 
@@ -15,7 +14,25 @@ namespace NuGet.Common.Test
     public class MigrationRunnerTests
     {
         [Fact]
-        public void WhenExecutedInParallelOnlyOneFileIsCreatedForEveryMigration_Success()
+        public void Run_WhenExecutedOnSingleThreadThenOneMigrationFileIsCreated_Success()
+        {
+            // Arrange
+            string directory = MigrationRunner.GetMigrationsDirectory();
+            if (Directory.Exists(directory))
+                Directory.Delete(path: directory, recursive: true);
+
+            // Act
+            MigrationRunner.Run();
+
+            // Assert
+            Assert.True(Directory.Exists(directory));
+            var files = Directory.GetFiles(directory);
+            Assert.Equal(1, files.Length);
+            Assert.Equal(Path.Combine(directory, "1"), files[0]);
+        }
+
+        [Fact]
+        public void Run_WhenExecutedInParallelThenOnlyOneMigrationFileIsCreated_Success()
         {
             var threads = new List<Thread>();
             int numThreads = 5;
@@ -41,7 +58,9 @@ public void WhenExecutedInParallelOnlyOneFileIsCreatedForEveryMigration_Success(
 
             // Assert
             Assert.True(Directory.Exists(directory));
-            Assert.Equal(1, Directory.GetFiles(directory).Length);
+            var files = Directory.GetFiles(directory);
+            Assert.Equal(1, files.Length);
+            Assert.Equal(Path.Combine(directory, "1"), files[0]);
         }
     }
 }