From f4f0d6efa9fa0cc401e073e0c33cca4f27cefe17 Mon Sep 17 00:00:00 2001 From: lilla28 Date: Thu, 7 Mar 2024 09:24:33 +0100 Subject: [PATCH] fix(fdc3) - Fixing DesktopAgent's flaky tests: waiting for StartRequests to finish before raising the intent --- .../src/DesktopAgent/Fdc3DesktopAgent.cs | 49 +++++++++++++++++-- .../Fdc3DesktopAgentTests.cs | 12 ++++- ...3DesktopAgentMessageRouterService.Tests.cs | 35 ++++++------- 3 files changed, 70 insertions(+), 26 deletions(-) diff --git a/src/fdc3/dotnet/DesktopAgent/src/DesktopAgent/Fdc3DesktopAgent.cs b/src/fdc3/dotnet/DesktopAgent/src/DesktopAgent/Fdc3DesktopAgent.cs index a28cad97d..41297ed7d 100644 --- a/src/fdc3/dotnet/DesktopAgent/src/DesktopAgent/Fdc3DesktopAgent.cs +++ b/src/fdc3/dotnet/DesktopAgent/src/DesktopAgent/Fdc3DesktopAgent.cs @@ -45,6 +45,7 @@ internal class Fdc3DesktopAgent : IFdc3DesktopAgentBridge private readonly IModuleLoader _moduleLoader; private readonly ConcurrentDictionary _runningModules = new(); private readonly ConcurrentDictionary _raisedIntentResolutions = new(); + private readonly ConcurrentDictionary> _pendingStartRequests = new(); private IAsyncDisposable? _subscription; public Fdc3DesktopAgent( @@ -97,6 +98,13 @@ public async Task StopAsync(CancellationToken cancellationToken) _runningModules.Clear(); await _subscription.DisposeAsync(); } + + foreach (var pendingStartRequest in _pendingStartRequests) + { + pendingStartRequest.Value.TrySetCanceled(); + } + + _pendingStartRequests.Clear(); } public bool FindChannel(string channelId, ChannelType channelType) @@ -461,13 +469,33 @@ private async ValueTask> RaiseIntentToApp try { var fdc3InstanceId = Guid.NewGuid(); - var moduleInstance = await _moduleLoader.StartModule( - new StartRequest( - targetAppMetadata.AppId, //TODO: possible remove some identifier like @"fdc3." + var startRequest = new StartRequest( + targetAppMetadata.AppId, //TODO: possible remove some identifier like @"fdc3." new List>() { { new(Fdc3StartupParameters.Fdc3InstanceId, fdc3InstanceId.ToString()) } - })) ?? throw ThrowHelper.TargetInstanceUnavailable(); + }); + + var taskCompletionSource = new TaskCompletionSource(); + + if (_pendingStartRequests.TryAdd(startRequest, taskCompletionSource)) + { + var moduleInstance = await _moduleLoader.StartModule(startRequest); + + if (moduleInstance == null) + { + var exception = ThrowHelper.TargetInstanceUnavailable(); + + if (!_pendingStartRequests.TryRemove(startRequest, out _)) + { + _logger.LogWarning($"Could not remove {nameof(StartRequest)} from the pending requests. ModuleId: {startRequest.ModuleId}."); + } + + taskCompletionSource.TrySetException(exception); + } + + await taskCompletionSource.Task; + } var raisedIntentMessageId = StoreRaisedIntentForTarget(messageId, fdc3InstanceId.ToString(), intent, context, sourceFdc3InstanceId); @@ -496,7 +524,7 @@ private async ValueTask> RaiseIntentToApp return new() { - Response = RaiseIntentResponse.Failure(exception.Message), + Response = RaiseIntentResponse.Failure(exception.ToString()), }; } } @@ -698,10 +726,16 @@ private Task RemoveModuleAsync(IModuleInstance instance) try { var fdc3InstanceId = GetFdc3InstanceId(instance); + if (!_runningModules.TryRemove(new(fdc3InstanceId), out _)) { _logger.LogError($"Could not remove the closed window with instanceId: {fdc3InstanceId}."); } + + if (_pendingStartRequests.TryRemove(instance.StartRequest, out var taskCompletionSource)) + { + taskCompletionSource.SetException(ThrowHelper.TargetInstanceUnavailable()); + } } catch (Fdc3DesktopAgentException exception) { @@ -717,6 +751,11 @@ private async Task AddOrUpdateModuleAsync(IModuleInstance instance) try { fdc3App = await _appDirectory.GetApp(instance.Manifest.Id); + + if (_pendingStartRequests.TryRemove(instance.StartRequest, out var taskCompletionSource)) + { + taskCompletionSource.SetResult(instance); + } } catch (AppNotFoundException) { diff --git a/src/fdc3/dotnet/DesktopAgent/tests/DesktopAgent.Tests/Fdc3DesktopAgentTests.cs b/src/fdc3/dotnet/DesktopAgent/tests/DesktopAgent.Tests/Fdc3DesktopAgentTests.cs index 4a9dbee76..65ddc9821 100644 --- a/src/fdc3/dotnet/DesktopAgent/tests/DesktopAgent.Tests/Fdc3DesktopAgentTests.cs +++ b/src/fdc3/dotnet/DesktopAgent/tests/DesktopAgent.Tests/Fdc3DesktopAgentTests.cs @@ -31,7 +31,7 @@ namespace MorganStanley.ComposeUI.Fdc3.DesktopAgent.Tests; -public class Fdc3DesktopAgentTests +public class Fdc3DesktopAgentTests : IAsyncLifetime { private readonly MockModuleLoader _mockModuleLoader = new(); private readonly IAppDirectory _appDirectory = new AppDirectory.AppDirectory( @@ -593,4 +593,14 @@ public async Task RaiseIntent_returns_one_running_app() result!.RaiseIntentResolutionMessages!.Should().HaveCount(1); result!.RaiseIntentResolutionMessages!.First().TargetModuleInstanceId.Should().Be(targetFdc3InstanceId); } + + public async Task InitializeAsync() + { + await _fdc3.StartAsync(CancellationToken.None); + } + + public async Task DisposeAsync() + { + await _fdc3.StopAsync(CancellationToken.None); + } } diff --git a/src/fdc3/dotnet/DesktopAgent/tests/DesktopAgent.Tests/Infrastructure/Internal/Fdc3DesktopAgentMessageRouterService.Tests.cs b/src/fdc3/dotnet/DesktopAgent/tests/DesktopAgent.Tests/Infrastructure/Internal/Fdc3DesktopAgentMessageRouterService.Tests.cs index df492b320..321323d08 100644 --- a/src/fdc3/dotnet/DesktopAgent/tests/DesktopAgent.Tests/Infrastructure/Internal/Fdc3DesktopAgentMessageRouterService.Tests.cs +++ b/src/fdc3/dotnet/DesktopAgent/tests/DesktopAgent.Tests/Infrastructure/Internal/Fdc3DesktopAgentMessageRouterService.Tests.cs @@ -30,7 +30,7 @@ namespace MorganStanley.ComposeUI.Fdc3.DesktopAgent.Tests.Infrastructure.Internal; -public class Fdc3DesktopAgentMessageRouterServiceTests +public class Fdc3DesktopAgentMessageRouterServiceTests : IAsyncLifetime { private readonly Mock _mockMessageRouter = new(); private readonly MockModuleLoader _mockModuleLoader = new(); @@ -78,7 +78,6 @@ public async Task RaiseIntent_returns_one_app_by_AppIdentifier() var result = await _fdc3.HandleRaiseIntent(raiseIntentRequest, new MessageContext()); result.Should().NotBeNull(); - result!.AppMetadata.Should().HaveCount(1); result!.AppMetadata!.First()!.AppId.Should().Be("appId4"); result!.AppMetadata!.First()!.InstanceId.Should().NotBeNull(); @@ -88,7 +87,6 @@ public async Task RaiseIntent_returns_one_app_by_AppIdentifier() public async Task RaiseIntent_fails_by_request_delivery_error() { var result = await _fdc3.HandleRaiseIntent(null, new MessageContext()); - result.Should().NotBeNull(); result!.Error.Should().Be(ResolveError.IntentDeliveryFailed); } @@ -106,9 +104,7 @@ public async Task RaiseIntent_returns_one_app_by_Context() }; var result = await _fdc3.HandleRaiseIntent(request, new MessageContext()); - result.Should().NotBeNull(); - result!.AppMetadata.Should().HaveCount(1); result!.AppMetadata!.First()!.AppId.Should().Be("appId4"); result!.AppMetadata!.First()!.InstanceId.Should().NotBeNull(); @@ -133,9 +129,7 @@ public async Task RaiseIntent_returns_one_app_by_AppIdentifier_and_saves_context }; var result = await _fdc3.HandleRaiseIntent(request, new MessageContext()); - result.Should().NotBeNull(); - result!.AppMetadata.Should().HaveCount(1); result!.AppMetadata!.First()!.AppId.Should().Be("appId4"); result!.AppMetadata!.First()!.InstanceId.Should().Be(targetFdc3InstanceId); @@ -169,7 +163,6 @@ public async Task RaiseIntent_returns_one_app_by_AppIdentifier_and_publishes_con var addIntentListenerResult = await _fdc3.HandleAddIntentListener(addIntentListenerRequest, new MessageContext()); addIntentListenerResult.Should().NotBeNull(); - addIntentListenerResult!.Stored.Should().BeTrue(); var request = new RaiseIntentRequest() @@ -184,7 +177,6 @@ public async Task RaiseIntent_returns_one_app_by_AppIdentifier_and_publishes_con var result = await _fdc3.HandleRaiseIntent(request, new MessageContext()); result.Should().NotBeNull(); - result!.AppMetadata.Should().HaveCount(1); result!.AppMetadata!.First()!.AppId.Should().Be("appId4"); result!.AppMetadata!.First()!.InstanceId.Should().Be(targetFdc3InstanceId); @@ -209,7 +201,6 @@ public async Task RaiseIntent_returns_first_app_by_Context() var result = await _fdc3.HandleRaiseIntent(raiseIntentRequest, new MessageContext()); result.Should().NotBeNull(); - result!.AppMetadata.Should().HaveCount(1); result!.AppMetadata!.First().AppId.Should().Be("appId4"); } @@ -230,7 +221,6 @@ public async Task RaiseIntent_returns_first_app_by_Context_if_fdc3_nothing() var result = await _fdc3.HandleRaiseIntent(raiseIntentRequest, new MessageContext()); result.Should().NotBeNull(); - result!.AppMetadata.Should().HaveCount(1); result!.AppMetadata!.First().AppId.Should().Be("appId4"); } @@ -266,7 +256,6 @@ public async Task RaiseIntent_fails_as_no_apps_found_by_Context() }; var result = await _fdc3.HandleRaiseIntent(raiseIntentRequest, new MessageContext()); - result.Should().NotBeNull(); result!.Error.Should().Be(ResolveError.NoAppsFound); } @@ -284,7 +273,6 @@ public async Task RaiseIntent_fails_as_no_apps_found_by_Intent() }; var result = await _fdc3.HandleRaiseIntent(raiseIntentRequest, new MessageContext()); - result.Should().NotBeNull(); result!.Error.Should().Be(ResolveError.NoAppsFound); } @@ -302,7 +290,6 @@ public async Task RaiseIntent_fails_as_multiple_IAppIntents_found() }; var result = await _fdc3.HandleRaiseIntent(raiseIntentRequest, new MessageContext()); - result.Should().NotBeNull(); result!.Error.Should().Be(ResolveError.IntentDeliveryFailed); } @@ -320,7 +307,6 @@ public async Task RaiseIntent_fails_as_request_specifies_error() }; var result = await _fdc3.HandleRaiseIntent(raiseIntentRequest, new MessageContext()); - result!.Error.Should().Be("Some weird error"); } @@ -616,7 +602,6 @@ public async Task GetIntentResult_succeeds_with_context() }; var storeResult = await _fdc3.HandleStoreIntentResult(storeIntentRequest, new MessageContext()); - storeResult.Should().NotBeNull(); storeResult!.Should().BeEquivalentTo(StoreIntentResultResponse.Success()); @@ -667,7 +652,6 @@ public async Task GetIntentResult_succeeds_with_channel() }; var storeResult = await _fdc3.HandleStoreIntentResult(storeIntentRequest, new MessageContext()); - storeResult.Should().NotBeNull(); storeResult!.Should().BeEquivalentTo(StoreIntentResultResponse.Success()); @@ -687,8 +671,8 @@ public async Task GetIntentResult_succeeds_with_channel() public async Task GetIntentResult_succeeds_with_voidResult() { await _fdc3.StartAsync(CancellationToken.None); - var originFdc3InstanceId = Guid.NewGuid().ToString(); + var originFdc3InstanceId = Guid.NewGuid().ToString(); var target = await _mockModuleLoader.Object.StartModule(new("appId4")); var targetFdc3InstanceId = Fdc3InstanceIdRetriever.Get(target); @@ -748,6 +732,7 @@ public async Task AddIntentListener_fails_due_missing_id() Fdc3InstanceId = Guid.NewGuid().ToString(), State = SubscribeState.Unsubscribe }; + var result = await _fdc3.HandleAddIntentListener(request, new()); result.Should().NotBeNull(); result!.Should().BeEquivalentTo(IntentListenerResponse.Failure(Fdc3DesktopAgentErrors.MissingId)); @@ -885,8 +870,8 @@ public async Task AddIntentListener_unsubscribes() public async Task FindIntent_edge_case_tests(FindIntentTestCase testCase) { var request = testCase.Request; - var result = await _fdc3.HandleFindIntent(request, new MessageContext()); + var result = await _fdc3.HandleFindIntent(request, new MessageContext()); result.Should().NotBeNull(); if (testCase.ExpectedAppCount > 0) @@ -902,8 +887,8 @@ public async Task FindIntent_edge_case_tests(FindIntentTestCase testCase) public async Task FindIntentsByContext_edge_case_tests(FindIntentsByContextTestCase testCase) { var request = testCase.Request; - var result = await _fdc3.HandleFindIntentsByContext(request, new MessageContext()); + var result = await _fdc3.HandleFindIntentsByContext(request, new MessageContext()); result.Should().NotBeNull(); if (testCase.ExpectedAppIntentsCount > 0) @@ -914,6 +899,16 @@ public async Task FindIntentsByContext_edge_case_tests(FindIntentsByContextTestC result!.Should().BeEquivalentTo(testCase.ExpectedResponse); } + public async Task InitializeAsync() + { + await _fdc3.StartAsync(CancellationToken.None); + } + + public async Task DisposeAsync() + { + await _fdc3.StopAsync(CancellationToken.None); + } + public class FindIntentsByContextTheoryData : TheoryData { public FindIntentsByContextTheoryData()