From ffb0a46ac819ce246c66ac56cc8bb46b43aab118 Mon Sep 17 00:00:00 2001 From: Gabriela Trutan Date: Mon, 23 Sep 2024 10:22:56 +0200 Subject: [PATCH 1/9] SLVS-1396 Rename method --- .../BindingToConnectionMigrationTests.cs | 20 +++++++++---------- .../Migration/BindingToConnectionMigration.cs | 5 ++--- .../SonarLintDaemonPackage.cs | 2 +- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/src/ConnectedMode.UnitTests/Migration/BindingToConnectionMigrationTests.cs b/src/ConnectedMode.UnitTests/Migration/BindingToConnectionMigrationTests.cs index f0d163639..d4acdf8dc 100644 --- a/src/ConnectedMode.UnitTests/Migration/BindingToConnectionMigrationTests.cs +++ b/src/ConnectedMode.UnitTests/Migration/BindingToConnectionMigrationTests.cs @@ -93,7 +93,7 @@ public async Task MigrateBindingToServerConnectionIfNeeded_RunsOnBackgroundThrea mockedThreadHandling, logger); - await migrateBindingToServer.MigrateBindingToServerConnectionIfNeededAsync(); + await migrateBindingToServer.MigrateAllBindingsToServerConnectionsIfNeededAsync(); mockedThreadHandling.ReceivedCalls().Should().Contain(call => call.GetMethodInfo().Name == nameof(IThreadHandling.RunOnBackgroundThread)); } @@ -103,7 +103,7 @@ public async Task MigrateBindingToServerConnectionIfNeeded_ConnectionsStorageFil { fileSystem.File.Exists(serverConnectionsRepository.ConnectionsStorageFilePath).Returns(true); - await testSubject.MigrateBindingToServerConnectionIfNeededAsync(); + await testSubject.MigrateAllBindingsToServerConnectionsIfNeededAsync(); fileSystem.File.Received(1).Exists(serverConnectionsRepository.ConnectionsStorageFilePath); serverConnectionsRepository.DidNotReceiveWithAnyArgs().TryAdd(default); @@ -115,7 +115,7 @@ public async Task MigrateBindingToServerConnectionIfNeeded_ConnectionsStorageDoe { CreateTwoBindingPathsToMockedBoundProject(); - await testSubject.MigrateBindingToServerConnectionIfNeededAsync(); + await testSubject.MigrateAllBindingsToServerConnectionsIfNeededAsync(); Received.InOrder(() => { @@ -132,7 +132,7 @@ public async Task MigrateBindingToServerConnectionIfNeeded_MigrationIsExecuted_C { var boundProjects = CreateTwoBindingPathsToMockedBoundProject().Values.ToList(); - await testSubject.MigrateBindingToServerConnectionIfNeededAsync(); + await testSubject.MigrateAllBindingsToServerConnectionsIfNeededAsync(); serverConnectionsRepository.Received(1).TryAdd(Arg.Is(conn => conn.Id == boundProjects[0].ServerUri.ToString())); serverConnectionsRepository.Received(1).TryAdd(Arg.Is(conn => conn.Id == boundProjects[1].ServerUri.ToString())); @@ -143,7 +143,7 @@ public async Task MigrateBindingToServerConnectionIfNeeded_MigrationIsExecuted_U { var bindingPathToBoundProjectDictionary = CreateTwoBindingPathsToMockedBoundProject(); - await testSubject.MigrateBindingToServerConnectionIfNeededAsync(); + await testSubject.MigrateAllBindingsToServerConnectionsIfNeededAsync(); CheckBindingsWereMigrated(bindingPathToBoundProjectDictionary); } @@ -157,7 +157,7 @@ public async Task MigrateBindingToServerConnectionIfNeeded_MigrationIsExecutedFo serverConnectionsRepository.TryGet(expectedServerConnectionId, out _).Returns(true); - await testSubject.MigrateBindingToServerConnectionIfNeededAsync(); + await testSubject.MigrateAllBindingsToServerConnectionsIfNeededAsync(); logger.Received(1).WriteLine(string.Format(MigrationStrings.ConnectionMigration_ExistingServerConnectionNotMigrated, expectedServerConnectionId)); serverConnectionsRepository.DidNotReceive().TryAdd(Arg.Is(conn => conn.Id == expectedServerConnectionId)); @@ -171,7 +171,7 @@ public async Task MigrateBindingToServerConnectionIfNeeded_ReadingBindingReturns var bindingPathToExclude = boundProjects.Keys.First(); legacyBindingRepository.Read(Arg.Is(path => path == bindingPathToExclude)).Returns((BoundSonarQubeProject)null); - await testSubject.MigrateBindingToServerConnectionIfNeededAsync(); + await testSubject.MigrateAllBindingsToServerConnectionsIfNeededAsync(); logger.Received(1).WriteLine(string.Format(MigrationStrings.ConnectionMigration_BindingNotMigrated, bindingPathToExclude, "legacyBoundProject was not found")); serverConnectionsRepository.DidNotReceive().TryAdd(Arg.Is(conn => IsExpectedServerConnection(conn, boundProjects.First().Value))); @@ -184,7 +184,7 @@ public async Task MigrateBindingToServerConnectionIfNeeded_MigratingConnectionFa var boundPathToBoundProject = CreateTwoBindingPathsToMockedBoundProject().First(); serverConnectionsRepository.TryAdd(Arg.Is(conn => IsExpectedServerConnection(conn, boundPathToBoundProject.Value))).Returns(false); - await testSubject.MigrateBindingToServerConnectionIfNeededAsync(); + await testSubject.MigrateAllBindingsToServerConnectionsIfNeededAsync(); logger.Received(1).WriteLine(string.Format(MigrationStrings.ConnectionMigration_ServerConnectionNotMigrated, boundPathToBoundProject.Value.ServerUri)); serverConnectionsRepository.Received(1).TryAdd(Arg.Is(conn => IsExpectedServerConnection(conn, boundPathToBoundProject.Value))); @@ -196,7 +196,7 @@ public async Task MigrateBindingToServerConnectionIfNeeded_MigrationIsExecuted_C { var boundProjects = CreateTwoBindingPathsToMockedBoundProject().Values.ToList(); - await testSubject.MigrateBindingToServerConnectionIfNeededAsync(); + await testSubject.MigrateAllBindingsToServerConnectionsIfNeededAsync(); CheckCredentialsAreLoaded(boundProjects[0]); CheckCredentialsAreLoaded(boundProjects[1]); @@ -209,7 +209,7 @@ public async Task MigrateBindingToServerConnectionIfNeeded_MigrationThrowsExcept var errorMessage = "loading failed"; solutionBindingRepository.When(repo => repo.Write(Arg.Any(), Arg.Any())).Throw(new Exception(errorMessage)); - await testSubject.MigrateBindingToServerConnectionIfNeededAsync(); + await testSubject.MigrateAllBindingsToServerConnectionsIfNeededAsync(); logger.Received(1).WriteLine(string.Format(MigrationStrings.ConnectionMigration_BindingNotMigrated, boundProjects.First().Key, errorMessage)); logger.Received(1).WriteLine(string.Format(MigrationStrings.ConnectionMigration_BindingNotMigrated, boundProjects.Last().Key, errorMessage)); diff --git a/src/ConnectedMode/Migration/BindingToConnectionMigration.cs b/src/ConnectedMode/Migration/BindingToConnectionMigration.cs index 4b114f570..d182c5b16 100644 --- a/src/ConnectedMode/Migration/BindingToConnectionMigration.cs +++ b/src/ConnectedMode/Migration/BindingToConnectionMigration.cs @@ -24,13 +24,12 @@ using SonarLint.VisualStudio.ConnectedMode.Binding; using SonarLint.VisualStudio.Core; using SonarLint.VisualStudio.Infrastructure.VS; -using SonarLint.VisualStudio.ConnectedMode.Persistence; namespace SonarLint.VisualStudio.ConnectedMode.Migration; public interface IBindingToConnectionMigration { - Task MigrateBindingToServerConnectionIfNeededAsync(); + Task MigrateAllBindingsToServerConnectionsIfNeededAsync(); } /// @@ -84,7 +83,7 @@ public BindingToConnectionMigration( this.logger = logger; } - public Task MigrateBindingToServerConnectionIfNeededAsync() + public Task MigrateAllBindingsToServerConnectionsIfNeededAsync() { return threadHandling.RunOnBackgroundThread(MigrateBindingToServerConnectionIfNeeded); } diff --git a/src/Integration.Vsix/SonarLintDaemonPackage.cs b/src/Integration.Vsix/SonarLintDaemonPackage.cs index 5aa3cafd8..1bea58870 100644 --- a/src/Integration.Vsix/SonarLintDaemonPackage.cs +++ b/src/Integration.Vsix/SonarLintDaemonPackage.cs @@ -118,7 +118,7 @@ private async Task InitAsync() private async Task MigrateBindingsToServerConnectionsIfNeededAsync() { var bindingToConnectionMigration = await this.GetMefServiceAsync(); - await bindingToConnectionMigration.MigrateBindingToServerConnectionIfNeededAsync(); + await bindingToConnectionMigration.MigrateAllBindingsToServerConnectionsIfNeededAsync(); } protected override void Dispose(bool disposing) From 9698fecbe31c228fe3c49c0efc546502bd77b1d8 Mon Sep 17 00:00:00 2001 From: Gabriela Trutan Date: Mon, 23 Sep 2024 12:41:15 +0200 Subject: [PATCH 2/9] SLVS-1396 Move method needed only for the migration of the server connection away from the UnintrusiveBindingController and into the ConnectedModeMigration class --- .../UnintrusiveBindingControllerTests.cs | 99 ------------ .../Migration/ConnectedModeMigrationTests.cs | 141 ++++++++++++++++-- .../Binding/IUnintrusiveBindingController.cs | 34 +---- .../Migration/ConnectedModeMigration.cs | 40 ++++- 4 files changed, 164 insertions(+), 150 deletions(-) diff --git a/src/ConnectedMode.UnitTests/Binding/UnintrusiveBindingControllerTests.cs b/src/ConnectedMode.UnitTests/Binding/UnintrusiveBindingControllerTests.cs index 208811bfc..66e60b69e 100644 --- a/src/ConnectedMode.UnitTests/Binding/UnintrusiveBindingControllerTests.cs +++ b/src/ConnectedMode.UnitTests/Binding/UnintrusiveBindingControllerTests.cs @@ -36,7 +36,6 @@ public class UnintrusiveBindingControllerTests { private static readonly CancellationToken ACancellationToken = CancellationToken.None; private static readonly BasicAuthCredentials ValidToken = new ("TOKEN", new SecureString()); - private static readonly BoundSonarQubeProject OldBoundProject = new (new Uri("http://any"), "any", "any"); private static readonly BoundServerProject AnyBoundProject = new ("any", "any", new ServerConnection.SonarCloud("any", credentials: ValidToken)); [TestMethod] @@ -48,8 +47,6 @@ public void MefCtor_IUnintrusiveBindingController_CheckIsExported() { MefTestHelpers.CheckTypeCanBeImported( MefTestHelpers.CreateExport(), - MefTestHelpers.CreateExport(), - MefTestHelpers.CreateExport(), MefTestHelpers.CreateExport(), MefTestHelpers.CreateExport()); } @@ -59,8 +56,6 @@ public void MefCtor_IBindingController_CheckIsExported() { MefTestHelpers.CheckTypeCanBeImported( MefTestHelpers.CreateExport(), - MefTestHelpers.CreateExport(), - MefTestHelpers.CreateExport(), MefTestHelpers.CreateExport(), MefTestHelpers.CreateExport()); } @@ -116,87 +111,12 @@ public async Task BindAsync_CallsBindingProcessInOrder() bindingProcess.SaveServerExclusionsAsync(cancellationToken); }); } - - [TestMethod] - public async Task BindWithMigrationAsync_OldProject_ConnectionExists_EstablishesBinding() - { - var cancellationToken = CancellationToken.None; - var bindingProcess = Substitute.For(); - var bindingProcessFactory = CreateBindingProcessFactory(bindingProcess); - var convertedConnection = ServerConnection.FromBoundSonarQubeProject(OldBoundProject); - var storedConnection = new ServerConnection.SonarQube(new Uri("http://any")); - var serverConnectionsRepository = CreateServerConnectionsRepository(convertedConnection.Id, storedConnection); - var solutionInfoProvider = CreateSolutionInfoProvider(); - var testSubject = CreateTestSubject(bindingProcessFactory, serverConnectionsRepository, solutionInfoProvider); - - await testSubject.BindWithMigrationAsync(OldBoundProject, null, cancellationToken); - - Received.InOrder(() => - { - serverConnectionsRepository.TryGet(convertedConnection.Id, out Arg.Any()); - solutionInfoProvider.GetSolutionNameAsync(); - bindingProcessFactory.Create(Arg.Is(b => b.ProjectToBind.ServerProjectKey == OldBoundProject.ProjectKey && b.ProjectToBind.ServerConnection == storedConnection)); - bindingProcess.DownloadQualityProfileAsync(null, cancellationToken); - bindingProcess.SaveServerExclusionsAsync(cancellationToken); - }); - } - - [TestMethod] - public async Task BindWithMigrationAsync_OldProject_ConnectionDoesNotExist_AddsConnectionAndEstablishesBinding() - { - var cancellationToken = CancellationToken.None; - var bindingProcess = Substitute.For(); - var bindingProcessFactory = CreateBindingProcessFactory(bindingProcess); - var convertedConnection = ServerConnection.FromBoundSonarQubeProject(OldBoundProject); - var serverConnectionsRepository = CreateServerConnectionsRepository(); - serverConnectionsRepository.TryAdd(Arg.Is(s => s.Id == convertedConnection.Id)).Returns(true); - var solutionInfoProvider = CreateSolutionInfoProvider(); - var testSubject = CreateTestSubject(bindingProcessFactory, serverConnectionsRepository, solutionInfoProvider); - - await testSubject.BindWithMigrationAsync(OldBoundProject, null, cancellationToken); - - Received.InOrder(() => - { - serverConnectionsRepository.TryGet(convertedConnection.Id, out Arg.Any()); - serverConnectionsRepository.TryAdd(Arg.Is(c => c.Id == convertedConnection.Id)); - solutionInfoProvider.GetSolutionNameAsync(); - bindingProcessFactory.Create(Arg.Is(b => b.ProjectToBind.ServerProjectKey == OldBoundProject.ProjectKey && b.ProjectToBind.ServerConnection.Id == convertedConnection.Id)); - bindingProcess.DownloadQualityProfileAsync(null, cancellationToken); - bindingProcess.SaveServerExclusionsAsync(cancellationToken); - }); - } - - [TestMethod] - public void BindWithMigrationAsync_OldProject_ConnectionDoesNotExist_CannotAdd_Throws() - { - var convertedConnection = ServerConnection.FromBoundSonarQubeProject(OldBoundProject); - var serverConnectionsRepository = CreateServerConnectionsRepository(convertedConnection.Id); - var testSubject = CreateTestSubject(serverConnectionsRepository: serverConnectionsRepository); - - Func act = async () => await testSubject.BindWithMigrationAsync(OldBoundProject, null, CancellationToken.None); - - act.Should().Throw().WithMessage(BindingStrings.UnintrusiveController_CantMigrateConnection); - } - [TestMethod] - public void BindWithMigrationAsync_OldProject_InvalidServerInformation_Throws() - { - var testSubject = CreateTestSubject(); - - Func act = async () => await testSubject.BindWithMigrationAsync(new BoundSonarQubeProject(), null, CancellationToken.None); - - act.Should().Throw().WithMessage(BindingStrings.UnintrusiveController_InvalidConnection); - } - private UnintrusiveBindingController CreateTestSubject(IBindingProcessFactory bindingProcessFactory = null, - IServerConnectionsRepository serverConnectionsRepository = null, - ISolutionInfoProvider solutionInfoProvider = null, ISonarQubeService sonarQubeService = null, IActiveSolutionChangedHandler activeSolutionChangedHandler = null) { var testSubject = new UnintrusiveBindingController(bindingProcessFactory ?? CreateBindingProcessFactory(), - serverConnectionsRepository ?? Substitute.For(), - solutionInfoProvider ?? Substitute.For(), sonarQubeService ?? Substitute.For(), activeSolutionChangedHandler ?? Substitute.For()); @@ -212,23 +132,4 @@ private static IBindingProcessFactory CreateBindingProcessFactory(IBindingProces return bindingProcessFactory; } - - private static IServerConnectionsRepository CreateServerConnectionsRepository(string id = null, ServerConnection.SonarQube storedConnection = null) - { - var serverConnectionsRepository = Substitute.For(); - serverConnectionsRepository.TryGet(id ?? Arg.Any(), out Arg.Any()) - .Returns(info => - { - info[1] = storedConnection; - return storedConnection != null; - }); - return serverConnectionsRepository; - } - - private static ISolutionInfoProvider CreateSolutionInfoProvider() - { - var solutionInfoProvider = Substitute.For(); - solutionInfoProvider.GetSolutionNameAsync().Returns("solution"); - return solutionInfoProvider; - } } diff --git a/src/ConnectedMode.UnitTests/Migration/ConnectedModeMigrationTests.cs b/src/ConnectedMode.UnitTests/Migration/ConnectedModeMigrationTests.cs index 99ed5476e..e22eeb10c 100644 --- a/src/ConnectedMode.UnitTests/Migration/ConnectedModeMigrationTests.cs +++ b/src/ConnectedMode.UnitTests/Migration/ConnectedModeMigrationTests.cs @@ -18,10 +18,6 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using System; -using System.Collections.Generic; -using System.Threading; -using System.Threading.Tasks; using SonarLint.VisualStudio.ConnectedMode.Binding; using SonarLint.VisualStudio.ConnectedMode.Migration; using SonarLint.VisualStudio.ConnectedMode.Shared; @@ -59,7 +55,9 @@ public void MefCtor_CheckIsExported() MefTestHelpers.CreateExport(), MefTestHelpers.CreateExport(), MefTestHelpers.CreateExport(), - MefTestHelpers.CreateExport()); + MefTestHelpers.CreateExport(), + MefTestHelpers.CreateExport(), + MefTestHelpers.CreateExport()); } [TestMethod] @@ -261,12 +259,83 @@ public async Task Migrate_CallBindAsync() { var unintrusiveBindingController = new Mock(); var cancellationToken = CancellationToken.None; - var migrationProgress = Mock.Of>(); - var testSubject = CreateTestSubject(unintrusiveBindingController: unintrusiveBindingController.Object); - await testSubject.MigrateAsync(AnyBoundProject, migrationProgress, false, cancellationToken); - unintrusiveBindingController.Verify(x => x.BindWithMigrationAsync(AnyBoundProject, It.IsAny>(), cancellationToken), Times.Once); + await testSubject.MigrateAsync(AnyBoundProject, Mock.Of>(), false, cancellationToken); + + unintrusiveBindingController.Verify( + x => x.BindAsync( + It.Is(proj => IsExpectedBoundServerProject(proj)), + It.IsAny>(), cancellationToken), Times.Once); + } + + [TestMethod] + public async Task Migrate_ConnectionExists_EstablishesBinding() + { + var storedConnection = new ServerConnection.SonarQube(AnyBoundProject.ServerUri); + var unintrusiveBindingControllerMock = new Mock(); + var serverConnectionsRepositoryMock = CreateServerConnectionsRepositoryMock(); + MockIServerConnectionsRepositoryTryGet(serverConnectionsRepositoryMock, storedConnection.Id, storedConnection); + var solutionInfoProviderMock = CreateSolutionInfoProviderMock(); + var testSubject = CreateTestSubject(unintrusiveBindingController: unintrusiveBindingControllerMock.Object, + serverConnectionsRepository: serverConnectionsRepositoryMock.Object, solutionInfoProvider: solutionInfoProviderMock.Object); + + await testSubject.MigrateAsync(AnyBoundProject, Mock.Of>(), false, CancellationToken.None); + + serverConnectionsRepositoryMock.Verify(mock => mock.TryGet(storedConnection.Id, out It.Ref.IsAny), Times.Once); + serverConnectionsRepositoryMock.Verify(mock => mock.TryAdd(IsExpectedServerConnection(It.IsAny())), Times.Never); + solutionInfoProviderMock.Verify(mock => mock.GetSolutionNameAsync(), Times.Once); + unintrusiveBindingControllerMock.Verify( + x => x.BindAsync( + It.Is(proj => IsExpectedBoundServerProject(proj)), + It.IsAny>(), It.IsAny()), Times.Once); + } + + [TestMethod] + public async Task Migrate_ConnectionDoesNotExist_AddsConnectionAndEstablishesBinding() + { + var unintrusiveBindingControllerMock = new Mock(); + var convertedConnection = ServerConnection.FromBoundSonarQubeProject(AnyBoundProject); + var serverConnectionsRepositoryMock = CreateServerConnectionsRepositoryMock(); + var solutionInfoProviderMock = CreateSolutionInfoProviderMock(); + var testSubject = CreateTestSubject(unintrusiveBindingController: unintrusiveBindingControllerMock.Object, + serverConnectionsRepository: serverConnectionsRepositoryMock.Object, solutionInfoProvider: solutionInfoProviderMock.Object); + serverConnectionsRepositoryMock.Setup(x => x.TryAdd(IsExpectedServerConnection(convertedConnection))).Returns(true); + + await testSubject.MigrateAsync(AnyBoundProject, Mock.Of>(), false, CancellationToken.None); + + serverConnectionsRepositoryMock.Verify(mock => mock.TryGet(convertedConnection.Id, out It.Ref.IsAny), Times.Once); + serverConnectionsRepositoryMock.Verify(mock => mock.TryAdd(IsExpectedServerConnection(convertedConnection)), Times.Once); + solutionInfoProviderMock.Verify(mock => mock.GetSolutionNameAsync(), Times.Once); + unintrusiveBindingControllerMock.Verify( + x => x.BindAsync( + It.Is(proj => IsExpectedBoundServerProject(proj)), + It.IsAny>(), It.IsAny()), Times.Once); + } + + [TestMethod] + public void Migrate_ConnectionDoesNotExist_CannotAdd_Throws() + { + var unintrusiveBindingController = new Mock(); + var convertedConnection = ServerConnection.FromBoundSonarQubeProject(AnyBoundProject); + var serverConnectionsRepositoryMock = new Mock(); + serverConnectionsRepositoryMock.Setup(x => x.TryAdd(convertedConnection)).Throws(new Exception()); + + var testSubject = CreateTestSubject(unintrusiveBindingController: unintrusiveBindingController.Object, serverConnectionsRepository: serverConnectionsRepositoryMock.Object); + + Func act = async () => await testSubject.MigrateAsync(AnyBoundProject, Mock.Of>(), false, CancellationToken.None); + + act.Should().Throw().WithMessage(BindingStrings.UnintrusiveController_CantMigrateConnection); + } + + [TestMethod] + public void Migrate_InvalidServerInformation_Throws() + { + var testSubject = CreateTestSubject(); + + Func act = async () => await testSubject.MigrateAsync(new BoundSonarQubeProject(), Mock.Of>(), false, CancellationToken.None); + + act.Should().Throw(); } [TestMethod] @@ -303,7 +372,9 @@ private static ConnectedModeMigration CreateTestSubject( ISuppressionIssueStoreUpdater suppressionIssueStoreUpdater = null, ISharedBindingConfigProvider sharedBindingConfigProvider = null, ILogger logger = null, - IThreadHandling threadHandling = null) + IThreadHandling threadHandling = null, + ISolutionInfoProvider solutionInfoProvider = null, + IServerConnectionsRepository serverConnectionsRepository = null) { fileProvider ??= Mock.Of(); fileCleaner ??= Mock.Of(); @@ -313,11 +384,24 @@ private static ConnectedModeMigration CreateTestSubject( suppressionIssueStoreUpdater ??= Mock.Of(); settingsProvider ??= CreateSettingsProvider(DefaultTestLegacySettings).Object; sharedBindingConfigProvider ??= Mock.Of(); + solutionInfoProvider ??= CreateSolutionInfoProviderMock().Object; + serverConnectionsRepository ??= CreateServerConnectionsRepositoryMock().Object; logger ??= new TestLogger(logToConsole: true); threadHandling ??= new NoOpThreadHandler(); - return new ConnectedModeMigration(settingsProvider, fileProvider, fileCleaner, fileSystem, sonarQubeService, unintrusiveBindingController, suppressionIssueStoreUpdater, sharedBindingConfigProvider, logger, threadHandling); + return new ConnectedModeMigration(settingsProvider, + fileProvider, + fileCleaner, + fileSystem, + sonarQubeService, + unintrusiveBindingController, + suppressionIssueStoreUpdater, + sharedBindingConfigProvider, + logger, + threadHandling, + solutionInfoProvider, + serverConnectionsRepository); } private static Mock CreateFileProvider(params string[] filesToReturn) @@ -345,6 +429,41 @@ private static Mock CreateSettingsProvider(LegacySet settingsProvider.Setup(x => x.GetAsync(It.IsAny())).Returns(Task.FromResult(settingsToReturn)); return settingsProvider; } + + private static Mock CreateServerConnectionsRepositoryMock() + { + var serverConnectionsRepositoryMock = new Mock(); + serverConnectionsRepositoryMock.Setup(x => x.TryAdd(It.IsAny())).Returns(true); + + return serverConnectionsRepositoryMock; + } + + private static void MockIServerConnectionsRepositoryTryGet(Mock serverConnectionsRepositoryMock, string id = null, ServerConnection.SonarQube storedConnection = null) + { + serverConnectionsRepositoryMock.Setup(service => service.TryGet(id ?? It.IsAny(), out It.Ref.IsAny)) + .Returns((string _, out ServerConnection value) => + { + value = storedConnection; + return storedConnection != null; + }); + } + + private static Mock CreateSolutionInfoProviderMock() + { + var createSolutionInfoProviderMock = new Mock(); + createSolutionInfoProviderMock.Setup(mock => mock.GetSolutionNameAsync()).ReturnsAsync("solution"); + return createSolutionInfoProviderMock; + } + + private static bool IsExpectedBoundServerProject(BoundServerProject proj) + { + return proj.ServerProjectKey == AnyBoundProject.ProjectKey && proj.ServerConnection.ServerUri == AnyBoundProject.ServerUri; + } + + private static ServerConnection IsExpectedServerConnection(ServerConnection convertedConnection) + { + return It.Is(conn => conn.Id == convertedConnection.Id); + } } // Extension methods to make the mocks easier to work with. diff --git a/src/ConnectedMode/Binding/IUnintrusiveBindingController.cs b/src/ConnectedMode/Binding/IUnintrusiveBindingController.cs index 15b29fa29..ef1b2b575 100644 --- a/src/ConnectedMode/Binding/IUnintrusiveBindingController.cs +++ b/src/ConnectedMode/Binding/IUnintrusiveBindingController.cs @@ -19,11 +19,8 @@ */ using System.ComponentModel.Composition; -using SonarLint.VisualStudio.ConnectedMode.Persistence; -using SonarLint.VisualStudio.Core; using SonarLint.VisualStudio.Core.Binding; using SonarQube.Client; -using SonarQube.Client.Models; using Task = System.Threading.Tasks.Task; namespace SonarLint.VisualStudio.ConnectedMode.Binding @@ -35,7 +32,6 @@ public interface IBindingController internal interface IUnintrusiveBindingController { - Task BindWithMigrationAsync(BoundSonarQubeProject project, IProgress progress, CancellationToken token); Task BindAsync(BoundServerProject project, IProgress progress, CancellationToken token); } @@ -45,17 +41,13 @@ internal interface IUnintrusiveBindingController internal class UnintrusiveBindingController : IUnintrusiveBindingController, IBindingController { private readonly IBindingProcessFactory bindingProcessFactory; - private readonly IServerConnectionsRepository serverConnectionsRepository; - private readonly ISolutionInfoProvider solutionInfoProvider; private readonly ISonarQubeService sonarQubeService; private readonly IActiveSolutionChangedHandler activeSolutionChangedHandler; [ImportingConstructor] - public UnintrusiveBindingController(IBindingProcessFactory bindingProcessFactory, IServerConnectionsRepository serverConnectionsRepository, ISolutionInfoProvider solutionInfoProvider, ISonarQubeService sonarQubeService, IActiveSolutionChangedHandler activeSolutionChangedHandler) + public UnintrusiveBindingController(IBindingProcessFactory bindingProcessFactory, ISonarQubeService sonarQubeService, IActiveSolutionChangedHandler activeSolutionChangedHandler) { this.bindingProcessFactory = bindingProcessFactory; - this.serverConnectionsRepository = serverConnectionsRepository; - this.solutionInfoProvider = solutionInfoProvider; this.sonarQubeService = sonarQubeService; this.activeSolutionChangedHandler = activeSolutionChangedHandler; } @@ -75,30 +67,6 @@ public async Task BindAsync(BoundServerProject project, IProgress progress, CancellationToken token) - { - var proposedConnection = ServerConnection.FromBoundSonarQubeProject(project); - - if (proposedConnection is null) - { - throw new InvalidOperationException(BindingStrings.UnintrusiveController_InvalidConnection); - } - - var connection = GetExistingConnection(proposedConnection) ?? MigrateConnection(proposedConnection); - - await BindAsync(BoundServerProject.FromBoundSonarQubeProject(project, await solutionInfoProvider.GetSolutionNameAsync(), connection), progress, token); - } - - private ServerConnection GetExistingConnection(ServerConnection proposedConnection) => - serverConnectionsRepository.TryGet(proposedConnection.Id, out var connection) - ? connection - : null; - - private ServerConnection MigrateConnection(ServerConnection proposedConnection) => - serverConnectionsRepository.TryAdd(proposedConnection) - ? proposedConnection - : throw new InvalidOperationException(BindingStrings.UnintrusiveController_CantMigrateConnection); - private IBindingProcess CreateBindingProcess(BoundServerProject project) { var bindingProcess = bindingProcessFactory.Create(new BindCommandArgs(project)); diff --git a/src/ConnectedMode/Migration/ConnectedModeMigration.cs b/src/ConnectedMode/Migration/ConnectedModeMigration.cs index 91cf43b30..1af4c3bff 100644 --- a/src/ConnectedMode/Migration/ConnectedModeMigration.cs +++ b/src/ConnectedMode/Migration/ConnectedModeMigration.cs @@ -18,12 +18,7 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using System; -using System.Collections.Generic; using System.ComponentModel.Composition; -using System.Diagnostics; -using System.Linq; -using System.Threading; using SonarLint.VisualStudio.ConnectedMode.Binding; using SonarLint.VisualStudio.ConnectedMode.Shared; using SonarLint.VisualStudio.ConnectedMode.Suppressions; @@ -51,6 +46,8 @@ private sealed class ChangedFiles : List> { } private readonly ISharedBindingConfigProvider sharedBindingConfigProvider; private readonly ILogger logger; private readonly IThreadHandling threadHandling; + private readonly ISolutionInfoProvider solutionInfoProvider; + private readonly IServerConnectionsRepository serverConnectionsRepository; // The user can have both the legacy and new connected mode files. In that case, we expect the SonarQubeService to already be connected. private bool isAlreadyConnectedToServer; @@ -65,7 +62,9 @@ public ConnectedModeMigration(IMigrationSettingsProvider settingsProvider, ISuppressionIssueStoreUpdater suppressionIssueStoreUpdater, ISharedBindingConfigProvider sharedBindingConfigProvider, ILogger logger, - IThreadHandling threadHandling) + IThreadHandling threadHandling, + ISolutionInfoProvider solutionInfoProvider, + IServerConnectionsRepository serverConnectionsRepository) { this.settingsProvider = settingsProvider; this.fileProvider = fileProvider; @@ -78,6 +77,8 @@ public ConnectedModeMigration(IMigrationSettingsProvider settingsProvider, this.logger = logger; this.threadHandling = threadHandling; + this.solutionInfoProvider = solutionInfoProvider; + this.serverConnectionsRepository = serverConnectionsRepository; } public async Task MigrateAsync(BoundSonarQubeProject oldBinding, IProgress progress, bool shareBinding, CancellationToken token) @@ -136,7 +137,7 @@ private async Task MigrateImplAsync(BoundSonarQubeProject oldBinding, IProgress< logger.WriteLine(MigrationStrings.Process_ProcessingNewBinding); var progressAdapter = new FixedStepsProgressToMigrationProgressAdapter(progress); - await unintrusiveBindingController.BindWithMigrationAsync(oldBinding, progressAdapter, token); + await BindWithMigrationAsync(oldBinding, progressAdapter, token); // Now make all of the files changes required to remove the legacy settings // i.e. update project files and delete .sonarlint folder @@ -263,5 +264,30 @@ private async Task SaveChangedFilesAsync(ChangedFiles changedFiles, IProgress progress, CancellationToken token) + { + var proposedConnection = ServerConnection.FromBoundSonarQubeProject(project); + if (proposedConnection is null) + { + throw new InvalidOperationException(BindingStrings.UnintrusiveController_InvalidConnection); + } + + // at this point we expect that the connections were already migrated to the newer format. See IBindingToConnectionMigration + var connection = GetExistingConnection(proposedConnection) ?? MigrateConnection(proposedConnection); + + await unintrusiveBindingController.BindAsync(BoundServerProject.FromBoundSonarQubeProject(project, await solutionInfoProvider.GetSolutionNameAsync(), connection), progress, token); + } + + private ServerConnection GetExistingConnection(ServerConnection proposedConnection) => + serverConnectionsRepository.TryGet(proposedConnection.Id, out var connection) + ? connection + : null; + + private ServerConnection MigrateConnection(ServerConnection proposedConnection) => + serverConnectionsRepository.TryAdd(proposedConnection) + ? proposedConnection + : throw new InvalidOperationException(BindingStrings.UnintrusiveController_CantMigrateConnection); + } } From a6534318d6380e9a0e431c96c3940fde0d7f22f4 Mon Sep 17 00:00:00 2001 From: Gabriela Trutan Date: Mon, 23 Sep 2024 15:25:25 +0200 Subject: [PATCH 3/9] SLVS-1396 Refactor ServerConnectionsRepository to not expose the file path, but instead only if the file exists. The callers do not need to know the name of the file, only if the file exists --- .../BindingToConnectionMigrationTests.cs | 12 +++--------- .../ServerConnectionsRepositoryTests.cs | 17 ++++++++++++++++- .../Migration/BindingToConnectionMigration.cs | 5 +---- .../Persistence/ServerConnectionsRepository.cs | 16 +++++++++++----- .../Binding/IServerConnectionsRepository.cs | 2 +- 5 files changed, 32 insertions(+), 20 deletions(-) diff --git a/src/ConnectedMode.UnitTests/Migration/BindingToConnectionMigrationTests.cs b/src/ConnectedMode.UnitTests/Migration/BindingToConnectionMigrationTests.cs index d4acdf8dc..94e723027 100644 --- a/src/ConnectedMode.UnitTests/Migration/BindingToConnectionMigrationTests.cs +++ b/src/ConnectedMode.UnitTests/Migration/BindingToConnectionMigrationTests.cs @@ -18,13 +18,11 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ -using System.IO.Abstractions; using SonarLint.VisualStudio.ConnectedMode.Binding; using SonarLint.VisualStudio.ConnectedMode.Migration; using SonarLint.VisualStudio.ConnectedMode.Persistence; using SonarLint.VisualStudio.Core; using SonarLint.VisualStudio.Core.Binding; -using SonarLint.VisualStudio.Infrastructure.VS; using SonarLint.VisualStudio.TestInfrastructure; using SonarQube.Client.Helpers; @@ -34,7 +32,6 @@ namespace SonarLint.VisualStudio.ConnectedMode.UnitTests.Migration; public class BindingToConnectionMigrationTests { private BindingToConnectionMigration testSubject; - private IFileSystem fileSystem; private IServerConnectionsRepository serverConnectionsRepository; private ILegacySolutionBindingRepository legacyBindingRepository; private IUnintrusiveBindingPathProvider unintrusiveBindingPathProvider; @@ -45,7 +42,6 @@ public class BindingToConnectionMigrationTests [TestInitialize] public void TestInitialize() { - fileSystem = Substitute.For(); serverConnectionsRepository = Substitute.For(); legacyBindingRepository = Substitute.For(); solutionBindingRepository = Substitute.For(); @@ -54,7 +50,6 @@ public void TestInitialize() threadHandling = new NoOpThreadHandler(); testSubject = new BindingToConnectionMigration( - fileSystem, serverConnectionsRepository, legacyBindingRepository, solutionBindingRepository, @@ -85,7 +80,6 @@ public async Task MigrateBindingToServerConnectionIfNeeded_RunsOnBackgroundThrea { var mockedThreadHandling = Substitute.For(); var migrateBindingToServer = new BindingToConnectionMigration( - fileSystem, serverConnectionsRepository, legacyBindingRepository, solutionBindingRepository, @@ -101,11 +95,11 @@ public async Task MigrateBindingToServerConnectionIfNeeded_RunsOnBackgroundThrea [TestMethod] public async Task MigrateBindingToServerConnectionIfNeeded_ConnectionsStorageFileExists_ShouldNotMigrate() { - fileSystem.File.Exists(serverConnectionsRepository.ConnectionsStorageFilePath).Returns(true); + serverConnectionsRepository.IsConnectionsFileExisting().Returns(true); await testSubject.MigrateAllBindingsToServerConnectionsIfNeededAsync(); - fileSystem.File.Received(1).Exists(serverConnectionsRepository.ConnectionsStorageFilePath); + serverConnectionsRepository.Received(1).IsConnectionsFileExisting(); serverConnectionsRepository.DidNotReceiveWithAnyArgs().TryAdd(default); unintrusiveBindingPathProvider.DidNotReceive().GetBindingPaths(); } @@ -119,7 +113,7 @@ public async Task MigrateBindingToServerConnectionIfNeeded_ConnectionsStorageDoe Received.InOrder(() => { - fileSystem.File.Exists(serverConnectionsRepository.ConnectionsStorageFilePath); + serverConnectionsRepository.IsConnectionsFileExisting(); logger.WriteLine(MigrationStrings.ConnectionMigration_StartMigration); unintrusiveBindingPathProvider.GetBindingPaths(); serverConnectionsRepository.TryAdd(Arg.Any()); diff --git a/src/ConnectedMode.UnitTests/Persistence/ServerConnectionsRepositoryTests.cs b/src/ConnectedMode.UnitTests/Persistence/ServerConnectionsRepositoryTests.cs index 7f838cda6..8276f17eb 100644 --- a/src/ConnectedMode.UnitTests/Persistence/ServerConnectionsRepositoryTests.cs +++ b/src/ConnectedMode.UnitTests/Persistence/ServerConnectionsRepositoryTests.cs @@ -19,6 +19,7 @@ */ using System.IO; +using System.IO.Abstractions; using SonarLint.VisualStudio.ConnectedMode.Binding; using SonarLint.VisualStudio.ConnectedMode.Persistence; using SonarLint.VisualStudio.Core; @@ -40,6 +41,7 @@ public class ServerConnectionsRepositoryTests private ISolutionBindingCredentialsLoader credentialsLoader; private readonly SonarCloud sonarCloudServerConnection = new("myOrganization", new ServerConnectionSettings(true), Substitute.For()); private readonly ServerConnection.SonarQube sonarQubeServerConnection = new(new Uri("http://localhost"), new ServerConnectionSettings(true), Substitute.For()); + private IFileSystem fileSystem; [TestInitialize] public void TestInitialize() @@ -49,8 +51,9 @@ public void TestInitialize() credentialsLoader = Substitute.For(); environmentVariableProvider = Substitute.For(); logger = Substitute.For(); + fileSystem = Substitute.For(); - testSubject = new ServerConnectionsRepository(jsonFileHandler, serverConnectionModelMapper, credentialsLoader, environmentVariableProvider, logger); + testSubject = new ServerConnectionsRepository(jsonFileHandler, serverConnectionModelMapper, credentialsLoader, environmentVariableProvider, fileSystem, logger); } [TestMethod] @@ -537,6 +540,18 @@ public void TryUpdateCredentialsById_SonarQubeConnectionExists_UpdatesCredential credentialsLoader.Received(1).Save(newCredentials, sonarQube.ServerUri); } + [TestMethod] + [DataRow(true)] + [DataRow(false)] + public void IsConnectionsFileExisting_ReturnsTrueOnlyIfTheConnectionsFileExists(bool fileExists) + { + fileSystem.File.Exists(Arg.Any()).Returns(fileExists); + + var result = testSubject.IsConnectionsFileExisting(); + + result.Should().Be(fileExists); + } + [TestMethod] public void TryUpdateCredentialsById_SavingCredentialsThrows_ReturnsFalseAndLogs() { diff --git a/src/ConnectedMode/Migration/BindingToConnectionMigration.cs b/src/ConnectedMode/Migration/BindingToConnectionMigration.cs index d182c5b16..53b577484 100644 --- a/src/ConnectedMode/Migration/BindingToConnectionMigration.cs +++ b/src/ConnectedMode/Migration/BindingToConnectionMigration.cs @@ -56,7 +56,6 @@ public BindingToConnectionMigration( IUnintrusiveBindingPathProvider unintrusiveBindingPathProvider, ILogger logger) : this( - new FileSystem(), serverConnectionsRepository, legacyBindingRepository, solutionBindingRepository, @@ -66,7 +65,6 @@ public BindingToConnectionMigration( { } internal /* for testing */ BindingToConnectionMigration( - IFileSystem fileSystem, IServerConnectionsRepository serverConnectionsRepository, ILegacySolutionBindingRepository legacyBindingRepository, ISolutionBindingRepository solutionBindingRepository, @@ -74,7 +72,6 @@ public BindingToConnectionMigration( IThreadHandling threadHandling, ILogger logger) { - this.fileSystem = fileSystem; this.serverConnectionsRepository = serverConnectionsRepository; this.legacyBindingRepository = legacyBindingRepository; this.solutionBindingRepository = solutionBindingRepository; @@ -90,7 +87,7 @@ public Task MigrateAllBindingsToServerConnectionsIfNeededAsync() private void MigrateBindingToServerConnectionIfNeeded() { - if (fileSystem.File.Exists(serverConnectionsRepository.ConnectionsStorageFilePath)) + if (serverConnectionsRepository.IsConnectionsFileExisting()) { return; } diff --git a/src/ConnectedMode/Persistence/ServerConnectionsRepository.cs b/src/ConnectedMode/Persistence/ServerConnectionsRepository.cs index e7e47a8bd..4e4b737d1 100644 --- a/src/ConnectedMode/Persistence/ServerConnectionsRepository.cs +++ b/src/ConnectedMode/Persistence/ServerConnectionsRepository.cs @@ -22,6 +22,7 @@ using SonarLint.VisualStudio.Core.Binding; using System.ComponentModel.Composition; using System.IO; +using System.IO.Abstractions; using SonarLint.VisualStudio.Core.Persistence; using SonarLint.VisualStudio.ConnectedMode.Binding; @@ -35,13 +36,13 @@ internal class ServerConnectionsRepository : IServerConnectionsRepository internal const string ConnectionsFileName = "connections.json"; private readonly ISolutionBindingCredentialsLoader credentialsLoader; + private readonly IFileSystem fileSystem; private readonly ILogger logger; private readonly IJsonFileHandler jsonFileHandle; private readonly IServerConnectionModelMapper serverConnectionModelMapper; + private readonly string connectionsStorageFilePath; private static readonly object LockObject = new(); - public string ConnectionsStorageFilePath { get; } - [ImportingConstructor] public ServerConnectionsRepository( IJsonFileHandler jsonFileHandle, @@ -51,6 +52,7 @@ public ServerConnectionsRepository( serverConnectionModelMapper, new SolutionBindingCredentialsLoader(credentialStoreService), EnvironmentVariableProvider.Instance, + new FileSystem(), logger) { } internal /* for testing */ ServerConnectionsRepository( @@ -58,13 +60,15 @@ public ServerConnectionsRepository( IServerConnectionModelMapper serverConnectionModelMapper, ISolutionBindingCredentialsLoader credentialsLoader, IEnvironmentVariableProvider environmentVariables, + IFileSystem fileSystem, ILogger logger) { this.jsonFileHandle = jsonFileHandle; this.serverConnectionModelMapper = serverConnectionModelMapper; this.credentialsLoader = credentialsLoader; + this.fileSystem = fileSystem; this.logger = logger; - ConnectionsStorageFilePath = GetStorageFilePath(environmentVariables); + connectionsStorageFilePath = GetStorageFilePath(environmentVariables); } public bool TryGet(string connectionId, out ServerConnection serverConnection) @@ -133,6 +137,8 @@ public bool TryUpdateCredentialsById(string connectionId, ICredentials credentia return false; } + public bool IsConnectionsFileExisting() => fileSystem.File.Exists(connectionsStorageFilePath); + private bool TryAddConnection(List connections, ServerConnection connection) { if (connection.Credentials is null) @@ -196,7 +202,7 @@ private List ReadServerConnectionsFromFile() { try { - var model = jsonFileHandle.ReadFile(ConnectionsStorageFilePath); + var model = jsonFileHandle.ReadFile(connectionsStorageFilePath); return model.ServerConnections.Select(serverConnectionModelMapper.GetServerConnection).ToList(); } catch (FileNotFoundException) @@ -223,7 +229,7 @@ private bool SafeUpdateConnectionsFile(Func, bool> tryUpd if (tryUpdateConnectionModels(serverConnections)) { var model = serverConnectionModelMapper.GetServerConnectionsListJsonModel(serverConnections); - return jsonFileHandle.TryWriteToFile(ConnectionsStorageFilePath, model); + return jsonFileHandle.TryWriteToFile(connectionsStorageFilePath, model); } } catch (Exception ex) when (!ErrorHandler.IsCriticalException(ex)) diff --git a/src/Core/Binding/IServerConnectionsRepository.cs b/src/Core/Binding/IServerConnectionsRepository.cs index aab4e409b..c73fecb30 100644 --- a/src/Core/Binding/IServerConnectionsRepository.cs +++ b/src/Core/Binding/IServerConnectionsRepository.cs @@ -28,5 +28,5 @@ public interface IServerConnectionsRepository bool TryDelete(string connectionId); bool TryUpdateSettingsById(string connectionId, ServerConnectionSettings connectionSettings); bool TryUpdateCredentialsById(string connectionId, ICredentials credentials); - string ConnectionsStorageFilePath { get; } + bool IsConnectionsFileExisting(); } From 08a3d458ef3a8490c268f6e95cc30728f2555a74 Mon Sep 17 00:00:00 2001 From: Gabriela Trutan Date: Mon, 23 Sep 2024 15:32:55 +0200 Subject: [PATCH 4/9] SLVS-1396 Validate that the old migration does not create the connection.json, which would prevent the new migration to migrate the existing bindings. --- .../Migration/ConnectedModeMigrationTests.cs | 14 ++++++++++++++ .../Migration/ConnectedModeMigration.cs | 9 ++++++++- .../Migration/MigrationStrings.Designer.cs | 9 +++++++++ src/ConnectedMode/Migration/MigrationStrings.resx | 3 +++ 4 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/ConnectedMode.UnitTests/Migration/ConnectedModeMigrationTests.cs b/src/ConnectedMode.UnitTests/Migration/ConnectedModeMigrationTests.cs index e22eeb10c..371d9f781 100644 --- a/src/ConnectedMode.UnitTests/Migration/ConnectedModeMigrationTests.cs +++ b/src/ConnectedMode.UnitTests/Migration/ConnectedModeMigrationTests.cs @@ -319,6 +319,7 @@ public void Migrate_ConnectionDoesNotExist_CannotAdd_Throws() var unintrusiveBindingController = new Mock(); var convertedConnection = ServerConnection.FromBoundSonarQubeProject(AnyBoundProject); var serverConnectionsRepositoryMock = new Mock(); + serverConnectionsRepositoryMock.Setup(x => x.IsConnectionsFileExisting()).Returns(true); serverConnectionsRepositoryMock.Setup(x => x.TryAdd(convertedConnection)).Throws(new Exception()); var testSubject = CreateTestSubject(unintrusiveBindingController: unintrusiveBindingController.Object, serverConnectionsRepository: serverConnectionsRepositoryMock.Object); @@ -328,6 +329,18 @@ public void Migrate_ConnectionDoesNotExist_CannotAdd_Throws() act.Should().Throw().WithMessage(BindingStrings.UnintrusiveController_CantMigrateConnection); } + [TestMethod] + public void Migrate_ConnectionsJsonFileDoesNotExist_Throws() + { + var serverConnectionsRepositoryMock = new Mock(); + serverConnectionsRepositoryMock.Setup(mock => mock.IsConnectionsFileExisting()).Returns(false); + var testSubject = CreateTestSubject(serverConnectionsRepository: serverConnectionsRepositoryMock.Object); + + Func act = async () => await testSubject.MigrateAsync(AnyBoundProject, Mock.Of>(), false, CancellationToken.None); + + act.Should().Throw().WithMessage(MigrationStrings.ConnectionsJson_DoesNotExist); + } + [TestMethod] public void Migrate_InvalidServerInformation_Throws() { @@ -434,6 +447,7 @@ private static Mock CreateServerConnectionsReposit { var serverConnectionsRepositoryMock = new Mock(); serverConnectionsRepositoryMock.Setup(x => x.TryAdd(It.IsAny())).Returns(true); + serverConnectionsRepositoryMock.Setup(x => x.IsConnectionsFileExisting()).Returns(true); return serverConnectionsRepositoryMock; } diff --git a/src/ConnectedMode/Migration/ConnectedModeMigration.cs b/src/ConnectedMode/Migration/ConnectedModeMigration.cs index 1af4c3bff..ff1bf7e80 100644 --- a/src/ConnectedMode/Migration/ConnectedModeMigration.cs +++ b/src/ConnectedMode/Migration/ConnectedModeMigration.cs @@ -267,13 +267,20 @@ private async Task SaveChangedFilesAsync(ChangedFiles changedFiles, IProgress progress, CancellationToken token) { + // at this point we expect that the connections file exist, meaning that all the existing bindings are already migrated to the newer format + // if the file doesn't exist, creating it now will prevent the migration of all existing bindings. + // The order being important, we throw an exception. For more info see IBindingToConnectionMigration + if (!serverConnectionsRepository.IsConnectionsFileExisting()) + { + throw new InvalidOperationException(MigrationStrings.ConnectionsJson_DoesNotExist); + } + var proposedConnection = ServerConnection.FromBoundSonarQubeProject(project); if (proposedConnection is null) { throw new InvalidOperationException(BindingStrings.UnintrusiveController_InvalidConnection); } - // at this point we expect that the connections were already migrated to the newer format. See IBindingToConnectionMigration var connection = GetExistingConnection(proposedConnection) ?? MigrateConnection(proposedConnection); await unintrusiveBindingController.BindAsync(BoundServerProject.FromBoundSonarQubeProject(project, await solutionInfoProvider.GetSolutionNameAsync(), connection), progress, token); diff --git a/src/ConnectedMode/Migration/MigrationStrings.Designer.cs b/src/ConnectedMode/Migration/MigrationStrings.Designer.cs index 64416e942..09637d9c7 100644 --- a/src/ConnectedMode/Migration/MigrationStrings.Designer.cs +++ b/src/ConnectedMode/Migration/MigrationStrings.Designer.cs @@ -141,6 +141,15 @@ internal static string ConnectionMigration_StartMigration { } } + /// + /// Looks up a localized string similar to [Migration] - Connections.json file does not exist. The new migration has to be performed first.. + /// + internal static string ConnectionsJson_DoesNotExist { + get { + return ResourceManager.GetString("ConnectionsJson_DoesNotExist", resourceCulture); + } + } + /// /// Looks up a localized string similar to [Migration] Error during migration: {0} /// Run migration again with verbose logging enabled for more information.. diff --git a/src/ConnectedMode/Migration/MigrationStrings.resx b/src/ConnectedMode/Migration/MigrationStrings.resx index 89eae9e97..ff1db17db 100644 --- a/src/ConnectedMode/Migration/MigrationStrings.resx +++ b/src/ConnectedMode/Migration/MigrationStrings.resx @@ -227,4 +227,7 @@ [Connection Migration] The server connection with the id {0} was not migrated as it already exists. + + [Migration] - Connections.json file does not exist. The new migration has to be performed first. + \ No newline at end of file From c3d8b689e154a2fa7bc9f65d2a3607f19abe8c1e Mon Sep 17 00:00:00 2001 From: Gabriela Trutan Date: Mon, 23 Sep 2024 15:41:06 +0200 Subject: [PATCH 5/9] SLVS-1396 Remove not needed white space --- src/ConnectedMode/Migration/MigrationStrings.Designer.cs | 2 +- src/ConnectedMode/Migration/MigrationStrings.resx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ConnectedMode/Migration/MigrationStrings.Designer.cs b/src/ConnectedMode/Migration/MigrationStrings.Designer.cs index 09637d9c7..c0979f7d7 100644 --- a/src/ConnectedMode/Migration/MigrationStrings.Designer.cs +++ b/src/ConnectedMode/Migration/MigrationStrings.Designer.cs @@ -142,7 +142,7 @@ internal static string ConnectionMigration_StartMigration { } /// - /// Looks up a localized string similar to [Migration] - Connections.json file does not exist. The new migration has to be performed first.. + /// Looks up a localized string similar to [Migration] Connections.json file does not exist. The new migration has to be performed first.. /// internal static string ConnectionsJson_DoesNotExist { get { diff --git a/src/ConnectedMode/Migration/MigrationStrings.resx b/src/ConnectedMode/Migration/MigrationStrings.resx index ff1db17db..c37f8f0b6 100644 --- a/src/ConnectedMode/Migration/MigrationStrings.resx +++ b/src/ConnectedMode/Migration/MigrationStrings.resx @@ -228,6 +228,6 @@ [Connection Migration] The server connection with the id {0} was not migrated as it already exists. - [Migration] - Connections.json file does not exist. The new migration has to be performed first. + [Migration] Connections.json file does not exist. The new migration has to be performed first. \ No newline at end of file From f0a23a780c3d0be1d00d76c53c96afa0b32f00f7 Mon Sep 17 00:00:00 2001 From: Gabriela Trutan Date: Mon, 23 Sep 2024 15:54:50 +0200 Subject: [PATCH 6/9] SLVS-1396 Remove no longer needed field. --- src/ConnectedMode/Migration/BindingToConnectionMigration.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ConnectedMode/Migration/BindingToConnectionMigration.cs b/src/ConnectedMode/Migration/BindingToConnectionMigration.cs index 53b577484..57e1c7c05 100644 --- a/src/ConnectedMode/Migration/BindingToConnectionMigration.cs +++ b/src/ConnectedMode/Migration/BindingToConnectionMigration.cs @@ -40,7 +40,6 @@ public interface IBindingToConnectionMigration [PartCreationPolicy(CreationPolicy.Shared)] internal class BindingToConnectionMigration : IBindingToConnectionMigration { - private readonly IFileSystem fileSystem; private readonly IServerConnectionsRepository serverConnectionsRepository; private readonly ILegacySolutionBindingRepository legacyBindingRepository; private readonly ISolutionBindingRepository solutionBindingRepository; From 6d6c53fe5a12b8acc71c00413e3e94e8fbfb64a5 Mon Sep 17 00:00:00 2001 From: Gabriela Trutan Date: Wed, 25 Sep 2024 11:47:34 +0200 Subject: [PATCH 7/9] SLVS-1396 Ensure old migration succeeds even if the corresponding ServerConnection could not be migrated. If it can not be migrate, due to missing credentials, for example, the old migration should still succeed. The user will just have to manually add the migration --- .../Migration/ConnectedModeMigrationTests.cs | 82 +++++++++++++++++-- .../Binding/BindingStrings.Designer.cs | 10 +-- src/ConnectedMode/Binding/BindingStrings.resx | 3 - .../Migration/ConnectedModeMigration.cs | 36 ++++---- 4 files changed, 90 insertions(+), 41 deletions(-) diff --git a/src/ConnectedMode.UnitTests/Migration/ConnectedModeMigrationTests.cs b/src/ConnectedMode.UnitTests/Migration/ConnectedModeMigrationTests.cs index 371d9f781..ae38e2a20 100644 --- a/src/ConnectedMode.UnitTests/Migration/ConnectedModeMigrationTests.cs +++ b/src/ConnectedMode.UnitTests/Migration/ConnectedModeMigrationTests.cs @@ -269,6 +269,28 @@ public async Task Migrate_CallBindAsync() It.IsAny>(), cancellationToken), Times.Once); } + [TestMethod] + public async Task Migrate_BoundProjectCanNotBeConvertedToServerConnection_LogsAndDoesB() + { + var storedConnection = new ServerConnection.SonarQube(AnyBoundProject.ServerUri); + var unintrusiveBindingControllerMock = new Mock(); + var serverConnectionsRepositoryMock = CreateServerConnectionsRepositoryMock(); + MockIServerConnectionsRepositoryTryGet(serverConnectionsRepositoryMock, storedConnection.Id, storedConnection); + var solutionInfoProviderMock = CreateSolutionInfoProviderMock(); + var testSubject = CreateTestSubject(unintrusiveBindingController: unintrusiveBindingControllerMock.Object, + serverConnectionsRepository: serverConnectionsRepositoryMock.Object, solutionInfoProvider: solutionInfoProviderMock.Object); + + await testSubject.MigrateAsync(AnyBoundProject, Mock.Of>(), false, CancellationToken.None); + + serverConnectionsRepositoryMock.Verify(mock => mock.TryGet(storedConnection.Id, out It.Ref.IsAny), Times.Once); + serverConnectionsRepositoryMock.Verify(mock => mock.TryAdd(IsExpectedServerConnection(It.IsAny())), Times.Never); + solutionInfoProviderMock.Verify(mock => mock.GetSolutionNameAsync(), Times.Once); + unintrusiveBindingControllerMock.Verify( + x => x.BindAsync( + It.Is(proj => IsExpectedBoundServerProject(proj)), + It.IsAny>(), It.IsAny()), Times.Once); + } + [TestMethod] public async Task Migrate_ConnectionExists_EstablishesBinding() { @@ -313,32 +335,74 @@ public async Task Migrate_ConnectionDoesNotExist_AddsConnectionAndEstablishesBin It.IsAny>(), It.IsAny()), Times.Once); } + /// + /// If the connection can not be added in the new format (for example, credentials do not exist or are invalid), the old migration should still succeed. + /// The user can add manually add the connection at a later time. + /// [TestMethod] - public void Migrate_ConnectionDoesNotExist_CannotAdd_Throws() + public async Task Migrate_ConnectionDoesNotExist_CannotAdd_DoesNotCreateConnection() { var unintrusiveBindingController = new Mock(); var convertedConnection = ServerConnection.FromBoundSonarQubeProject(AnyBoundProject); var serverConnectionsRepositoryMock = new Mock(); serverConnectionsRepositoryMock.Setup(x => x.IsConnectionsFileExisting()).Returns(true); - serverConnectionsRepositoryMock.Setup(x => x.TryAdd(convertedConnection)).Throws(new Exception()); + serverConnectionsRepositoryMock.Setup(x => x.TryAdd(convertedConnection)).Returns(false); + var testSubject = CreateTestSubject(unintrusiveBindingController: unintrusiveBindingController.Object, serverConnectionsRepository: serverConnectionsRepositoryMock.Object); + + await testSubject.MigrateAsync(AnyBoundProject, Mock.Of>(), false, CancellationToken.None); + serverConnectionsRepositoryMock.Verify(mock => mock.TryGet(convertedConnection.Id, out It.Ref.IsAny), Times.Once); + serverConnectionsRepositoryMock.Verify(mock => mock.TryAdd(IsExpectedServerConnection(convertedConnection)), Times.Once); + unintrusiveBindingController.Verify( + x => x.BindAsync( + It.Is(proj => IsExpectedBoundServerProject(proj)), + It.IsAny>(), It.IsAny()), Times.Once); + } + + /// + /// If the connection can not be added in the new format for whatever reason, the old migration should still succeed. + /// The user can add manually add the connection at a later time. + /// + [TestMethod] + public async Task Migrate_ConnectionDoesNotExist_Throws_DoesNotCreateConnection() + { + var unintrusiveBindingController = new Mock(); + var convertedConnection = ServerConnection.FromBoundSonarQubeProject(AnyBoundProject); + var serverConnectionsRepositoryMock = new Mock(); + serverConnectionsRepositoryMock.Setup(x => x.IsConnectionsFileExisting()).Returns(true); + serverConnectionsRepositoryMock.Setup(x => x.TryAdd(convertedConnection)).Throws(new Exception()); var testSubject = CreateTestSubject(unintrusiveBindingController: unintrusiveBindingController.Object, serverConnectionsRepository: serverConnectionsRepositoryMock.Object); - - Func act = async () => await testSubject.MigrateAsync(AnyBoundProject, Mock.Of>(), false, CancellationToken.None); - act.Should().Throw().WithMessage(BindingStrings.UnintrusiveController_CantMigrateConnection); + await testSubject.MigrateAsync(AnyBoundProject, Mock.Of>(), false, CancellationToken.None); + + serverConnectionsRepositoryMock.Verify(mock => mock.TryGet(convertedConnection.Id, out It.Ref.IsAny), Times.Once); + serverConnectionsRepositoryMock.Verify(mock => mock.TryAdd(IsExpectedServerConnection(convertedConnection)), Times.Once); + unintrusiveBindingController.Verify( + x => x.BindAsync( + It.Is(proj => IsExpectedBoundServerProject(proj)), + It.IsAny>(), It.IsAny()), Times.Once); } [TestMethod] - public void Migrate_ConnectionsJsonFileDoesNotExist_Throws() + public async Task Migrate_ConnectionsJsonFileDoesNotExist_DoesNotCreateServerConnection() { var serverConnectionsRepositoryMock = new Mock(); + var unintrusiveBindingControllerMock = new Mock(); + var logger = new Mock(); serverConnectionsRepositoryMock.Setup(mock => mock.IsConnectionsFileExisting()).Returns(false); - var testSubject = CreateTestSubject(serverConnectionsRepository: serverConnectionsRepositoryMock.Object); + var testSubject = CreateTestSubject( + serverConnectionsRepository: serverConnectionsRepositoryMock.Object, + unintrusiveBindingController: unintrusiveBindingControllerMock.Object, + logger:logger.Object); - Func act = async () => await testSubject.MigrateAsync(AnyBoundProject, Mock.Of>(), false, CancellationToken.None); + await testSubject.MigrateAsync(AnyBoundProject, Mock.Of>(), false, CancellationToken.None); - act.Should().Throw().WithMessage(MigrationStrings.ConnectionsJson_DoesNotExist); + logger.Verify(x=> x.WriteLine(MigrationStrings.ConnectionsJson_DoesNotExist)); + serverConnectionsRepositoryMock.Verify(mock => mock.TryAdd(It.IsAny()), Times.Never); + unintrusiveBindingControllerMock.Verify( + x => x.BindAsync( + It.Is(proj => IsExpectedBoundServerProject(proj)), + It.IsAny>(), It.IsAny()), Times.Once); } [TestMethod] diff --git a/src/ConnectedMode/Binding/BindingStrings.Designer.cs b/src/ConnectedMode/Binding/BindingStrings.Designer.cs index bcf0708f3..93b400fb1 100644 --- a/src/ConnectedMode/Binding/BindingStrings.Designer.cs +++ b/src/ConnectedMode/Binding/BindingStrings.Designer.cs @@ -1,6 +1,7 @@ //------------------------------------------------------------------------------ // // This code was generated by a tool. +// Runtime Version:4.0.30319.42000 // // Changes to this file may cause incorrect behavior and will be lost if // the code is regenerated. @@ -114,15 +115,6 @@ internal static string SubTextPaddingFormat { } } - /// - /// Looks up a localized string similar to Could not migrate server connection. - /// - internal static string UnintrusiveController_CantMigrateConnection { - get { - return ResourceManager.GetString("UnintrusiveController_CantMigrateConnection", resourceCulture); - } - } - /// /// Looks up a localized string similar to Could not convert connection information for the binding. /// diff --git a/src/ConnectedMode/Binding/BindingStrings.resx b/src/ConnectedMode/Binding/BindingStrings.resx index 3bf68c1a2..c3cd2aee3 100644 --- a/src/ConnectedMode/Binding/BindingStrings.resx +++ b/src/ConnectedMode/Binding/BindingStrings.resx @@ -141,7 +141,4 @@ Could not convert connection information for the binding - - Could not migrate server connection - \ No newline at end of file diff --git a/src/ConnectedMode/Migration/ConnectedModeMigration.cs b/src/ConnectedMode/Migration/ConnectedModeMigration.cs index ff1bf7e80..2830eef28 100644 --- a/src/ConnectedMode/Migration/ConnectedModeMigration.cs +++ b/src/ConnectedMode/Migration/ConnectedModeMigration.cs @@ -18,7 +18,10 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ +using System; using System.ComponentModel.Composition; +using Microsoft.Alm.Authentication; +using Microsoft.VisualStudio.LanguageServer.Client; using SonarLint.VisualStudio.ConnectedMode.Binding; using SonarLint.VisualStudio.ConnectedMode.Shared; using SonarLint.VisualStudio.ConnectedMode.Suppressions; @@ -137,7 +140,8 @@ private async Task MigrateImplAsync(BoundSonarQubeProject oldBinding, IProgress< logger.WriteLine(MigrationStrings.Process_ProcessingNewBinding); var progressAdapter = new FixedStepsProgressToMigrationProgressAdapter(progress); - await BindWithMigrationAsync(oldBinding, progressAdapter, token); + var serverConnection = GetServerConnectionWithMigration(oldBinding); + await unintrusiveBindingController.BindAsync(BoundServerProject.FromBoundSonarQubeProject(oldBinding, await solutionInfoProvider.GetSolutionNameAsync(), serverConnection), progressAdapter, token); // Now make all of the files changes required to remove the legacy settings // i.e. update project files and delete .sonarlint folder @@ -265,36 +269,28 @@ private async Task SaveChangedFilesAsync(ChangedFiles changedFiles, IProgress progress, CancellationToken token) + private ServerConnection GetServerConnectionWithMigration(BoundSonarQubeProject project) { + if (ServerConnection.FromBoundSonarQubeProject(project) is not {} proposedConnection) + { + throw new InvalidOperationException(BindingStrings.UnintrusiveController_InvalidConnection); + } + // at this point we expect that the connections file exist, meaning that all the existing bindings are already migrated to the newer format // if the file doesn't exist, creating it now will prevent the migration of all existing bindings. // The order being important, we throw an exception. For more info see IBindingToConnectionMigration if (!serverConnectionsRepository.IsConnectionsFileExisting()) { - throw new InvalidOperationException(MigrationStrings.ConnectionsJson_DoesNotExist); + logger.WriteLine(MigrationStrings.ConnectionsJson_DoesNotExist); + return proposedConnection; } - var proposedConnection = ServerConnection.FromBoundSonarQubeProject(project); - if (proposedConnection is null) + if(!serverConnectionsRepository.TryGet(proposedConnection.Id, out _)) { - throw new InvalidOperationException(BindingStrings.UnintrusiveController_InvalidConnection); + serverConnectionsRepository.TryAdd(proposedConnection); } - var connection = GetExistingConnection(proposedConnection) ?? MigrateConnection(proposedConnection); - - await unintrusiveBindingController.BindAsync(BoundServerProject.FromBoundSonarQubeProject(project, await solutionInfoProvider.GetSolutionNameAsync(), connection), progress, token); + return proposedConnection; } - - private ServerConnection GetExistingConnection(ServerConnection proposedConnection) => - serverConnectionsRepository.TryGet(proposedConnection.Id, out var connection) - ? connection - : null; - - private ServerConnection MigrateConnection(ServerConnection proposedConnection) => - serverConnectionsRepository.TryAdd(proposedConnection) - ? proposedConnection - : throw new InvalidOperationException(BindingStrings.UnintrusiveController_CantMigrateConnection); - } } From 5e49a4b57a9cd5eb50f409623ed3028fde5197a8 Mon Sep 17 00:00:00 2001 From: Gabriela Trutan Date: Wed, 25 Sep 2024 11:53:54 +0200 Subject: [PATCH 8/9] SLVS-1396 Make sure serverConnection is migrated if the connections.json does not exist and no bindings in the new binding exist. --- .../Migration/ConnectedModeMigrationTests.cs | 53 ++++++++++++++++--- .../Migration/ConnectedModeMigration.cs | 12 +++-- 2 files changed, 54 insertions(+), 11 deletions(-) diff --git a/src/ConnectedMode.UnitTests/Migration/ConnectedModeMigrationTests.cs b/src/ConnectedMode.UnitTests/Migration/ConnectedModeMigrationTests.cs index ae38e2a20..cb2f49ca6 100644 --- a/src/ConnectedMode.UnitTests/Migration/ConnectedModeMigrationTests.cs +++ b/src/ConnectedMode.UnitTests/Migration/ConnectedModeMigrationTests.cs @@ -18,6 +18,7 @@ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ +using Microsoft.VisualStudio.TestTools.UnitTesting.Logging; using SonarLint.VisualStudio.ConnectedMode.Binding; using SonarLint.VisualStudio.ConnectedMode.Migration; using SonarLint.VisualStudio.ConnectedMode.Shared; @@ -57,7 +58,8 @@ public void MefCtor_CheckIsExported() MefTestHelpers.CreateExport(), MefTestHelpers.CreateExport(), MefTestHelpers.CreateExport(), - MefTestHelpers.CreateExport()); + MefTestHelpers.CreateExport(), + MefTestHelpers.CreateExport()); } [TestMethod] @@ -383,21 +385,27 @@ public async Task Migrate_ConnectionDoesNotExist_Throws_DoesNotCreateConnection( It.IsAny>(), It.IsAny()), Times.Once); } + /// + /// If bindings exist in the new format (in the Bindings folder), it means that the old migration is being executed before the new migration. But we expect it the other way around + /// [TestMethod] - public async Task Migrate_ConnectionsJsonFileDoesNotExist_DoesNotCreateServerConnection() + public async Task Migrate_ConnectionsJsonFileDoesNotExistAndNewBindingsExist_DoesNotMigrateServerConnection() { - var serverConnectionsRepositoryMock = new Mock(); var unintrusiveBindingControllerMock = new Mock(); + var serverConnectionsRepositoryMock = new Mock(); + var bindingPathProvider = new Mock(); var logger = new Mock(); - serverConnectionsRepositoryMock.Setup(mock => mock.IsConnectionsFileExisting()).Returns(false); var testSubject = CreateTestSubject( serverConnectionsRepository: serverConnectionsRepositoryMock.Object, unintrusiveBindingController: unintrusiveBindingControllerMock.Object, + unintrusiveBindingPathProvider: bindingPathProvider.Object, logger:logger.Object); + serverConnectionsRepositoryMock.Setup(mock => mock.IsConnectionsFileExisting()).Returns(false); + bindingPathProvider.Setup(mock => mock.GetBindingPaths()).Returns(["binding1"]); await testSubject.MigrateAsync(AnyBoundProject, Mock.Of>(), false, CancellationToken.None); - logger.Verify(x=> x.WriteLine(MigrationStrings.ConnectionsJson_DoesNotExist)); + logger.Verify(x=> x.WriteLine(MigrationStrings.ConnectionsJson_DoesNotExist), Times.Once); serverConnectionsRepositoryMock.Verify(mock => mock.TryAdd(It.IsAny()), Times.Never); unintrusiveBindingControllerMock.Verify( x => x.BindAsync( @@ -405,6 +413,34 @@ public async Task Migrate_ConnectionsJsonFileDoesNotExist_DoesNotCreateServerCon It.IsAny>(), It.IsAny()), Times.Once); } + /// + /// If no bindings exist in the new format (in the Bindings folder), even if the new migration is executed first no connections.json will be created, so we can safely proceed with the old migration + /// + [TestMethod] + public async Task Migrate_ConnectionsJsonFileDoesNotExistAndNoNewBindingsExist_MigratesConnection() + { + var unintrusiveBindingControllerMock = new Mock(); + var serverConnectionsRepositoryMock = new Mock(); + var bindingPathProvider = new Mock(); + var logger = new Mock(); + var testSubject = CreateTestSubject( + serverConnectionsRepository: serverConnectionsRepositoryMock.Object, + unintrusiveBindingController: unintrusiveBindingControllerMock.Object, + unintrusiveBindingPathProvider:bindingPathProvider.Object, + logger: logger.Object); + serverConnectionsRepositoryMock.Setup(mock => mock.IsConnectionsFileExisting()).Returns(false); + bindingPathProvider.Setup(mock => mock.GetBindingPaths()).Returns([]); + + await testSubject.MigrateAsync(AnyBoundProject, Mock.Of>(), false, CancellationToken.None); + + logger.Verify(x => x.WriteLine(MigrationStrings.ConnectionsJson_DoesNotExist), Times.Never); + serverConnectionsRepositoryMock.Verify(mock => mock.TryAdd(It.IsAny()), Times.Once); + unintrusiveBindingControllerMock.Verify( + x => x.BindAsync( + It.Is(proj => IsExpectedBoundServerProject(proj)), + It.IsAny>(), It.IsAny()), Times.Once); + } + [TestMethod] public void Migrate_InvalidServerInformation_Throws() { @@ -451,7 +487,8 @@ private static ConnectedModeMigration CreateTestSubject( ILogger logger = null, IThreadHandling threadHandling = null, ISolutionInfoProvider solutionInfoProvider = null, - IServerConnectionsRepository serverConnectionsRepository = null) + IServerConnectionsRepository serverConnectionsRepository = null, + IUnintrusiveBindingPathProvider unintrusiveBindingPathProvider = null) { fileProvider ??= Mock.Of(); fileCleaner ??= Mock.Of(); @@ -463,6 +500,7 @@ private static ConnectedModeMigration CreateTestSubject( sharedBindingConfigProvider ??= Mock.Of(); solutionInfoProvider ??= CreateSolutionInfoProviderMock().Object; serverConnectionsRepository ??= CreateServerConnectionsRepositoryMock().Object; + unintrusiveBindingPathProvider ??= Mock.Of(); logger ??= new TestLogger(logToConsole: true); threadHandling ??= new NoOpThreadHandler(); @@ -478,7 +516,8 @@ private static ConnectedModeMigration CreateTestSubject( logger, threadHandling, solutionInfoProvider, - serverConnectionsRepository); + serverConnectionsRepository, + unintrusiveBindingPathProvider); } private static Mock CreateFileProvider(params string[] filesToReturn) diff --git a/src/ConnectedMode/Migration/ConnectedModeMigration.cs b/src/ConnectedMode/Migration/ConnectedModeMigration.cs index 2830eef28..0b2d84076 100644 --- a/src/ConnectedMode/Migration/ConnectedModeMigration.cs +++ b/src/ConnectedMode/Migration/ConnectedModeMigration.cs @@ -51,6 +51,7 @@ private sealed class ChangedFiles : List> { } private readonly IThreadHandling threadHandling; private readonly ISolutionInfoProvider solutionInfoProvider; private readonly IServerConnectionsRepository serverConnectionsRepository; + private readonly IUnintrusiveBindingPathProvider unintrusiveBindingPathProvider; // The user can have both the legacy and new connected mode files. In that case, we expect the SonarQubeService to already be connected. private bool isAlreadyConnectedToServer; @@ -65,9 +66,10 @@ public ConnectedModeMigration(IMigrationSettingsProvider settingsProvider, ISuppressionIssueStoreUpdater suppressionIssueStoreUpdater, ISharedBindingConfigProvider sharedBindingConfigProvider, ILogger logger, - IThreadHandling threadHandling, + IThreadHandling threadHandling, ISolutionInfoProvider solutionInfoProvider, - IServerConnectionsRepository serverConnectionsRepository) + IServerConnectionsRepository serverConnectionsRepository, + IUnintrusiveBindingPathProvider unintrusiveBindingPathProvider) { this.settingsProvider = settingsProvider; this.fileProvider = fileProvider; @@ -82,6 +84,7 @@ public ConnectedModeMigration(IMigrationSettingsProvider settingsProvider, this.threadHandling = threadHandling; this.solutionInfoProvider = solutionInfoProvider; this.serverConnectionsRepository = serverConnectionsRepository; + this.unintrusiveBindingPathProvider = unintrusiveBindingPathProvider; } public async Task MigrateAsync(BoundSonarQubeProject oldBinding, IProgress progress, bool shareBinding, CancellationToken token) @@ -276,10 +279,11 @@ private ServerConnection GetServerConnectionWithMigration(BoundSonarQubeProject throw new InvalidOperationException(BindingStrings.UnintrusiveController_InvalidConnection); } - // at this point we expect that the connections file exist, meaning that all the existing bindings are already migrated to the newer format + // at this point we expect that the connections file exist if there are bindings in new format, meaning that all the existing bindings are already migrated to the newer format // if the file doesn't exist, creating it now will prevent the migration of all existing bindings. // The order being important, we throw an exception. For more info see IBindingToConnectionMigration - if (!serverConnectionsRepository.IsConnectionsFileExisting()) + // But if there are no bindings in the new format, then creating the connections file is safe, because the new migration did not do anything in this case + if (!serverConnectionsRepository.IsConnectionsFileExisting() && unintrusiveBindingPathProvider.GetBindingPaths().Any()) { logger.WriteLine(MigrationStrings.ConnectionsJson_DoesNotExist); return proposedConnection; From 51a24853dc5a6d0f432d0680f04f0a2c14797f4e Mon Sep 17 00:00:00 2001 From: Gabriela Trutan Date: Wed, 25 Sep 2024 12:03:17 +0200 Subject: [PATCH 9/9] SLVS-1396 Rename method --- .../Migration/BindingToConnectionMigrationTests.cs | 6 +++--- .../Migration/ConnectedModeMigrationTests.cs | 10 +++++----- .../Persistence/ServerConnectionsRepositoryTests.cs | 4 ++-- .../Migration/BindingToConnectionMigration.cs | 2 +- src/ConnectedMode/Migration/ConnectedModeMigration.cs | 2 +- .../Persistence/ServerConnectionsRepository.cs | 2 +- src/Core/Binding/IServerConnectionsRepository.cs | 2 +- 7 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/ConnectedMode.UnitTests/Migration/BindingToConnectionMigrationTests.cs b/src/ConnectedMode.UnitTests/Migration/BindingToConnectionMigrationTests.cs index 94e723027..be465df1f 100644 --- a/src/ConnectedMode.UnitTests/Migration/BindingToConnectionMigrationTests.cs +++ b/src/ConnectedMode.UnitTests/Migration/BindingToConnectionMigrationTests.cs @@ -95,11 +95,11 @@ public async Task MigrateBindingToServerConnectionIfNeeded_RunsOnBackgroundThrea [TestMethod] public async Task MigrateBindingToServerConnectionIfNeeded_ConnectionsStorageFileExists_ShouldNotMigrate() { - serverConnectionsRepository.IsConnectionsFileExisting().Returns(true); + serverConnectionsRepository.ConnectionsFileExists().Returns(true); await testSubject.MigrateAllBindingsToServerConnectionsIfNeededAsync(); - serverConnectionsRepository.Received(1).IsConnectionsFileExisting(); + serverConnectionsRepository.Received(1).ConnectionsFileExists(); serverConnectionsRepository.DidNotReceiveWithAnyArgs().TryAdd(default); unintrusiveBindingPathProvider.DidNotReceive().GetBindingPaths(); } @@ -113,7 +113,7 @@ public async Task MigrateBindingToServerConnectionIfNeeded_ConnectionsStorageDoe Received.InOrder(() => { - serverConnectionsRepository.IsConnectionsFileExisting(); + serverConnectionsRepository.ConnectionsFileExists(); logger.WriteLine(MigrationStrings.ConnectionMigration_StartMigration); unintrusiveBindingPathProvider.GetBindingPaths(); serverConnectionsRepository.TryAdd(Arg.Any()); diff --git a/src/ConnectedMode.UnitTests/Migration/ConnectedModeMigrationTests.cs b/src/ConnectedMode.UnitTests/Migration/ConnectedModeMigrationTests.cs index cb2f49ca6..c786ff72a 100644 --- a/src/ConnectedMode.UnitTests/Migration/ConnectedModeMigrationTests.cs +++ b/src/ConnectedMode.UnitTests/Migration/ConnectedModeMigrationTests.cs @@ -347,7 +347,7 @@ public async Task Migrate_ConnectionDoesNotExist_CannotAdd_DoesNotCreateConnecti var unintrusiveBindingController = new Mock(); var convertedConnection = ServerConnection.FromBoundSonarQubeProject(AnyBoundProject); var serverConnectionsRepositoryMock = new Mock(); - serverConnectionsRepositoryMock.Setup(x => x.IsConnectionsFileExisting()).Returns(true); + serverConnectionsRepositoryMock.Setup(x => x.ConnectionsFileExists()).Returns(true); serverConnectionsRepositoryMock.Setup(x => x.TryAdd(convertedConnection)).Returns(false); var testSubject = CreateTestSubject(unintrusiveBindingController: unintrusiveBindingController.Object, serverConnectionsRepository: serverConnectionsRepositoryMock.Object); @@ -371,7 +371,7 @@ public async Task Migrate_ConnectionDoesNotExist_Throws_DoesNotCreateConnection( var unintrusiveBindingController = new Mock(); var convertedConnection = ServerConnection.FromBoundSonarQubeProject(AnyBoundProject); var serverConnectionsRepositoryMock = new Mock(); - serverConnectionsRepositoryMock.Setup(x => x.IsConnectionsFileExisting()).Returns(true); + serverConnectionsRepositoryMock.Setup(x => x.ConnectionsFileExists()).Returns(true); serverConnectionsRepositoryMock.Setup(x => x.TryAdd(convertedConnection)).Throws(new Exception()); var testSubject = CreateTestSubject(unintrusiveBindingController: unintrusiveBindingController.Object, serverConnectionsRepository: serverConnectionsRepositoryMock.Object); @@ -400,7 +400,7 @@ public async Task Migrate_ConnectionsJsonFileDoesNotExistAndNewBindingsExist_Doe unintrusiveBindingController: unintrusiveBindingControllerMock.Object, unintrusiveBindingPathProvider: bindingPathProvider.Object, logger:logger.Object); - serverConnectionsRepositoryMock.Setup(mock => mock.IsConnectionsFileExisting()).Returns(false); + serverConnectionsRepositoryMock.Setup(mock => mock.ConnectionsFileExists()).Returns(false); bindingPathProvider.Setup(mock => mock.GetBindingPaths()).Returns(["binding1"]); await testSubject.MigrateAsync(AnyBoundProject, Mock.Of>(), false, CancellationToken.None); @@ -428,7 +428,7 @@ public async Task Migrate_ConnectionsJsonFileDoesNotExistAndNoNewBindingsExist_M unintrusiveBindingController: unintrusiveBindingControllerMock.Object, unintrusiveBindingPathProvider:bindingPathProvider.Object, logger: logger.Object); - serverConnectionsRepositoryMock.Setup(mock => mock.IsConnectionsFileExisting()).Returns(false); + serverConnectionsRepositoryMock.Setup(mock => mock.ConnectionsFileExists()).Returns(false); bindingPathProvider.Setup(mock => mock.GetBindingPaths()).Returns([]); await testSubject.MigrateAsync(AnyBoundProject, Mock.Of>(), false, CancellationToken.None); @@ -550,7 +550,7 @@ private static Mock CreateServerConnectionsReposit { var serverConnectionsRepositoryMock = new Mock(); serverConnectionsRepositoryMock.Setup(x => x.TryAdd(It.IsAny())).Returns(true); - serverConnectionsRepositoryMock.Setup(x => x.IsConnectionsFileExisting()).Returns(true); + serverConnectionsRepositoryMock.Setup(x => x.ConnectionsFileExists()).Returns(true); return serverConnectionsRepositoryMock; } diff --git a/src/ConnectedMode.UnitTests/Persistence/ServerConnectionsRepositoryTests.cs b/src/ConnectedMode.UnitTests/Persistence/ServerConnectionsRepositoryTests.cs index 8276f17eb..3afb6587f 100644 --- a/src/ConnectedMode.UnitTests/Persistence/ServerConnectionsRepositoryTests.cs +++ b/src/ConnectedMode.UnitTests/Persistence/ServerConnectionsRepositoryTests.cs @@ -543,11 +543,11 @@ public void TryUpdateCredentialsById_SonarQubeConnectionExists_UpdatesCredential [TestMethod] [DataRow(true)] [DataRow(false)] - public void IsConnectionsFileExisting_ReturnsTrueOnlyIfTheConnectionsFileExists(bool fileExists) + public void ConnectionsFileExists_ReturnsTrueOnlyIfTheConnectionsFileExists(bool fileExists) { fileSystem.File.Exists(Arg.Any()).Returns(fileExists); - var result = testSubject.IsConnectionsFileExisting(); + var result = testSubject.ConnectionsFileExists(); result.Should().Be(fileExists); } diff --git a/src/ConnectedMode/Migration/BindingToConnectionMigration.cs b/src/ConnectedMode/Migration/BindingToConnectionMigration.cs index 57e1c7c05..0591663af 100644 --- a/src/ConnectedMode/Migration/BindingToConnectionMigration.cs +++ b/src/ConnectedMode/Migration/BindingToConnectionMigration.cs @@ -86,7 +86,7 @@ public Task MigrateAllBindingsToServerConnectionsIfNeededAsync() private void MigrateBindingToServerConnectionIfNeeded() { - if (serverConnectionsRepository.IsConnectionsFileExisting()) + if (serverConnectionsRepository.ConnectionsFileExists()) { return; } diff --git a/src/ConnectedMode/Migration/ConnectedModeMigration.cs b/src/ConnectedMode/Migration/ConnectedModeMigration.cs index 0b2d84076..270fca0a9 100644 --- a/src/ConnectedMode/Migration/ConnectedModeMigration.cs +++ b/src/ConnectedMode/Migration/ConnectedModeMigration.cs @@ -283,7 +283,7 @@ private ServerConnection GetServerConnectionWithMigration(BoundSonarQubeProject // if the file doesn't exist, creating it now will prevent the migration of all existing bindings. // The order being important, we throw an exception. For more info see IBindingToConnectionMigration // But if there are no bindings in the new format, then creating the connections file is safe, because the new migration did not do anything in this case - if (!serverConnectionsRepository.IsConnectionsFileExisting() && unintrusiveBindingPathProvider.GetBindingPaths().Any()) + if (!serverConnectionsRepository.ConnectionsFileExists() && unintrusiveBindingPathProvider.GetBindingPaths().Any()) { logger.WriteLine(MigrationStrings.ConnectionsJson_DoesNotExist); return proposedConnection; diff --git a/src/ConnectedMode/Persistence/ServerConnectionsRepository.cs b/src/ConnectedMode/Persistence/ServerConnectionsRepository.cs index 4e4b737d1..3578bc375 100644 --- a/src/ConnectedMode/Persistence/ServerConnectionsRepository.cs +++ b/src/ConnectedMode/Persistence/ServerConnectionsRepository.cs @@ -137,7 +137,7 @@ public bool TryUpdateCredentialsById(string connectionId, ICredentials credentia return false; } - public bool IsConnectionsFileExisting() => fileSystem.File.Exists(connectionsStorageFilePath); + public bool ConnectionsFileExists() => fileSystem.File.Exists(connectionsStorageFilePath); private bool TryAddConnection(List connections, ServerConnection connection) { diff --git a/src/Core/Binding/IServerConnectionsRepository.cs b/src/Core/Binding/IServerConnectionsRepository.cs index c73fecb30..13a859b1b 100644 --- a/src/Core/Binding/IServerConnectionsRepository.cs +++ b/src/Core/Binding/IServerConnectionsRepository.cs @@ -28,5 +28,5 @@ public interface IServerConnectionsRepository bool TryDelete(string connectionId); bool TryUpdateSettingsById(string connectionId, ServerConnectionSettings connectionSettings); bool TryUpdateCredentialsById(string connectionId, ICredentials credentials); - bool IsConnectionsFileExisting(); + bool ConnectionsFileExists(); }