From d6f1e7a1dab3d8e78b9da7646d96a28d53639351 Mon Sep 17 00:00:00 2001 From: Paul Johnson Date: Fri, 27 Aug 2021 10:42:36 +0100 Subject: [PATCH 1/3] Mark IUserComposer obsolete. --- src/Umbraco.Core/Composing/IUserComposer.cs | 6 ++---- src/Umbraco.TestData/LoadTestComposer.cs | 2 +- src/Umbraco.Tests.Integration/ComponentRuntimeTests.cs | 2 +- .../Umbraco.Core/Components/ComponentTests.cs | 4 ++-- 4 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/Umbraco.Core/Composing/IUserComposer.cs b/src/Umbraco.Core/Composing/IUserComposer.cs index 8e07dae672..fe5af3a985 100644 --- a/src/Umbraco.Core/Composing/IUserComposer.cs +++ b/src/Umbraco.Core/Composing/IUserComposer.cs @@ -1,11 +1,9 @@ -namespace Umbraco.Cms.Core.Composing +namespace Umbraco.Cms.Core.Composing { /// /// Represents a user . /// - /// - /// User composers compose after core composers, and before the final composer. - /// + [System.Obsolete("This interface is obsolete. Use IComposer instead.")] public interface IUserComposer : IComposer { } } diff --git a/src/Umbraco.TestData/LoadTestComposer.cs b/src/Umbraco.TestData/LoadTestComposer.cs index 8d66c4965d..3d3302b881 100644 --- a/src/Umbraco.TestData/LoadTestComposer.cs +++ b/src/Umbraco.TestData/LoadTestComposer.cs @@ -6,7 +6,7 @@ using Umbraco.TestData.Extensions; namespace Umbraco.TestData { - public class LoadTestComposer : IUserComposer + public class LoadTestComposer : IComposer { public void Compose(IUmbracoBuilder builder) => builder.AddUmbracoTestData(); } diff --git a/src/Umbraco.Tests.Integration/ComponentRuntimeTests.cs b/src/Umbraco.Tests.Integration/ComponentRuntimeTests.cs index 2331e5079f..acefa12796 100644 --- a/src/Umbraco.Tests.Integration/ComponentRuntimeTests.cs +++ b/src/Umbraco.Tests.Integration/ComponentRuntimeTests.cs @@ -46,7 +46,7 @@ namespace Umbraco.Cms.Tests.Integration Assert.IsTrue(myComponent.IsTerminated, "The component was not terminated"); } - public class MyComposer : IUserComposer + public class MyComposer : IComposer { public void Compose(IUmbracoBuilder builder) => builder.Components().Append(); } diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Core/Components/ComponentTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Components/ComponentTests.cs index 9e1669a846..69059eb0b2 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Core/Components/ComponentTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Components/ComponentTests.cs @@ -478,7 +478,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Components { } - public class Composer3 : TestComposerBase, IUserComposer + public class Composer3 : TestComposerBase, IComposer { } @@ -539,7 +539,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Components { } - public interface ITestComposer : IUserComposer + public interface ITestComposer : IComposer { } From 7216c6140fcda3f96cc7c4320310da904bbad3a2 Mon Sep 17 00:00:00 2001 From: Paul Johnson Date: Fri, 27 Aug 2021 10:56:21 +0100 Subject: [PATCH 2/3] Rename Composers.cs and perform minimal spring cleaning. --- .../{Composers.cs => ComposerGraph.cs} | 46 ++++++++----------- .../UmbracoBuilder.Composers.cs | 5 +- .../Umbraco.Core/Components/ComponentTests.cs | 46 +++++++++---------- 3 files changed, 45 insertions(+), 52 deletions(-) rename src/Umbraco.Core/Composing/{Composers.cs => ComposerGraph.cs} (93%) diff --git a/src/Umbraco.Core/Composing/Composers.cs b/src/Umbraco.Core/Composing/ComposerGraph.cs similarity index 93% rename from src/Umbraco.Core/Composing/Composers.cs rename to src/Umbraco.Core/Composing/ComposerGraph.cs index 2250d022d5..ebbc053712 100644 --- a/src/Umbraco.Core/Composing/Composers.cs +++ b/src/Umbraco.Core/Composing/ComposerGraph.cs @@ -9,28 +9,25 @@ using Umbraco.Cms.Core.DependencyInjection; namespace Umbraco.Cms.Core.Composing { - // note: this class is NOT thread-safe in any ways + // note: this class is NOT thread-safe in any way /// /// Handles the composers. /// - public class Composers + public class ComposerGraph { private readonly IUmbracoBuilder _builder; - private readonly ILogger _logger; + private readonly ILogger _logger; private readonly IEnumerable _composerTypes; private readonly IEnumerable _enableDisableAttributes; - private const int LogThresholdMilliseconds = 100; - /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class. /// /// The composition. /// The types. /// The and/or attributes. /// The logger. - /// The profiling logger. /// composition /// or /// composerTypes @@ -38,7 +35,7 @@ namespace Umbraco.Cms.Core.Composing /// enableDisableAttributes /// or /// logger - public Composers(IUmbracoBuilder builder, IEnumerable composerTypes, IEnumerable enableDisableAttributes, ILogger logger) + public ComposerGraph(IUmbracoBuilder builder, IEnumerable composerTypes, IEnumerable enableDisableAttributes, ILogger logger) { _builder = builder ?? throw new ArgumentNullException(nameof(builder)); _composerTypes = composerTypes ?? throw new ArgumentNullException(nameof(composerTypes)); @@ -48,8 +45,8 @@ namespace Umbraco.Cms.Core.Composing private class EnableInfo { - public bool Enabled; - public int Weight = -1; + public bool Enabled { get; set; } + public int Weight { get; set; } = -1; } /// @@ -60,14 +57,9 @@ namespace Umbraco.Cms.Core.Composing // make sure it is there _builder.WithCollectionBuilder(); - IEnumerable orderedComposerTypes; + IEnumerable orderedComposerTypes = PrepareComposerTypes(); - orderedComposerTypes = PrepareComposerTypes(); - - var composers = InstantiateComposers(orderedComposerTypes); - - - foreach (var composer in composers) + foreach (IComposer composer in InstantiateComposers(orderedComposerTypes)) { composer.Compose(_builder); } @@ -78,7 +70,7 @@ namespace Umbraco.Cms.Core.Composing var requirements = GetRequirements(); // only for debugging, this is verbose - //_logger.Debug(GetComposersReport(requirements)); + //_logger.Debug(GetComposersReport(requirements)); var sortedComposerTypes = SortComposers(requirements); @@ -341,17 +333,19 @@ namespace Umbraco.Cms.Core.Composing } } - private IEnumerable InstantiateComposers(IEnumerable types) + private static IEnumerable InstantiateComposers(IEnumerable types) { - IComposer InstantiateComposer(Type type) + foreach (Type type in types) { - var ctor = type.GetConstructor(Array.Empty()); - if (ctor == null) - throw new InvalidOperationException($"Composer {type.FullName} does not have a parameter-less constructor."); - return (IComposer)ctor.Invoke(Array.Empty()); - } + ConstructorInfo ctor = type.GetConstructor(Array.Empty()); - return types.Select(InstantiateComposer).ToArray(); + if (ctor == null) + { + throw new InvalidOperationException($"Composer {type.FullName} does not have a parameter-less constructor."); + } + + yield return (IComposer) ctor.Invoke(Array.Empty()); + } } } } diff --git a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Composers.cs b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Composers.cs index b6f9e7ae88..81a1bbac32 100644 --- a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Composers.cs +++ b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Composers.cs @@ -15,11 +15,10 @@ namespace Umbraco.Cms.Core.DependencyInjection /// public static IUmbracoBuilder AddComposers(this IUmbracoBuilder builder) { - // TODO: Should have a better name - IEnumerable composerTypes = builder.TypeLoader.GetTypes(); IEnumerable enableDisable = builder.TypeLoader.GetAssemblyAttributes(typeof(EnableComposerAttribute), typeof(DisableComposerAttribute)); - new Composers(builder, composerTypes, enableDisable, builder.BuilderLoggerFactory.CreateLogger()).Compose(); + + new ComposerGraph(builder, composerTypes, enableDisable, builder.BuilderLoggerFactory.CreateLogger()).Compose(); return builder; } diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Core/Components/ComponentTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Components/ComponentTests.cs index 69059eb0b2..5a02fbee2b 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Core/Components/ComponentTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Components/ComponentTests.cs @@ -94,7 +94,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Components var composition = new UmbracoBuilder(register, Mock.Of(), TestHelper.GetMockedTypeLoader()); Type[] types = TypeArray(); - var composers = new Composers(composition, types, Enumerable.Empty(), Mock.Of>()); + var composers = new ComposerGraph(composition, types, Enumerable.Empty(), Mock.Of>()); Composed.Clear(); // 2 is Core and requires 4 @@ -155,7 +155,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Components var composition = new UmbracoBuilder(register, Mock.Of(), TestHelper.GetMockedTypeLoader()); Type[] types = TypeArray(); - var composers = new Composers(composition, types, Enumerable.Empty(), Mock.Of>()); + var composers = new ComposerGraph(composition, types, Enumerable.Empty(), Mock.Of>()); Composed.Clear(); // 2 is Core and requires 4 @@ -172,7 +172,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Components var composition = new UmbracoBuilder(register, Mock.Of(), TestHelper.GetMockedTypeLoader()); Type[] types = TypeArray(); - var composers = new Composers(composition, types, Enumerable.Empty(), Mock.Of>()); + var composers = new ComposerGraph(composition, types, Enumerable.Empty(), Mock.Of>()); Composed.Clear(); // 21 is required by 20 @@ -188,7 +188,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Components var composition = new UmbracoBuilder(register, Mock.Of(), TestHelper.GetMockedTypeLoader()); Type[] types = TypeArray(); - var composers = new Composers(composition, types, Enumerable.Empty(), Mock.Of>()); + var composers = new ComposerGraph(composition, types, Enumerable.Empty(), Mock.Of>()); Composed.Clear(); // i23 requires 22 @@ -206,7 +206,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Components var composition = new UmbracoBuilder(register, Mock.Of(), TestHelper.GetMockedTypeLoader()); Type[] types = TypeArray(); - var composers = new Composers(composition, types, Enumerable.Empty(), Mock.Of>()); + var composers = new ComposerGraph(composition, types, Enumerable.Empty(), Mock.Of>()); Composed.Clear(); try { @@ -229,7 +229,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Components var composition = new UmbracoBuilder(register, Mock.Of(), TestHelper.GetMockedTypeLoader()); Type[] types = TypeArray(); - var composers = new Composers(composition, types, Enumerable.Empty(), Mock.Of>()); + var composers = new ComposerGraph(composition, types, Enumerable.Empty(), Mock.Of>()); Composed.Clear(); // 2 is Core and requires 4 @@ -295,7 +295,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Components var composition = new UmbracoBuilder(register, Mock.Of(), TestHelper.GetMockedTypeLoader()); Type[] types = new[] { typeof(Composer1), typeof(Composer5), typeof(Composer5a) }; - var composers = new Composers(composition, types, Enumerable.Empty(), Mock.Of>()); + var composers = new ComposerGraph(composition, types, Enumerable.Empty(), Mock.Of>()); Assert.IsEmpty(Composed); composers.Compose(); @@ -321,7 +321,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Components var composition = new UmbracoBuilder(register, Mock.Of(), TestHelper.GetMockedTypeLoader()); Type[] types = new[] { typeof(Composer6), typeof(Composer7), typeof(Composer8) }; - var composers = new Composers(composition, types, Enumerable.Empty(), Mock.Of>()); + var composers = new ComposerGraph(composition, types, Enumerable.Empty(), Mock.Of>()); Composed.Clear(); composers.Compose(); Assert.AreEqual(2, Composed.Count); @@ -336,7 +336,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Components var composition = new UmbracoBuilder(register, Mock.Of(), TestHelper.GetMockedTypeLoader()); Type[] types = new[] { typeof(Composer9), typeof(Composer2), typeof(Composer4) }; - var composers = new Composers(composition, types, Enumerable.Empty(), Mock.Of>()); + var composers = new ComposerGraph(composition, types, Enumerable.Empty(), Mock.Of>()); Composed.Clear(); composers.Compose(); Assert.AreEqual(3, Composed.Count); @@ -353,7 +353,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Components var composition = new UmbracoBuilder(register, Mock.Of(), TestHelper.GetMockedTypeLoader()); Type[] types = new[] { typeof(Composer9), typeof(Composer2), typeof(Composer4) }; - var composers = new Composers(composition, types, Enumerable.Empty(), Mock.Of>()); + var composers = new ComposerGraph(composition, types, Enumerable.Empty(), Mock.Of>()); Composed.Clear(); composers.Compose(); ComponentCollectionBuilder builder = composition.WithCollectionBuilder(); @@ -372,32 +372,32 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Components var composition = new UmbracoBuilder(register, Mock.Of(), TestHelper.GetMockedTypeLoader()); Type[] types = new[] { typeof(Composer10) }; - var composers = new Composers(composition, types, Enumerable.Empty(), Mock.Of>()); + var composers = new ComposerGraph(composition, types, Enumerable.Empty(), Mock.Of>()); Composed.Clear(); composers.Compose(); Assert.AreEqual(1, Composed.Count); Assert.AreEqual(typeof(Composer10), Composed[0]); types = new[] { typeof(Composer11) }; - composers = new Composers(composition, types, Enumerable.Empty(), Mock.Of>()); + composers = new ComposerGraph(composition, types, Enumerable.Empty(), Mock.Of>()); Composed.Clear(); Assert.Throws(() => composers.Compose()); Console.WriteLine("throws:"); - composers = new Composers(composition, types, Enumerable.Empty(), Mock.Of>()); + composers = new ComposerGraph(composition, types, Enumerable.Empty(), Mock.Of>()); Dictionary> requirements = composers.GetRequirements(false); - Console.WriteLine(Composers.GetComposersReport(requirements)); + Console.WriteLine(ComposerGraph.GetComposersReport(requirements)); types = new[] { typeof(Composer2) }; - composers = new Composers(composition, types, Enumerable.Empty(), Mock.Of>()); + composers = new ComposerGraph(composition, types, Enumerable.Empty(), Mock.Of>()); Composed.Clear(); Assert.Throws(() => composers.Compose()); Console.WriteLine("throws:"); - composers = new Composers(composition, types, Enumerable.Empty(), Mock.Of>()); + composers = new ComposerGraph(composition, types, Enumerable.Empty(), Mock.Of>()); requirements = composers.GetRequirements(false); - Console.WriteLine(Composers.GetComposersReport(requirements)); + Console.WriteLine(ComposerGraph.GetComposersReport(requirements)); types = new[] { typeof(Composer12) }; - composers = new Composers(composition, types, Enumerable.Empty(), Mock.Of>()); + composers = new ComposerGraph(composition, types, Enumerable.Empty(), Mock.Of>()); Composed.Clear(); composers.Compose(); Assert.AreEqual(1, Composed.Count); @@ -411,7 +411,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Components var composition = new UmbracoBuilder(register, Mock.Of(), TestHelper.GetMockedTypeLoader()); Type[] types = new[] { typeof(Composer6), typeof(Composer8) }; // 8 disables 7 which is not in the list - var composers = new Composers(composition, types, Enumerable.Empty(), Mock.Of>()); + var composers = new ComposerGraph(composition, types, Enumerable.Empty(), Mock.Of>()); Composed.Clear(); composers.Compose(); Assert.AreEqual(2, Composed.Count); @@ -427,13 +427,13 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Components Type[] types = new[] { typeof(Composer26) }; DisableComposerAttribute[] enableDisableAttributes = new[] { new DisableComposerAttribute(typeof(Composer26)) }; - var composers = new Composers(composition, types, enableDisableAttributes, Mock.Of>()); + var composers = new ComposerGraph(composition, types, enableDisableAttributes, Mock.Of>()); Composed.Clear(); composers.Compose(); Assert.AreEqual(0, Composed.Count); // 26 gone types = new[] { typeof(Composer26), typeof(Composer27) }; // 26 disabled by assembly attribute, enabled by 27 - composers = new Composers(composition, types, enableDisableAttributes, Mock.Of>()); + composers = new ComposerGraph(composition, types, enableDisableAttributes, Mock.Of>()); Composed.Clear(); composers.Compose(); Assert.AreEqual(2, Composed.Count); // both @@ -452,9 +452,9 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Components var allComposers = typeLoader.GetTypes().ToList(); var types = allComposers.Where(x => x.FullName.StartsWith("Umbraco.Core.") || x.FullName.StartsWith("Umbraco.Web")).ToList(); - var composers = new Composers(builder, types, Enumerable.Empty(), Mock.Of>()); + var composers = new ComposerGraph(builder, types, Enumerable.Empty(), Mock.Of>()); Dictionary> requirements = composers.GetRequirements(); - string report = Composers.GetComposersReport(requirements); + string report = ComposerGraph.GetComposersReport(requirements); Console.WriteLine(report); IEnumerable composerTypes = composers.SortComposers(requirements); From d6d853326d649b7f498bcbc1786e51bd5b739691 Mon Sep 17 00:00:00 2001 From: Paul Johnson Date: Mon, 6 Sep 2021 09:44:33 +0100 Subject: [PATCH 3/3] Make ComposerGraph internal --- src/Umbraco.Core/Composing/ComposerGraph.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Core/Composing/ComposerGraph.cs b/src/Umbraco.Core/Composing/ComposerGraph.cs index ebbc053712..71470d2d73 100644 --- a/src/Umbraco.Core/Composing/ComposerGraph.cs +++ b/src/Umbraco.Core/Composing/ComposerGraph.cs @@ -14,7 +14,7 @@ namespace Umbraco.Cms.Core.Composing /// /// Handles the composers. /// - public class ComposerGraph + internal class ComposerGraph { private readonly IUmbracoBuilder _builder; private readonly ILogger _logger;