Add PackageId to package manifest and PackageMigrationPlan (#14047)

* Add VersionAssemblyName to package manifest

* Fix/improve nullability

* Ensure package version from manifest is set when package migration exists

* Set versionAssemblyName in umbracopackage template

* Use Assembly.Load instead of ITypeFinder

* Use AssemblyLoadContext to get asesmbly by name

* Add PackageId to package manifest

* Show ID on installed packages overview

* Fallback to package ID to get assembly version

* Include package ID in telemetry data

* Set id in umbracopackage template

* Add PackageId to PackageMigrationPlan

* Get version from package migration assembly

* Hide unknown package version

* Always use package name from manifest

* Use IPackagingService to return package telemety data

* Set versionAssemblyName in umbracopackage-rcl template

* Set id in umbracopackage-rcl template
This commit is contained in:
Ronald Barendse
2023-05-31 11:31:37 +02:00
committed by GitHub
parent 3b433e6658
commit 41e51d6e71
13 changed files with 229 additions and 87 deletions

View File

@@ -13,7 +13,16 @@ public class PackageManifest
private string? _packageName;
/// <summary>
/// Gets or sets the name of the package. If not specified, uses the directory name instead.
/// Gets or sets the package identifier.
/// </summary>
/// <value>
/// The package identifier.
/// </value>
[DataMember(Name = "id")]
public string? PackageId { get; set; }
/// <summary>
/// Gets or sets the name of the package. If not specified, uses the package identifier or directory name instead.
/// </summary>
/// <value>
/// The name of the package.
@@ -27,6 +36,11 @@ public class PackageManifest
{
return _packageName;
}
if (!PackageId.IsNullOrWhiteSpace())
{
_packageName = PackageId;
}
if (!Source.IsNullOrWhiteSpace())
{

View File

@@ -6,30 +6,40 @@ namespace Umbraco.Cms.Core.Packaging;
[DataContract(Name = "installedPackage")]
public class InstalledPackage
{
[DataMember(Name = "id")]
public string? PackageId { get; set; }
[DataMember(Name = "name", IsRequired = true)]
[Required]
public string? PackageName { get; set; }
// TODO: Version? Icon? Other metadata? This would need to come from querying the package on Our
[DataMember(Name = "packageView")]
public string? PackageView { get; set; }
[DataMember(Name = "version")]
public string? Version { get; set; }
[DataMember(Name = "allowPackageTelemetry")]
public bool AllowPackageTelemetry { get; set; } = true;
[DataMember(Name = "plans")]
public IEnumerable<InstalledPackageMigrationPlans> PackageMigrationPlans { get; set; } =
Enumerable.Empty<InstalledPackageMigrationPlans>();
public IEnumerable<InstalledPackageMigrationPlans> PackageMigrationPlans { get; set; } = Enumerable.Empty<InstalledPackageMigrationPlans>();
/// <summary>
/// It the package contains any migrations at all
/// Gets a value indicating whether this package has migrations.
/// </summary>
/// <value>
/// <c>true</c> if this package has migrations; otherwise, <c>false</c>.
/// </value>
[DataMember(Name = "hasMigrations")]
public bool HasMigrations => PackageMigrationPlans.Any();
/// <summary>
/// If the package has any pending migrations to run
/// Gets a value indicating whether this package has pending migrations.
/// </summary>
/// <value>
/// <c>true</c> if this package has pending migrations; otherwise, <c>false</c>.
/// </value>
[DataMember(Name = "hasPendingMigrations")]
public bool HasPendingMigrations => PackageMigrationPlans.Any(x => x.HasPendingMigrations);
}

View File

@@ -3,23 +3,35 @@ using System.Runtime.Serialization;
namespace Umbraco.Cms.Core.Telemetry.Models;
/// <summary>
/// Serializable class containing information about an installed package.
/// Serializable class containing information about an installed package.
/// </summary>
[DataContract(Name = "packageTelemetry")]
public class PackageTelemetry
{
/// <summary>
/// Gets or sets the name of the installed package.
/// Gets or sets the identifier of the installed package.
/// </summary>
/// <value>
/// The identifier.
/// </value>
[DataMember(Name = "id")]
public string? Id { get; set; }
/// <summary>
/// Gets or sets the name of the installed package.
/// </summary>
/// <value>
/// The name.
/// </value>
[DataMember(Name = "name")]
public string? Name { get; set; }
/// <summary>
/// Gets or sets the version of the installed package.
/// Gets or sets the version of the installed package.
/// </summary>
/// <remarks>
/// This may be an empty string if no version is specified, or if package telemetry has been restricted.
/// </remarks>
/// <value>
/// The version.
/// </value>
[DataMember(Name = "version")]
public string? Version { get; set; }
}

View File

@@ -2,8 +2,8 @@
// See LICENSE for more details.
using Umbraco.Cms.Core.Configuration;
using Umbraco.Cms.Core.Manifest;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Packaging;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Telemetry.Models;
using Umbraco.Extensions;
@@ -13,7 +13,7 @@ namespace Umbraco.Cms.Core.Telemetry;
/// <inheritdoc />
internal class TelemetryService : ITelemetryService
{
private readonly IManifestParser _manifestParser;
private readonly IPackagingService _packagingService;
private readonly IMetricsConsentService _metricsConsentService;
private readonly ISiteIdentifierService _siteIdentifierService;
private readonly IUmbracoVersion _umbracoVersion;
@@ -23,13 +23,13 @@ internal class TelemetryService : ITelemetryService
/// Initializes a new instance of the <see cref="TelemetryService" /> class.
/// </summary>
public TelemetryService(
IManifestParser manifestParser,
IPackagingService packagingService,
IUmbracoVersion umbracoVersion,
ISiteIdentifierService siteIdentifierService,
IUsageInformationService usageInformationService,
IMetricsConsentService metricsConsentService)
{
_manifestParser = manifestParser;
_packagingService = packagingService;
_umbracoVersion = umbracoVersion;
_siteIdentifierService = siteIdentifierService;
_usageInformationService = usageInformationService;
@@ -73,19 +73,20 @@ internal class TelemetryService : ITelemetryService
}
List<PackageTelemetry> packages = new();
IEnumerable<PackageManifest> manifests = _manifestParser.GetManifests();
IEnumerable<InstalledPackage> installedPackages = _packagingService.GetAllInstalledPackages();
foreach (PackageManifest manifest in manifests)
foreach (InstalledPackage installedPackage in installedPackages)
{
if (manifest.AllowPackageTelemetry is false)
if (installedPackage.AllowPackageTelemetry is false)
{
continue;
}
packages.Add(new PackageTelemetry
{
Name = manifest.PackageName,
Version = manifest.Version ?? string.Empty,
Id = installedPackage.PackageId,
Name = installedPackage.PackageName,
Version = installedPackage.Version,
});
}

View File

@@ -179,11 +179,20 @@ public class ManifestParser : IManifestParser
new ValueValidatorConverter(_validators),
new DashboardAccessRuleConverter())!;
if (string.IsNullOrEmpty(manifest.Version) &&
!string.IsNullOrEmpty(manifest.VersionAssemblyName) &&
TryGetAssemblyInformationalVersion(manifest.VersionAssemblyName, out string? version))
if (string.IsNullOrEmpty(manifest.Version))
{
manifest.Version = version;
string? assemblyName = manifest.VersionAssemblyName;
if (string.IsNullOrEmpty(assemblyName))
{
// Fallback to package ID
assemblyName = manifest.PackageId;
}
if (!string.IsNullOrEmpty(assemblyName) &&
TryGetAssemblyInformationalVersion(assemblyName, out string? version))
{
manifest.Version = version;
}
}
// scripts and stylesheets are raw string, must process here

View File

@@ -11,20 +11,41 @@ using Umbraco.Extensions;
namespace Umbraco.Cms.Infrastructure.Packaging;
/// <summary>
/// Used to automatically indicate that a package has an embedded package data manifest that needs to be installed
/// Represents a package migration plan that automatically imports an embedded package data manifest.
/// </summary>
public abstract class AutomaticPackageMigrationPlan : PackageMigrationPlan
{
/// <summary>
/// Initializes a new instance of the <see cref="AutomaticPackageMigrationPlan" /> class.
/// </summary>
/// <param name="packageName">The package name that the plan is for. If the package has a package.manifest these must match.</param>
protected AutomaticPackageMigrationPlan(string packageName)
: this(packageName, packageName)
{
}
/// <summary>
/// Initializes a new instance of the <see cref="AutomaticPackageMigrationPlan" /> class.
/// </summary>
/// <param name="packageName">The package name that the plan is for. If the package has a package.manifest these must match.</param>
/// <param name="planName">The plan name for the package. This should be the same name as the package name, if there is only one plan in the package.</param>
protected AutomaticPackageMigrationPlan(string packageName, string planName)
: base(packageName, planName)
: this(null!, packageName, planName)
{
}
/// <summary>
/// Initializes a new instance of the <see cref="AutomaticPackageMigrationPlan" /> class.
/// </summary>
/// <param name="packageId">The package identifier that the plan is for. If the package has a package.manifest these must match.</param>
/// <param name="packageName">The package name that the plan is for. If the package has a package.manifest these must match.</param>
/// <param name="planName">The plan name for the package. This should be the same name as the package name, if there is only one plan in the package.</param>
protected AutomaticPackageMigrationPlan(string packageId, string packageName, string planName)
: base(packageId, packageName, planName)
{
}
/// <inheritdoc />
protected sealed override void DefinePlan()
{
// calculate the final state based on the hash value of the embedded resource
@@ -35,8 +56,22 @@ public abstract class AutomaticPackageMigrationPlan : PackageMigrationPlan
To<MigrateToPackageData>(finalId);
}
/// <summary>
/// Provides a migration that imports an embedded package data manifest.
/// </summary>
private class MigrateToPackageData : PackageMigrationBase
{
/// <summary>
/// Initializes a new instance of the <see cref="MigrateToPackageData" /> class.
/// </summary>
/// <param name="packagingService">The packaging service.</param>
/// <param name="mediaService">The media service.</param>
/// <param name="mediaFileManager">The media file manager.</param>
/// <param name="mediaUrlGenerators">The media URL generators.</param>
/// <param name="shortStringHelper">The short string helper.</param>
/// <param name="contentTypeBaseServiceProvider">The content type base service provider.</param>
/// <param name="context">The migration context.</param>
/// <param name="options">The package migration settings.</param>
public MigrateToPackageData(
IPackagingService packagingService,
IMediaService mediaService,
@@ -44,11 +79,13 @@ public abstract class AutomaticPackageMigrationPlan : PackageMigrationPlan
MediaUrlGeneratorCollection mediaUrlGenerators,
IShortStringHelper shortStringHelper,
IContentTypeBaseServiceProvider contentTypeBaseServiceProvider,
IMigrationContext context, IOptions<PackageMigrationSettings> options)
IMigrationContext context,
IOptions<PackageMigrationSettings> options)
: base(packagingService, mediaService, mediaFileManager, mediaUrlGenerators, shortStringHelper, contentTypeBaseServiceProvider, context, options)
{
}
/// <inheritdoc />
protected override void Migrate()
{
var plan = (AutomaticPackageMigrationPlan)Context.Plan;

View File

@@ -4,50 +4,70 @@ using Umbraco.Cms.Infrastructure.Migrations;
namespace Umbraco.Cms.Core.Packaging;
/// <summary>
/// Base class for package migration plans
/// Represents a package migration plan.
/// </summary>
public abstract class PackageMigrationPlan : MigrationPlan, IDiscoverable
{
/// <summary>
/// Creates a package migration plan
/// Initializes a new instance of the <see cref="PackageMigrationPlan" /> class.
/// </summary>
/// <param name="packageName">The name of the package. If the package has a package.manifest these must match.</param>
/// <param name="packageName">The package name that the plan is for. If the package has a package.manifest these must match.</param>
protected PackageMigrationPlan(string packageName)
: this(packageName, packageName)
{
}
/// <summary>
/// Create a plan for a Package Name
/// Initializes a new instance of the <see cref="PackageMigrationPlan" /> class.
/// </summary>
/// <param name="packageName">
/// The package name that the plan is for. If the package has a package.manifest these must
/// match.
/// </param>
/// <param name="planName">
/// The plan name for the package. This should be the same name as the
/// package name if there is only one plan in the package.
/// </param>
/// <param name="packageName">The package name that the plan is for. If the package has a package.manifest these must match.</param>
/// <param name="planName">The plan name for the package. This should be the same name as the package name, if there is only one plan in the package.</param>
protected PackageMigrationPlan(string packageName, string planName)
: base(planName)
: this(null!, packageName, planName)
{
// A call to From must be done first
From(string.Empty);
DefinePlan();
PackageName = packageName;
}
/// <summary>
/// Inform the plan executor to ignore all saved package state and
/// run the migration from initial state to it's end state.
/// Initializes a new instance of the <see cref="PackageMigrationPlan" /> class.
/// </summary>
/// <param name="packageId">The package identifier that the plan is for. If the package has a package.manifest these must match.</param>
/// <param name="packageName">The package name that the plan is for. If the package has a package.manifest these must match.</param>
/// <param name="planName">The plan name for the package. This should be the same name as the package name, if there is only one plan in the package.</param>
protected PackageMigrationPlan(string packageId, string packageName, string planName)
: base(planName)
{
PackageId = packageId;
PackageName = packageName;
// A call to From must be done first
From(string.Empty);
DefinePlan();
}
/// <summary>
/// Inform the plan executor to ignore all saved package state and
/// run the migration from initial state to it's end state.
/// </summary>
public override bool IgnoreCurrentState => true;
/// <summary>
/// Returns the Package Name for this plan
/// Gets the package identifier.
/// </summary>
public string PackageName { get; }
/// <value>
/// The package identifier.
/// </value>
public string? PackageId { get; init; }
/// <summary>
/// Gets the package name.
/// </summary>
/// <value>
/// The package name.
/// </value>
public string PackageName { get; init; }
/// <summary>
/// Defines the plan.
/// </summary>
protected abstract void DefinePlan();
}

View File

@@ -118,26 +118,41 @@ public class PackagingService : IPackagingService
{
IReadOnlyDictionary<string, string?>? keyValues = _keyValueService.FindByKeyPrefix(Constants.Conventions.Migrations.KeyValuePrefix);
var installedPackages = new Dictionary<string, InstalledPackage>();
var installedPackages = new List<InstalledPackage>();
// Collect the package from the package migration plans
foreach (PackageMigrationPlan plan in _packageMigrationPlans)
{
if (!installedPackages.TryGetValue(plan.PackageName, out InstalledPackage? installedPackage))
InstalledPackage installedPackage;
if (plan.PackageId is not null && installedPackages.FirstOrDefault(x => x.PackageId == plan.PackageId) is InstalledPackage installedPackageById)
{
installedPackage = installedPackageById;
}
else if (installedPackages.FirstOrDefault(x => x.PackageName == plan.PackageName) is InstalledPackage installedPackageByName)
{
installedPackage = installedPackageByName;
// Ensure package ID is set
installedPackage.PackageId ??= plan.PackageId;
}
else
{
installedPackage = new InstalledPackage
{
PackageId = plan.PackageId,
PackageName = plan.PackageName,
};
if (plan.GetType().Assembly.TryGetInformationalVersion(out string? version))
{
installedPackage.Version = version;
}
installedPackages.Add(plan.PackageName, installedPackage);
installedPackages.Add(installedPackage);
}
if (installedPackage.Version is null &&
plan.GetType().Assembly.TryGetInformationalVersion(out string? version))
{
installedPackage.Version = version;
}
// Combine all package migration plans for a package
var currentPlans = installedPackage.PackageMigrationPlans.ToList();
if (keyValues is null || keyValues.TryGetValue(Constants.Conventions.Migrations.KeyValuePrefix + plan.Name, out var currentState) is false)
{
@@ -156,32 +171,49 @@ public class PackagingService : IPackagingService
// Collect and merge the packages from the manifests
foreach (PackageManifest package in _manifestParser.GetManifests())
{
if (package.PackageName is null)
if (package.PackageId is null && package.PackageName is null)
{
continue;
}
if (!installedPackages.TryGetValue(package.PackageName, out InstalledPackage? installedPackage))
InstalledPackage installedPackage;
if (package.PackageId is not null && installedPackages.FirstOrDefault(x => x.PackageId == package.PackageId) is InstalledPackage installedPackageById)
{
installedPackage = installedPackageById;
// Always use package name from manifest
installedPackage.PackageName = package.PackageName;
}
else if (installedPackages.FirstOrDefault(x => x.PackageName == package.PackageName) is InstalledPackage installedPackageByName)
{
installedPackage = installedPackageByName;
// Ensure package ID is set
installedPackage.PackageId ??= package.PackageId;
}
else
{
installedPackage = new InstalledPackage
{
PackageId = package.PackageId,
PackageName = package.PackageName,
};
installedPackages.Add(package.PackageName, installedPackage);
installedPackages.Add(installedPackage);
}
// Set additional values
installedPackage.AllowPackageTelemetry = package.AllowPackageTelemetry;
installedPackage.PackageView = package.PackageView;
if (!string.IsNullOrEmpty(package.Version))
{
installedPackage.Version = package.Version;
}
installedPackage.PackageView = package.PackageView;
}
// Return all packages with a name in the package.manifest or package migrations
return installedPackages.Values;
// Return all packages with an ID or name in the package.manifest or package migrations
return installedPackages;
}
#endregion

View File

@@ -546,6 +546,11 @@
font-weight: bold;
}
.umb-package-list__item-id {
font-size: 12px;
color: @gray-6;
}
.umb-package-list__item-description {
font-size: 14px;
color: @gray-4;

View File

@@ -17,6 +17,7 @@
</div>
<div class="umb-package-list__item-content">
<div class="umb-package-list__item-name">{{ installedPackage.name }}</div>
<div class="umb-package-list__item-id" ng-if="installedPackage.id">{{ installedPackage.id }}</div>
<div class="umb-package-list__item-description" ng-if="installedPackage.version">Version: {{ installedPackage.version }}</div>
<div class="umb-package-list__item-description">
<localize ng-if="installedPackage.hasMigrations && !installedPackage.hasPendingMigrations"

View File

@@ -1,5 +1,5 @@
{
"id": "UmbracoPackage",
"name": "UmbracoPackage",
"versionAssemblyName": "UmbracoPackage",
"allowPackageTelemetry": true
}

View File

@@ -1,5 +1,5 @@
{
"id": "UmbracoPackage",
"name": "UmbracoPackage",
"versionAssemblyName": "UmbracoPackage",
"allowPackageTelemetry": true
}

View File

@@ -1,10 +1,8 @@
using System.Collections.Generic;
using System.Linq;
using Moq;
using NUnit.Framework;
using Umbraco.Cms.Core.Configuration;
using Umbraco.Cms.Core.Manifest;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Packaging;
using Umbraco.Cms.Core.Semver;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Telemetry;
@@ -21,7 +19,7 @@ public class TelemetryServiceTests
var siteIdentifierServiceMock = new Mock<ISiteIdentifierService>();
var usageInformationServiceMock = new Mock<IUsageInformationService>();
var sut = new TelemetryService(
Mock.Of<IManifestParser>(),
Mock.Of<IPackagingService>(),
version,
siteIdentifierServiceMock.Object,
usageInformationServiceMock.Object,
@@ -37,7 +35,7 @@ public class TelemetryServiceTests
{
var version = CreateUmbracoVersion(9, 3, 1);
var sut = new TelemetryService(
Mock.Of<IManifestParser>(),
Mock.Of<IPackagingService>(),
version,
CreateSiteIdentifierService(false),
Mock.Of<IUsageInformationService>(),
@@ -57,7 +55,7 @@ public class TelemetryServiceTests
var metricsConsentService = new Mock<IMetricsConsentService>();
metricsConsentService.Setup(x => x.GetConsentLevel()).Returns(TelemetryLevel.Detailed);
var sut = new TelemetryService(
Mock.Of<IManifestParser>(),
Mock.Of<IPackagingService>(),
version,
CreateSiteIdentifierService(),
Mock.Of<IUsageInformationService>(),
@@ -73,19 +71,20 @@ public class TelemetryServiceTests
public void CanGatherPackageTelemetry()
{
var version = CreateUmbracoVersion(9, 1, 1);
var versionPackageName = "VersionPackage";
var versionPackageId = "VersionPackageId";
var versionPackageName = "VersionPackageName";
var packageVersion = "1.0.0";
var noVersionPackageName = "NoVersionPackage";
PackageManifest[] manifests =
var noVersionPackageName = "NoVersionPackageName";
InstalledPackage[] installedPackages =
{
new() { PackageName = versionPackageName, Version = packageVersion },
new() { PackageId = versionPackageId, PackageName = versionPackageName, Version = packageVersion },
new() { PackageName = noVersionPackageName },
};
var manifestParser = CreateManifestParser(manifests);
var packagingService = CreatePackagingService(installedPackages);
var metricsConsentService = new Mock<IMetricsConsentService>();
metricsConsentService.Setup(x => x.GetConsentLevel()).Returns(TelemetryLevel.Detailed);
var sut = new TelemetryService(
manifestParser,
packagingService,
version,
CreateSiteIdentifierService(),
Mock.Of<IUsageInformationService>(),
@@ -98,12 +97,14 @@ public class TelemetryServiceTests
{
Assert.AreEqual(2, telemetry.Packages.Count());
var versionPackage = telemetry.Packages.FirstOrDefault(x => x.Name == versionPackageName);
Assert.AreEqual(versionPackageId, versionPackage.Id);
Assert.AreEqual(versionPackageName, versionPackage.Name);
Assert.AreEqual(packageVersion, versionPackage.Version);
var noVersionPackage = telemetry.Packages.FirstOrDefault(x => x.Name == noVersionPackageName);
Assert.AreEqual(null, noVersionPackage.Id);
Assert.AreEqual(noVersionPackageName, noVersionPackage.Name);
Assert.AreEqual(string.Empty, noVersionPackage.Version);
Assert.AreEqual(null, noVersionPackage.Version);
});
}
@@ -111,16 +112,16 @@ public class TelemetryServiceTests
public void RespectsAllowPackageTelemetry()
{
var version = CreateUmbracoVersion(9, 1, 1);
PackageManifest[] manifests =
InstalledPackage[] installedPackages =
{
new() { PackageName = "DoNotTrack", AllowPackageTelemetry = false },
new() { PackageName = "TrackingAllowed", AllowPackageTelemetry = true },
};
var manifestParser = CreateManifestParser(manifests);
var packagingService = CreatePackagingService(installedPackages);
var metricsConsentService = new Mock<IMetricsConsentService>();
metricsConsentService.Setup(x => x.GetConsentLevel()).Returns(TelemetryLevel.Detailed);
var sut = new TelemetryService(
manifestParser,
packagingService,
version,
CreateSiteIdentifierService(),
Mock.Of<IUsageInformationService>(),
@@ -136,11 +137,11 @@ public class TelemetryServiceTests
});
}
private IManifestParser CreateManifestParser(IEnumerable<PackageManifest> manifests)
private IPackagingService CreatePackagingService(IEnumerable<InstalledPackage> installedPackages)
{
var manifestParserMock = new Mock<IManifestParser>();
manifestParserMock.Setup(x => x.GetManifests()).Returns(manifests);
return manifestParserMock.Object;
var packagingServiceMock = new Mock<IPackagingService>();
packagingServiceMock.Setup(x => x.GetAllInstalledPackages()).Returns(installedPackages);
return packagingServiceMock.Object;
}
private IUmbracoVersion CreateUmbracoVersion(int major, int minor, int patch, string prerelease = "", string build = "")