V14: Fix templates not having set master template on package install (#…

…16978) * Reorder templates to save master templates first, and use new ITemplateService * Add obsoletion * Fix if statement * Refactor async calls into async method to avoid multiple get awaiters * Update interface * Avoid breaking changes --------- Co-authored-by: Sven Geusens <[email protected]> Co-authored-by: Bjarke Berg <[email protected]>
umbraco · Sep 17, 2024 · f72c39e · f72c39e
1 parent 991b9a7
commit f72c39e
Show file tree

Hide file tree

Showing 9 changed files with 110 additions and 26 deletions.
diff --git a/Directory.Build.props b/Directory.Build.props
@@ -28,7 +28,7 @@
 
   <!-- Package Validation -->
   <PropertyGroup>
-    <GenerateCompatibilitySuppressionFile>true</GenerateCompatibilitySuppressionFile>
+    <GenerateCompatibilitySuppressionFile>false</GenerateCompatibilitySuppressionFile>
     <EnablePackageValidation>true</EnablePackageValidation>
     <PackageValidationBaselineVersion>14.0.0</PackageValidationBaselineVersion>
     <EnableStrictModeForCompatibleFrameworksInPackage>true</EnableStrictModeForCompatibleFrameworksInPackage>

diff --git a/src/Umbraco.Core/Media/EmbedProviders/OEmbedResponse.cs b/src/Umbraco.Core/Media/EmbedProviders/OEmbedResponse.cs
@@ -6,5 +6,16 @@ namespace Umbraco.Cms.Core.Media.EmbedProviders;
 ///     Wrapper class for OEmbed response.
 /// </summary>
 [DataContract]
-public class OEmbedResponse : OEmbedResponseBase<double>;
+public class OEmbedResponse : OEmbedResponseBase<double>
+{
+
+    // these is only here to avoid breaking changes. In theory it should still be source code compatible to remove them.
+    public new double? ThumbnailHeight => base.ThumbnailHeight;
+
+    public new double? ThumbnailWidth => base.ThumbnailWidth;
+
+    public new double? Height => base.Height;
+
+    public new double? Width => base.Width;
+}
 
diff --git a/src/Umbraco.Core/Media/EmbedProviders/OEmbedResponseBase.cs b/src/Umbraco.Core/Media/EmbedProviders/OEmbedResponseBase.cs
@@ -34,7 +34,7 @@ public abstract class OEmbedResponseBase<T>
     public string? ThumbnailUrl { get; set; }
 
     [DataMember(Name = "thumbnail_height")]
-    public T? ThumbnailHeight { get; set; }
+    public virtual T? ThumbnailHeight { get; set; }
 
     [DataMember(Name = "thumbnail_width")]
     public T? ThumbnailWidth { get; set; }

diff --git a/src/Umbraco.Core/Services/FileService.cs b/src/Umbraco.Core/Services/FileService.cs
@@ -523,8 +523,6 @@ public void SaveTemplate(ITemplate template, int userId = Constants.Security.Sup
     /// </summary>
     /// <param name="templates">List of <see cref="Template" /> to save</param>
     /// <param name="userId">Optional id of the user</param>
-    // FIXME: we need to re-implement PackageDataInstallation.ImportTemplates so it imports templates in the correct order
-    //        instead of relying on being able to save invalid templates (child templates whose master has yet to be created)
     [Obsolete("Please use ITemplateService for template operations - will be removed in Umbraco 15")]
     public void SaveTemplate(IEnumerable<ITemplate> templates, int userId = Constants.Security.SuperUserId)
     {

diff --git a/src/Umbraco.Core/Services/IDataTypeService.cs b/src/Umbraco.Core/Services/IDataTypeService.cs
@@ -196,7 +196,7 @@ public interface IDataTypeService : IService
     /// </summary>
     /// <param name="propertyEditorAlias">Alias of the property editor</param>
     /// <returns>Collection of <see cref="IDataType" /> configured for the property editor</returns>
-    Task<IEnumerable<IDataType>> GetByEditorAliasAsync(string propertyEditorAlias);
+    Task<IEnumerable<IDataType>> GetByEditorAliasAsync(string propertyEditorAlias) => Task.FromResult(GetByEditorAlias(propertyEditorAlias));
 
     /// <summary>
     ///     Gets all <see cref="IDataType" /> for a given editor UI alias
@@ -246,5 +246,5 @@ public interface IDataTypeService : IService
     /// </summary>
     /// <param name="propertyEditorAlias">Aliases of the property editors</param>
     /// <returns>Collection of <see cref="IDataType" /> configured for the property editors</returns>
-    Task<IEnumerable<IDataType>> GetByEditorAliasAsync(string[] propertyEditorAlias);
+    Task<IEnumerable<IDataType>> GetByEditorAliasAsync(string[] propertyEditorAlias) => Task.FromResult(propertyEditorAlias.SelectMany(x=>GetByEditorAlias(x)));
 }
diff --git a/src/Umbraco.Core/Services/IPackageDataInstallation.cs b/src/Umbraco.Core/Services/IPackageDataInstallation.cs
@@ -64,16 +64,28 @@ IReadOnlyList<IDictionaryItem> ImportDictionaryItems(IEnumerable<XElement> dicti
     /// <returns>An enumerable list of generated languages</returns>
     IReadOnlyList<ILanguage> ImportLanguages(IEnumerable<XElement> languageElements, int userId);
 
+    [Obsolete("Use Async version instead, Scheduled to be removed in v17")]
     IEnumerable<ITemplate> ImportTemplate(XElement templateElement, int userId);
 
+    Task<IEnumerable<ITemplate>> ImportTemplateAsync(XElement templateElement, int userId) => Task.FromResult(ImportTemplate(templateElement, userId));
+
     /// <summary>
     /// Imports and saves package xml as <see cref="ITemplate"/>
     /// </summary>
     /// <param name="templateElements">Xml to import</param>
     /// <param name="userId">Optional user id</param>
     /// <returns>An enumerable list of generated Templates</returns>
+    [Obsolete("Use Async version instead, Scheduled to be removed in v17")]
     IReadOnlyList<ITemplate> ImportTemplates(IReadOnlyCollection<XElement> templateElements, int userId);
 
+    /// <summary>
+    /// Imports and saves package xml as <see cref="ITemplate"/>
+    /// </summary>
+    /// <param name="templateElements">Xml to import</param>
+    /// <param name="userId">Optional user id</param>
+    /// <returns>An enumerable list of generated Templates</returns>
+    Task<IReadOnlyList<ITemplate>> ImportTemplatesAsync(IReadOnlyCollection<XElement> templateElements, int userId) => Task.FromResult(ImportTemplates(templateElements, userId));
+
     Guid GetContentTypeKey(XElement contentType);
 
     string? GetEntityTypeAlias(XElement entityType);

diff --git a/src/Umbraco.Core/Services/OperationStatus/ContentTypeOperationStatus.cs b/src/Umbraco.Core/Services/OperationStatus/ContentTypeOperationStatus.cs
@@ -5,10 +5,7 @@ public enum ContentTypeOperationStatus
     Success,
     DuplicateAlias,
     InvalidAlias,
-    NameCannotBeEmpty,
-    NameTooLong,
     InvalidPropertyTypeAlias,
-    PropertyTypeAliasCannotEqualContentTypeAlias,
     DuplicatePropertyTypeAlias,
     DataTypeNotFound,
     InvalidInheritance,
@@ -21,6 +18,9 @@ public enum ContentTypeOperationStatus
     NotFound,
     NotAllowed,
     CancelledByNotification,
+    PropertyTypeAliasCannotEqualContentTypeAlias,
+    NameCannotBeEmpty,
+    NameTooLong,
     InvalidElementFlagDocumentHasContent,
     InvalidElementFlagElementIsUsedInPropertyEditorConfiguration,
     InvalidElementFlagComparedToParent,

diff --git a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.Services.cs b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.Services.cs
@@ -113,7 +113,9 @@ private static IPackageDataInstallation CreatePackageDataInstallation(IServicePr
             factory.GetRequiredService<IShortStringHelper>(),
             factory.GetRequiredService<IConfigurationEditorJsonSerializer>(),
             factory.GetRequiredService<IMediaService>(),
-            factory.GetRequiredService<IMediaTypeService>());
+            factory.GetRequiredService<IMediaTypeService>(),
+            factory.GetRequiredService<ITemplateContentParserService>(),
+            factory.GetRequiredService<ITemplateService>());
 
     private static LocalizedTextServiceFileSources CreateLocalizedTextServiceFileSourcesFactory(
         IServiceProvider container)

diff --git a/src/Umbraco.Infrastructure/Packaging/PackageDataInstallation.cs b/src/Umbraco.Infrastructure/Packaging/PackageDataInstallation.cs
@@ -2,11 +2,13 @@
 using System.Net;
 using System.Xml.Linq;
 using System.Xml.XPath;
+using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
 using Umbraco.Cms.Core;
 using Umbraco.Cms.Core.Collections;
 using Umbraco.Cms.Core.Configuration.Models;
+using Umbraco.Cms.Core.DependencyInjection;
 using Umbraco.Cms.Core.Hosting;
 using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Models.Entities;
@@ -34,6 +36,8 @@ public class PackageDataInstallation : IPackageDataInstallation
         private readonly IConfigurationEditorJsonSerializer _serializer;
         private readonly IMediaService _mediaService;
         private readonly IMediaTypeService _mediaTypeService;
+        private readonly ITemplateContentParserService _templateContentParserService;
+        private readonly ITemplateService _templateService;
         private readonly IEntityService _entityService;
         private readonly IContentTypeService _contentTypeService;
         private readonly IContentService _contentService;
@@ -52,7 +56,9 @@ public PackageDataInstallation(
             IShortStringHelper shortStringHelper,
             IConfigurationEditorJsonSerializer serializer,
             IMediaService mediaService,
-            IMediaTypeService mediaTypeService)
+            IMediaTypeService mediaTypeService,
+            ITemplateContentParserService templateContentParserService,
+            ITemplateService templateService)
         {
             _dataValueEditorFactory = dataValueEditorFactory;
             _logger = logger;
@@ -68,6 +74,44 @@ public PackageDataInstallation(
             _serializer = serializer;
             _mediaService = mediaService;
             _mediaTypeService = mediaTypeService;
+            _templateContentParserService = templateContentParserService;
+            _templateService = templateService;
+        }
+
+        [Obsolete("Please use new constructor, scheduled for removal in v15")]
+        public PackageDataInstallation(
+            IDataValueEditorFactory dataValueEditorFactory,
+            ILogger<PackageDataInstallation> logger,
+            IFileService fileService,
+            ILocalizationService localizationService,
+            IDataTypeService dataTypeService,
+            IEntityService entityService,
+            IContentTypeService contentTypeService,
+            IContentService contentService,
+            PropertyEditorCollection propertyEditors,
+            IScopeProvider scopeProvider,
+            IShortStringHelper shortStringHelper,
+            IConfigurationEditorJsonSerializer serializer,
+            IMediaService mediaService,
+            IMediaTypeService mediaTypeService)
+            : this(
+                dataValueEditorFactory,
+                logger,
+                fileService,
+                localizationService,
+                dataTypeService,
+                entityService,
+                contentTypeService,
+                contentService,
+                propertyEditors,
+                scopeProvider,
+                shortStringHelper,
+                serializer,
+                mediaService,
+                mediaTypeService,
+                StaticServiceProvider.Instance.GetRequiredService<ITemplateContentParserService>(),
+                StaticServiceProvider.Instance.GetRequiredService<ITemplateService>())
+        {
         }
 
         // Also remove factory service registration when this constructor is removed
@@ -103,7 +147,9 @@ public PackageDataInstallation(
                   shortStringHelper,
                   serializer,
                   mediaService,
-                  mediaTypeService)
+                  mediaTypeService,
+                  StaticServiceProvider.Instance.GetRequiredService<ITemplateContentParserService>(),
+                  StaticServiceProvider.Instance.GetRequiredService<ITemplateService>())
         { }
 
         #region Install/Uninstall
@@ -1651,16 +1697,25 @@ public IReadOnlyList<IFile> ImportStylesheets(IEnumerable<XElement> stylesheetEl
 
         #region Templates
 
+        [Obsolete("Use Async version instead, Scheduled to be removed in v17")]
         public IEnumerable<ITemplate> ImportTemplate(XElement templateElement, int userId)
             => ImportTemplates(new[] {templateElement}, userId);
 
+        public async Task<IEnumerable<ITemplate>> ImportTemplateAsync(XElement templateElement, int userId)
+            => ImportTemplatesAsync(new[] {templateElement}, userId).GetAwaiter().GetResult();
+
+
+        [Obsolete("Use Async version instead, Scheduled to be removed in v17")]
+        public IReadOnlyList<ITemplate> ImportTemplates(IReadOnlyCollection<XElement> templateElements, int userId)
+            => ImportTemplatesAsync(templateElements, userId).GetAwaiter().GetResult();
+
         /// <summary>
         /// Imports and saves package xml as <see cref="ITemplate"/>
         /// </summary>
         /// <param name="templateElements">Xml to import</param>
         /// <param name="userId">Optional user id</param>
         /// <returns>An enumerable list of generated Templates</returns>
-        public IReadOnlyList<ITemplate> ImportTemplates(IReadOnlyCollection<XElement> templateElements, int userId)
+        public async Task<IReadOnlyList<ITemplate>> ImportTemplatesAsync(IReadOnlyCollection<XElement> templateElements, int userId)
         {
             var templates = new List<ITemplate>();
 
@@ -1670,20 +1725,19 @@ public IReadOnlyList<ITemplate> ImportTemplates(IReadOnlyCollection<XElement> te
             {
                 var dependencies = new List<string>();
                 XElement elementCopy = tempElement;
-                //Ensure that the Master of the current template is part of the import, otherwise we ignore this dependency as part of the dependency sorting.
-                if (string.IsNullOrEmpty((string?)elementCopy.Element("Master")) == false &&
-                    templateElements.Any(x => (string?)x.Element("Alias") == (string?)elementCopy.Element("Master")))
+
+                //Ensure that the Master of the current template is part of the import, otherwise we ignore this dependency as part of the dependency sorting.'
+                var masterTemplate = _templateContentParserService.MasterTemplateAlias(tempElement.Value);
+                if (masterTemplate is not null && templateElements.Any(x => (string?)x.Element("Alias") == masterTemplate))
                 {
-                    dependencies.Add((string)elementCopy.Element("Master")!);
+                    dependencies.Add(masterTemplate);
                 }
-                else if (string.IsNullOrEmpty((string?)elementCopy.Element("Master")) == false &&
-                         templateElements.Any(x =>
-                             (string?)x.Element("Alias") == (string?)elementCopy.Element("Master")) == false)
+                else
                 {
                     _logger.LogInformation(
                         "Template '{TemplateAlias}' has an invalid Master '{TemplateMaster}', so the reference has been ignored.",
                         (string?)elementCopy.Element("Alias"),
-                        (string?)elementCopy.Element("Master"));
+                        masterTemplate);
                 }
 
                 graph.AddItem(TopoGraph.CreateNode((string)elementCopy.Element("Alias")!, elementCopy, dependencies));
@@ -1700,9 +1754,9 @@ public IReadOnlyList<ITemplate> ImportTemplates(IReadOnlyCollection<XElement> te
                 var design = templateElement.Element("Design")?.Value;
                 XElement? masterElement = templateElement.Element("Master");
 
-                var existingTemplate = _fileService.GetTemplate(alias) as Template;
+                var existingTemplate = await _templateService.GetAsync(alias) as Template;
 
-                Template? template = existingTemplate ?? new Template(_shortStringHelper, templateName, alias);
+                Template template = existingTemplate ?? new Template(_shortStringHelper, templateName, alias);
 
                 // For new templates, use the serialized key if avaialble.
                 if (existingTemplate == null && Guid.TryParse(templateElement.Element("Key")?.Value, out Guid key))
@@ -1725,9 +1779,16 @@ public IReadOnlyList<ITemplate> ImportTemplates(IReadOnlyCollection<XElement> te
                 templates.Add(template);
             }
 
-            if (templates.Any())
+            foreach (ITemplate template in templates)
             {
-                _fileService.SaveTemplate(templates, userId);
+                if (template.Id > 0)
+                {
+                    await _templateService.UpdateAsync(template, Constants.Security.SuperUserKey);
+                }
+                else
+                {
+                    await _templateService.CreateAsync(template, Constants.Security.SuperUserKey);
+                }
             }
 
             return templates;