Skip to content

Commit 2cb114f

Browse files
AndyButlandkjac
AndyButland
and
kjac
authored
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 <[email protected]>
1 parent fb2aad0 commit 2cb114f

File tree

7 files changed

+77
-20
lines changed

7 files changed

+77
-20
lines changed

‎src/Umbraco.Cms.Api.Management/Controllers/Package/AllMigrationStatusPackageController.cs

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
using Asp.Versioning;
22
using Microsoft.AspNetCore.Http;
33
using Microsoft.AspNetCore.Mvc;
4+
using Microsoft.Extensions.DependencyInjection;
45
using Umbraco.Cms.Api.Common.ViewModels.Pagination;
6+
using Umbraco.Cms.Api.Management.Factories;
57
using Umbraco.Cms.Api.Management.ViewModels.Package;
8+
using Umbraco.Cms.Core.DependencyInjection;
69
using Umbraco.Cms.Core.Mapping;
710
using Umbraco.Cms.Core.Packaging;
811
using Umbraco.Cms.Core.Services;
@@ -14,12 +17,25 @@ namespace Umbraco.Cms.Api.Management.Controllers.Package;
1417
public class AllMigrationStatusPackageController : PackageControllerBase
1518
{
1619
private readonly IPackagingService _packagingService;
17-
private readonly IUmbracoMapper _umbracoMapper;
20+
private readonly IPackagePresentationFactory _packagePresentationFactory;
1821

22+
[Obsolete("Please use the non-obsolete constructor. Scheduled for removal in V18.")]
1923
public AllMigrationStatusPackageController(IPackagingService packagingService, IUmbracoMapper umbracoMapper)
24+
: this(packagingService, StaticServiceProvider.Instance.GetRequiredService<IPackagePresentationFactory>())
25+
{
26+
}
27+
28+
[Obsolete("Please use the non-obsolete constructor. Scheduled for removal in V18.")]
29+
public AllMigrationStatusPackageController(IPackagingService packagingService, IUmbracoMapper umbracoMapper, IPackagePresentationFactory packagePresentationFactory)
30+
: this(packagingService, packagePresentationFactory)
31+
{
32+
}
33+
34+
[ActivatorUtilitiesConstructor]
35+
public AllMigrationStatusPackageController(IPackagingService packagingService, IPackagePresentationFactory packagePresentationFactory)
2036
{
2137
_packagingService = packagingService;
22-
_umbracoMapper = umbracoMapper;
38+
_packagePresentationFactory = packagePresentationFactory;
2339
}
2440

2541
/// <summary>
@@ -38,11 +54,7 @@ public async Task<ActionResult<PagedViewModel<PackageMigrationStatusResponseMode
3854
{
3955
PagedModel<InstalledPackage> migrationPlans = await _packagingService.GetInstalledPackagesFromMigrationPlansAsync(skip, take);
4056

41-
var viewModel = new PagedViewModel<PackageMigrationStatusResponseModel>
42-
{
43-
Total = migrationPlans.Total,
44-
Items = _umbracoMapper.MapEnumerable<InstalledPackage, PackageMigrationStatusResponseModel>(migrationPlans.Items)
45-
};
57+
PagedViewModel<PackageMigrationStatusResponseModel> viewModel = _packagePresentationFactory.CreatePackageMigrationStatusResponseModel(migrationPlans);
4658

4759
return Ok(viewModel);
4860
}

‎src/Umbraco.Cms.Api.Management/Factories/IPackagePresentationFactory.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1+
using Umbraco.Cms.Api.Common.ViewModels.Pagination;
12
using Umbraco.Cms.Api.Management.ViewModels.Package;
3+
using Umbraco.Cms.Core.Models;
24
using Umbraco.Cms.Core.Packaging;
35

46
namespace Umbraco.Cms.Api.Management.Factories;
@@ -8,4 +10,6 @@ public interface IPackagePresentationFactory
810
PackageDefinition CreatePackageDefinition(CreatePackageRequestModel createPackageRequestModel);
911

1012
PackageConfigurationResponseModel CreateConfigurationResponseModel();
13+
14+
PagedViewModel<PackageMigrationStatusResponseModel> CreatePackageMigrationStatusResponseModel(PagedModel<InstalledPackage> installedPackages) => new();
1115
}

‎src/Umbraco.Cms.Api.Management/Factories/PackagePresentationFactory.cs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
using System.Collections.Specialized;
22
using System.Web;
33
using Microsoft.Extensions.Options;
4+
using Umbraco.Cms.Api.Common.ViewModels.Pagination;
45
using Umbraco.Cms.Api.Management.ViewModels.Package;
56
using Umbraco.Cms.Core;
67
using Umbraco.Cms.Core.Configuration.Models;
78
using Umbraco.Cms.Core.Mapping;
9+
using Umbraco.Cms.Core.Models;
810
using Umbraco.Cms.Core.Packaging;
911
using Umbraco.Cms.Core.Services;
1012
using Umbraco.Extensions;
@@ -42,6 +44,25 @@ public PackageConfigurationResponseModel CreateConfigurationResponseModel() =>
4244
MarketplaceUrl = GetMarketplaceUrl(),
4345
};
4446

47+
public PagedViewModel<PackageMigrationStatusResponseModel> CreatePackageMigrationStatusResponseModel(PagedModel<InstalledPackage> installedPackages)
48+
{
49+
InstalledPackage[] installedPackagesAsArray = installedPackages.Items as InstalledPackage[] ?? installedPackages.Items.ToArray();
50+
51+
return new PagedViewModel<PackageMigrationStatusResponseModel>
52+
{
53+
Total = installedPackages.Total,
54+
Items = installedPackagesAsArray
55+
.GroupBy(package => package.PackageName)
56+
.Select(packages => packages.OrderByDescending(package => package.HasPendingMigrations).First())
57+
.Select(package => new PackageMigrationStatusResponseModel
58+
{
59+
PackageName = package.PackageName ?? string.Empty,
60+
HasPendingMigrations = package.HasPendingMigrations,
61+
})
62+
.ToArray(),
63+
};
64+
}
65+
4566
private string GetMarketplaceUrl()
4667
{
4768
var uriBuilder = new UriBuilder(Constants.Marketplace.Url);

‎src/Umbraco.Cms.Api.Management/Mapping/Package/PackageViewModelMapDefinition.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ private static void Map(PackageDefinition source, PackageDefinitionResponseModel
5858
}
5959

6060
// Umbraco.Code.MapAll
61+
[Obsolete("Please use the IPackagePresentationFactory instead. Scheduled for removal in V18.")]
6162
private static void Map(InstalledPackage source, PackageMigrationStatusResponseModel target, MapperContext context)
6263
{
6364
if (source.PackageName is not null)

‎src/Umbraco.Infrastructure/Install/PackageMigrationRunner.cs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,9 @@ public async Task<IEnumerable<ExecutedMigrationPlan>> RunPackageMigrationsIfPend
7777
/// </summary>
7878
public async Task<Attempt<bool, PackageMigrationOperationStatus>> RunPendingPackageMigrations(string packageName)
7979
{
80-
// Check if there are any migrations
81-
if (_packageMigrationPlans.ContainsKey(packageName) == false)
80+
// Check if there are any migrations (note that the key for _packageMigrationPlans is the migration plan name, not the package name).
81+
if (_packageMigrationPlans.Values
82+
.Any(x => x.PackageName.InvariantEquals(packageName)) is false)
8283
{
8384
return Attempt.FailWithStatus(PackageMigrationOperationStatus.NotFound, false);
8485
}
@@ -121,8 +122,8 @@ public async Task<IEnumerable<ExecutedMigrationPlan>> RunPackagePlansAsync(IEnum
121122
}
122123

123124
using (_profilingLogger.TraceDuration<PackageMigrationRunner>(
124-
"Starting unattended package migration for " + migrationName,
125-
"Unattended upgrade completed for " + migrationName))
125+
"Starting package migration for " + migrationName,
126+
"Package migration completed for " + migrationName))
126127
{
127128
Upgrader upgrader = new(plan);
128129

‎src/Umbraco.Infrastructure/Services/Implement/PackagingService.cs

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -314,8 +314,9 @@ public async Task<IEnumerable<InstalledPackage>> GetAllInstalledPackagesAsync()
314314
/// <inheritdoc/>
315315
public Task<PagedModel<InstalledPackage>> GetInstalledPackagesFromMigrationPlansAsync(int skip, int take)
316316
{
317-
IReadOnlyDictionary<string, string?>? keyValues =
318-
_keyValueService.FindByKeyPrefix(Constants.Conventions.Migrations.KeyValuePrefix);
317+
IReadOnlyDictionary<string, string?> keyValues =
318+
_keyValueService.FindByKeyPrefix(Constants.Conventions.Migrations.KeyValuePrefix)
319+
?? new Dictionary<string, string?>();
319320

320321
InstalledPackage[] installedPackages = _packageMigrationPlans
321322
.GroupBy(plan => (plan.PackageName, plan.PackageId))
@@ -326,15 +327,21 @@ public Task<PagedModel<InstalledPackage>> GetInstalledPackagesFromMigrationPlans
326327
PackageName = group.Key.PackageName,
327328
};
328329

329-
var packageKey = Constants.Conventions.Migrations.KeyValuePrefix + (group.Key.PackageId ?? group.Key.PackageName);
330-
var currentState = keyValues?
331-
.GetValueOrDefault(packageKey);
332-
333330
package.PackageMigrationPlans = group
334-
.Select(plan => new InstalledPackageMigrationPlans
331+
.Select(plan =>
335332
{
336-
CurrentMigrationId = currentState,
337-
FinalMigrationId = plan.FinalState,
333+
// look for migration states in this order:
334+
// - plan name
335+
// - package identifier
336+
// - package name
337+
var currentState =
338+
keyValues.GetValueOrDefault($"{Constants.Conventions.Migrations.KeyValuePrefix}{plan.Name}")
339+
?? keyValues.GetValueOrDefault($"{Constants.Conventions.Migrations.KeyValuePrefix}{plan.PackageId ?? plan.PackageName}");
340+
341+
return new InstalledPackageMigrationPlans
342+
{
343+
CurrentMigrationId = currentState, FinalMigrationId = plan.FinalState,
344+
};
338345
});
339346

340347
return package;

‎tests/Umbraco.Tests.Integration/CompatibilitySuppressions.xml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<!-- https://learn.microsoft.com/dotnet/fundamentals/package-validation/diagnostic-ids -->
3+
<Suppressions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
4+
<Suppression>
5+
<DiagnosticId>CP0002</DiagnosticId>
6+
<Target>M:Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services.ContentEditingServiceTests.Updating_Single_Variant_Name_Does_Not_Change_Update_Dates_Of_Other_Vaiants</Target>
7+
<Left>lib/net9.0/Umbraco.Tests.Integration.dll</Left>
8+
<Right>lib/net9.0/Umbraco.Tests.Integration.dll</Right>
9+
<IsBaselineSuppression>true</IsBaselineSuppression>
10+
</Suppression>
11+
</Suppressions>

0 commit comments

Comments
 (0)