Include umbraco-package.json manifests in telemetry data (#16430)

* Include umbraco-package.json manifests in package telemetry and add ID

* Add package ID to ManifestResponseModel

* Use async service method and HTTP client factory

* Avoid breaking changes

src/Umbraco.Cms.Api.Management/Mapping/Manifest/ManifestViewModelMapDefinition.cs

@@ -1,4 +1,4 @@
-using Umbraco.Cms.Api.Management.ViewModels.Manifest;
+using Umbraco.Cms.Api.Management.ViewModels.Manifest;
 using Umbraco.Cms.Core.Manifest;
 using Umbraco.Cms.Core.Mapping;
 
@@ -13,6 +13,7 @@ public class ManifestViewModelMapDefinition : IMapDefinition
     private static void Map(PackageManifest source, ManifestResponseModel target, MapperContext context)
     {
         target.Name = source.Name;
+        target.Id = source.Id;
         target.Version = source.Version;
         target.Extensions = source.Extensions;
     }

src/Umbraco.Cms.Api.Management/ViewModels/Manifest/ManifestResponseModel.cs

@@ -1,4 +1,4 @@
-using System.ComponentModel.DataAnnotations;
+using System.ComponentModel.DataAnnotations;
 
 namespace Umbraco.Cms.Api.Management.ViewModels.Manifest;
 
@@ -7,6 +7,8 @@ public class ManifestResponseModel
     [Required]
     public string Name { get; set; } = string.Empty;
 
+    public string? Id { get; set; }
+
     public string? Version { get; set; }
 
     public object[] Extensions { get; set; } = Array.Empty<object>();

src/Umbraco.Core/Manifest/PackageManifest.cs

@@ -1,9 +1,11 @@
-namespace Umbraco.Cms.Core.Manifest;
+namespace Umbraco.Cms.Core.Manifest;
 
 public class PackageManifest
 {
     public required string Name { get; set; }
 
+    public string? Id { get; set; }
+
     public string? Version { get; set; }
 
     public bool AllowPublicAccess { get; set; }

src/Umbraco.Core/Services/IPackagingService.cs

@@ -29,8 +29,18 @@ public interface IPackagingService : IService
     ///     Returns the advertised installed packages
     /// </summary>
     /// <returns></returns>
+    [Obsolete("Use GetAllInstalledPackagesAsync instead. Scheduled for removal in Umbraco 15.")]
     IEnumerable<InstalledPackage> GetAllInstalledPackages();
 
+    /// <summary>
+    ///     Returns the advertised installed packages
+    /// </summary>
+    /// <returns></returns>
+    Task<IEnumerable<InstalledPackage>> GetAllInstalledPackagesAsync()
+#pragma warning disable CS0618 // Type or member is obsolete
+        => Task.FromResult(GetAllInstalledPackages());
+#pragma warning restore CS0618 // Type or member is obsolete
+
     /// <summary>
     ///     Returns installed packages collected from the package migration plans.
     /// </summary>

src/Umbraco.Core/Telemetry/TelemetryService.cs

@@ -1,10 +1,7 @@
 // Copyright (c) Umbraco.
 // See LICENSE for more details.
 
-using Microsoft.Extensions.DependencyInjection;
 using Umbraco.Cms.Core.Configuration;
-using Umbraco.Cms.Core.DependencyInjection;
-using Umbraco.Cms.Core.Manifest;
 using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Packaging;
 using Umbraco.Cms.Core.Services;
@@ -43,6 +40,7 @@ internal class TelemetryService : ITelemetryService
     public bool TryGetTelemetryReportData(out TelemetryReportData? telemetryReportData)
     {
         telemetryReportData = GetTelemetryReportDataAsync().GetAwaiter().GetResult();
+
         return telemetryReportData != null;
     }
 
@@ -58,7 +56,7 @@ internal class TelemetryService : ITelemetryService
         {
             Id = telemetryId,
             Version = GetVersion(),
-            Packages = await GetPackageTelemetryAsync(),
+            Packages = await GetPackageTelemetryAsync().ConfigureAwait(false),
             Detailed = _usageInformationService.GetDetailed(),
         };
     }
@@ -67,8 +65,6 @@ internal class TelemetryService : ITelemetryService
         ? null
         : _umbracoVersion.SemanticVersion.ToSemanticStringWithoutBuild();
 
-
-
     private async Task<IEnumerable<PackageTelemetry>?> GetPackageTelemetryAsync()
     {
         if (_metricsConsentService.GetConsentLevel() == TelemetryLevel.Minimal)
@@ -77,7 +73,7 @@ internal class TelemetryService : ITelemetryService
         }
 
         List<PackageTelemetry> packages = new();
-        IEnumerable<InstalledPackage> installedPackages = _packagingService.GetAllInstalledPackages();
+        IEnumerable<InstalledPackage> installedPackages = await _packagingService.GetAllInstalledPackagesAsync().ConfigureAwait(false);
 
         foreach (InstalledPackage installedPackage in installedPackages)
         {

src/Umbraco.Infrastructure/BackgroundJobs/Jobs/ReportSiteJob.cs

@@ -1,5 +1,7 @@
 using System.Text;
+using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
+using Umbraco.Cms.Core.DependencyInjection;
 using Umbraco.Cms.Core.Serialization;
 using Umbraco.Cms.Core.Sync;
 using Umbraco.Cms.Core.Telemetry;
@@ -9,7 +11,6 @@ namespace Umbraco.Cms.Infrastructure.BackgroundJobs.Jobs;
 
 public class ReportSiteJob : IRecurringBackgroundJob
 {
-
     public TimeSpan Period => TimeSpan.FromDays(1);
 
     public TimeSpan Delay => TimeSpan.FromMinutes(5);
@@ -19,25 +20,37 @@ public class ReportSiteJob : IRecurringBackgroundJob
     // No-op event as the period never changes on this job
     public event EventHandler PeriodChanged
     {
-        add { } remove { }
+        add { }
+        remove { }
     }
 
-    private static HttpClient _httpClient = new();
-
     private readonly ILogger<ReportSiteJob> _logger;
-
     private readonly ITelemetryService _telemetryService;
-private readonly IJsonSerializer _jsonSerializer;
+    private readonly IJsonSerializer _jsonSerializer;
+    private readonly IHttpClientFactory _httpClientFactory;
 
+    [Obsolete("Use the constructor with IHttpClientFactory instead.")]
     public ReportSiteJob(
         ILogger<ReportSiteJob> logger,
         ITelemetryService telemetryService,
         IJsonSerializer jsonSerializer)
+        : this(
+            logger,
+            telemetryService,
+            jsonSerializer,
+            StaticServiceProvider.Instance.GetRequiredService<IHttpClientFactory>())
+    { }
+
+    public ReportSiteJob(
+        ILogger<ReportSiteJob> logger,
+        ITelemetryService telemetryService,
+        IJsonSerializer jsonSerializer,
+        IHttpClientFactory httpClientFactory)
     {
         _logger = logger;
         _telemetryService = telemetryService;
         _jsonSerializer = jsonSerializer;
-        _httpClient = new HttpClient();
+        _httpClientFactory = httpClientFactory;
     }
 
     /// <summary>
@@ -46,7 +59,8 @@ private readonly IJsonSerializer _jsonSerializer;
     /// </summary>
     public async Task RunJobAsync()
     {
-        if (_telemetryService.TryGetTelemetryReportData(out TelemetryReportData? telemetryReportData) is false)
+        TelemetryReportData? telemetryReportData = await _telemetryService.GetTelemetryReportDataAsync().ConfigureAwait(false);
+        if (telemetryReportData is null)
         {
             _logger.LogWarning("No telemetry marker found");
 
@@ -55,31 +69,28 @@ private readonly IJsonSerializer _jsonSerializer;
 
         try
         {
-            if (_httpClient.BaseAddress is null)
+            HttpClient httpClient = _httpClientFactory.CreateClient();
+            if (httpClient.BaseAddress is null)
             {
                 // Send data to LIVE telemetry
-                _httpClient.BaseAddress = new Uri("https://telemetry.umbraco.com/");
+                httpClient.BaseAddress = new Uri("https://telemetry.umbraco.com/");
 
 #if DEBUG
                 // Send data to DEBUG telemetry service
-                _httpClient.BaseAddress = new Uri("https://telemetry.rainbowsrock.net/");
+                httpClient.BaseAddress = new Uri("https://telemetry.rainbowsrock.net/");
 #endif
             }
 
-            _httpClient.DefaultRequestHeaders.TryAddWithoutValidation("Content-Type", "application/json");
+            httpClient.DefaultRequestHeaders.TryAddWithoutValidation("Content-Type", "application/json");
 
-            using (var request = new HttpRequestMessage(HttpMethod.Post, "installs/"))
-            {
-                request.Content = new StringContent(_jsonSerializer.Serialize(telemetryReportData), Encoding.UTF8,
-                    "application/json");
+            using var request = new HttpRequestMessage(HttpMethod.Post, "installs/");
+            request.Content = new StringContent(_jsonSerializer.Serialize(telemetryReportData), Encoding.UTF8, "application/json");
 
-                // Make a HTTP Post to telemetry service
-                // https://telemetry.umbraco.com/installs/
-                // Fire & Forget, do not need to know if its a 200, 500 etc
-                using (await _httpClient.SendAsync(request))
-                {
-                }
-            }
+            // Make a HTTP Post to telemetry service
+            // https://telemetry.umbraco.com/installs/
+            // Fire & Forget, do not need to know if its a 200, 500 etc
+            using (await httpClient.SendAsync(request))
+            { }
         }
         catch
         {

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

@@ -1,7 +1,5 @@
 using System.Xml.Linq;
 using Microsoft.Extensions.DependencyInjection;
-using Umbraco.Cms.Core.DependencyInjection;
-using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.FileProviders;
 using Microsoft.Extensions.Hosting;
 using Umbraco.Cms.Core.DependencyInjection;
@@ -14,6 +12,7 @@ using Umbraco.Cms.Core.Notifications;
 using Umbraco.Cms.Core.Packaging;
 using Umbraco.Cms.Core.Scoping;
 using Umbraco.Cms.Core.Services.OperationStatus;
+using Umbraco.Cms.Infrastructure.Manifest;
 using Umbraco.Extensions;
 using File = System.IO.File;
 
@@ -31,10 +30,12 @@ public class PackagingService : IPackagingService
     private readonly IKeyValueService _keyValueService;
     private readonly IPackageInstallation _packageInstallation;
     private readonly PackageMigrationPlanCollection _packageMigrationPlans;
+    private readonly IPackageManifestReader _packageManifestReader;
     private readonly ICoreScopeProvider _coreScopeProvider;
     private readonly IHostEnvironment _hostEnvironment;
     private readonly IUserService _userService;
 
+    [Obsolete("Use the constructor with IPackageManifestReader instead.")]
     public PackagingService(
         IAuditService auditService,
         ICreatedPackagesRepository createdPackages,
@@ -45,6 +46,30 @@ public class PackagingService : IPackagingService
         PackageMigrationPlanCollection packageMigrationPlans,
         IHostEnvironment hostEnvironment,
         IUserService userService)
+        : this(
+            auditService,
+            createdPackages,
+            packageInstallation,
+            eventAggregator,
+            keyValueService,
+            coreScopeProvider,
+            packageMigrationPlans,
+            StaticServiceProvider.Instance.GetRequiredService<IPackageManifestReader>(),
+            hostEnvironment,
+            userService)
+    { }
+
+    public PackagingService(
+        IAuditService auditService,
+        ICreatedPackagesRepository createdPackages,
+        IPackageInstallation packageInstallation,
+        IEventAggregator eventAggregator,
+        IKeyValueService keyValueService,
+        ICoreScopeProvider coreScopeProvider,
+        PackageMigrationPlanCollection packageMigrationPlans,
+        IPackageManifestReader packageManifestReader,
+        IHostEnvironment hostEnvironment,
+        IUserService userService)
     {
         _auditService = auditService;
         _createdPackages = createdPackages;
@@ -52,6 +77,7 @@ public class PackagingService : IPackagingService
         _eventAggregator = eventAggregator;
         _keyValueService = keyValueService;
         _packageMigrationPlans = packageMigrationPlans;
+        _packageManifestReader = packageManifestReader;
         _coreScopeProvider = coreScopeProvider;
         _hostEnvironment = hostEnvironment;
         _userService = userService;
@@ -61,9 +87,7 @@ public class PackagingService : IPackagingService
 
     public CompiledPackage GetCompiledPackageInfo(XDocument? xml) => _packageInstallation.ReadPackage(xml);
 
-    public InstallationSummary InstallCompiledPackageData(
-        XDocument? packageXml,
-        int userId = Constants.Security.SuperUserId)
+    public InstallationSummary InstallCompiledPackageData(XDocument? packageXml, int userId = Constants.Security.SuperUserId)
     {
         CompiledPackage compiledPackage = GetCompiledPackageInfo(packageXml);
 
@@ -89,9 +113,7 @@ public class PackagingService : IPackagingService
         return summary;
     }
 
-    public InstallationSummary InstallCompiledPackageData(
-        FileInfo packageXmlFile,
-        int userId = Constants.Security.SuperUserId)
+    public InstallationSummary InstallCompiledPackageData(FileInfo packageXmlFile, int userId = Constants.Security.SuperUserId)
     {
         XDocument xml;
         using (StreamReader streamReader = File.OpenText(packageXmlFile.FullName))
@@ -109,10 +131,14 @@ public class PackagingService : IPackagingService
     [Obsolete("Use DeleteCreatedPackageAsync instead. Scheduled for removal in Umbraco 15.")]
     public void DeleteCreatedPackage(int id, int userId = Constants.Security.SuperUserId)
     {
-        using ICoreScope scope = _coreScopeProvider.CreateCoreScope();
-        PackageDefinition? package = GetCreatedPackageById(id);
-        Guid key = package?.PackageId ?? Guid.Empty;
-        Guid currentUserKey = _userService.GetUserById(id)?.Key ?? Constants.Security.SuperUserKey;
+        Guid key, currentUserKey;
+
+        using (ICoreScope scope = _coreScopeProvider.CreateCoreScope(autoComplete: true))
+        {
+            PackageDefinition? package = GetCreatedPackageById(id);
+            key = package?.PackageId ?? Guid.Empty;
+            currentUserKey = _userService.GetUserById(id)?.Key ?? Constants.Security.SuperUserKey;
+        }
 
         DeleteCreatedPackageAsync(key, currentUserKey).GetAwaiter().GetResult();
     }
@@ -139,6 +165,7 @@ public class PackagingService : IPackagingService
     public IEnumerable<PackageDefinition?> GetAllCreatedPackages()
     {
         using ICoreScope scope = _coreScopeProvider.CreateCoreScope(autoComplete: true);
+
         return _createdPackages.GetAll();
     }
 
@@ -146,6 +173,7 @@ public class PackagingService : IPackagingService
     public PackageDefinition? GetCreatedPackageById(int id)
     {
         using ICoreScope scope = _coreScopeProvider.CreateCoreScope(autoComplete: true);
+
         return _createdPackages.GetById(id);
     }
 
@@ -155,7 +183,8 @@ public class PackagingService : IPackagingService
         using ICoreScope scope = _coreScopeProvider.CreateCoreScope(autoComplete: true);
         PackageDefinition[] packages = _createdPackages.GetAll().WhereNotNull().ToArray();
         var pagedModel = new PagedModel<PackageDefinition>(packages.Length, packages.Skip(skip).Take(take));
-        return await Task.FromResult(pagedModel);
+
+        return pagedModel;
     }
 
     /// <inheritdoc/>
@@ -173,6 +202,7 @@ public class PackagingService : IPackagingService
 
         var success = _createdPackages.SavePackage(definition);
         scope.Complete();
+
         return success;
     }
 
@@ -192,9 +222,10 @@ public class PackagingService : IPackagingService
 
         int currentUserId = (await _userService.GetRequiredUserAsync(userKey)).Id;
         _auditService.Add(AuditType.New, currentUserId, -1, "Package", $"Created package '{package.Name}' created. Package key: {package.PackageId}");
-        scope.Complete();
-        return await Task.FromResult(Attempt.SucceedWithStatus(PackageOperationStatus.Success, package));
 
+        scope.Complete();
+
+        return Attempt.SucceedWithStatus(PackageOperationStatus.Success, package);
     }
 
     /// <inheritdoc/>
@@ -208,20 +239,27 @@ public class PackagingService : IPackagingService
 
         int currentUserId = (await _userService.GetRequiredUserAsync(userKey)).Id;
         _auditService.Add(AuditType.New, currentUserId, -1, "Package", $"Created package '{package.Name}' updated. Package key: {package.PackageId}");
+
         scope.Complete();
-        return await Task.FromResult(Attempt.SucceedWithStatus(PackageOperationStatus.Success, package));
+
+        return Attempt.SucceedWithStatus(PackageOperationStatus.Success, package);
     }
 
     public string ExportCreatedPackage(PackageDefinition definition)
     {
         using ICoreScope scope = _coreScopeProvider.CreateCoreScope(autoComplete: true);
+
         return _createdPackages.ExportPackage(definition);
     }
 
     public InstalledPackage? GetInstalledPackageByName(string packageName)
         => GetAllInstalledPackages().Where(x => x.PackageName?.InvariantEquals(packageName) ?? false).FirstOrDefault();
 
+    [Obsolete("Use GetAllInstalledPackagesAsync instead. Scheduled for removal in Umbraco 15.")]
     public IEnumerable<InstalledPackage> GetAllInstalledPackages()
+        => GetAllInstalledPackagesAsync().GetAwaiter().GetResult();
+
+    public async Task<IEnumerable<InstalledPackage>> GetAllInstalledPackagesAsync()
     {
         using ICoreScope scope = _coreScopeProvider.CreateCoreScope(autoComplete: true);
 
@@ -277,14 +315,57 @@ public class PackagingService : IPackagingService
             installedPackage.PackageMigrationPlans = currentPlans;
         }
 
-        // Return all packages with an ID or name in the package.manifest or package migrations
+        // Collect and merge the packages from the manifests
+        foreach (PackageManifest packageManifest in await _packageManifestReader.ReadPackageManifestsAsync().ConfigureAwait(false))
+        {
+            if (packageManifest.Id is null && string.IsNullOrEmpty(packageManifest.Name))
+            {
+                continue;
+            }
+
+            InstalledPackage installedPackage;
+            if (packageManifest.Id is not null && installedPackages.FirstOrDefault(x => x.PackageId == packageManifest.Id) is InstalledPackage installedPackageById)
+            {
+                installedPackage = installedPackageById;
+
+                // Always use package name from manifest
+                installedPackage.PackageName = packageManifest.Name;
+            }
+            else if (installedPackages.FirstOrDefault(x => x.PackageName == packageManifest.Name) is InstalledPackage installedPackageByName)
+            {
+                installedPackage = installedPackageByName;
+
+                // Ensure package ID is set
+                installedPackage.PackageId ??= packageManifest.Id;
+            }
+            else
+            {
+                installedPackage = new InstalledPackage
+                {
+                    PackageId = packageManifest.Id,
+                    PackageName = packageManifest.Name,
+                };
+
+                installedPackages.Add(installedPackage);
+            }
+
+            // Set additional values
+            installedPackage.AllowPackageTelemetry = packageManifest.AllowTelemetry;
+
+            if (!string.IsNullOrEmpty(packageManifest.Version))
+            {
+                installedPackage.Version = packageManifest.Version;
+            }
+        }
+
+        // Return all packages with an ID or name in the package manifest or package migrations
         return installedPackages;
     }
 
     #endregion
 
     /// <inheritdoc/>
-    public async Task<PagedModel<InstalledPackage>> GetInstalledPackagesFromMigrationPlansAsync(int skip, int take)
+    public Task<PagedModel<InstalledPackage>> GetInstalledPackagesFromMigrationPlansAsync(int skip, int take)
     {
         IReadOnlyDictionary<string, string?>? keyValues =
             _keyValueService.FindByKeyPrefix(Constants.Conventions.Migrations.KeyValuePrefix);
@@ -311,7 +392,7 @@ public class PackagingService : IPackagingService
                 return package;
             }).ToArray();
 
-        return await Task.FromResult(new PagedModel<InstalledPackage>
+        return Task.FromResult(new PagedModel<InstalledPackage>
         {
             Total = installedPackages.Count(),
             Items = installedPackages.Skip(skip).Take(take),

tests/Umbraco.Tests.UnitTests/Umbraco.Core/Telemetry/TelemetryServiceTests.cs

@@ -1,7 +1,6 @@
 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;
@@ -139,7 +138,7 @@ public class TelemetryServiceTests
     private IPackagingService CreatePackagingService(IEnumerable<InstalledPackage> installedPackages)
     {
         var packagingServiceMock = new Mock<IPackagingService>();
-        packagingServiceMock.Setup(x => x.GetAllInstalledPackages()).Returns(installedPackages);
+        packagingServiceMock.Setup(x => x.GetAllInstalledPackagesAsync()).ReturnsAsync(installedPackages);
         return packagingServiceMock.Object;
     }