From 0f95dbbfa664dd996f5276eddd44ef1850508c26 Mon Sep 17 00:00:00 2001 From: kjac Date: Tue, 28 Feb 2023 08:36:48 +0100 Subject: [PATCH] Review changes and fixes --- .../Models/PackageManifestSettings.cs | 10 ++++++++++ src/Umbraco.Core/Constants-Configuration.cs | 1 + .../UmbracoBuilder.Configuration.cs | 3 ++- .../UmbracoBuilder.CoreServices.cs | 2 +- .../Manifest/PackageManifestReader.cs | 7 ++++--- .../Manifest/PackageManifestService.cs | 15 +++++++++++---- .../Manifest/PackageManifestReaderTests.cs | 18 ++++++------------ .../Manifest/PackageManifestServiceTests.cs | 6 ++++-- 8 files changed, 39 insertions(+), 23 deletions(-) create mode 100644 src/Umbraco.Core/Configuration/Models/PackageManifestSettings.cs diff --git a/src/Umbraco.Core/Configuration/Models/PackageManifestSettings.cs b/src/Umbraco.Core/Configuration/Models/PackageManifestSettings.cs new file mode 100644 index 0000000000..3b712d8eb2 --- /dev/null +++ b/src/Umbraco.Core/Configuration/Models/PackageManifestSettings.cs @@ -0,0 +1,10 @@ +namespace Umbraco.Cms.Core.Configuration.Models; + +/// +/// Typed configuration options for package manifest settings. +/// +[UmbracoOptions(Constants.Configuration.ConfigGlobal)] +public class PackageManifestSettings +{ + public TimeSpan CacheTimeout { get; set; } = TimeSpan.FromMinutes(10); +} diff --git a/src/Umbraco.Core/Constants-Configuration.cs b/src/Umbraco.Core/Constants-Configuration.cs index 656bcd1cdf..782ead11b9 100644 --- a/src/Umbraco.Core/Constants-Configuration.cs +++ b/src/Umbraco.Core/Constants-Configuration.cs @@ -62,6 +62,7 @@ public static partial class Constants public const string ConfigHelpPage = ConfigPrefix + "HelpPage"; public const string ConfigInstallDefaultData = ConfigPrefix + "InstallDefaultData"; public const string ConfigDataTypes = ConfigPrefix + "DataTypes"; + public const string ConfigPackageManifests = ConfigPrefix + "PackageManifests"; public static class NamedOptions { diff --git a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs index 92e10c7b1c..a35b38a509 100644 --- a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs +++ b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs @@ -85,7 +85,8 @@ public static partial class UmbracoBuilderExtensions .AddUmbracoOptions() .AddUmbracoOptions() .AddUmbracoOptions() - .AddUmbracoOptions(); + .AddUmbracoOptions() + .AddUmbracoOptions(); // Configure connection string and ensure it's updated when the configuration changes builder.Services.AddSingleton, ConfigureConnectionStrings>(); diff --git a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs index 93d9c52143..73f0b0da49 100644 --- a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs +++ b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs @@ -128,7 +128,7 @@ public static partial class UmbracoBuilderExtensions // register manifest parser, will be injected in collection builders where needed builder.Services.AddSingleton(); - builder.Services.AddSingleton(); + builder.Services.AddSingleton(); builder.Services.AddSingleton(); // register the manifest filter collection builder (collection is empty by default) diff --git a/src/Umbraco.Infrastructure/Manifest/PackageManifestReader.cs b/src/Umbraco.Infrastructure/Manifest/PackageManifestReader.cs index b4de5c824c..c87f1ebecf 100644 --- a/src/Umbraco.Infrastructure/Manifest/PackageManifestReader.cs +++ b/src/Umbraco.Infrastructure/Manifest/PackageManifestReader.cs @@ -14,12 +14,12 @@ internal sealed class AppPluginsFileProviderPackageManifestReader : IPackageMani { private readonly IPackageManifestFileProviderFactory _packageManifestFileProviderFactory; private readonly IJsonSerializer _jsonSerializer; - private readonly ILogger _logger; + private readonly ILogger _logger; - public PackageManifestReader( + public AppPluginsFileProviderPackageManifestReader( IPackageManifestFileProviderFactory packageManifestFileProviderFactory, IJsonSerializer jsonSerializer, - ILogger logger) + ILogger logger) { _packageManifestFileProviderFactory = packageManifestFileProviderFactory; _jsonSerializer = jsonSerializer; @@ -90,6 +90,7 @@ internal sealed class AppPluginsFileProviderPackageManifestReader : IPackageMani catch (Exception ex) { _logger.LogError(ex, "Unable to load package manifest file: {FileName}", fileInfo.Name); + throw; } } diff --git a/src/Umbraco.Infrastructure/Manifest/PackageManifestService.cs b/src/Umbraco.Infrastructure/Manifest/PackageManifestService.cs index 180e8a0f10..70c691abe9 100644 --- a/src/Umbraco.Infrastructure/Manifest/PackageManifestService.cs +++ b/src/Umbraco.Infrastructure/Manifest/PackageManifestService.cs @@ -1,4 +1,6 @@ -using Umbraco.Cms.Core.Cache; +using Microsoft.Extensions.Options; +using Umbraco.Cms.Core.Cache; +using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.Manifest; using Umbraco.Extensions; @@ -8,10 +10,15 @@ internal sealed class PackageManifestService : IPackageManifestService { private readonly IEnumerable _packageManifestReaders; private readonly IAppPolicyCache _cache; + private readonly PackageManifestSettings _packageManifestSettings; - public PackageManifestService(IEnumerable packageManifestReaders, AppCaches appCaches) + public PackageManifestService( + IEnumerable packageManifestReaders, + AppCaches appCaches, + IOptions packageManifestSettings) { _packageManifestReaders = packageManifestReaders; + _packageManifestSettings = packageManifestSettings.Value; _cache = appCaches.RuntimeCache; } @@ -20,13 +27,13 @@ internal sealed class PackageManifestService : IPackageManifestService $"{nameof(PackageManifestService)}-PackageManifests", async () => { - var tasks = _packageManifestReaders + Task>[] tasks = _packageManifestReaders .Select(x => x.ReadPackageManifestsAsync()) .ToArray(); await Task.WhenAll(tasks); return tasks.SelectMany(x => x.Result); }, - TimeSpan.FromMinutes(10)) + _packageManifestSettings.CacheTimeout) ?? Array.Empty(); } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Manifest/PackageManifestReaderTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Manifest/PackageManifestReaderTests.cs index d7d7fb405a..a4b2586e82 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Manifest/PackageManifestReaderTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Manifest/PackageManifestReaderTests.cs @@ -16,7 +16,7 @@ public class PackageManifestReaderTests { private IPackageManifestReader _reader; private Mock _rootDirectoryContentsMock; - private Mock> _loggerMock; + private Mock> _loggerMock; private Mock _fileProviderMock; [SetUp] @@ -30,8 +30,8 @@ public class PackageManifestReaderTests var fileProviderFactoryMock = new Mock(); fileProviderFactoryMock.Setup(m => m.Create()).Returns(_fileProviderMock.Object); - _loggerMock = new Mock>(); - _reader = new PackageManifestReader(fileProviderFactoryMock.Object, new SystemTextJsonSerializer(), _loggerMock.Object); + _loggerMock = new Mock>(); + _reader = new AppPluginsFileProviderPackageManifestReader(fileProviderFactoryMock.Object, new SystemTextJsonSerializer(), _loggerMock.Object); } [Test] @@ -147,9 +147,7 @@ public class PackageManifestReaderTests .Setup(f => f.GetEnumerator()) .Returns(new List { CreatePackageManifestFile(content) }.GetEnumerator()); - var result = await _reader.ReadPackageManifestsAsync(); - Assert.AreEqual(0, result.Count()); - + Assert.ThrowsAsync(() => _reader.ReadPackageManifestsAsync()); EnsureLogErrorWasCalled(); } @@ -165,9 +163,7 @@ public class PackageManifestReaderTests .Setup(f => f.GetEnumerator()) .Returns(new List { CreatePackageManifestFile(content) }.GetEnumerator()); - var result = await _reader.ReadPackageManifestsAsync(); - Assert.AreEqual(0, result.Count()); - + Assert.ThrowsAsync(() => _reader.ReadPackageManifestsAsync()); EnsureLogErrorWasCalled(); } @@ -179,9 +175,7 @@ public class PackageManifestReaderTests .Setup(f => f.GetEnumerator()) .Returns(new List { CreatePackageManifestFile(content) }.GetEnumerator()); - var result = await _reader.ReadPackageManifestsAsync(); - Assert.AreEqual(0, result.Count()); - + Assert.ThrowsAsync(() => _reader.ReadPackageManifestsAsync()); EnsureLogErrorWasCalled(); } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Manifest/PackageManifestServiceTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Manifest/PackageManifestServiceTests.cs index cca026eb20..419e1e10dd 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Manifest/PackageManifestServiceTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Manifest/PackageManifestServiceTests.cs @@ -1,6 +1,8 @@ -using Moq; +using Microsoft.Extensions.Options; +using Moq; using NUnit.Framework; using Umbraco.Cms.Core.Cache; +using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.Manifest; using Umbraco.Cms.Infrastructure.Manifest; @@ -29,7 +31,7 @@ public class PackageManifestServiceTests NoAppCache.Instance, new IsolatedCaches(type => NoAppCache.Instance)); - _service = new PackageManifestService(_readerMock.Object, appCaches); + _service = new PackageManifestService(new[] { _readerMock.Object }, appCaches, new OptionsWrapper(new PackageManifestSettings())); } [Test]