From 2cb114ffafefdc7b1a082bb36a1b6a0cc2875da7 Mon Sep 17 00:00:00 2001 From: Andy Butland Date: Mon, 30 Jun 2025 12:20:11 +0200 Subject: [PATCH] Fix check for pending package migration to use the package not the plan name (#19509) * Fix check for pending package migration to use the package not plan name. * Cover all package name/identifier permutations and fix the API output for multiple plans * Adjusted log message to not refer to unattended migrations as migrations may be being run attended. --------- Co-authored-by: Kenn Jacobsen --- .../AllMigrationStatusPackageController.cs | 26 ++++++++++++++----- .../Factories/IPackagePresentationFactory.cs | 4 +++ .../Factories/PackagePresentationFactory.cs | 21 +++++++++++++++ .../Package/PackageViewModelMapDefinition.cs | 1 + .../Install/PackageMigrationRunner.cs | 9 ++++--- .../Services/Implement/PackagingService.cs | 25 +++++++++++------- .../CompatibilitySuppressions.xml | 11 ++++++++ 7 files changed, 77 insertions(+), 20 deletions(-) create mode 100644 tests/Umbraco.Tests.Integration/CompatibilitySuppressions.xml diff --git a/src/Umbraco.Cms.Api.Management/Controllers/Package/AllMigrationStatusPackageController.cs b/src/Umbraco.Cms.Api.Management/Controllers/Package/AllMigrationStatusPackageController.cs index a5fbf0e007..0bb2f830dc 100644 --- a/src/Umbraco.Cms.Api.Management/Controllers/Package/AllMigrationStatusPackageController.cs +++ b/src/Umbraco.Cms.Api.Management/Controllers/Package/AllMigrationStatusPackageController.cs @@ -1,8 +1,11 @@ using Asp.Versioning; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; +using Microsoft.Extensions.DependencyInjection; using Umbraco.Cms.Api.Common.ViewModels.Pagination; +using Umbraco.Cms.Api.Management.Factories; using Umbraco.Cms.Api.Management.ViewModels.Package; +using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Mapping; using Umbraco.Cms.Core.Packaging; using Umbraco.Cms.Core.Services; @@ -14,12 +17,25 @@ namespace Umbraco.Cms.Api.Management.Controllers.Package; public class AllMigrationStatusPackageController : PackageControllerBase { private readonly IPackagingService _packagingService; - private readonly IUmbracoMapper _umbracoMapper; + private readonly IPackagePresentationFactory _packagePresentationFactory; + [Obsolete("Please use the non-obsolete constructor. Scheduled for removal in V18.")] public AllMigrationStatusPackageController(IPackagingService packagingService, IUmbracoMapper umbracoMapper) + : this(packagingService, StaticServiceProvider.Instance.GetRequiredService()) + { + } + + [Obsolete("Please use the non-obsolete constructor. Scheduled for removal in V18.")] + public AllMigrationStatusPackageController(IPackagingService packagingService, IUmbracoMapper umbracoMapper, IPackagePresentationFactory packagePresentationFactory) + : this(packagingService, packagePresentationFactory) + { + } + + [ActivatorUtilitiesConstructor] + public AllMigrationStatusPackageController(IPackagingService packagingService, IPackagePresentationFactory packagePresentationFactory) { _packagingService = packagingService; - _umbracoMapper = umbracoMapper; + _packagePresentationFactory = packagePresentationFactory; } /// @@ -38,11 +54,7 @@ public class AllMigrationStatusPackageController : PackageControllerBase { PagedModel migrationPlans = await _packagingService.GetInstalledPackagesFromMigrationPlansAsync(skip, take); - var viewModel = new PagedViewModel - { - Total = migrationPlans.Total, - Items = _umbracoMapper.MapEnumerable(migrationPlans.Items) - }; + PagedViewModel viewModel = _packagePresentationFactory.CreatePackageMigrationStatusResponseModel(migrationPlans); return Ok(viewModel); } diff --git a/src/Umbraco.Cms.Api.Management/Factories/IPackagePresentationFactory.cs b/src/Umbraco.Cms.Api.Management/Factories/IPackagePresentationFactory.cs index d05617536a..f055416bb7 100644 --- a/src/Umbraco.Cms.Api.Management/Factories/IPackagePresentationFactory.cs +++ b/src/Umbraco.Cms.Api.Management/Factories/IPackagePresentationFactory.cs @@ -1,4 +1,6 @@ +using Umbraco.Cms.Api.Common.ViewModels.Pagination; using Umbraco.Cms.Api.Management.ViewModels.Package; +using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Packaging; namespace Umbraco.Cms.Api.Management.Factories; @@ -8,4 +10,6 @@ public interface IPackagePresentationFactory PackageDefinition CreatePackageDefinition(CreatePackageRequestModel createPackageRequestModel); PackageConfigurationResponseModel CreateConfigurationResponseModel(); + + PagedViewModel CreatePackageMigrationStatusResponseModel(PagedModel installedPackages) => new(); } diff --git a/src/Umbraco.Cms.Api.Management/Factories/PackagePresentationFactory.cs b/src/Umbraco.Cms.Api.Management/Factories/PackagePresentationFactory.cs index ded31a63f2..960b2fdbcd 100644 --- a/src/Umbraco.Cms.Api.Management/Factories/PackagePresentationFactory.cs +++ b/src/Umbraco.Cms.Api.Management/Factories/PackagePresentationFactory.cs @@ -1,10 +1,12 @@ using System.Collections.Specialized; using System.Web; using Microsoft.Extensions.Options; +using Umbraco.Cms.Api.Common.ViewModels.Pagination; using Umbraco.Cms.Api.Management.ViewModels.Package; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.Mapping; +using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Packaging; using Umbraco.Cms.Core.Services; using Umbraco.Extensions; @@ -42,6 +44,25 @@ internal class PackagePresentationFactory : IPackagePresentationFactory MarketplaceUrl = GetMarketplaceUrl(), }; + public PagedViewModel CreatePackageMigrationStatusResponseModel(PagedModel installedPackages) + { + InstalledPackage[] installedPackagesAsArray = installedPackages.Items as InstalledPackage[] ?? installedPackages.Items.ToArray(); + + return new PagedViewModel + { + Total = installedPackages.Total, + Items = installedPackagesAsArray + .GroupBy(package => package.PackageName) + .Select(packages => packages.OrderByDescending(package => package.HasPendingMigrations).First()) + .Select(package => new PackageMigrationStatusResponseModel + { + PackageName = package.PackageName ?? string.Empty, + HasPendingMigrations = package.HasPendingMigrations, + }) + .ToArray(), + }; + } + private string GetMarketplaceUrl() { var uriBuilder = new UriBuilder(Constants.Marketplace.Url); diff --git a/src/Umbraco.Cms.Api.Management/Mapping/Package/PackageViewModelMapDefinition.cs b/src/Umbraco.Cms.Api.Management/Mapping/Package/PackageViewModelMapDefinition.cs index 4ab05a910d..758580b464 100644 --- a/src/Umbraco.Cms.Api.Management/Mapping/Package/PackageViewModelMapDefinition.cs +++ b/src/Umbraco.Cms.Api.Management/Mapping/Package/PackageViewModelMapDefinition.cs @@ -58,6 +58,7 @@ public class PackageViewModelMapDefinition : IMapDefinition } // Umbraco.Code.MapAll + [Obsolete("Please use the IPackagePresentationFactory instead. Scheduled for removal in V18.")] private static void Map(InstalledPackage source, PackageMigrationStatusResponseModel target, MapperContext context) { if (source.PackageName is not null) diff --git a/src/Umbraco.Infrastructure/Install/PackageMigrationRunner.cs b/src/Umbraco.Infrastructure/Install/PackageMigrationRunner.cs index 01e2fbe595..0ea6a46a26 100644 --- a/src/Umbraco.Infrastructure/Install/PackageMigrationRunner.cs +++ b/src/Umbraco.Infrastructure/Install/PackageMigrationRunner.cs @@ -77,8 +77,9 @@ public class PackageMigrationRunner /// public async Task> RunPendingPackageMigrations(string packageName) { - // Check if there are any migrations - if (_packageMigrationPlans.ContainsKey(packageName) == false) + // Check if there are any migrations (note that the key for _packageMigrationPlans is the migration plan name, not the package name). + if (_packageMigrationPlans.Values + .Any(x => x.PackageName.InvariantEquals(packageName)) is false) { return Attempt.FailWithStatus(PackageMigrationOperationStatus.NotFound, false); } @@ -121,8 +122,8 @@ public class PackageMigrationRunner } using (_profilingLogger.TraceDuration( - "Starting unattended package migration for " + migrationName, - "Unattended upgrade completed for " + migrationName)) + "Starting package migration for " + migrationName, + "Package migration completed for " + migrationName)) { Upgrader upgrader = new(plan); diff --git a/src/Umbraco.Infrastructure/Services/Implement/PackagingService.cs b/src/Umbraco.Infrastructure/Services/Implement/PackagingService.cs index 3d46df8f01..e300ba6922 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/PackagingService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/PackagingService.cs @@ -314,8 +314,9 @@ public class PackagingService : IPackagingService /// public Task> GetInstalledPackagesFromMigrationPlansAsync(int skip, int take) { - IReadOnlyDictionary? keyValues = - _keyValueService.FindByKeyPrefix(Constants.Conventions.Migrations.KeyValuePrefix); + IReadOnlyDictionary keyValues = + _keyValueService.FindByKeyPrefix(Constants.Conventions.Migrations.KeyValuePrefix) + ?? new Dictionary(); InstalledPackage[] installedPackages = _packageMigrationPlans .GroupBy(plan => (plan.PackageName, plan.PackageId)) @@ -326,15 +327,21 @@ public class PackagingService : IPackagingService PackageName = group.Key.PackageName, }; - var packageKey = Constants.Conventions.Migrations.KeyValuePrefix + (group.Key.PackageId ?? group.Key.PackageName); - var currentState = keyValues? - .GetValueOrDefault(packageKey); - package.PackageMigrationPlans = group - .Select(plan => new InstalledPackageMigrationPlans + .Select(plan => { - CurrentMigrationId = currentState, - FinalMigrationId = plan.FinalState, + // look for migration states in this order: + // - plan name + // - package identifier + // - package name + var currentState = + keyValues.GetValueOrDefault($"{Constants.Conventions.Migrations.KeyValuePrefix}{plan.Name}") + ?? keyValues.GetValueOrDefault($"{Constants.Conventions.Migrations.KeyValuePrefix}{plan.PackageId ?? plan.PackageName}"); + + return new InstalledPackageMigrationPlans + { + CurrentMigrationId = currentState, FinalMigrationId = plan.FinalState, + }; }); return package; diff --git a/tests/Umbraco.Tests.Integration/CompatibilitySuppressions.xml b/tests/Umbraco.Tests.Integration/CompatibilitySuppressions.xml new file mode 100644 index 0000000000..f5c79dd905 --- /dev/null +++ b/tests/Umbraco.Tests.Integration/CompatibilitySuppressions.xml @@ -0,0 +1,11 @@ + + + + + CP0002 + M:Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services.ContentEditingServiceTests.Updating_Single_Variant_Name_Does_Not_Change_Update_Dates_Of_Other_Vaiants + lib/net9.0/Umbraco.Tests.Integration.dll + lib/net9.0/Umbraco.Tests.Integration.dll + true + + \ No newline at end of file