From 78cf8ade63c3092f83c90f56bf944096cd0ea9b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C5=99emek=20Vysok=C3=BD?= Date: Mon, 4 Nov 2024 14:36:07 +0100 Subject: [PATCH] Fix DI for the `darc vmr get-version` command (#4120) --- .../GetRepoVersionCommandLineOptions.cs | 7 ++ .../VirtualMonoRepo/VmrCommandLineOptions.cs | 34 +------- .../VmrCommandLineOptionsBase.cs | 37 +++++++++ test/Maestro.Web.Tests/TestDatabase.cs | 77 ++++++++---------- .../DependencyRegistrationTests.cs | 22 +++--- .../TestDatabase.cs | 79 ++++++++----------- 6 files changed, 124 insertions(+), 132 deletions(-) diff --git a/src/Microsoft.DotNet.Darc/Darc/Options/VirtualMonoRepo/GetRepoVersionCommandLineOptions.cs b/src/Microsoft.DotNet.Darc/Darc/Options/VirtualMonoRepo/GetRepoVersionCommandLineOptions.cs index b78203418d..5b500dd8c1 100644 --- 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 Repositories { get; set; } = Array.Empty(); + + 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 index 5777e64d79..325bc387b2 100644 --- 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 : VmrCommandLineOptionsBase 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(); + 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 index 6005938b0a..cf31484c2b 100644 --- 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 : CommandLineOptions 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(); + } } diff --git a/test/Maestro.Web.Tests/TestDatabase.cs b/test/Maestro.Web.Tests/TestDatabase.cs index 3a71ea2369..fe1849afd3 100644 --- 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> _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 GetConnectionString() + public async Task GetConnectionString() => BuildAssetRegistryContextFactory.GetConnectionString(await _databaseName.Value); + + private static async Task 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(new HostingEnvironment - {EnvironmentName = Environments.Development}); - collection.AddBuildAssetRegistry(o => - { - o.UseSqlServer(BuildAssetRegistryContextFactory.GetConnectionString(databaseName)); - o.EnableServiceProviderCaching(false); - }); + var collection = new ServiceCollection(); + collection.AddSingleton(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().Database.MigrateAsync(); + await using ServiceProvider provider = collection.BuildServiceProvider(); + await provider.GetRequiredService().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 index 7a0c790893..809938ca26 100644 --- 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,19 +14,20 @@ namespace Microsoft.DotNet.Darc.Tests; [TestFixture] public class DependencyRegistrationTests { + /// + /// Tests instantiating the operations + /// [Test] - public void AreDependenciesRegistered() + public void AreDarcOperationsRegistered() { - DependencyInjectionValidation.IsDependencyResolutionCoherent(services => + foreach (Type optionType in Program.GetOptions().Concat(Program.GetVmrOptions())) { - // Tests instantiating the operations - IEnumerable 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); @@ -35,10 +35,10 @@ public void AreDependenciesRegistered() // 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 index db10dad529..a8decba67b 100644 --- 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> _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 GetConnectionString() + public async Task GetConnectionString() => BuildAssetRegistryContextFactory.GetConnectionString(await _databaseName.Value); + + private static async Task 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(new HostingEnvironment - { EnvironmentName = Environments.Development }); - collection.AddBuildAssetRegistry(o => - { - o.UseSqlServer(BuildAssetRegistryContextFactory.GetConnectionString(databaseName)); - o.EnableServiceProviderCaching(false); - }); + var collection = new ServiceCollection(); + collection.AddSingleton(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().Database.MigrateAsync(); + await using ServiceProvider provider = collection.BuildServiceProvider(); + await provider.GetRequiredService().Database.MigrateAsync(); - _databaseName = databaseName; - return ConnectionString; - } - finally - { - _createLock.Dispose(); - } + return databaseName; } private static async Task DropAllTestDatabases(SqlConnection connection) @@ -109,7 +100,7 @@ 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(); @@ -117,6 +108,4 @@ private static async Task DropAllTestDatabases(SqlConnection connection) await command.ExecuteNonQueryAsync(); } } - - private string ConnectionString => BuildAssetRegistryContextFactory.GetConnectionString(_databaseName); }