diff --git a/src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageManagerControl.xaml.cs b/src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageManagerControl.xaml.cs index a81dfdc7111..7e29dec9842 100644 --- a/src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageManagerControl.xaml.cs +++ b/src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageManagerControl.xaml.cs @@ -64,6 +64,8 @@ public partial class PackageManagerControl : UserControl, IVsWindowSearch, IDisp // This tells the operation execution part that it needs to trigger a refresh when done. private bool _isRefreshRequired; private bool _isExecutingAction; // Signifies where an action is being executed. Should be updated in a coordinated fashion with IsEnabled + private bool _projectUpdateOccurredDuringRestore; + private IVsNuGetProjectUpdateEvents _projectUpdateEvents; private RestartRequestBar _restartBar; private bool _missingPackageStatus; private bool _loadedAndInitialized = false; @@ -207,7 +209,9 @@ private async ValueTask InitializeAsync(PackageManagerModel model, INuGetUILogge solutionManager.ProjectRemoved += OnProjectChanged; solutionManager.ProjectUpdated += OnProjectUpdated; solutionManager.ProjectRenamed += OnProjectRenamed; - solutionManager.AfterNuGetCacheUpdated += OnNuGetCacheUpdated; + _projectUpdateEvents = await ServiceLocator.GetComponentModelServiceAsync(); + _projectUpdateEvents.ProjectUpdateFinished += OnProjectUpdateFinished; + _projectUpdateEvents.SolutionRestoreFinished += OnSolutionRestoreFinished; Model.Context.ProjectActionsExecuted += OnProjectActionsExecuted; @@ -406,47 +410,77 @@ private async ValueTask RefreshProjectAfterActionAsync(TimeSpan timeSpan, IReadO } } - private void OnNuGetCacheUpdated(object sender, string e) + private void OnProjectUpdateFinished(string projectUniqueName, IReadOnlyList updatedFiles) { var timeSpan = GetTimeSinceLastRefreshAndRestart(); - // Do not refresh if the UI is not visible. It will be refreshed later when the loaded event is called. - if (IsVisible) + + if (Model.IsSolution) { - NuGetUIThreadHelper.JoinableTaskFactory - .RunAsync(() => SolutionManager_CacheUpdatedAsync(timeSpan, e)) - .PostOnFailure(nameof(PackageManagerControl), nameof(OnNuGetCacheUpdated)); + // Solution-level PM UI: record that a non-no-op project update occurred. + // The actual refresh will happen in OnSolutionRestoreFinished. + _projectUpdateOccurredDuringRestore = true; + return; } - else + + if (!IsVisible) { - EmitRefreshEvent(timeSpan, RefreshOperationSource.CacheUpdated, RefreshOperationStatus.NoOp); + _isRefreshRequired = true; + EmitRefreshEvent(timeSpan, RefreshOperationSource.RestoreCompleted, RefreshOperationStatus.NoOp); + return; } + + // Project-level PM UI: only refresh when the updated project matches the viewed project. + NuGetUIThreadHelper.JoinableTaskFactory + .RunAsync(() => ProjectUpdateFinishedAsync(timeSpan, projectUniqueName)) + .PostOnFailure(nameof(PackageManagerControl), nameof(OnProjectUpdateFinished)); } - private async Task SolutionManager_CacheUpdatedAsync(TimeSpan timeSpan, string eventProjectFullName) + private async Task ProjectUpdateFinishedAsync(TimeSpan timeSpan, string projectUniqueName) { - if (Model.IsSolution) + IProjectContextInfo project = Model.Context.Projects.First(); + IProjectMetadataContextInfo projectMetadata = await project.GetMetadataAsync( + Model.Context.ServiceBroker, + CancellationToken.None); + + if (string.Equals(projectMetadata.FullPath, projectUniqueName, StringComparison.OrdinalIgnoreCase)) { - await RefreshWhenNotExecutingActionAsync(RefreshOperationSource.CacheUpdated, timeSpan); + await RefreshWhenNotExecutingActionAsync(RefreshOperationSource.RestoreCompleted, timeSpan); } else { - // This is a project package manager, so there is one and only one project. - IProjectContextInfo project = Model.Context.Projects.First(); - IProjectMetadataContextInfo projectMetadata = await project.GetMetadataAsync( - Model.Context.ServiceBroker, - CancellationToken.None); + EmitRefreshEvent(timeSpan, RefreshOperationSource.RestoreCompleted, RefreshOperationStatus.NotApplicable); + } + } - // This ensures that we refresh the UI only if the event.project.FullName matches the NuGetProject.FullName. - // We also refresh the UI if projectFullPath is not present. - if (projectMetadata.FullPath == eventProjectFullName) - { - await RefreshWhenNotExecutingActionAsync(RefreshOperationSource.CacheUpdated, timeSpan); - } - else - { - EmitRefreshEvent(timeSpan, RefreshOperationSource.CacheUpdated, RefreshOperationStatus.NotApplicable); - } + private void OnSolutionRestoreFinished(IReadOnlyList projects) + { + var timeSpan = GetTimeSinceLastRefreshAndRestart(); + + if (!Model.IsSolution) + { + // Project-level PM UI handles refresh via OnProjectUpdateFinished. + return; + } + + // Only refresh if at least one project had a non-no-op restore. + if (!_projectUpdateOccurredDuringRestore) + { + EmitRefreshEvent(timeSpan, RefreshOperationSource.RestoreCompleted, RefreshOperationStatus.NoOp); + return; + } + + _projectUpdateOccurredDuringRestore = false; + + if (!IsVisible) + { + _isRefreshRequired = true; + EmitRefreshEvent(timeSpan, RefreshOperationSource.RestoreCompleted, RefreshOperationStatus.NoOp); + return; } + + NuGetUIThreadHelper.JoinableTaskFactory + .RunAsync(async () => await RefreshWhenNotExecutingActionAsync(RefreshOperationSource.RestoreCompleted, timeSpan)) + .PostOnFailure(nameof(PackageManagerControl), nameof(OnSolutionRestoreFinished)); } private async ValueTask RefreshWhenNotExecutingActionAsync(RefreshOperationSource source, TimeSpan timeSpanSinceLastRefresh) @@ -542,6 +576,11 @@ await RunAndEmitRefreshAsync(async () => }, RefreshOperationSource.PackageManagerLoaded, timeSpan, sw); } + else if (_isRefreshRequired) + { + _isRefreshRequired = false; + await RunAndEmitRefreshAsync(async () => await RefreshAsync(), RefreshOperationSource.PackageManagerLoaded, timeSpan, sw); + } else { EmitRefreshEvent(timeSpan, RefreshOperationSource.PackageManagerLoaded, RefreshOperationStatus.NoOp, isUIFiltering: false, 0); @@ -1603,7 +1642,12 @@ private void CleanUp() solutionManager.ProjectRemoved -= OnProjectChanged; solutionManager.ProjectUpdated -= OnProjectUpdated; solutionManager.ProjectRenamed -= OnProjectRenamed; - solutionManager.AfterNuGetCacheUpdated -= OnNuGetCacheUpdated; + + if (_projectUpdateEvents != null) + { + _projectUpdateEvents.ProjectUpdateFinished -= OnProjectUpdateFinished; + _projectUpdateEvents.SolutionRestoreFinished -= OnSolutionRestoreFinished; + } Model.Context.ProjectActionsExecuted -= OnProjectActionsExecuted; diff --git a/src/NuGet.Clients/NuGet.PackageManagement.VisualStudio/Telemetry/PackageManagerUIRefreshEvent.cs b/src/NuGet.Clients/NuGet.PackageManagement.VisualStudio/Telemetry/PackageManagerUIRefreshEvent.cs index f053689e80d..36f8727cdfc 100644 --- a/src/NuGet.Clients/NuGet.PackageManagement.VisualStudio/Telemetry/PackageManagerUIRefreshEvent.cs +++ b/src/NuGet.Clients/NuGet.PackageManagement.VisualStudio/Telemetry/PackageManagerUIRefreshEvent.cs @@ -104,6 +104,7 @@ public enum RefreshOperationSource { ActionsExecuted, CacheUpdated, + RestoreCompleted, CheckboxPrereleaseChanged, ClearSearch, ExecuteAction, diff --git a/test/NuGet.Clients.Tests/NuGet.PackageManagement.VisualStudio.Test/Telemetry/NuGetTelemetryServiceTests.cs b/test/NuGet.Clients.Tests/NuGet.PackageManagement.VisualStudio.Test/Telemetry/NuGetTelemetryServiceTests.cs index f48c17163c0..b8092d17c08 100644 --- a/test/NuGet.Clients.Tests/NuGet.PackageManagement.VisualStudio.Test/Telemetry/NuGetTelemetryServiceTests.cs +++ b/test/NuGet.Clients.Tests/NuGet.PackageManagement.VisualStudio.Test/Telemetry/NuGetTelemetryServiceTests.cs @@ -91,6 +91,9 @@ public void NuGetTelemetryService_EmitProjectInformation(NuGetProjectType projec [InlineData(RefreshOperationSource.PackageSourcesChanged, RefreshOperationStatus.Success)] [InlineData(RefreshOperationSource.ProjectsChanged, RefreshOperationStatus.Success)] [InlineData(RefreshOperationSource.ProjectsChanged, RefreshOperationStatus.Failed)] + [InlineData(RefreshOperationSource.RestoreCompleted, RefreshOperationStatus.Success)] + [InlineData(RefreshOperationSource.RestoreCompleted, RefreshOperationStatus.NoOp)] + [InlineData(RefreshOperationSource.RestoreCompleted, RefreshOperationStatus.NotApplicable)] [InlineData(RefreshOperationSource.RestartSearchCommand, RefreshOperationStatus.Success)] [InlineData(RefreshOperationSource.SourceSelectionChanged, RefreshOperationStatus.Success)] public void NuGetTelemetryService_EmitsPMUIRefreshEvent(RefreshOperationSource expectedRefreshSource, RefreshOperationStatus expectedRefreshStatus, bool expectedUiFiltering = false) diff --git a/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/VsRestoreProgressEventsTests.cs b/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/VsRestoreProgressEventsTests.cs index 461015add94..5c08c4e1a61 100644 --- a/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/VsRestoreProgressEventsTests.cs +++ b/test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/VsRestoreProgressEventsTests.cs @@ -246,5 +246,87 @@ public void EndProjectUpdate_WhenBatchEventEndIsRaisedWithNonProject_DoesNotFire Assert.Equal(0, invocations); } + + [Fact] + public void EndProjectUpdate_WhenSubscriberThrows_OtherSubscribersStillReceiveEvent() + { + var restoreProgressEvents = new VsRestoreProgressEvents(_packageProjectProvider.Object, new Mock().Object); + + var expectedProjectName = "projectName.csproj"; + var expectedFileList = new List() { "project.assets.json" }; + bool secondHandlerCalled = false; + + restoreProgressEvents.ProjectUpdateFinished += (projectUniqueName, updatedFiles) => + { + throw new InvalidOperationException("Simulated handler failure"); + }; + + restoreProgressEvents.ProjectUpdateFinished += (projectUniqueName, updatedFiles) => + { + secondHandlerCalled = true; + }; + + restoreProgressEvents.EndProjectUpdate(expectedProjectName, expectedFileList); + + Assert.True(secondHandlerCalled); + } + + [Fact] + public void EndSolutionRestore_WhenSubscriberThrows_OtherSubscribersStillReceiveEvent() + { + var restoreProgressEvents = new VsRestoreProgressEvents(_packageProjectProvider.Object, new Mock().Object); + + var expectedProjectList = new List() { "projectName.csproj" }; + bool secondHandlerCalled = false; + + restoreProgressEvents.SolutionRestoreFinished += (projects) => + { + throw new InvalidOperationException("Simulated handler failure"); + }; + + restoreProgressEvents.SolutionRestoreFinished += (projects) => + { + secondHandlerCalled = true; + }; + + restoreProgressEvents.EndSolutionRestore(expectedProjectList); + + Assert.True(secondHandlerCalled); + } + + [Fact] + public void EndProjectUpdate_WithMultipleSubscribers_AllReceiveEvent() + { + var restoreProgressEvents = new VsRestoreProgressEvents(_packageProjectProvider.Object, new Mock().Object); + + var expectedProjectName = "projectName.csproj"; + var expectedFileList = new List() { "project.assets.json" }; + int handlerCallCount = 0; + + restoreProgressEvents.ProjectUpdateFinished += (projectUniqueName, updatedFiles) => handlerCallCount++; + restoreProgressEvents.ProjectUpdateFinished += (projectUniqueName, updatedFiles) => handlerCallCount++; + restoreProgressEvents.ProjectUpdateFinished += (projectUniqueName, updatedFiles) => handlerCallCount++; + + restoreProgressEvents.EndProjectUpdate(expectedProjectName, expectedFileList); + + Assert.Equal(3, handlerCallCount); + } + + [Fact] + public void EndSolutionRestore_WithMultipleSubscribers_AllReceiveEvent() + { + var restoreProgressEvents = new VsRestoreProgressEvents(_packageProjectProvider.Object, new Mock().Object); + + var expectedProjectList = new List() { "projectA.csproj", "projectB.csproj" }; + int handlerCallCount = 0; + + restoreProgressEvents.SolutionRestoreFinished += (projects) => handlerCallCount++; + restoreProgressEvents.SolutionRestoreFinished += (projects) => handlerCallCount++; + restoreProgressEvents.SolutionRestoreFinished += (projects) => handlerCallCount++; + + restoreProgressEvents.EndSolutionRestore(expectedProjectList); + + Assert.Equal(3, handlerCallCount); + } } }