Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SLVS-1406 Add binding changed trigger to ActiveSolutionBoundTracker #5642

Merged
merged 2 commits into from
Aug 30, 2024
Merged

SLVS-1406 Add binding changed trigger to ActiveSolutionBoundTracker #5642

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fb19f07
SLVS-1406 Add binding changed trigger to ActiveSolutionBoundTracker
georgii-borovinskikh-sonarsource Aug 28, 2024
8ed1dea
Replace hardcoded strings with nameof in tests
georgii-borovinskikh-sonarsource Aug 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/Core/Binding/IActiveSolutionBoundTracker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@

namespace SonarLint.VisualStudio.Core.Binding
{
public interface IActiveSolutionChangedHandler
{
void HandleBindingChange(bool isBindingCleared);
}

/// <summary>
/// Allows checking if the current Visual Studio solution is bound to a SonarQube project or not
/// </summary>
Expand Down
88 changes: 85 additions & 3 deletions src/Integration.UnitTests/MefServices/ActiveSolutionBoundTrackerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ public void ActiveSolutionBoundTracker_Changes()
ConfigureSolutionBinding(null);

// Act
host.VisualStateManager.ClearBoundProject();
testSubject.HandleBindingChange(true);

// Assert
testSubject.CurrentConfiguration.Mode.Should().Be(SonarLintMode.Standalone, "Unbound solution should report false activation");
Expand All @@ -235,7 +235,7 @@ public void ActiveSolutionBoundTracker_Changes()
// Case 2: Set bound project
ConfigureSolutionBinding(boundProject);
// Act
host.VisualStateManager.SetBoundProject(new Uri("http://localhost"), null, "project123");
testSubject.HandleBindingChange(false);

// Assert
testSubject.CurrentConfiguration.Mode.Should().Be(SonarLintMode.Connected, "Bound solution should report true activation");
Expand Down Expand Up @@ -307,7 +307,7 @@ public void ActiveSolutionBoundTracker_Changes()
// Act
testSubject.Dispose();
ConfigureSolutionBinding(boundProject);
host.VisualStateManager.ClearBoundProject();
testSubject.HandleBindingChange(true);

// Assert
eventCounter.PreSolutionBindingChangedCount.Should().Be(5, "Once disposed should stop raising the event");
Expand Down Expand Up @@ -349,8 +349,48 @@ public void OnBindingStateChanged_NewConfiguration_EventsRaisedInCorrectOrder()
// Assert
// Different config so event should be raised
eventCounter.PreSolutionBindingChangedCount.Should().Be(1);
eventCounter.PreSolutionBindingUpdatedCount.Should().Be(0);
eventCounter.SolutionBindingChangedCount.Should().Be(1);
eventCounter.SolutionBindingUpdatedCount.Should().Be(0);

eventCounter.RaisedEventNames.Should().HaveCount(2);
eventCounter.RaisedEventNames[0].Should().Be("PreSolutionBindingChanged");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use name of instead of hardcoded string for event name.
Maybe search and replace in the whole file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't change these, but I can try to fix this.

eventCounter.RaisedEventNames[1].Should().Be("SolutionBindingChanged");
}
}

[TestMethod]
public void HandleBindingChange_NewConfiguration_EventsRaisedInCorrectOrder()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test seems to be a copy-past of OnBindingStateChanged_NewConfiguration_EventsRaisedInCorrectOrder.
Can we maybe extract the whole content in a method and provide the act as a parameter to the method?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The event-based test will be removed once we get rid of the old binding window

{
// Arrange
var initialProject = new BoundServerProject(
"solution",
"projectKey",
new ServerConnection.SonarCloud("myOrgKey"));

// Set the current configuration used by the tracker
ConfigureSolutionBinding(initialProject);
using (var testSubject = CreateTestSubject(this.host, this.activeSolutionTracker, this.configProvider, loggerMock.Object))
{
var eventCounter = new EventCounter(testSubject);

// Now configure the provider to return a different configuration
var newProject = new BoundServerProject(
"solution",
"projectKey",
new ServerConnection.SonarCloud("myOrgKey_DIFFERENT"));
ConfigureSolutionBinding(newProject);

// Act - simulate the binding state changing in the Team explorer section.
// The project configuration hasn't changed (it doesn't matter what properties
// we pass here; they aren't used when raising the event.)
testSubject.HandleBindingChange(false);

// Assert
// Different config so event should be raised
eventCounter.PreSolutionBindingChangedCount.Should().Be(1);
eventCounter.PreSolutionBindingUpdatedCount.Should().Be(0);
eventCounter.SolutionBindingChangedCount.Should().Be(1);
eventCounter.SolutionBindingUpdatedCount.Should().Be(0);

eventCounter.RaisedEventNames.Should().HaveCount(2);
Expand Down Expand Up @@ -521,6 +561,25 @@ public void SolutionBindingUpdated_WhenClearBoundProject_NotRaised()
eventCounter.SolutionBindingChangedCount.Should().Be(0);
}
}

[TestMethod]
public void HandleBindingChange_WhenClearBoundProject_NotRaised()
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test seems to be a copy-past of SolutionBindingUpdated_WhenClearBoundProject_NotRaised.
Can we maybe extract the whole content in a method and provide the act as a parameter to the method?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The event-based test will be removed once we get rid of the old binding window

// Arrange
using (var testSubject = CreateTestSubject(this.host, this.activeSolutionTracker, this.configProvider, loggerMock.Object))
{
var eventCounter = new EventCounter(testSubject);

// Act
testSubject.HandleBindingChange(true);

// Assert
eventCounter.PreSolutionBindingUpdatedCount.Should().Be(0);
eventCounter.SolutionBindingUpdatedCount.Should().Be(0);
eventCounter.PreSolutionBindingChangedCount.Should().Be(0);
eventCounter.SolutionBindingChangedCount.Should().Be(0);
}
}

[TestMethod]
public void SolutionBindingUpdated_WhenSetBoundProject_EventsRaisedInExpectedOrder()
Expand All @@ -544,6 +603,29 @@ public void SolutionBindingUpdated_WhenSetBoundProject_EventsRaisedInExpectedOrd
eventCounter.RaisedEventNames[1].Should().Be("SolutionBindingUpdated");
}
}

[TestMethod]
public void HandleBindingChange_WhenSetBoundProject_EventsRaisedInExpectedOrder()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test seems to be a copy-past of SolutionBindingUpdated_WhenSetBoundProject_EventsRaisedInExpectedOrder.
Can we maybe extract the whole content in a method and provide the act as a parameter to the method?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The event-based test will be removed once we get rid of the old binding window

{
// Arrange
using (var testSubject = CreateTestSubject(this.host, this.activeSolutionTracker, this.configProvider, loggerMock.Object))
{
var eventCounter = new EventCounter(testSubject);

// Act
testSubject.HandleBindingChange(false);

// Assert
eventCounter.PreSolutionBindingUpdatedCount.Should().Be(1);
eventCounter.SolutionBindingUpdatedCount.Should().Be(1);
eventCounter.PreSolutionBindingChangedCount.Should().Be(0);
eventCounter.SolutionBindingChangedCount.Should().Be(0);

eventCounter.RaisedEventNames.Should().HaveCount(2);
eventCounter.RaisedEventNames[0].Should().Be("PreSolutionBindingUpdated");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use name of instead of hardcoded string for event name.
Both in here and in the line 626

eventCounter.RaisedEventNames[1].Should().Be("SolutionBindingUpdated");
}
}

[TestMethod]
public void GitRepoUpdated_SolutionBindingUpdatedEventsRaised()
Expand Down
25 changes: 19 additions & 6 deletions src/Integration/MefServices/ActiveSolutionBoundTracker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,9 @@ namespace SonarLint.VisualStudio.Integration
/// UIContext.
/// </remarks>
[Export(typeof(IActiveSolutionBoundTracker))]
[Export(typeof(IActiveSolutionChangedHandler))]
[PartCreationPolicy(CreationPolicy.Shared)]
internal sealed class ActiveSolutionBoundTracker : IActiveSolutionBoundTracker, IDisposable, IPartImportsSatisfiedNotification
internal sealed class ActiveSolutionBoundTracker : IActiveSolutionBoundTracker, IActiveSolutionChangedHandler, IDisposable, IPartImportsSatisfiedNotification
{
private readonly IHost extensionHost;
private readonly IActiveSolutionTracker solutionTracker;
Expand All @@ -52,6 +53,7 @@ internal sealed class ActiveSolutionBoundTracker : IActiveSolutionBoundTracker,
private readonly IConfigScopeUpdater configScopeUpdater;
private readonly ILogger logger;
private readonly uint boundSolutionContextCookie;
private bool disposed;

public event EventHandler<ActiveSolutionBindingEventArgs> PreSolutionBindingChanged;
public event EventHandler<ActiveSolutionBindingEventArgs> SolutionBindingChanged;
Expand Down Expand Up @@ -95,6 +97,21 @@ public ActiveSolutionBoundTracker([Import(typeof(SVsServiceProvider))] IServiceP
this.gitEventsMonitor.HeadChanged += GitEventsMonitor_HeadChanged;
}

public void HandleBindingChange(bool isBindingCleared)
{
if (disposed)
{
return;
}

this.RaiseAnalyzersChangedIfBindingChanged(GetBindingConfiguration(), isBindingCleared);
}

private void OnBindingStateChanged(object sender, BindingStateEventArgs e)
{
HandleBindingChange(e.IsBindingCleared);
}

private BindingConfiguration GetBindingConfiguration()
{
return configurationProvider.GetConfiguration();
Expand Down Expand Up @@ -159,11 +176,6 @@ await Core.WebServiceHelper.SafeServiceCallAsync(() => sonarQubeService.ConnectA
}
}

private void OnBindingStateChanged(object sender, BindingStateEventArgs e)
{
this.RaiseAnalyzersChangedIfBindingChanged(GetBindingConfiguration(), e.IsBindingCleared);
}

private void RaiseAnalyzersChangedIfBindingChanged(BindingConfiguration newBindingConfiguration, bool? isBindingCleared = null)
{
configScopeUpdater.UpdateConfigScopeForCurrentSolution(newBindingConfiguration.Project);
Expand Down Expand Up @@ -206,6 +218,7 @@ private void Dispose(bool disposing)
{
if (disposing)
{
this.disposed = true;
this.solutionTracker.ActiveSolutionChanged -= this.OnActiveSolutionChanged;
this.extensionHost.VisualStateManager.BindingStateChanged -= this.OnBindingStateChanged;
this.gitEventsMonitor.HeadChanged -= GitEventsMonitor_HeadChanged;
Expand Down
Loading