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

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)
     {

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);
+
     /// <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);
+
     Guid GetContentTypeKey(XElement contentType);
 
     string? GetEntityTypeAlias(XElement entityType);

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)

src/Umbraco.Infrastructure/Packaging/PackageDataInstallation.cs

@@ -1,12 +1,14 @@
 using System.Globalization;
 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 @@
         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,22 +56,62 @@
             IShortStringHelper shortStringHelper,
             IConfigurationEditorJsonSerializer serializer,
             IMediaService mediaService,
-            IMediaTypeService mediaTypeService)
+            IMediaTypeService mediaTypeService,
+            ITemplateContentParserService templateContentParserService,
+            ITemplateService templateService)
         {
             _dataValueEditorFactory = dataValueEditorFactory;
             _logger = logger;
             _fileService = fileService;
             _localizationService = localizationService;
             _dataTypeService = dataTypeService;
             _entityService = entityService;
             _contentTypeService = contentTypeService;
             _contentService = contentService;
             _propertyEditors = propertyEditors;
             _scopeProvider = scopeProvider;
             _shortStringHelper = shortStringHelper;
             _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 @@
                   shortStringHelper,
                   serializer,
                   mediaService,
-                  mediaTypeService)
+                  mediaTypeService,
+                  StaticServiceProvider.Instance.GetRequiredService<ITemplateContentParserService>(),
+                  StaticServiceProvider.Instance.GetRequiredService<ITemplateService>())
         { }
 
         #region Install/Uninstall
@@ -1651,83 +1697,98 @@
 
         #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>();
 
             var graph = new TopoGraph<string, TopoGraph.Node<string, XElement>>(x => x.Key, x => x.Dependencies);
 
             foreach (XElement tempElement in templateElements)
             {
                 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));
             }
 
             //Sort templates by dependencies to a potential master template
             IEnumerable<TopoGraph.Node<string, XElement>> sorted = graph.GetSortedItems();
             foreach (TopoGraph.Node<string, XElement>? item in sorted)
             {
                 XElement templateElement = item.Item;
 
                 var templateName = templateElement.Element("Name")?.Value;
                 var alias = templateElement.Element("Alias")!.Value;
                 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))
                 {
                     template.Key = key;
                 }
 
                 template.Content = design;
 
                 if (masterElement != null && string.IsNullOrEmpty((string)masterElement) == false)
                 {
                     template.MasterTemplateAlias = masterElement.Value;
                     ITemplate? masterTemplate = templates.FirstOrDefault(x => x.Alias == masterElement.Value);
                     if (masterTemplate != null)
                     {
                         template.MasterTemplateId = new Lazy<int>(() => masterTemplate.Id);
                     }
                 }
 
                 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;