Skip to content

Commit bde9562

Browse files
authored
Revert PM UI refresh regression for SDK-style project install/uninstall flows (#7294)
This reverts commit 76b3a60.
1 parent de3b92c commit bde9562

4 files changed

Lines changed: 28 additions & 158 deletions

File tree

src/NuGet.Clients/NuGet.PackageManagement.UI/Xamls/PackageManagerControl.xaml.cs

Lines changed: 28 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,6 @@ public partial class PackageManagerControl : UserControl, IVsWindowSearch, IDisp
6464
// This tells the operation execution part that it needs to trigger a refresh when done.
6565
private bool _isRefreshRequired;
6666
private bool _isExecutingAction; // Signifies where an action is being executed. Should be updated in a coordinated fashion with IsEnabled
67-
private bool _projectUpdateOccurredDuringRestore;
68-
private IVsNuGetProjectUpdateEvents _projectUpdateEvents;
6967
private RestartRequestBar _restartBar;
7068
private bool _missingPackageStatus;
7169
private bool _loadedAndInitialized = false;
@@ -209,9 +207,7 @@ private async ValueTask InitializeAsync(PackageManagerModel model, INuGetUILogge
209207
solutionManager.ProjectRemoved += OnProjectChanged;
210208
solutionManager.ProjectUpdated += OnProjectUpdated;
211209
solutionManager.ProjectRenamed += OnProjectRenamed;
212-
_projectUpdateEvents = await ServiceLocator.GetComponentModelServiceAsync<IVsNuGetProjectUpdateEvents>();
213-
_projectUpdateEvents.ProjectUpdateFinished += OnProjectUpdateFinished;
214-
_projectUpdateEvents.SolutionRestoreFinished += OnSolutionRestoreFinished;
210+
solutionManager.AfterNuGetCacheUpdated += OnNuGetCacheUpdated;
215211

216212
Model.Context.ProjectActionsExecuted += OnProjectActionsExecuted;
217213

@@ -410,77 +406,47 @@ private async ValueTask RefreshProjectAfterActionAsync(TimeSpan timeSpan, IReadO
410406
}
411407
}
412408

413-
private void OnProjectUpdateFinished(string projectUniqueName, IReadOnlyList<string> updatedFiles)
409+
private void OnNuGetCacheUpdated(object sender, string e)
414410
{
415411
var timeSpan = GetTimeSinceLastRefreshAndRestart();
416-
417-
if (Model.IsSolution)
418-
{
419-
// Solution-level PM UI: record that a non-no-op project update occurred.
420-
// The actual refresh will happen in OnSolutionRestoreFinished.
421-
_projectUpdateOccurredDuringRestore = true;
422-
return;
423-
}
424-
425-
if (!IsVisible)
426-
{
427-
_isRefreshRequired = true;
428-
EmitRefreshEvent(timeSpan, RefreshOperationSource.RestoreCompleted, RefreshOperationStatus.NoOp);
429-
return;
430-
}
431-
432-
// Project-level PM UI: only refresh when the updated project matches the viewed project.
433-
NuGetUIThreadHelper.JoinableTaskFactory
434-
.RunAsync(() => ProjectUpdateFinishedAsync(timeSpan, projectUniqueName))
435-
.PostOnFailure(nameof(PackageManagerControl), nameof(OnProjectUpdateFinished));
436-
}
437-
438-
private async Task ProjectUpdateFinishedAsync(TimeSpan timeSpan, string projectUniqueName)
439-
{
440-
IProjectContextInfo project = Model.Context.Projects.First();
441-
IProjectMetadataContextInfo projectMetadata = await project.GetMetadataAsync(
442-
Model.Context.ServiceBroker,
443-
CancellationToken.None);
444-
445-
if (string.Equals(projectMetadata.FullPath, projectUniqueName, StringComparison.OrdinalIgnoreCase))
412+
// Do not refresh if the UI is not visible. It will be refreshed later when the loaded event is called.
413+
if (IsVisible)
446414
{
447-
await RefreshWhenNotExecutingActionAsync(RefreshOperationSource.RestoreCompleted, timeSpan);
415+
NuGetUIThreadHelper.JoinableTaskFactory
416+
.RunAsync(() => SolutionManager_CacheUpdatedAsync(timeSpan, e))
417+
.PostOnFailure(nameof(PackageManagerControl), nameof(OnNuGetCacheUpdated));
448418
}
449419
else
450420
{
451-
EmitRefreshEvent(timeSpan, RefreshOperationSource.RestoreCompleted, RefreshOperationStatus.NotApplicable);
421+
EmitRefreshEvent(timeSpan, RefreshOperationSource.CacheUpdated, RefreshOperationStatus.NoOp);
452422
}
453423
}
454424

455-
private void OnSolutionRestoreFinished(IReadOnlyList<string> projects)
425+
private async Task SolutionManager_CacheUpdatedAsync(TimeSpan timeSpan, string eventProjectFullName)
456426
{
457-
var timeSpan = GetTimeSinceLastRefreshAndRestart();
458-
459-
if (!Model.IsSolution)
427+
if (Model.IsSolution)
460428
{
461-
// Project-level PM UI handles refresh via OnProjectUpdateFinished.
462-
return;
429+
await RefreshWhenNotExecutingActionAsync(RefreshOperationSource.CacheUpdated, timeSpan);
463430
}
464-
465-
// Only refresh if at least one project had a non-no-op restore.
466-
if (!_projectUpdateOccurredDuringRestore)
431+
else
467432
{
468-
EmitRefreshEvent(timeSpan, RefreshOperationSource.RestoreCompleted, RefreshOperationStatus.NoOp);
469-
return;
470-
}
471-
472-
_projectUpdateOccurredDuringRestore = false;
433+
// This is a project package manager, so there is one and only one project.
434+
IProjectContextInfo project = Model.Context.Projects.First();
435+
IProjectMetadataContextInfo projectMetadata = await project.GetMetadataAsync(
436+
Model.Context.ServiceBroker,
437+
CancellationToken.None);
473438

474-
if (!IsVisible)
475-
{
476-
_isRefreshRequired = true;
477-
EmitRefreshEvent(timeSpan, RefreshOperationSource.RestoreCompleted, RefreshOperationStatus.NoOp);
478-
return;
439+
// This ensures that we refresh the UI only if the event.project.FullName matches the NuGetProject.FullName.
440+
// We also refresh the UI if projectFullPath is not present.
441+
if (projectMetadata.FullPath == eventProjectFullName)
442+
{
443+
await RefreshWhenNotExecutingActionAsync(RefreshOperationSource.CacheUpdated, timeSpan);
444+
}
445+
else
446+
{
447+
EmitRefreshEvent(timeSpan, RefreshOperationSource.CacheUpdated, RefreshOperationStatus.NotApplicable);
448+
}
479449
}
480-
481-
NuGetUIThreadHelper.JoinableTaskFactory
482-
.RunAsync(async () => await RefreshWhenNotExecutingActionAsync(RefreshOperationSource.RestoreCompleted, timeSpan))
483-
.PostOnFailure(nameof(PackageManagerControl), nameof(OnSolutionRestoreFinished));
484450
}
485451

486452
private async ValueTask RefreshWhenNotExecutingActionAsync(RefreshOperationSource source, TimeSpan timeSpanSinceLastRefresh)
@@ -576,11 +542,6 @@ await RunAndEmitRefreshAsync(async () =>
576542
},
577543
RefreshOperationSource.PackageManagerLoaded, timeSpan, sw);
578544
}
579-
else if (_isRefreshRequired)
580-
{
581-
_isRefreshRequired = false;
582-
await RunAndEmitRefreshAsync(async () => await RefreshAsync(), RefreshOperationSource.PackageManagerLoaded, timeSpan, sw);
583-
}
584545
else
585546
{
586547
EmitRefreshEvent(timeSpan, RefreshOperationSource.PackageManagerLoaded, RefreshOperationStatus.NoOp, isUIFiltering: false, 0);
@@ -1642,12 +1603,7 @@ private void CleanUp()
16421603
solutionManager.ProjectRemoved -= OnProjectChanged;
16431604
solutionManager.ProjectUpdated -= OnProjectUpdated;
16441605
solutionManager.ProjectRenamed -= OnProjectRenamed;
1645-
1646-
if (_projectUpdateEvents != null)
1647-
{
1648-
_projectUpdateEvents.ProjectUpdateFinished -= OnProjectUpdateFinished;
1649-
_projectUpdateEvents.SolutionRestoreFinished -= OnSolutionRestoreFinished;
1650-
}
1606+
solutionManager.AfterNuGetCacheUpdated -= OnNuGetCacheUpdated;
16511607

16521608
Model.Context.ProjectActionsExecuted -= OnProjectActionsExecuted;
16531609

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,6 @@ public enum RefreshOperationSource
104104
{
105105
ActionsExecuted,
106106
CacheUpdated,
107-
RestoreCompleted,
108107
CheckboxPrereleaseChanged,
109108
ClearSearch,
110109
ExecuteAction,

test/NuGet.Clients.Tests/NuGet.PackageManagement.VisualStudio.Test/Telemetry/NuGetTelemetryServiceTests.cs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,9 +91,6 @@ public void NuGetTelemetryService_EmitProjectInformation(NuGetProjectType projec
9191
[InlineData(RefreshOperationSource.PackageSourcesChanged, RefreshOperationStatus.Success)]
9292
[InlineData(RefreshOperationSource.ProjectsChanged, RefreshOperationStatus.Success)]
9393
[InlineData(RefreshOperationSource.ProjectsChanged, RefreshOperationStatus.Failed)]
94-
[InlineData(RefreshOperationSource.RestoreCompleted, RefreshOperationStatus.Success)]
95-
[InlineData(RefreshOperationSource.RestoreCompleted, RefreshOperationStatus.NoOp)]
96-
[InlineData(RefreshOperationSource.RestoreCompleted, RefreshOperationStatus.NotApplicable)]
9794
[InlineData(RefreshOperationSource.RestartSearchCommand, RefreshOperationStatus.Success)]
9895
[InlineData(RefreshOperationSource.SourceSelectionChanged, RefreshOperationStatus.Success)]
9996
public void NuGetTelemetryService_EmitsPMUIRefreshEvent(RefreshOperationSource expectedRefreshSource, RefreshOperationStatus expectedRefreshStatus, bool expectedUiFiltering = false)

test/NuGet.Clients.Tests/NuGet.SolutionRestoreManager.Test/VsRestoreProgressEventsTests.cs

Lines changed: 0 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -246,87 +246,5 @@ public void EndProjectUpdate_WhenBatchEventEndIsRaisedWithNonProject_DoesNotFire
246246

247247
Assert.Equal(0, invocations);
248248
}
249-
250-
[Fact]
251-
public void EndProjectUpdate_WhenSubscriberThrows_OtherSubscribersStillReceiveEvent()
252-
{
253-
var restoreProgressEvents = new VsRestoreProgressEvents(_packageProjectProvider.Object, new Mock<INuGetTelemetryProvider>().Object);
254-
255-
var expectedProjectName = "projectName.csproj";
256-
var expectedFileList = new List<string>() { "project.assets.json" };
257-
bool secondHandlerCalled = false;
258-
259-
restoreProgressEvents.ProjectUpdateFinished += (projectUniqueName, updatedFiles) =>
260-
{
261-
throw new InvalidOperationException("Simulated handler failure");
262-
};
263-
264-
restoreProgressEvents.ProjectUpdateFinished += (projectUniqueName, updatedFiles) =>
265-
{
266-
secondHandlerCalled = true;
267-
};
268-
269-
restoreProgressEvents.EndProjectUpdate(expectedProjectName, expectedFileList);
270-
271-
Assert.True(secondHandlerCalled);
272-
}
273-
274-
[Fact]
275-
public void EndSolutionRestore_WhenSubscriberThrows_OtherSubscribersStillReceiveEvent()
276-
{
277-
var restoreProgressEvents = new VsRestoreProgressEvents(_packageProjectProvider.Object, new Mock<INuGetTelemetryProvider>().Object);
278-
279-
var expectedProjectList = new List<string>() { "projectName.csproj" };
280-
bool secondHandlerCalled = false;
281-
282-
restoreProgressEvents.SolutionRestoreFinished += (projects) =>
283-
{
284-
throw new InvalidOperationException("Simulated handler failure");
285-
};
286-
287-
restoreProgressEvents.SolutionRestoreFinished += (projects) =>
288-
{
289-
secondHandlerCalled = true;
290-
};
291-
292-
restoreProgressEvents.EndSolutionRestore(expectedProjectList);
293-
294-
Assert.True(secondHandlerCalled);
295-
}
296-
297-
[Fact]
298-
public void EndProjectUpdate_WithMultipleSubscribers_AllReceiveEvent()
299-
{
300-
var restoreProgressEvents = new VsRestoreProgressEvents(_packageProjectProvider.Object, new Mock<INuGetTelemetryProvider>().Object);
301-
302-
var expectedProjectName = "projectName.csproj";
303-
var expectedFileList = new List<string>() { "project.assets.json" };
304-
int handlerCallCount = 0;
305-
306-
restoreProgressEvents.ProjectUpdateFinished += (projectUniqueName, updatedFiles) => handlerCallCount++;
307-
restoreProgressEvents.ProjectUpdateFinished += (projectUniqueName, updatedFiles) => handlerCallCount++;
308-
restoreProgressEvents.ProjectUpdateFinished += (projectUniqueName, updatedFiles) => handlerCallCount++;
309-
310-
restoreProgressEvents.EndProjectUpdate(expectedProjectName, expectedFileList);
311-
312-
Assert.Equal(3, handlerCallCount);
313-
}
314-
315-
[Fact]
316-
public void EndSolutionRestore_WithMultipleSubscribers_AllReceiveEvent()
317-
{
318-
var restoreProgressEvents = new VsRestoreProgressEvents(_packageProjectProvider.Object, new Mock<INuGetTelemetryProvider>().Object);
319-
320-
var expectedProjectList = new List<string>() { "projectA.csproj", "projectB.csproj" };
321-
int handlerCallCount = 0;
322-
323-
restoreProgressEvents.SolutionRestoreFinished += (projects) => handlerCallCount++;
324-
restoreProgressEvents.SolutionRestoreFinished += (projects) => handlerCallCount++;
325-
restoreProgressEvents.SolutionRestoreFinished += (projects) => handlerCallCount++;
326-
327-
restoreProgressEvents.EndSolutionRestore(expectedProjectList);
328-
329-
Assert.Equal(3, handlerCallCount);
330-
}
331249
}
332250
}

0 commit comments

Comments
 (0)