Update the PM UI when a successful restore finishes (#7209)

* Refresh PM UI when restore finishes

* Skip PM UI refresh on no-op solution restores

Author: martinrrm

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<IVsNuGetProjectUpdateEvents>();
+            _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<string> 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<string> 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;
 

src/NuGet.Clients/NuGet.PackageManagement.VisualStudio/Telemetry/PackageManagerUIRefreshEvent.cs

@@ -104,6 +104,7 @@ public enum RefreshOperationSource
     {
         ActionsExecuted,
         CacheUpdated,
+        RestoreCompleted,
         CheckboxPrereleaseChanged,
         ClearSearch,
         ExecuteAction,

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)

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<INuGetTelemetryProvider>().Object);
+
+            var expectedProjectName = "projectName.csproj";
+            var expectedFileList = new List<string>() { "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<INuGetTelemetryProvider>().Object);
+
+            var expectedProjectList = new List<string>() { "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<INuGetTelemetryProvider>().Object);
+
+            var expectedProjectName = "projectName.csproj";
+            var expectedFileList = new List<string>() { "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<INuGetTelemetryProvider>().Object);
+
+            var expectedProjectList = new List<string>() { "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);
+        }
     }
 }