From cb32f7853103674a7f7900df48aa3dee1431389c Mon Sep 17 00:00:00 2001 From: Alexis Nowikowski Date: Sat, 26 Oct 2024 14:45:04 +0200 Subject: [PATCH 1/6] Added test cases for issue #906 --- .../ConductorWithCollectionOneActiveTests.cs | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/src/Caliburn.Micro.Core.Tests/ConductorWithCollectionOneActiveTests.cs b/src/Caliburn.Micro.Core.Tests/ConductorWithCollectionOneActiveTests.cs index 6e3371cd0..a176a56a3 100644 --- a/src/Caliburn.Micro.Core.Tests/ConductorWithCollectionOneActiveTests.cs +++ b/src/Caliburn.Micro.Core.Tests/ConductorWithCollectionOneActiveTests.cs @@ -24,6 +24,45 @@ protected override async Task OnDeactivateAsync(bool close, CancellationToken ca } } + private class AsyncActivationScreen : Screen + { + private readonly bool _simulateAsyncOnActivate; + + private readonly bool _simulateAsyncOnDeactivate; + + private readonly TimeSpan _simulateAsyncTaskDuration; + + public AsyncActivationScreen(bool simulateAsyncOnActivate, bool simulateAsyncOnDeactivate, + TimeSpan simulateAsyncTaskDuration) + { + _simulateAsyncOnActivate = simulateAsyncOnActivate; + _simulateAsyncOnDeactivate = simulateAsyncOnDeactivate; + _simulateAsyncTaskDuration = simulateAsyncTaskDuration; + } + + protected override async Task OnActivateAsync(CancellationToken cancellationToken) + { + await base.OnActivateAsync(cancellationToken); + + if (_simulateAsyncOnActivate) + { + // Task.Delay doesn't run within captured context + await Task.Delay(_simulateAsyncTaskDuration, cancellationToken); + } + } + + protected override async Task OnDeactivateAsync(bool close, CancellationToken cancellationToken) + { + if (_simulateAsyncOnDeactivate) + { + // Task.Delay doesn't run within captured context + await Task.Delay(_simulateAsyncTaskDuration, cancellationToken); + } + + await base.OnDeactivateAsync(close, cancellationToken); + } + } + [Fact] public void AddedItemAppearsInChildren() { @@ -150,5 +189,33 @@ public void ParentItemIsUnsetOnReplaceConductedItem() Assert.NotEqual(conductor, conducted.Parent); Assert.Equal(conductor, conducted2.Parent); } + + [Fact] + public async Task ActiveItemSetterShouldSetThePropertySynchronouslyWhenOnActivateIsLongRunningTask() + { + var conductor = new Conductor.Collection.OneActive(); + var conducted = new AsyncActivationScreen(true, false, TimeSpan.FromSeconds(1)); + conductor.Items.Add(conducted); + await ((IActivate)conductor).ActivateAsync(CancellationToken.None); + conductor.ActiveItem = conducted; + Assert.NotNull(conductor.ActiveItem); + Assert.Equal(conducted, conductor.ActiveItem); + } + + [Fact] + public async Task ActiveItemSetterShouldSetThePropertySynchronouslyWhenOnDeactivateIsLongRunningTask() + { + var conductor = new Conductor.Collection.OneActive(); + var conducted1 = new AsyncActivationScreen(false, true, TimeSpan.FromSeconds(1)); + conductor.Items.Add(conducted1); + var conducted2 = new Screen(); + conductor.Items.Add(conducted2); + await((IActivate)conductor).ActivateAsync(CancellationToken.None); + conductor.ActiveItem = conducted1; + conductor.ActiveItem = conducted2; + Assert.NotNull(conductor.ActiveItem); + Assert.NotEqual(conducted1, conductor.ActiveItem); + Assert.Equal(conducted2, conductor.ActiveItem); + } } } From 37d249ca3cc8e6613cce43dc7323137940758cf4 Mon Sep 17 00:00:00 2001 From: Alexis Nowikowski Date: Sat, 26 Oct 2024 14:46:30 +0200 Subject: [PATCH 2/6] Bugfix issue #906 - ConductorBaseWithActiveItem: set ActiveItem underlying field synchronously --- src/Caliburn.Micro.Core/ConductorBaseWithActiveItem.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Caliburn.Micro.Core/ConductorBaseWithActiveItem.cs b/src/Caliburn.Micro.Core/ConductorBaseWithActiveItem.cs index 1249eac40..f54c446a1 100644 --- a/src/Caliburn.Micro.Core/ConductorBaseWithActiveItem.cs +++ b/src/Caliburn.Micro.Core/ConductorBaseWithActiveItem.cs @@ -39,12 +39,12 @@ object IHaveActiveItem.ActiveItem /// A task that represents the asynchronous operation. protected virtual async Task ChangeActiveItemAsync(T newItem, bool closePrevious, CancellationToken cancellationToken) { - await ScreenExtensions.TryDeactivateAsync(_activeItem, closePrevious, cancellationToken); - newItem = EnsureItem(newItem); - _activeItem = newItem; NotifyOfPropertyChange(nameof(ActiveItem)); + await ScreenExtensions.TryDeactivateAsync(_activeItem, closePrevious, cancellationToken); + newItem = EnsureItem(newItem); + if (IsActive) await ScreenExtensions.TryActivateAsync(newItem, cancellationToken); From 10180d1d3b1d8a71716ecd919debe60b4eb957ea Mon Sep 17 00:00:00 2001 From: Alexis Nowikowski Date: Sat, 26 Oct 2024 22:04:41 +0200 Subject: [PATCH 3/6] ConductorBaseWithActiveItem: ensure item is ready to be activated before setting ActiveItem backing field --- src/Caliburn.Micro.Core/ConductorBaseWithActiveItem.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Caliburn.Micro.Core/ConductorBaseWithActiveItem.cs b/src/Caliburn.Micro.Core/ConductorBaseWithActiveItem.cs index f54c446a1..e2005495f 100644 --- a/src/Caliburn.Micro.Core/ConductorBaseWithActiveItem.cs +++ b/src/Caliburn.Micro.Core/ConductorBaseWithActiveItem.cs @@ -39,11 +39,12 @@ object IHaveActiveItem.ActiveItem /// A task that represents the asynchronous operation. protected virtual async Task ChangeActiveItemAsync(T newItem, bool closePrevious, CancellationToken cancellationToken) { + newItem = EnsureItem(newItem); + _activeItem = newItem; NotifyOfPropertyChange(nameof(ActiveItem)); await ScreenExtensions.TryDeactivateAsync(_activeItem, closePrevious, cancellationToken); - newItem = EnsureItem(newItem); if (IsActive) await ScreenExtensions.TryActivateAsync(newItem, cancellationToken); From f4b0595579f691e805a4afe2dfe07889d215d19e Mon Sep 17 00:00:00 2001 From: Alexis Nowikowski Date: Sat, 26 Oct 2024 14:45:04 +0200 Subject: [PATCH 4/6] Added test cases for issue #906 --- .../ConductorWithCollectionOneActiveTests.cs | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/src/Caliburn.Micro.Core.Tests/ConductorWithCollectionOneActiveTests.cs b/src/Caliburn.Micro.Core.Tests/ConductorWithCollectionOneActiveTests.cs index 6e3371cd0..a176a56a3 100644 --- a/src/Caliburn.Micro.Core.Tests/ConductorWithCollectionOneActiveTests.cs +++ b/src/Caliburn.Micro.Core.Tests/ConductorWithCollectionOneActiveTests.cs @@ -24,6 +24,45 @@ protected override async Task OnDeactivateAsync(bool close, CancellationToken ca } } + private class AsyncActivationScreen : Screen + { + private readonly bool _simulateAsyncOnActivate; + + private readonly bool _simulateAsyncOnDeactivate; + + private readonly TimeSpan _simulateAsyncTaskDuration; + + public AsyncActivationScreen(bool simulateAsyncOnActivate, bool simulateAsyncOnDeactivate, + TimeSpan simulateAsyncTaskDuration) + { + _simulateAsyncOnActivate = simulateAsyncOnActivate; + _simulateAsyncOnDeactivate = simulateAsyncOnDeactivate; + _simulateAsyncTaskDuration = simulateAsyncTaskDuration; + } + + protected override async Task OnActivateAsync(CancellationToken cancellationToken) + { + await base.OnActivateAsync(cancellationToken); + + if (_simulateAsyncOnActivate) + { + // Task.Delay doesn't run within captured context + await Task.Delay(_simulateAsyncTaskDuration, cancellationToken); + } + } + + protected override async Task OnDeactivateAsync(bool close, CancellationToken cancellationToken) + { + if (_simulateAsyncOnDeactivate) + { + // Task.Delay doesn't run within captured context + await Task.Delay(_simulateAsyncTaskDuration, cancellationToken); + } + + await base.OnDeactivateAsync(close, cancellationToken); + } + } + [Fact] public void AddedItemAppearsInChildren() { @@ -150,5 +189,33 @@ public void ParentItemIsUnsetOnReplaceConductedItem() Assert.NotEqual(conductor, conducted.Parent); Assert.Equal(conductor, conducted2.Parent); } + + [Fact] + public async Task ActiveItemSetterShouldSetThePropertySynchronouslyWhenOnActivateIsLongRunningTask() + { + var conductor = new Conductor.Collection.OneActive(); + var conducted = new AsyncActivationScreen(true, false, TimeSpan.FromSeconds(1)); + conductor.Items.Add(conducted); + await ((IActivate)conductor).ActivateAsync(CancellationToken.None); + conductor.ActiveItem = conducted; + Assert.NotNull(conductor.ActiveItem); + Assert.Equal(conducted, conductor.ActiveItem); + } + + [Fact] + public async Task ActiveItemSetterShouldSetThePropertySynchronouslyWhenOnDeactivateIsLongRunningTask() + { + var conductor = new Conductor.Collection.OneActive(); + var conducted1 = new AsyncActivationScreen(false, true, TimeSpan.FromSeconds(1)); + conductor.Items.Add(conducted1); + var conducted2 = new Screen(); + conductor.Items.Add(conducted2); + await((IActivate)conductor).ActivateAsync(CancellationToken.None); + conductor.ActiveItem = conducted1; + conductor.ActiveItem = conducted2; + Assert.NotNull(conductor.ActiveItem); + Assert.NotEqual(conducted1, conductor.ActiveItem); + Assert.Equal(conducted2, conductor.ActiveItem); + } } } From c2121aa1cab0e634285cb8c840bd8d68aafd597c Mon Sep 17 00:00:00 2001 From: Alexis Nowikowski Date: Sat, 26 Oct 2024 14:46:30 +0200 Subject: [PATCH 5/6] Bugfix issue #906 - ConductorBaseWithActiveItem: set ActiveItem underlying field synchronously --- src/Caliburn.Micro.Core/ConductorBaseWithActiveItem.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Caliburn.Micro.Core/ConductorBaseWithActiveItem.cs b/src/Caliburn.Micro.Core/ConductorBaseWithActiveItem.cs index 1249eac40..f54c446a1 100644 --- a/src/Caliburn.Micro.Core/ConductorBaseWithActiveItem.cs +++ b/src/Caliburn.Micro.Core/ConductorBaseWithActiveItem.cs @@ -39,12 +39,12 @@ object IHaveActiveItem.ActiveItem /// A task that represents the asynchronous operation. protected virtual async Task ChangeActiveItemAsync(T newItem, bool closePrevious, CancellationToken cancellationToken) { - await ScreenExtensions.TryDeactivateAsync(_activeItem, closePrevious, cancellationToken); - newItem = EnsureItem(newItem); - _activeItem = newItem; NotifyOfPropertyChange(nameof(ActiveItem)); + await ScreenExtensions.TryDeactivateAsync(_activeItem, closePrevious, cancellationToken); + newItem = EnsureItem(newItem); + if (IsActive) await ScreenExtensions.TryActivateAsync(newItem, cancellationToken); From d5b5bb840a6177d9e9cf8271d1de4e4b6bd379a3 Mon Sep 17 00:00:00 2001 From: Alexis Nowikowski Date: Sat, 26 Oct 2024 22:04:41 +0200 Subject: [PATCH 6/6] ConductorBaseWithActiveItem: ensure item is ready to be activated before setting ActiveItem backing field --- src/Caliburn.Micro.Core/ConductorBaseWithActiveItem.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Caliburn.Micro.Core/ConductorBaseWithActiveItem.cs b/src/Caliburn.Micro.Core/ConductorBaseWithActiveItem.cs index f54c446a1..e2005495f 100644 --- a/src/Caliburn.Micro.Core/ConductorBaseWithActiveItem.cs +++ b/src/Caliburn.Micro.Core/ConductorBaseWithActiveItem.cs @@ -39,11 +39,12 @@ object IHaveActiveItem.ActiveItem /// A task that represents the asynchronous operation. protected virtual async Task ChangeActiveItemAsync(T newItem, bool closePrevious, CancellationToken cancellationToken) { + newItem = EnsureItem(newItem); + _activeItem = newItem; NotifyOfPropertyChange(nameof(ActiveItem)); await ScreenExtensions.TryDeactivateAsync(_activeItem, closePrevious, cancellationToken); - newItem = EnsureItem(newItem); if (IsActive) await ScreenExtensions.TryActivateAsync(newItem, cancellationToken);