Review changes and fixes

src/Umbraco.Core/Configuration/Models/PackageManifestSettings.cs

@@ -0,0 +1,10 @@
+namespace Umbraco.Cms.Core.Configuration.Models;
+
+/// <summary>
+///     Typed configuration options for package manifest settings.
+/// </summary>
+[UmbracoOptions(Constants.Configuration.ConfigGlobal)]
+public class PackageManifestSettings
+{
+    public TimeSpan CacheTimeout { get; set; } = TimeSpan.FromMinutes(10);
+}

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
         {

src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs

@@ -85,7 +85,8 @@ public static partial class UmbracoBuilderExtensions
             .AddUmbracoOptions<PackageMigrationSettings>()
             .AddUmbracoOptions<ContentDashboardSettings>()
             .AddUmbracoOptions<HelpPageSettings>()
-            .AddUmbracoOptions<DataTypesSettings>();
+            .AddUmbracoOptions<DataTypesSettings>()
+            .AddUmbracoOptions<PackageManifestSettings>();
 
         // Configure connection string and ensure it's updated when the configuration changes
         builder.Services.AddSingleton<IConfigureOptions<ConnectionStrings>, ConfigureConnectionStrings>();

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<ILegacyManifestParser, LegacyManifestParser>();
-        builder.Services.AddSingleton<IPackageManifestReader, PackageManifestReader>();
+        builder.Services.AddSingleton<IPackageManifestReader, AppPluginsFileProviderPackageManifestReader>();
         builder.Services.AddSingleton<IPackageManifestService, PackageManifestService>();
 
         // register the manifest filter collection builder (collection is empty by default)

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<PackageManifestReader> _logger;
+    private readonly ILogger<AppPluginsFileProviderPackageManifestReader> _logger;
 
-    public PackageManifestReader(
+    public AppPluginsFileProviderPackageManifestReader(
         IPackageManifestFileProviderFactory packageManifestFileProviderFactory,
         IJsonSerializer jsonSerializer,
-        ILogger<PackageManifestReader> logger)
+        ILogger<AppPluginsFileProviderPackageManifestReader> 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;
             }
         }
 

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<IPackageManifestReader> _packageManifestReaders;
     private readonly IAppPolicyCache _cache;
+    private readonly PackageManifestSettings _packageManifestSettings;
 
-    public PackageManifestService(IEnumerable<IPackageManifestReader> packageManifestReaders, AppCaches appCaches)
+    public PackageManifestService(
+        IEnumerable<IPackageManifestReader> packageManifestReaders,
+        AppCaches appCaches,
+        IOptions<PackageManifestSettings> 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<IEnumerable<PackageManifest>>[] tasks = _packageManifestReaders
                        .Select(x => x.ReadPackageManifestsAsync())
                        .ToArray();
                    await Task.WhenAll(tasks);
 
                    return tasks.SelectMany(x => x.Result);
                },
-               TimeSpan.FromMinutes(10))
+               _packageManifestSettings.CacheTimeout)
            ?? Array.Empty<PackageManifest>();
 }

tests/Umbraco.Tests.UnitTests/Umbraco.Core/Manifest/PackageManifestReaderTests.cs

@@ -16,7 +16,7 @@ public class PackageManifestReaderTests
 {
     private IPackageManifestReader _reader;
     private Mock<IDirectoryContents> _rootDirectoryContentsMock;
-    private Mock<ILogger<PackageManifestReader>> _loggerMock;
+    private Mock<ILogger<AppPluginsFileProviderPackageManifestReader>> _loggerMock;
     private Mock<IFileProvider> _fileProviderMock;
 
     [SetUp]
@@ -30,8 +30,8 @@ public class PackageManifestReaderTests
         var fileProviderFactoryMock = new Mock<IPackageManifestFileProviderFactory>();
         fileProviderFactoryMock.Setup(m => m.Create()).Returns(_fileProviderMock.Object);
 
-        _loggerMock = new Mock<ILogger<PackageManifestReader>>();
-        _reader = new PackageManifestReader(fileProviderFactoryMock.Object, new SystemTextJsonSerializer(), _loggerMock.Object);
+        _loggerMock = new Mock<ILogger<AppPluginsFileProviderPackageManifestReader>>();
+        _reader = new AppPluginsFileProviderPackageManifestReader(fileProviderFactoryMock.Object, new SystemTextJsonSerializer(), _loggerMock.Object);
     }
 
     [Test]
@@ -147,9 +147,7 @@ public class PackageManifestReaderTests
             .Setup(f => f.GetEnumerator())
             .Returns(new List<IFileInfo> { CreatePackageManifestFile(content) }.GetEnumerator());
 
-        var result = await _reader.ReadPackageManifestsAsync();
-        Assert.AreEqual(0, result.Count());
-
+        Assert.ThrowsAsync<JsonException>(() => _reader.ReadPackageManifestsAsync());
         EnsureLogErrorWasCalled();
     }
 
@@ -165,9 +163,7 @@ public class PackageManifestReaderTests
             .Setup(f => f.GetEnumerator())
             .Returns(new List<IFileInfo> { CreatePackageManifestFile(content) }.GetEnumerator());
 
-        var result = await _reader.ReadPackageManifestsAsync();
-        Assert.AreEqual(0, result.Count());
-
+        Assert.ThrowsAsync<JsonException>(() => _reader.ReadPackageManifestsAsync());
         EnsureLogErrorWasCalled();
     }
 
@@ -179,9 +175,7 @@ public class PackageManifestReaderTests
             .Setup(f => f.GetEnumerator())
             .Returns(new List<IFileInfo> { CreatePackageManifestFile(content) }.GetEnumerator());
 
-        var result = await _reader.ReadPackageManifestsAsync();
-        Assert.AreEqual(0, result.Count());
-
+        Assert.ThrowsAsync<JsonException>(() => _reader.ReadPackageManifestsAsync());
         EnsureLogErrorWasCalled();
     }
 

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<PackageManifestSettings>(new PackageManifestSettings()));
     }
 
     [Test]