From e0b182ebf1303bb003d4311cede8522e2a9a622f Mon Sep 17 00:00:00 2001 From: lilla28 Date: Fri, 12 Apr 2024 22:45:48 +0200 Subject: [PATCH 1/3] fix(module-loader) - Fixed ModuleLoader behavior with multiple module catalogs --- .../IModuleCatalog.cs | 1 + .../ModuleLoaderException.cs | 36 +++++ .../ModuleNotFoundException.cs | 22 +++ .../AggregateModuleCatalog.cs | 83 ++++++++++ .../ModuleLoader.cs | 20 +-- ...organStanley.ComposeUI.ModuleLoader.csproj | 1 + .../Runners/NativeModuleRunner.cs | 2 +- .../AggregateModuleCatalogTests.cs | 147 ++++++++++++++++++ .../ModuleLoaderTests.cs | 9 +- ...tanley.ComposeUI.ModuleLoader.Tests.csproj | 1 + 10 files changed, 308 insertions(+), 14 deletions(-) create mode 100644 src/module-loader/dotnet/src/MorganStanley.ComposeUI.ModuleLoader.Abstractions/ModuleLoaderException.cs create mode 100644 src/module-loader/dotnet/src/MorganStanley.ComposeUI.ModuleLoader.Abstractions/ModuleNotFoundException.cs create mode 100644 src/module-loader/dotnet/src/MorganStanley.ComposeUI.ModuleLoader/AggregateModuleCatalog.cs create mode 100644 src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Tests/AggregateModuleCatalogTests.cs diff --git a/src/module-loader/dotnet/src/MorganStanley.ComposeUI.ModuleLoader.Abstractions/IModuleCatalog.cs b/src/module-loader/dotnet/src/MorganStanley.ComposeUI.ModuleLoader.Abstractions/IModuleCatalog.cs index 7c84e19d9..6d2715886 100644 --- a/src/module-loader/dotnet/src/MorganStanley.ComposeUI.ModuleLoader.Abstractions/IModuleCatalog.cs +++ b/src/module-loader/dotnet/src/MorganStanley.ComposeUI.ModuleLoader.Abstractions/IModuleCatalog.cs @@ -15,6 +15,7 @@ namespace MorganStanley.ComposeUI.ModuleLoader; /// /// Represents a collection of modules that can be started in the scope of the application. /// +/// The requested module was not found in the catalog public interface IModuleCatalog { /// diff --git a/src/module-loader/dotnet/src/MorganStanley.ComposeUI.ModuleLoader.Abstractions/ModuleLoaderException.cs b/src/module-loader/dotnet/src/MorganStanley.ComposeUI.ModuleLoader.Abstractions/ModuleLoaderException.cs new file mode 100644 index 000000000..04c432e04 --- /dev/null +++ b/src/module-loader/dotnet/src/MorganStanley.ComposeUI.ModuleLoader.Abstractions/ModuleLoaderException.cs @@ -0,0 +1,36 @@ +/* +* Morgan Stanley makes this available to you under the Apache License, +* Version 2.0 (the "License"). You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0. +* +* See the NOTICE file distributed with this work for additional information +* regarding copyright ownership. Unless required by applicable law or agreed +* to in writing, software distributed under the License is distributed on an +* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express +* or implied. See the License for the specific language governing permissions +* and limitations under the License. +*/ + +using System.Runtime.Serialization; + +namespace MorganStanley.ComposeUI.ModuleLoader; + +public class ModuleLoaderException : Exception +{ + public ModuleLoaderException() + { + } + + protected ModuleLoaderException(SerializationInfo info, StreamingContext context) : base(info, context) + { + } + + public ModuleLoaderException(string? message) : base(message) + { + } + + public ModuleLoaderException(string? message, Exception? innerException) : base(message, innerException) + { + } +} diff --git a/src/module-loader/dotnet/src/MorganStanley.ComposeUI.ModuleLoader.Abstractions/ModuleNotFoundException.cs b/src/module-loader/dotnet/src/MorganStanley.ComposeUI.ModuleLoader.Abstractions/ModuleNotFoundException.cs new file mode 100644 index 000000000..12d7bd821 --- /dev/null +++ b/src/module-loader/dotnet/src/MorganStanley.ComposeUI.ModuleLoader.Abstractions/ModuleNotFoundException.cs @@ -0,0 +1,22 @@ +/* +* Morgan Stanley makes this available to you under the Apache License, +* Version 2.0 (the "License"). You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0. +* +* See the NOTICE file distributed with this work for additional information +* regarding copyright ownership. Unless required by applicable law or agreed +* to in writing, software distributed under the License is distributed on an +* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express +* or implied. See the License for the specific language governing permissions +* and limitations under the License. +*/ + +namespace MorganStanley.ComposeUI.ModuleLoader; + +public class ModuleNotFoundException : ModuleLoaderException +{ + public ModuleNotFoundException(string moduleId) : base($"Unknown module id: {moduleId}") + { + } +} diff --git a/src/module-loader/dotnet/src/MorganStanley.ComposeUI.ModuleLoader/AggregateModuleCatalog.cs b/src/module-loader/dotnet/src/MorganStanley.ComposeUI.ModuleLoader/AggregateModuleCatalog.cs new file mode 100644 index 000000000..4784ed943 --- /dev/null +++ b/src/module-loader/dotnet/src/MorganStanley.ComposeUI.ModuleLoader/AggregateModuleCatalog.cs @@ -0,0 +1,83 @@ +// Morgan Stanley makes this available to you under the Apache License, +// Version 2.0 (the "License"). You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0. +// +// See the NOTICE file distributed with this work for additional information +// regarding copyright ownership. Unless required by applicable law or agreed +// to in writing, software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express +// or implied. See the License for the specific language governing permissions +// and limitations under the License. + +using System.Collections.Concurrent; +using System.Reflection; +using System.Runtime.CompilerServices; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; + +namespace MorganStanley.ComposeUI.ModuleLoader; + +internal class AggregateModuleCatalog : IModuleCatalog +{ + private readonly IEnumerable _moduleCatalogs; + private readonly ILogger _logger; + + public AggregateModuleCatalog( + IEnumerable moduleCatalogs, + ILogger? logger = null) + { + _moduleCatalogs = moduleCatalogs; + _logger = logger ?? NullLogger.Instance; + _lazyEnumerateModules = new Lazy(EnumerateModulesCore); + } + + public async Task GetManifest(string moduleId) + { + await EnumerateModules(); + + return _moduleIdToCatalog.TryGetValue(moduleId, out var catalog) + ? await catalog.GetManifest(moduleId) + : throw new ModuleNotFoundException(moduleId); + } + + public async Task> GetModuleIds() + { + await EnumerateModules(); + + return _moduleIdToCatalog.Keys; + } + + private readonly Lazy _lazyEnumerateModules; + private readonly ConcurrentDictionary _moduleIdToCatalog = new(); + + private Task EnumerateModules() => _lazyEnumerateModules.Value; + + private async Task EnumerateModulesCore() + { + var moduleCatalogs = _moduleCatalogs.ToDictionary(x => x, y => y.GetModuleIds()); + // Services/DI registrations appear in the order they were registered when resolved via IEnumerable<{SERVICE}> + // https://learn.microsoft.com/en-us/dotnet/core/extensions/dependency-injection + foreach (var moduleCatalog in moduleCatalogs) + { + var moduleIds = await moduleCatalog.Value; + + foreach (var moduleId in moduleIds) + { + if (!_moduleIdToCatalog.TryGetValue(moduleId, out var catalog)) + { + _moduleIdToCatalog[moduleId] = moduleCatalog.Key; + } + else + { + if (_logger.IsEnabled(LogLevel.Warning)) + { + var moduleManifest = await catalog.GetManifest(moduleId); + _logger.LogWarning( + $"ModuleId: {moduleId} is already contained by an another {nameof(IModuleCatalog)} with name {moduleManifest.Name}. Please consider using unique ids for modules. The first occurrence of the module will be saved and used by the {nameof(ModuleLoader)}."); + } + } + } + } + } +} diff --git a/src/module-loader/dotnet/src/MorganStanley.ComposeUI.ModuleLoader/ModuleLoader.cs b/src/module-loader/dotnet/src/MorganStanley.ComposeUI.ModuleLoader/ModuleLoader.cs index 2e250f719..87e83fa5a 100644 --- a/src/module-loader/dotnet/src/MorganStanley.ComposeUI.ModuleLoader/ModuleLoader.cs +++ b/src/module-loader/dotnet/src/MorganStanley.ComposeUI.ModuleLoader/ModuleLoader.cs @@ -12,11 +12,14 @@ using System.Collections.Concurrent; using System.Reactive.Subjects; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; namespace MorganStanley.ComposeUI.ModuleLoader; internal sealed class ModuleLoader : IModuleLoader, IAsyncDisposable { + private readonly ILogger _logger; private readonly Subject _lifetimeEvents = new(); private readonly ConcurrentDictionary _modules = new(); private readonly Dictionary _moduleRunners; @@ -24,17 +27,20 @@ internal sealed class ModuleLoader : IModuleLoader, IAsyncDisposable private readonly IReadOnlyList _startupActions; public ModuleLoader( - IModuleCatalog moduleCatalog, + IEnumerable moduleCatalogs, IEnumerable moduleRunners, - IEnumerable startupActions) + IEnumerable startupActions, + ILogger? logger = null) { - ArgumentNullException.ThrowIfNull(moduleCatalog); + ArgumentNullException.ThrowIfNull(moduleCatalogs); ArgumentNullException.ThrowIfNull(moduleRunners); ArgumentNullException.ThrowIfNull(startupActions); - - _moduleCatalog = moduleCatalog; + + _logger = logger ?? NullLogger.Instance; + _moduleCatalog = new AggregateModuleCatalog(moduleCatalogs, _logger); _moduleRunners = moduleRunners.GroupBy(runner => runner.ModuleType).ToDictionary(g => g.Key, g => g.First()); _startupActions = new List(startupActions); + } public IObservable LifetimeEvents => _lifetimeEvents; @@ -42,10 +48,6 @@ public ModuleLoader( public async Task StartModule(StartRequest request) { var manifest = await _moduleCatalog.GetManifest(request.ModuleId); - if (manifest == null) - { - throw new Exception($"Unknown Module id: {request.ModuleId}"); - } if (!_moduleRunners.TryGetValue(manifest.ModuleType, out var moduleRunner)) { diff --git a/src/module-loader/dotnet/src/MorganStanley.ComposeUI.ModuleLoader/MorganStanley.ComposeUI.ModuleLoader.csproj b/src/module-loader/dotnet/src/MorganStanley.ComposeUI.ModuleLoader/MorganStanley.ComposeUI.ModuleLoader.csproj index 555caf8fd..5af1fb99d 100644 --- a/src/module-loader/dotnet/src/MorganStanley.ComposeUI.ModuleLoader/MorganStanley.ComposeUI.ModuleLoader.csproj +++ b/src/module-loader/dotnet/src/MorganStanley.ComposeUI.ModuleLoader/MorganStanley.ComposeUI.ModuleLoader.csproj @@ -18,6 +18,7 @@ See the License for the specific language governing permissions and limitations + diff --git a/src/module-loader/dotnet/src/MorganStanley.ComposeUI.ModuleLoader/Runners/NativeModuleRunner.cs b/src/module-loader/dotnet/src/MorganStanley.ComposeUI.ModuleLoader/Runners/NativeModuleRunner.cs index 3c0dbbc4c..7a01ccd51 100644 --- a/src/module-loader/dotnet/src/MorganStanley.ComposeUI.ModuleLoader/Runners/NativeModuleRunner.cs +++ b/src/module-loader/dotnet/src/MorganStanley.ComposeUI.ModuleLoader/Runners/NativeModuleRunner.cs @@ -22,7 +22,7 @@ public async Task Start(StartupContext startupContext, Func pipeline) { if (!startupContext.ModuleInstance.Manifest.TryGetDetails(out NativeManifestDetails details)) { - throw new Exception("Unable to get native maifest details"); + throw new Exception("Unable to get native manifest details"); } startupContext.AddProperty(new EnvironmentVariables(details.EnvironmentVariables)); diff --git a/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Tests/AggregateModuleCatalogTests.cs b/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Tests/AggregateModuleCatalogTests.cs new file mode 100644 index 000000000..490acf153 --- /dev/null +++ b/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Tests/AggregateModuleCatalogTests.cs @@ -0,0 +1,147 @@ +// Morgan Stanley makes this available to you under the Apache License, +// Version 2.0 (the "License"). You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0. +// +// See the NOTICE file distributed with this work for additional information +// regarding copyright ownership. Unless required by applicable law or agreed +// to in writing, software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express +// or implied. See the License for the specific language governing permissions +// and limitations under the License. + +using FluentAssertions; + +namespace MorganStanley.ComposeUI.ModuleLoader.Tests; + +public class AggregateModuleCatalogTests +{ + private readonly IModuleCatalog _moduleCatalog = new AggregateModuleCatalog( + new[] + { + new MockModuleCatalog(new[] + { + new MockModuleManifest("testModuleId", "testModuleName"), + new MockModuleManifest("testModuleId1", "testModuleName1"), + new MockModuleManifest("testModuleId2", "testModuleName2"), + new MockModuleManifest("testModuleId3", "testModuleName3"), + new MockModuleManifest("testModuleId4", "testModuleName4") + }), + new MockModuleCatalog(new[] + { + new MockModuleManifest("testModuleId5", "testModuleName5"), + new MockModuleManifest("testModuleId", "testModuleName8"), + new MockModuleManifest("testModuleId6", "testModuleName6"), + }), + new MockModuleCatalog(new[] + { + new MockModuleManifest("testModuleId", "testModuleName9"), + }), + new MockModuleCatalog(new[] + { + new MockModuleManifest("testModuleId7", "testModuleName7"), + }), + new MockModuleCatalog(Enumerable.Empty()), + }); + + [Theory] + [ClassData(typeof(AggregateModuleCatalogReturnManifestTheoryData))] + public async Task GetManifest_ReturnsModuleManifest_WhenModuleIdPassed(string moduleId, IModuleManifest expectedModuleManifest) + { + var result = await _moduleCatalog.GetManifest(moduleId); + result.Should().NotBeNull(); + result.Should().BeEquivalentTo(expectedModuleManifest); + } + + [Theory] + [InlineData("noModuleId")] + [InlineData("testModuleId8")] + public async Task GetManifest_ThrowsModuleNotFoundException_WhenNoModuleIdFound(string moduleId) + { + var action = () => _moduleCatalog.GetManifest(moduleId); + await action.Should().ThrowAsync(); + } + + [Theory] + [InlineData("testModuleId")] + public async Task GetManifest_ReturnsTheFirstOccurrence_WhenMultipleCatalogContainsTheSameModuleId(string moduleId) + { + var result = await _moduleCatalog.GetManifest(moduleId); + result.Should().NotBeNull(); + result.Should().BeEquivalentTo(new MockModuleManifest("testModuleId", "testModuleName")); + } + + [Fact] + public async Task GetModuleIds_ReturnsTheUniqueModules() + { + var result = await _moduleCatalog.GetModuleIds(); + result.Should().NotBeNull(); + result.Should().HaveCount(8); + result.Should().BeEquivalentTo( + new[] + { + "testModuleId", + "testModuleId1", + "testModuleId2", + "testModuleId3", + "testModuleId4", + "testModuleId5", + "testModuleId6", + "testModuleId7", + }); + } + + private class MockModuleCatalog : IModuleCatalog + { + private readonly IEnumerable _modules; + + public MockModuleCatalog(IEnumerable modules) + { + _modules = modules; + } + + public Task GetManifest(string moduleId) + { + var module = _modules.FirstOrDefault(module => module.Id == moduleId); + if (module == null) + { + throw new NullReferenceException(moduleId); + } + + return Task.FromResult(module); + } + + public Task> GetModuleIds() + { + return Task.FromResult(_modules.Select(module => module.Id)); + } + } + + private class MockModuleManifest : IModuleManifest + { + private string _id; + private string _name; + + public MockModuleManifest(string id, string name) + { + _id = id; + _name = name; + } + + public string Id => _id; + + public string Name => _name; + + public string ModuleType => "dummy"; + } + + private class AggregateModuleCatalogReturnManifestTheoryData : TheoryData + { + public AggregateModuleCatalogReturnManifestTheoryData() + { + AddRow("testModuleId1", new MockModuleManifest("testModuleId1", "testModuleName1")); + AddRow("testModuleId2", new MockModuleManifest("testModuleId2", "testModuleName2")); + AddRow("testModuleId5", new MockModuleManifest("testModuleId5", "testModuleName5")); + } + } +} \ No newline at end of file diff --git a/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Tests/ModuleLoaderTests.cs b/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Tests/ModuleLoaderTests.cs index 26c62db2e..5d09f16c0 100644 --- a/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Tests/ModuleLoaderTests.cs +++ b/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Tests/ModuleLoaderTests.cs @@ -21,8 +21,8 @@ public class ModuleLoaderTests public void GivenNullArguments_WhenCtor_ThrowsArgumentNullException() { Assert.Throws(() => new ModuleLoader(null!, Enumerable.Empty(), Enumerable.Empty())); - Assert.Throws(() => new ModuleLoader(new Mock().Object, null!, Enumerable.Empty())); - Assert.Throws(() => new ModuleLoader(new Mock().Object, Enumerable.Empty(), null!)); + Assert.Throws(() => new ModuleLoader(new Mock>().Object, null!, Enumerable.Empty())); + Assert.Throws(() => new ModuleLoader(new Mock>().Object, Enumerable.Empty(), null!)); } [Fact] @@ -31,7 +31,7 @@ public void GivenUnknownModuleId_WhenStart_ThrowsException() var moduleCatalogMock = new Mock(); moduleCatalogMock.Setup(c => c.GetManifest(It.IsAny())).Returns(Task.FromResult(null)); - var moduleLoader = new ModuleLoader(moduleCatalogMock.Object, Enumerable.Empty(), Enumerable.Empty()); + var moduleLoader = new ModuleLoader(new[] { moduleCatalogMock.Object }, Enumerable.Empty(), Enumerable.Empty()); Assert.ThrowsAsync(async () => await moduleLoader.StartModule(new StartRequest("invalid"))); } @@ -45,7 +45,7 @@ public void WhenNoModuleRunnerAvailable_WhenStart_ThrowsException() var testModuleRunnerMock = new Mock(); testModuleRunnerMock.Setup(r => r.ModuleType).Returns("other"); - var moduleLoader = new ModuleLoader(moduleCatalogMock.Object, new[] { testModuleRunnerMock.Object }, Enumerable.Empty()); + var moduleLoader = new ModuleLoader(new[] { moduleCatalogMock.Object }, new[] { testModuleRunnerMock.Object }, Enumerable.Empty()); Assert.ThrowsAsync(async () => await moduleLoader.StartModule(new StartRequest("valid"))); } @@ -67,6 +67,7 @@ public async Task StartModule_EndToEndTest() moduleManifestMock.Setup(m => m.ModuleType).Returns(ModuleType.Web); moduleManifestMock.Setup(m => m.Details).Returns(webManifestDetails); moduleCatalogMock.Setup(c => c.GetManifest(moduleId)).Returns(Task.FromResult(moduleManifestMock.Object)); + moduleCatalogMock.Setup(catalog => catalog.GetModuleIds()).Returns(Task.FromResult>(new[] { moduleId })); startupActionMock.Setup(s => s.InvokeAsync(It.IsAny(), It.IsAny>())) .Callback>((startupContext, next) => { diff --git a/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Tests/MorganStanley.ComposeUI.ModuleLoader.Tests.csproj b/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Tests/MorganStanley.ComposeUI.ModuleLoader.Tests.csproj index c32e12dc8..59f183196 100644 --- a/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Tests/MorganStanley.ComposeUI.ModuleLoader.Tests.csproj +++ b/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Tests/MorganStanley.ComposeUI.ModuleLoader.Tests.csproj @@ -11,6 +11,7 @@ + From 4ea883c44aaef8cba8207deb91f37c39259da39e Mon Sep 17 00:00:00 2001 From: Lilla Orban Date: Fri, 12 Apr 2024 22:28:53 +0200 Subject: [PATCH 2/3] fix(module-loader) - Using FluentAssertions in ModuleLoaderTests --- ...eUI.ModuleLoader.Abstractions.Tests.csproj | 1 + .../StartupContextTests.cs | 12 +++-- .../ServiceCollectionExtensionsTests.cs | 7 +-- .../ModuleInstanceTests.cs | 13 +++-- .../ModuleLoaderTests.cs | 48 +++++++++++-------- .../Runners/NativeModuleRunnerTests.cs | 20 ++++---- .../Runners/WebModuleRunnerTests.cs | 9 ++-- 7 files changed, 63 insertions(+), 47 deletions(-) diff --git a/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Abstractions.Tests/MorganStanley.ComposeUI.ModuleLoader.Abstractions.Tests.csproj b/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Abstractions.Tests/MorganStanley.ComposeUI.ModuleLoader.Abstractions.Tests.csproj index e134c5bb3..588edca04 100644 --- a/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Abstractions.Tests/MorganStanley.ComposeUI.ModuleLoader.Abstractions.Tests.csproj +++ b/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Abstractions.Tests/MorganStanley.ComposeUI.ModuleLoader.Abstractions.Tests.csproj @@ -10,6 +10,7 @@ + diff --git a/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Abstractions.Tests/StartupContextTests.cs b/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Abstractions.Tests/StartupContextTests.cs index f66010475..b83a49e38 100644 --- a/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Abstractions.Tests/StartupContextTests.cs +++ b/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Abstractions.Tests/StartupContextTests.cs @@ -10,6 +10,7 @@ // or implied. See the License for the specific language governing permissions // and limitations under the License. +using FluentAssertions; using Moq; namespace MorganStanley.ComposeUI.ModuleLoader.Abstractions.Tests; @@ -25,20 +26,21 @@ public void WhenAdd_AddedValuesCanBeRetrieved() new MyContextInfo { Name = "Test2" } }; - StartupContext context = new StartupContext(new StartRequest("test"), Mock.Of()); + var context = new StartupContext(new StartRequest("test"), Mock.Of()); context.AddProperty(expected[0]); context.AddProperty(expected[1]); var result = context.GetProperties(); - Assert.NotNull(result); - Assert.Equal(expected, result); + result.Should().NotBeNull(); + result.Should().BeEquivalentTo(expected); } [Fact] public void GivenNullArgument_WhenAdd_ThrowsArgumentNullException() { - StartupContext context = new StartupContext(new StartRequest("test"), Mock.Of()); - Assert.Throws(() => context.AddProperty(null!)); + var context = new StartupContext(new StartRequest("test"), Mock.Of()); + var action = () => context.AddProperty(null!); + action.Should().Throw(); } private class MyContextInfo diff --git a/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Tests/DependencyInjection/ServiceCollectionExtensionsTests.cs b/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Tests/DependencyInjection/ServiceCollectionExtensionsTests.cs index 680b579c3..f4d062647 100644 --- a/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Tests/DependencyInjection/ServiceCollectionExtensionsTests.cs +++ b/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Tests/DependencyInjection/ServiceCollectionExtensionsTests.cs @@ -10,6 +10,7 @@ // or implied. See the License for the specific language governing permissions // and limitations under the License. +using FluentAssertions; using Microsoft.Extensions.DependencyInjection; using MorganStanley.ComposeUI.ModuleLoader.Runners; @@ -23,8 +24,8 @@ public void WhenAddModuleLoader_TypesAreAddedToServiceCollection() var services = new ServiceCollection() .AddModuleLoader(); - Assert.Equal(1, services.Count(s => s.ServiceType == typeof(IModuleLoader) && s.ImplementationType == typeof(ModuleLoader))); - Assert.Equal(1, services.Count(s => s.ServiceType == typeof(IModuleRunner) && s.ImplementationType == typeof(WebModuleRunner))); - Assert.Equal(1, services.Count(s => s.ServiceType == typeof(IModuleRunner) && s.ImplementationType == typeof(NativeModuleRunner))); + services.Count(s => s.ServiceType == typeof(IModuleLoader) && s.ImplementationType == typeof(ModuleLoader)).Should().Be(1); + services.Count(s => s.ServiceType == typeof(IModuleRunner) && s.ImplementationType == typeof(WebModuleRunner)).Should().Be(1); + services.Count(s => s.ServiceType == typeof(IModuleRunner) && s.ImplementationType == typeof(NativeModuleRunner)).Should().Be(1); } } diff --git a/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Tests/ModuleInstanceTests.cs b/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Tests/ModuleInstanceTests.cs index b193ba778..52989d79e 100644 --- a/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Tests/ModuleInstanceTests.cs +++ b/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Tests/ModuleInstanceTests.cs @@ -11,6 +11,7 @@ // and limitations under the License. +using FluentAssertions; using Moq; namespace MorganStanley.ComposeUI.ModuleLoader.Tests; @@ -22,17 +23,19 @@ public class ModuleInstanceTests [Fact] public void GivenNullArguments_WhenCtor_ThrowsArgumentNullException() { - Assert.Throws(() => new ModuleInstance(_testGuid, null!, new StartRequest("test"))); - Assert.Throws(() => new ModuleInstance(_testGuid, new Mock().Object, null!)); + var action1 = () => new ModuleInstance(_testGuid, null!, new StartRequest("test")); + action1.Should().Throw(); + + var action2 = () => new ModuleInstance(_testGuid, new Mock().Object, null!); + action2.Should().Throw(); } [Fact] public void WhenNewModuleInstance_GetProperties_ReturnsEmptyCollection() { var moduleInstance = new ModuleInstance(_testGuid, new Mock().Object, new StartRequest("test")); - var properties = moduleInstance.GetProperties(); - Assert.Empty(properties); + properties.Should().BeEmpty(); } [Fact] @@ -44,6 +47,6 @@ public void WhenSetProperties_GetPropertiesReturnThePropertiesAdded() moduleInstance.AddProperties(testProperties); var properties = moduleInstance.GetProperties(); - Assert.Equal(testProperties, properties); + testProperties.Should().BeEquivalentTo(properties); } } diff --git a/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Tests/ModuleLoaderTests.cs b/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Tests/ModuleLoaderTests.cs index 5d09f16c0..448e07b95 100644 --- a/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Tests/ModuleLoaderTests.cs +++ b/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Tests/ModuleLoaderTests.cs @@ -10,6 +10,7 @@ // or implied. See the License for the specific language governing permissions // and limitations under the License. +using FluentAssertions; using Microsoft.Extensions.DependencyInjection; using Moq; @@ -20,23 +21,28 @@ public class ModuleLoaderTests [Fact] public void GivenNullArguments_WhenCtor_ThrowsArgumentNullException() { - Assert.Throws(() => new ModuleLoader(null!, Enumerable.Empty(), Enumerable.Empty())); - Assert.Throws(() => new ModuleLoader(new Mock>().Object, null!, Enumerable.Empty())); - Assert.Throws(() => new ModuleLoader(new Mock>().Object, Enumerable.Empty(), null!)); + var action1 = () => new ModuleLoader(null!, Enumerable.Empty(), Enumerable.Empty()); + var action2 = () => new ModuleLoader(new Mock>().Object, null!, Enumerable.Empty()); + var action3 = () => new ModuleLoader(new Mock>().Object, Enumerable.Empty(), null!); + + action1.Should().Throw(); + action2.Should().Throw(); + action3.Should().Throw(); } [Fact] - public void GivenUnknownModuleId_WhenStart_ThrowsException() + public async void GivenUnknownModuleId_WhenStart_ThrowsException() { var moduleCatalogMock = new Mock(); moduleCatalogMock.Setup(c => c.GetManifest(It.IsAny())).Returns(Task.FromResult(null)); var moduleLoader = new ModuleLoader(new[] { moduleCatalogMock.Object }, Enumerable.Empty(), Enumerable.Empty()); - Assert.ThrowsAsync(async () => await moduleLoader.StartModule(new StartRequest("invalid"))); + var action = () => moduleLoader.StartModule(new StartRequest("invalid")); + await action.Should().ThrowAsync(); } [Fact] - public void WhenNoModuleRunnerAvailable_WhenStart_ThrowsException() + public async Task WhenNoModuleRunnerAvailable_WhenStart_ThrowsException() { var moduleManifestMock = new Mock(); moduleManifestMock.Setup(m => m.ModuleType).Returns("test"); @@ -46,7 +52,8 @@ public void WhenNoModuleRunnerAvailable_WhenStart_ThrowsException() testModuleRunnerMock.Setup(r => r.ModuleType).Returns("other"); var moduleLoader = new ModuleLoader(new[] { moduleCatalogMock.Object }, new[] { testModuleRunnerMock.Object }, Enumerable.Empty()); - Assert.ThrowsAsync(async () => await moduleLoader.StartModule(new StartRequest("valid"))); + var action = () => moduleLoader.StartModule(new StartRequest("valid")); + await action.Should().ThrowAsync(); } [Fact] @@ -77,13 +84,13 @@ public async Task StartModule_EndToEndTest() var serviceProvider = new ServiceCollection() .AddModuleLoader() - .AddSingleton(moduleCatalogMock.Object) + .AddSingleton>(new[] { moduleCatalogMock.Object }) .AddSingleton(startupActionMock.Object) .BuildServiceProvider(); var moduleLoader = serviceProvider.GetRequiredService(); - bool startingEventReceived = false; - bool startedEventReceived = false; + var startingEventReceived = false; + var startedEventReceived = false; moduleLoader.LifetimeEvents.Subscribe(x => { if (x.EventType == LifetimeEventType.Starting) @@ -95,20 +102,19 @@ public async Task StartModule_EndToEndTest() var startRequest = new StartRequest(moduleId); var moduleInstance = await moduleLoader.StartModule(startRequest); - Assert.NotNull(moduleInstance); - Assert.Equal(moduleManifestMock.Object, moduleInstance.Manifest); - Assert.Equal(startRequest, moduleInstance.StartRequest); + moduleInstance.Should().NotBeNull(); + moduleManifestMock.Object.Should().BeEquivalentTo(moduleInstance.Manifest); + startRequest.Should().BeEquivalentTo(moduleInstance.StartRequest); + var allProperties = moduleInstance.GetProperties(); - var webProperties = allProperties.OfType().Single(); - Assert.Equal(webManifestDetails.IconUrl, webProperties.IconUrl); - Assert.Equal(webManifestDetails.Url, webProperties.Url); + webManifestDetails.IconUrl.Should().BeEquivalentTo(webProperties.IconUrl); + webManifestDetails.Url.Should().BeEquivalentTo(webProperties.Url); var stringProperty = allProperties.OfType().Single(); - Assert.Equal(testProperty, stringProperty); - - Assert.True(startingEventReceived); - Assert.True(startedEventReceived); + testProperty.Should().BeEquivalentTo(stringProperty); + startingEventReceived.Should().BeTrue(); + startedEventReceived.Should().BeTrue(); } - } + } } \ No newline at end of file diff --git a/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Tests/Runners/NativeModuleRunnerTests.cs b/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Tests/Runners/NativeModuleRunnerTests.cs index 03e4fdbac..ae2661290 100644 --- a/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Tests/Runners/NativeModuleRunnerTests.cs +++ b/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Tests/Runners/NativeModuleRunnerTests.cs @@ -11,6 +11,8 @@ // and limitations under the License. using System.Diagnostics; +using FluentAssertions; +using FluentAssertions.Execution; using Moq; using MorganStanley.ComposeUI.ModuleLoader.Runners; using MorganStanley.ComposeUI.ModuleLoader.Tests.TestUtils; @@ -26,7 +28,7 @@ public class NativeModuleRunnerTests : IDisposable [Fact] public void ModuleType_is_Native() { - Assert.Equal(ModuleType.Native, _runner.ModuleType); + _runner.ModuleType.Should().Be(ModuleType.Native); } [Fact] @@ -53,13 +55,13 @@ public async Task ModuleIsLaunchedWithProvidedArguments() var processInfo = startupContext.GetOrAddProperty(() => { - Assert.Fail("No MainProcessInfo found"); + Execute.Assertion.FailWith("No MainProcessInfo found"); return null; }); _mainProcess = processInfo.MainProcess; var result = await _mainProcess.WaitForExitAsync(Timeout); - Assert.Equal($"Hello ComposeUI! I am {randomString}", result.Output.Trim()); + result.Output.Trim().Should().Be($"Hello ComposeUI! I am {randomString}"); } [Fact] @@ -86,13 +88,13 @@ public async Task AbsolutePathModuleIsLaunchedWithEnvironmentVariablesFromManife var processInfo = startupContext.GetOrAddProperty(() => { - Assert.Fail("No MainProcessInfo found"); + Execute.Assertion.FailWith("No MainProcessInfo found"); return null; }); _mainProcess = processInfo.MainProcess; var result = await _mainProcess.WaitForExitAsync(Timeout); - Assert.Contains($"{variableName}={randomString}", result.Output); + result.Output.Should().Contain($"{variableName}={randomString}"); } [Fact] @@ -119,13 +121,13 @@ public async Task RelativePathModuleIsLaunchedWithEnvironmentVariablesFromManife var processInfo = startupContext.GetOrAddProperty(() => { - Assert.Fail("No MainProcessInfo found"); + Execute.Assertion.FailWith("No MainProcessInfo found"); return null; }); _mainProcess = processInfo.MainProcess; var result = await _mainProcess.WaitForExitAsync(Timeout); - Assert.Contains($"{variableName}={randomString}", result.Output); + result.Output.Should().Contain($"{variableName}={randomString}"); } [Fact] @@ -156,13 +158,13 @@ await _runner.Start(startupContext, () => var processInfo = startupContext.GetOrAddProperty(() => { - Assert.Fail("No MainProcessInfo found"); + Execute.Assertion.FailWith("No MainProcessInfo found"); return null; }); _mainProcess = processInfo.MainProcess; var result = await _mainProcess.WaitForExitAsync(Timeout); - Assert.Contains($"{variableName}={randomString}", result.Output); + result.Output.Should().Contain($"{variableName}={randomString}"); } private Task RedirectMainProcessOutput(StartupContext startupContext) diff --git a/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Tests/Runners/WebModuleRunnerTests.cs b/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Tests/Runners/WebModuleRunnerTests.cs index 3935fedb8..7caa0c392 100644 --- a/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Tests/Runners/WebModuleRunnerTests.cs +++ b/src/module-loader/dotnet/tests/MorganStanley.ComposeUI.ModuleLoader.Tests/Runners/WebModuleRunnerTests.cs @@ -11,6 +11,7 @@ // and limitations under the License. +using FluentAssertions; using Moq; using MorganStanley.ComposeUI.ModuleLoader.Runners; @@ -22,7 +23,7 @@ public class WebModuleRunnerTests public void ModuleType_Is_Web() { var runner = new WebModuleRunner(); - Assert.Equal(ModuleType.Web, runner.ModuleType); + runner.ModuleType.Should().Be(ModuleType.Web); } [Fact] @@ -40,10 +41,10 @@ public async Task WhenStart_WebStartupPropertiesAreAddedToStartupContext() await runner.Start(startupContext, MockPipeline); var result = startupContext.GetProperties(); - Assert.NotNull(result); + result.Should().NotBeNull(); var webProperties = result.OfType().Single(); - Assert.Equal(details.IconUrl, webProperties.IconUrl); - Assert.Equal(details.Url, webProperties.Url); + details.IconUrl.Should().BeEquivalentTo(webProperties.IconUrl); + details.Url.Should().BeEquivalentTo(webProperties.Url); } private class MockModuleManifest : IModuleManifest From fa881f39f0368e49b3386291d125f69ffec41cda Mon Sep 17 00:00:00 2001 From: Lilla Orban Date: Fri, 12 Apr 2024 22:30:13 +0200 Subject: [PATCH 3/3] fix(ADR-014) - Add ADR about multiple ModuleCatalogs --- architecture/adr-014-module-catalog.md | 28 ++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 architecture/adr-014-module-catalog.md diff --git a/architecture/adr-014-module-catalog.md b/architecture/adr-014-module-catalog.md new file mode 100644 index 000000000..80f490bad --- /dev/null +++ b/architecture/adr-014-module-catalog.md @@ -0,0 +1,28 @@ + + +| Type | ID | Status | Title | +| ------------- | ------------- | ------------- | ----------------------- | +| ADR | adr-014 | Accepted | Multiple ModuleCatalogs | + + +# Architecture Decision Record: Multiple ModuleCatalogs + +## Context +ComposeUI aims to support many different kind of modules. These modules, that can be started by the ModuleLoader should be stored in ModuleCatalogs and every module should have its unique module id. The user should have the ability to handle multiple ModuleCatalogs with the ModuleLoader. + +General problem statement: +- When multiple ModuleCatalogs are used, some modules could have the same module id in different ModuleCatalogs which could cause issues while starting or stopping the module via the ModuleLoader. + +## Decision +- If the same module id is being used for multiple modules (likely in different ModuleCatalogs), the first occurrence will be used. In case of multiple ModuleCatalog registrations, the order of the registration determines which module can be used. + +# Alternative solutions that we considered +- Throwing an exception when multiple modules have the same moduleId either in one or multiple ModuleCatalogs. This might cause an issue that is unresolvable by the developer or the enduser. +- Prefixing the modules with the name of the ModuleCatalog, but prefixing ids would be non-standard. +- Aggregating the registered ModuleCatalogs should be implemented by the developer of the application. + +## Consequences +- Our implementation is more permissive than the standard. +- We must ensure keeping the order of the ModuleCatalog registrations as the order of registrations will determine which module is contained by our aggregated ModuleCatalog in case of duplicate moduleIds. +- We must ensure keeping the order of the modules within the invidual ModuleCatalogs as the order of the registered modules will determine which module is contained by our aggregated ModuleCatalog in case of duplicate moduleIds. +- We must ensure this order when fetching modules asynchronously from the ModuleCatalogs. \ No newline at end of file