Fix DI for the darc vmr get-version command (#4120)

dotnet · Nov 4, 2024 · 78cf8ad · 78cf8ad
1 parent 110f190
commit 78cf8ad
Show file tree

Hide file tree

Showing 6 changed files with 124 additions and 132 deletions.
diff --git a/src/Microsoft.DotNet.Darc/Darc/Options/VirtualMonoRepo/GetRepoVersionCommandLineOptions.cs b/src/Microsoft.DotNet.Darc/Darc/Options/VirtualMonoRepo/GetRepoVersionCommandLineOptions.cs
@@ -5,6 +5,7 @@
 using System.Collections.Generic;
 using CommandLine;
 using Microsoft.DotNet.Darc.Operations.VirtualMonoRepo;
+using Microsoft.Extensions.DependencyInjection;
 
 #nullable enable
 namespace Microsoft.DotNet.Darc.Options.VirtualMonoRepo;
@@ -14,4 +15,10 @@ internal class GetRepoVersionCommandLineOptions : VmrCommandLineOptionsBase<GetR
 {
     [Value(0, HelpText = "Repository names (e.g. runtime) to get the versions for.")]
     public IEnumerable<string> Repositories { get; set; } = Array.Empty<string>();
+
+    public override IServiceCollection RegisterServices(IServiceCollection services)
+    {
+        RegisterVmrServices(services, tmpPath: null);
+        return base.RegisterServices(services);
+    }
 }
diff --git a/src/Microsoft.DotNet.Darc/Darc/Options/VirtualMonoRepo/VmrCommandLineOptions.cs b/src/Microsoft.DotNet.Darc/Darc/Options/VirtualMonoRepo/VmrCommandLineOptions.cs
@@ -1,15 +1,9 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System.IO;
 using CommandLine;
-using Microsoft.DotNet.Darc.Helpers;
-using Microsoft.DotNet.DarcLib.VirtualMonoRepo;
-using Microsoft.DotNet.DarcLib;
-using Microsoft.Extensions.DependencyInjection;
-using Microsoft.Extensions.Logging.Abstractions;
-using Microsoft.Extensions.DependencyInjection.Extensions;
 using Microsoft.DotNet.Darc.Operations;
+using Microsoft.Extensions.DependencyInjection;
 
 namespace Microsoft.DotNet.Darc.Options.VirtualMonoRepo;
 
@@ -20,31 +14,7 @@ internal abstract class VmrCommandLineOptions<T> : VmrCommandLineOptionsBase<T>
 
     public override IServiceCollection RegisterServices(IServiceCollection services)
     {
-        string tmpPath = Path.GetFullPath(TmpPath ?? Path.GetTempPath());
-        LocalSettings localDarcSettings = null;
-
-        var gitHubToken = GitHubPat;
-        var azureDevOpsToken = AzureDevOpsPat;
-
-        // Read tokens from local settings if not provided
-        // We silence errors because the VMR synchronization often works with public repositories where tokens are not required
-        if (gitHubToken == null || azureDevOpsToken == null)
-        {
-            try
-            {
-                localDarcSettings = LocalSettings.GetSettings(this, NullLogger.Instance);
-            }
-            catch (DarcException)
-            {
-                // The VMR synchronization often works with public repositories where tokens are not required
-            }
-
-            gitHubToken ??= localDarcSettings?.GitHubToken;
-            azureDevOpsToken ??= localDarcSettings?.AzureDevOpsToken;
-        }
-
-        services.AddVmrManagers(GitLocation, VmrPath, tmpPath, gitHubToken, azureDevOpsToken);
-        services.TryAddTransient<IVmrScanner, VmrCloakedFileScanner>();
+        RegisterVmrServices(services, TmpPath);
         return base.RegisterServices(services);
     }
 }
diff --git a/src/Microsoft.DotNet.Darc/Darc/Options/VirtualMonoRepo/VmrCommandLineOptionsBase.cs b/src/Microsoft.DotNet.Darc/Darc/Options/VirtualMonoRepo/VmrCommandLineOptionsBase.cs
@@ -3,7 +3,14 @@
 
 using System;
 using CommandLine;
+using Microsoft.DotNet.Darc.Helpers;
 using Microsoft.DotNet.Darc.Operations;
+using Microsoft.DotNet.DarcLib.VirtualMonoRepo;
+using Microsoft.DotNet.DarcLib;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.DependencyInjection.Extensions;
+using Microsoft.Extensions.Logging.Abstractions;
+using System.IO;
 
 #nullable enable
 namespace Microsoft.DotNet.Darc.Options.VirtualMonoRepo;
@@ -12,4 +19,34 @@ internal abstract class VmrCommandLineOptionsBase<T> : CommandLineOptions<T> whe
 {
     [Option("vmr", HelpText = "Path to the VMR; defaults to nearest git root above the current working directory.")]
     public string VmrPath { get; set; } = Environment.CurrentDirectory;
+
+    protected void RegisterVmrServices(IServiceCollection services, string? tmpPath)
+    {
+        LocalSettings? localDarcSettings = null;
+
+        var gitHubToken = GitHubPat;
+        var azureDevOpsToken = AzureDevOpsPat;
+
+        // Read tokens from local settings if not provided
+        // We silence errors because the VMR synchronization often works with public repositories where tokens are not required
+        if (gitHubToken == null || azureDevOpsToken == null)
+        {
+            try
+            {
+                localDarcSettings = LocalSettings.GetSettings(this, NullLogger.Instance);
+            }
+            catch (DarcException)
+            {
+                // The VMR synchronization often works with public repositories where tokens are not required
+            }
+
+            gitHubToken ??= localDarcSettings?.GitHubToken;
+            azureDevOpsToken ??= localDarcSettings?.AzureDevOpsToken;
+        }
+
+        tmpPath = Path.GetFullPath(tmpPath ?? Path.GetTempPath());
+
+        services.AddVmrManagers(GitLocation, VmrPath, tmpPath, gitHubToken, azureDevOpsToken);
+        services.TryAddTransient<IVmrScanner, VmrCloakedFileScanner>();
+    }
 }
diff --git a/test/Maestro.Web.Tests/TestDatabase.cs b/test/Maestro.Web.Tests/TestDatabase.cs
@@ -42,64 +42,55 @@ private class SharedTestDatabase : TestDatabase
 public class TestDatabase : IDisposable
 {
     private const string TestDatabasePrefix = "TFD_";
-    private string _databaseName;
-    private readonly SemaphoreSlim _createLock = new(1);
-
-    protected TestDatabase()
-    {
-    }
+    private readonly Lazy<Task<string>> _databaseName = new(
+        InitializeDatabaseAsync,
+        LazyThreadSafetyMode.ExecutionAndPublication);
 
     public void Dispose()
     {
         using var connection = new SqlConnection(BuildAssetRegistryContextFactory.GetConnectionString("master"));
         connection.Open();
         DropAllTestDatabases(connection).GetAwaiter().GetResult();
+        GC.SuppressFinalize(this);
     }
 
-    public async Task<string> GetConnectionString()
+    public async Task<string> GetConnectionString() => BuildAssetRegistryContextFactory.GetConnectionString(await _databaseName.Value);
+
+    private static async Task<string> InitializeDatabaseAsync()
     {
-        if (_databaseName != null)
-        {
-            return ConnectionString;
-        }
+        string databaseName = TestDatabasePrefix +
+            $"_{TestContext.CurrentContext.Test.ClassName!.Split('.').Last()}" +
+            $"_{TestContext.CurrentContext.Test.MethodName}" +
+            $"_{DateTime.Now:yyyyMMddHHmmss}";
+
+        TestContext.WriteLine($"Creating database '{databaseName}'");
 
-        await _createLock.WaitAsync();
-        try
+        await using (var connection = new SqlConnection(BuildAssetRegistryContextFactory.GetConnectionString("master")))
         {
-            string databaseName = $"{TestDatabasePrefix}_{TestContext.CurrentContext.Test.ClassName.Split('.').Last()}_{TestContext.CurrentContext.Test.MethodName}_{DateTime.Now:yyyyMMddHHmmss}";
-            TestContext.WriteLine($"Creating database '{databaseName}'");
-            await using (var connection = new SqlConnection(BuildAssetRegistryContextFactory.GetConnectionString("master")))
+            await connection.OpenAsync();
+            await DropAllTestDatabases(connection);
+            await using (SqlCommand createCommand = connection.CreateCommand())
             {
-                await connection.OpenAsync();
-
-                await DropAllTestDatabases(connection);
-
-                await using (SqlCommand createCommand = connection.CreateCommand())
-                {
-                    createCommand.CommandText = $"CREATE DATABASE {databaseName}";
-                    await createCommand.ExecuteNonQueryAsync();
-                }
+                createCommand.CommandText = $"CREATE DATABASE {databaseName}";
+                await createCommand.ExecuteNonQueryAsync();
             }
+        }
 
-            var collection = new ServiceCollection();
-            collection.AddSingleton<IHostEnvironment>(new HostingEnvironment
-                {EnvironmentName = Environments.Development});
-            collection.AddBuildAssetRegistry(o =>
-            {
-                o.UseSqlServer(BuildAssetRegistryContextFactory.GetConnectionString(databaseName));
-                o.EnableServiceProviderCaching(false);
-            });
+        var collection = new ServiceCollection();
+        collection.AddSingleton<IHostEnvironment>(new HostingEnvironment
+        {
+            EnvironmentName = Environments.Development
+        });
+        collection.AddBuildAssetRegistry(o =>
+        {
+            o.UseSqlServer(BuildAssetRegistryContextFactory.GetConnectionString(databaseName));
+            o.EnableServiceProviderCaching(false);
+        });
 
-            await using ServiceProvider provider = collection.BuildServiceProvider();
-            await provider.GetRequiredService<BuildAssetRegistryContext>().Database.MigrateAsync();
+        await using ServiceProvider provider = collection.BuildServiceProvider();
+        await provider.GetRequiredService<BuildAssetRegistryContext>().Database.MigrateAsync();
 
-            _databaseName = databaseName;
-            return ConnectionString;
-        }
-        finally
-        {
-            _createLock.Dispose();
-        }
+        return databaseName;
     }
 
     private static async Task DropAllTestDatabases(SqlConnection connection)
@@ -123,6 +114,4 @@ private static async Task DropAllTestDatabases(SqlConnection connection)
             await command.ExecuteNonQueryAsync();
         }
     }
-
-    private string ConnectionString => BuildAssetRegistryContextFactory.GetConnectionString(_databaseName);
 }
diff --git a/test/Microsoft.DotNet.Darc.Tests/DependencyRegistrationTests.cs b/test/Microsoft.DotNet.Darc.Tests/DependencyRegistrationTests.cs
@@ -2,7 +2,6 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
-using System.Collections.Generic;
 using System.Linq;
 using FluentAssertions;
 using Microsoft.DotNet.Darc.Options;
@@ -15,30 +14,31 @@ namespace Microsoft.DotNet.Darc.Tests;
 [TestFixture]
 public class DependencyRegistrationTests
 {
+    /// <summary>
+    /// Tests instantiating the operations
+    /// </summary>
     [Test]
-    public void AreDependenciesRegistered()
+    public void AreDarcOperationsRegistered()
     {
-        DependencyInjectionValidation.IsDependencyResolutionCoherent(services =>
+        foreach (Type optionType in Program.GetOptions().Concat(Program.GetVmrOptions()))
         {
-            // Tests instantiating the operations
-            IEnumerable<Type> optionTypes = Program.GetOptions().Concat(Program.GetVmrOptions());
-            foreach (Type optionType in optionTypes)
+            DependencyInjectionValidation.IsDependencyResolutionCoherent(services =>
             {
                 // Register the option type
                 services.AddTransient(optionType);
 
-                var operationOption = (CommandLineOptions) Activator.CreateInstance(optionType);
+                var operationOption = (CommandLineOptions)Activator.CreateInstance(optionType);
                 // Set IsCi to true to avoid login pop up
                 operationOption.IsCi = true;
                 operationOption.RegisterServices(services);
                 var provider = services.BuildServiceProvider();
 
                 // Verify we can create the operation
                 var operation = operationOption.GetOperation(provider);
-                operation.Should().NotBeNull($"Operation of {optionType.Name} could not be created");
+                operation.Should().NotBeNull($"Operation for {optionType.Name} could not be created");
                 services.AddTransient(operation.GetType());
-            }
-        },
-        out string message).Should().BeTrue(message);
+            },
+            out string message).Should().BeTrue(message);
+        }
     }
 }
diff --git a/test/ProductConstructionService.Api.Tests/TestDatabase.cs b/test/ProductConstructionService.Api.Tests/TestDatabase.cs
@@ -36,64 +36,55 @@ private class SharedTestDatabase : TestDatabase
 public class TestDatabase : IDisposable
 {
     private const string TestDatabasePrefix = "TFD_";
-    private string _databaseName = null!;
-    private readonly SemaphoreSlim _createLock = new(1);
-
-    protected TestDatabase()
-    {
-    }
+    private readonly Lazy<Task<string>> _databaseName = new(
+        InitializeDatabaseAsync,
+        LazyThreadSafetyMode.ExecutionAndPublication);
 
     public void Dispose()
     {
         using var connection = new SqlConnection(BuildAssetRegistryContextFactory.GetConnectionString("master"));
         connection.Open();
         DropAllTestDatabases(connection).GetAwaiter().GetResult();
+        GC.SuppressFinalize(this);
     }
 
-    public async Task<string> GetConnectionString()
+    public async Task<string> GetConnectionString() => BuildAssetRegistryContextFactory.GetConnectionString(await _databaseName.Value);
+
+    private static async Task<string> InitializeDatabaseAsync()
     {
-        if (_databaseName != null)
-        {
-            return ConnectionString;
-        }
+        string databaseName = TestDatabasePrefix +
+            $"_{TestContext.CurrentContext.Test.ClassName!.Split('.').Last()}" +
+            $"_{TestContext.CurrentContext.Test.MethodName}" +
+            $"_{DateTime.Now:yyyyMMddHHmmss}";
+
+        TestContext.WriteLine($"Creating database '{databaseName}'");
 
-        await _createLock.WaitAsync();
-        try
+        await using (var connection = new SqlConnection(BuildAssetRegistryContextFactory.GetConnectionString("master")))
         {
-            var databaseName = $"{TestDatabasePrefix}_{TestContext.CurrentContext.Test.ClassName!.Split('.').Last()}_{TestContext.CurrentContext.Test.MethodName}_{DateTime.Now:yyyyMMddHHmmss}";
-            TestContext.WriteLine($"Creating database '{databaseName}'");
-            await using (var connection = new SqlConnection(BuildAssetRegistryContextFactory.GetConnectionString("master")))
+            await connection.OpenAsync();
+            await DropAllTestDatabases(connection);
+            await using (SqlCommand createCommand = connection.CreateCommand())
             {
-                await connection.OpenAsync();
-
-                await DropAllTestDatabases(connection);
-
-                await using (SqlCommand createCommand = connection.CreateCommand())
-                {
-                    createCommand.CommandText = $"CREATE DATABASE {databaseName}";
-                    await createCommand.ExecuteNonQueryAsync();
-                }
+                createCommand.CommandText = $"CREATE DATABASE {databaseName}";
+                await createCommand.ExecuteNonQueryAsync();
             }
+        }
 
-            var collection = new ServiceCollection();
-            collection.AddSingleton<IHostEnvironment>(new HostingEnvironment
-            { EnvironmentName = Environments.Development });
-            collection.AddBuildAssetRegistry(o =>
-            {
-                o.UseSqlServer(BuildAssetRegistryContextFactory.GetConnectionString(databaseName));
-                o.EnableServiceProviderCaching(false);
-            });
+        var collection = new ServiceCollection();
+        collection.AddSingleton<IHostEnvironment>(new HostingEnvironment
+        {
+            EnvironmentName = Environments.Development
+        });
+        collection.AddBuildAssetRegistry(o =>
+        {
+            o.UseSqlServer(BuildAssetRegistryContextFactory.GetConnectionString(databaseName));
+            o.EnableServiceProviderCaching(false);
+        });
 
-            await using ServiceProvider provider = collection.BuildServiceProvider();
-            await provider.GetRequiredService<BuildAssetRegistryContext>().Database.MigrateAsync();
+        await using ServiceProvider provider = collection.BuildServiceProvider();
+        await provider.GetRequiredService<BuildAssetRegistryContext>().Database.MigrateAsync();
 
-            _databaseName = databaseName;
-            return ConnectionString;
-        }
-        finally
-        {
-            _createLock.Dispose();
-        }
+        return databaseName;
     }
 
     private static async Task DropAllTestDatabases(SqlConnection connection)
@@ -109,14 +100,12 @@ private static async Task DropAllTestDatabases(SqlConnection connection)
             }
         }
 
-        foreach (var db in previousTestDbs)
+        foreach (string db in previousTestDbs)
         {
             TestContext.WriteLine($"Dropping test database '{db}'");
             await using SqlCommand command = connection.CreateCommand();
             command.CommandText = $"ALTER DATABASE {db} SET single_user with rollback immediate; DROP DATABASE {db}";
             await command.ExecuteNonQueryAsync();
         }
     }
-
-    private string ConnectionString => BuildAssetRegistryContextFactory.GetConnectionString(_databaseName);
 }