diff --git a/src/Umbraco.Core/Manifest/PackageManifest.cs b/src/Umbraco.Core/Manifest/PackageManifest.cs index eb732509ce..1ef8e4fb08 100644 --- a/src/Umbraco.Core/Manifest/PackageManifest.cs +++ b/src/Umbraco.Core/Manifest/PackageManifest.cs @@ -13,7 +13,16 @@ public class PackageManifest private string? _packageName; /// - /// Gets or sets the name of the package. If not specified, uses the directory name instead. + /// Gets or sets the package identifier. + /// + /// + /// The package identifier. + /// + [DataMember(Name = "id")] + public string? PackageId { get; set; } + + /// + /// Gets or sets the name of the package. If not specified, uses the package identifier or directory name instead. /// /// /// The name of the package. @@ -27,6 +36,11 @@ public class PackageManifest { return _packageName; } + + if (!PackageId.IsNullOrWhiteSpace()) + { + _packageName = PackageId; + } if (!Source.IsNullOrWhiteSpace()) { diff --git a/src/Umbraco.Core/Packaging/InstalledPackage.cs b/src/Umbraco.Core/Packaging/InstalledPackage.cs index 8a18cd1da6..42d62317c5 100644 --- a/src/Umbraco.Core/Packaging/InstalledPackage.cs +++ b/src/Umbraco.Core/Packaging/InstalledPackage.cs @@ -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 PackageMigrationPlans { get; set; } = - Enumerable.Empty(); + public IEnumerable PackageMigrationPlans { get; set; } = Enumerable.Empty(); /// - /// It the package contains any migrations at all + /// Gets a value indicating whether this package has migrations. /// + /// + /// true if this package has migrations; otherwise, false. + /// [DataMember(Name = "hasMigrations")] public bool HasMigrations => PackageMigrationPlans.Any(); /// - /// If the package has any pending migrations to run + /// Gets a value indicating whether this package has pending migrations. /// + /// + /// true if this package has pending migrations; otherwise, false. + /// [DataMember(Name = "hasPendingMigrations")] public bool HasPendingMigrations => PackageMigrationPlans.Any(x => x.HasPendingMigrations); } diff --git a/src/Umbraco.Core/Telemetry/Models/PackageTelemetry.cs b/src/Umbraco.Core/Telemetry/Models/PackageTelemetry.cs index 53c07766e8..b0500c0127 100644 --- a/src/Umbraco.Core/Telemetry/Models/PackageTelemetry.cs +++ b/src/Umbraco.Core/Telemetry/Models/PackageTelemetry.cs @@ -3,23 +3,35 @@ using System.Runtime.Serialization; namespace Umbraco.Cms.Core.Telemetry.Models; /// -/// Serializable class containing information about an installed package. +/// Serializable class containing information about an installed package. /// [DataContract(Name = "packageTelemetry")] public class PackageTelemetry { /// - /// Gets or sets the name of the installed package. + /// Gets or sets the identifier of the installed package. /// + /// + /// The identifier. + /// + [DataMember(Name = "id")] + public string? Id { get; set; } + + /// + /// Gets or sets the name of the installed package. + /// + /// + /// The name. + /// [DataMember(Name = "name")] public string? Name { get; set; } /// - /// Gets or sets the version of the installed package. + /// Gets or sets the version of the installed package. /// - /// - /// This may be an empty string if no version is specified, or if package telemetry has been restricted. - /// + /// + /// The version. + /// [DataMember(Name = "version")] public string? Version { get; set; } } diff --git a/src/Umbraco.Core/Telemetry/TelemetryService.cs b/src/Umbraco.Core/Telemetry/TelemetryService.cs index 4ebf1ba0b9..82d1b19493 100644 --- a/src/Umbraco.Core/Telemetry/TelemetryService.cs +++ b/src/Umbraco.Core/Telemetry/TelemetryService.cs @@ -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; /// 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 class. /// 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 packages = new(); - IEnumerable manifests = _manifestParser.GetManifests(); + IEnumerable 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, }); } diff --git a/src/Umbraco.Infrastructure/Manifest/ManifestParser.cs b/src/Umbraco.Infrastructure/Manifest/ManifestParser.cs index 0108fa107d..18da814b59 100644 --- a/src/Umbraco.Infrastructure/Manifest/ManifestParser.cs +++ b/src/Umbraco.Infrastructure/Manifest/ManifestParser.cs @@ -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 diff --git a/src/Umbraco.Infrastructure/Packaging/AutomaticPackageMigrationPlan.cs b/src/Umbraco.Infrastructure/Packaging/AutomaticPackageMigrationPlan.cs index 3e3c2cfae1..d8becf0bfa 100644 --- a/src/Umbraco.Infrastructure/Packaging/AutomaticPackageMigrationPlan.cs +++ b/src/Umbraco.Infrastructure/Packaging/AutomaticPackageMigrationPlan.cs @@ -11,20 +11,41 @@ using Umbraco.Extensions; namespace Umbraco.Cms.Infrastructure.Packaging; /// -/// 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. /// public abstract class AutomaticPackageMigrationPlan : PackageMigrationPlan { + /// + /// Initializes a new instance of the class. + /// + /// The package name that the plan is for. If the package has a package.manifest these must match. protected AutomaticPackageMigrationPlan(string packageName) : this(packageName, packageName) { } + /// + /// Initializes a new instance of the class. + /// + /// The package name that the plan is for. If the package has a package.manifest these must match. + /// 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. protected AutomaticPackageMigrationPlan(string packageName, string planName) - : base(packageName, planName) + : this(null!, packageName, planName) { } + /// + /// Initializes a new instance of the class. + /// + /// The package identifier that the plan is for. If the package has a package.manifest these must match. + /// The package name that the plan is for. If the package has a package.manifest these must match. + /// 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. + protected AutomaticPackageMigrationPlan(string packageId, string packageName, string planName) + : base(packageId, packageName, planName) + { + } + + /// 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(finalId); } + /// + /// Provides a migration that imports an embedded package data manifest. + /// private class MigrateToPackageData : PackageMigrationBase { + /// + /// Initializes a new instance of the class. + /// + /// The packaging service. + /// The media service. + /// The media file manager. + /// The media URL generators. + /// The short string helper. + /// The content type base service provider. + /// The migration context. + /// The package migration settings. public MigrateToPackageData( IPackagingService packagingService, IMediaService mediaService, @@ -44,11 +79,13 @@ public abstract class AutomaticPackageMigrationPlan : PackageMigrationPlan MediaUrlGeneratorCollection mediaUrlGenerators, IShortStringHelper shortStringHelper, IContentTypeBaseServiceProvider contentTypeBaseServiceProvider, - IMigrationContext context, IOptions options) + IMigrationContext context, + IOptions options) : base(packagingService, mediaService, mediaFileManager, mediaUrlGenerators, shortStringHelper, contentTypeBaseServiceProvider, context, options) { } + /// protected override void Migrate() { var plan = (AutomaticPackageMigrationPlan)Context.Plan; diff --git a/src/Umbraco.Infrastructure/Packaging/PackageMigrationPlan.cs b/src/Umbraco.Infrastructure/Packaging/PackageMigrationPlan.cs index bdbce82fcb..be7caa1aad 100644 --- a/src/Umbraco.Infrastructure/Packaging/PackageMigrationPlan.cs +++ b/src/Umbraco.Infrastructure/Packaging/PackageMigrationPlan.cs @@ -4,50 +4,70 @@ using Umbraco.Cms.Infrastructure.Migrations; namespace Umbraco.Cms.Core.Packaging; /// -/// Base class for package migration plans +/// Represents a package migration plan. /// public abstract class PackageMigrationPlan : MigrationPlan, IDiscoverable { /// - /// Creates a package migration plan + /// Initializes a new instance of the class. /// - /// The name of the package. If the package has a package.manifest these must match. + /// The package name that the plan is for. If the package has a package.manifest these must match. protected PackageMigrationPlan(string packageName) : this(packageName, packageName) { } /// - /// Create a plan for a Package Name + /// Initializes a new instance of the class. /// - /// - /// The package name that the plan is for. If the package has a package.manifest these must - /// match. - /// - /// - /// 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. - /// + /// The package name that the plan is for. If the package has a package.manifest these must match. + /// 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. 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; } /// - /// 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 class. + /// + /// The package identifier that the plan is for. If the package has a package.manifest these must match. + /// The package name that the plan is for. If the package has a package.manifest these must match. + /// 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. + 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(); + } + + /// + /// Inform the plan executor to ignore all saved package state and + /// run the migration from initial state to it's end state. /// public override bool IgnoreCurrentState => true; /// - /// Returns the Package Name for this plan + /// Gets the package identifier. /// - public string PackageName { get; } + /// + /// The package identifier. + /// + public string? PackageId { get; init; } + /// + /// Gets the package name. + /// + /// + /// The package name. + /// + public string PackageName { get; init; } + + /// + /// Defines the plan. + /// protected abstract void DefinePlan(); } diff --git a/src/Umbraco.Infrastructure/Services/Implement/PackagingService.cs b/src/Umbraco.Infrastructure/Services/Implement/PackagingService.cs index 5047bae233..5ed48329fd 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/PackagingService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/PackagingService.cs @@ -118,26 +118,41 @@ public class PackagingService : IPackagingService { IReadOnlyDictionary? keyValues = _keyValueService.FindByKeyPrefix(Constants.Conventions.Migrations.KeyValuePrefix); - var installedPackages = new Dictionary(); + var installedPackages = new List(); // 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 diff --git a/src/Umbraco.Web.UI.Client/src/less/components/umb-packages.less b/src/Umbraco.Web.UI.Client/src/less/components/umb-packages.less index 14cfa5f007..d4415ce0f6 100644 --- a/src/Umbraco.Web.UI.Client/src/less/components/umb-packages.less +++ b/src/Umbraco.Web.UI.Client/src/less/components/umb-packages.less @@ -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; diff --git a/src/Umbraco.Web.UI.Client/src/views/packages/views/installed.html b/src/Umbraco.Web.UI.Client/src/views/packages/views/installed.html index a166f7ab2d..ef420921fe 100644 --- a/src/Umbraco.Web.UI.Client/src/views/packages/views/installed.html +++ b/src/Umbraco.Web.UI.Client/src/views/packages/views/installed.html @@ -17,6 +17,7 @@
{{ installedPackage.name }}
+
{{ installedPackage.id }}
Version: {{ installedPackage.version }}
(); var usageInformationServiceMock = new Mock(); var sut = new TelemetryService( - Mock.Of(), + Mock.Of(), version, siteIdentifierServiceMock.Object, usageInformationServiceMock.Object, @@ -37,7 +35,7 @@ public class TelemetryServiceTests { var version = CreateUmbracoVersion(9, 3, 1); var sut = new TelemetryService( - Mock.Of(), + Mock.Of(), version, CreateSiteIdentifierService(false), Mock.Of(), @@ -57,7 +55,7 @@ public class TelemetryServiceTests var metricsConsentService = new Mock(); metricsConsentService.Setup(x => x.GetConsentLevel()).Returns(TelemetryLevel.Detailed); var sut = new TelemetryService( - Mock.Of(), + Mock.Of(), version, CreateSiteIdentifierService(), Mock.Of(), @@ -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(); metricsConsentService.Setup(x => x.GetConsentLevel()).Returns(TelemetryLevel.Detailed); var sut = new TelemetryService( - manifestParser, + packagingService, version, CreateSiteIdentifierService(), Mock.Of(), @@ -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(); metricsConsentService.Setup(x => x.GetConsentLevel()).Returns(TelemetryLevel.Detailed); var sut = new TelemetryService( - manifestParser, + packagingService, version, CreateSiteIdentifierService(), Mock.Of(), @@ -136,11 +137,11 @@ public class TelemetryServiceTests }); } - private IManifestParser CreateManifestParser(IEnumerable manifests) + private IPackagingService CreatePackagingService(IEnumerable installedPackages) { - var manifestParserMock = new Mock(); - manifestParserMock.Setup(x => x.GetManifests()).Returns(manifests); - return manifestParserMock.Object; + var packagingServiceMock = new Mock(); + packagingServiceMock.Setup(x => x.GetAllInstalledPackages()).Returns(installedPackages); + return packagingServiceMock.Object; } private IUmbracoVersion CreateUmbracoVersion(int major, int minor, int patch, string prerelease = "", string build = "")