Skip to content

Commit

Permalink
V14: Fix templates not having set master template on package install (#…
Browse files Browse the repository at this point in the history
…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]>
  • Loading branch information
3 people authored Sep 17, 2024
1 parent 991b9a7 commit f72c39e
Show file tree
Hide file tree
Showing 9 changed files with 110 additions and 26 deletions.
2 changes: 1 addition & 1 deletion Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@

<!-- Package Validation -->
<PropertyGroup>
<GenerateCompatibilitySuppressionFile>true</GenerateCompatibilitySuppressionFile>
<GenerateCompatibilitySuppressionFile>false</GenerateCompatibilitySuppressionFile>
<EnablePackageValidation>true</EnablePackageValidation>
<PackageValidationBaselineVersion>14.0.0</PackageValidationBaselineVersion>
<EnableStrictModeForCompatibleFrameworksInPackage>true</EnableStrictModeForCompatibleFrameworksInPackage>
Expand Down
13 changes: 12 additions & 1 deletion src/Umbraco.Core/Media/EmbedProviders/OEmbedResponse.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down
2 changes: 0 additions & 2 deletions src/Umbraco.Core/Services/FileService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
4 changes: 2 additions & 2 deletions src/Umbraco.Core/Services/IDataTypeService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)));
}
12 changes: 12 additions & 0 deletions src/Umbraco.Core/Services/IPackageDataInstallation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,7 @@ public enum ContentTypeOperationStatus
Success,
DuplicateAlias,
InvalidAlias,
NameCannotBeEmpty,
NameTooLong,
InvalidPropertyTypeAlias,
PropertyTypeAliasCannotEqualContentTypeAlias,
DuplicatePropertyTypeAlias,
DataTypeNotFound,
InvalidInheritance,
Expand All @@ -21,6 +18,9 @@ public enum ContentTypeOperationStatus
NotFound,
NotAllowed,
CancelledByNotification,
PropertyTypeAliasCannotEqualContentTypeAlias,
NameCannotBeEmpty,
NameTooLong,
InvalidElementFlagDocumentHasContent,
InvalidElementFlagElementIsUsedInPropertyEditorConfiguration,
InvalidElementFlagComparedToParent,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
91 changes: 76 additions & 15 deletions src/Umbraco.Infrastructure/Packaging/PackageDataInstallation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -52,7 +56,9 @@ public PackageDataInstallation(
IShortStringHelper shortStringHelper,
IConfigurationEditorJsonSerializer serializer,
IMediaService mediaService,
IMediaTypeService mediaTypeService)
IMediaTypeService mediaTypeService,
ITemplateContentParserService templateContentParserService,
ITemplateService templateService)
{
_dataValueEditorFactory = dataValueEditorFactory;
_logger = logger;
Expand All @@ -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
Expand Down Expand Up @@ -103,7 +147,9 @@ public PackageDataInstallation(
shortStringHelper,
serializer,
mediaService,
mediaTypeService)
mediaTypeService,
StaticServiceProvider.Instance.GetRequiredService<ITemplateContentParserService>(),
StaticServiceProvider.Instance.GetRequiredService<ITemplateService>())
{ }

#region Install/Uninstall
Expand Down Expand Up @@ -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>();

Expand All @@ -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));
Expand All @@ -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))
Expand All @@ -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;
Expand Down

0 comments on commit f72c39e

Please sign in to comment.