Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions uSync.BackOffice/SyncHandlers/Handlers/TemplateHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
using uSync.BackOffice.SyncHandlers.Models;
using uSync.Core;
using uSync.Core.Serialization;
using uSync.Core.Templates;

using static Umbraco.Cms.Core.Constants;

Expand All @@ -42,7 +43,7 @@ public class TemplateHandler : SyncHandlerLevelBase<ITemplate>, ISyncHandler, IS
INotificationAsyncHandler<MovingNotification<ITemplate>>
{
private readonly IFileSystem? _viewFileSystem;
private readonly ITemplateService _templateService;
private readonly ISyncTemplateService _templateService;

private readonly ITemplateContentParserService _templateContentParserService;

Expand All @@ -51,20 +52,20 @@ public class TemplateHandler : SyncHandlerLevelBase<ITemplate>, ISyncHandler, IS
public TemplateHandler(
ILogger<TemplateHandler> logger,
IEntityService entityService,
ITemplateService templateService,
FileSystems fileSystems,
ITemplateContentParserService templateContentParserService,
AppCaches appCaches,
IShortStringHelper shortStringHelper,
ISyncFileService syncFileService,
ISyncEventService mutexService,
ISyncConfigService uSyncConfig,
ISyncItemFactory syncItemFactory)
ISyncItemFactory syncItemFactory,
ISyncTemplateService syncTemplateService)
: base(logger, entityService, appCaches, shortStringHelper, syncFileService, mutexService, uSyncConfig, syncItemFactory)
{
_templateService = templateService;
_viewFileSystem = fileSystems.MvcViewsFileSystem;
_templateContentParserService = templateContentParserService;
_templateService = syncTemplateService;
}

/// <inheritdoc/>
Expand Down
11 changes: 6 additions & 5 deletions uSync.Core/Serialization/Serializers/ContentSerializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using uSync.Core.Extensions;
using uSync.Core.Mapping;
using uSync.Core.Models;
using uSync.Core.Templates;

namespace uSync.Core.Serialization.Serializers;

Expand All @@ -23,7 +24,7 @@ public class ContentSerializer : ContentSerializerBase<IContent>, ISyncSerialize
protected readonly IContentService contentService;
protected readonly IUserService userService;

protected readonly ITemplateService _templateService;
protected readonly ISyncTemplateService _templateService;
protected readonly ISyncDocumentUrlCleaner? _urlCleaner;

[Obsolete("Use the constructor with urlCleaner, will be removed in v19")]
Expand All @@ -36,8 +37,8 @@ public ContentSerializer(
IContentService contentService,
SyncValueMapperCollection syncMappers,
IUserService userService,
ITemplateService templateService
) : this(entityService, languageService, relationService, shortStringHelper, logger, contentService, syncMappers, userService, templateService, null)
ISyncTemplateService syncTemplateService
) : this(entityService, languageService, relationService, shortStringHelper, logger, contentService, syncMappers, userService, syncTemplateService, null)
{ }

public ContentSerializer(
Expand All @@ -49,15 +50,15 @@ public ContentSerializer(
IContentService contentService,
SyncValueMapperCollection syncMappers,
IUserService userService,
ITemplateService templateService,
ISyncTemplateService syncTemplateService,
ISyncDocumentUrlCleaner? urlCleaner)
: base(entityService, languageService, relationService, shortStringHelper, logger, UmbracoObjectTypes.Document, syncMappers)
{
this.contentService = contentService;

this.relationAlias = Constants.Conventions.RelationTypes.RelateParentDocumentOnDeleteAlias;
this.userService = userService;
_templateService = templateService;
_templateService = syncTemplateService;
_urlCleaner = urlCleaner;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using uSync.Core.Extensions;
using uSync.Core.Mapping;
using uSync.Core.Models;
using uSync.Core.Templates;

namespace uSync.Core.Serialization.Serializers;

Expand All @@ -30,7 +31,7 @@ public ContentTemplateSerializer(
IContentTypeService contentTypeService,
SyncValueMapperCollection syncMappers,
IUserService userService,
ITemplateService templateService,
ISyncTemplateService templateService,
ISyncDocumentUrlCleaner urlCleaner)
: base(entityService, languageService, relationService, shortStringHelper, logger, contentService, syncMappers, userService, templateService, urlCleaner)
{
Expand Down
24 changes: 14 additions & 10 deletions uSync.Core/Serialization/Serializers/TemplateSerializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,17 @@
using Umbraco.Extensions;

using uSync.Core.Models;
using uSync.Core.Templates;
using uSync.Core.Versions;

namespace uSync.Core.Serialization.Serializers;

[SyncSerializer("D0E0769D-CCAE-47B4-AD34-4182C587B08A", "Template Serializer", uSyncConstants.Serialization.Template)]
public class TemplateSerializer : SyncSerializerBase<ITemplate>, ISyncSerializer<ITemplate>
{
private readonly IShortStringHelper _shortStringHelper;
private readonly IFileSystem? _viewFileSystem;

private readonly ITemplateService _templateService;
private readonly ISyncTemplateService _templateService;
private readonly IUserIdKeyResolver _userIdKeyResolver;

private readonly uSyncCapabilityChecker _capabilityChecker;
Expand All @@ -38,17 +38,15 @@ public TemplateSerializer(
FileSystems fileSystems,
IConfiguration configuration,
uSyncCapabilityChecker capabilityChecker,
ITemplateService templateService,
IUserIdKeyResolver userIdKeyResolver)
IUserIdKeyResolver userIdKeyResolver,
ISyncTemplateService syncTemplateService)
Comment thread
KevinJump marked this conversation as resolved.
: base(entityService, logger)
{
_shortStringHelper = shortStringHelper;

_viewFileSystem = fileSystems.MvcViewsFileSystem;
_configuration = configuration;
_capabilityChecker = capabilityChecker;
_templateService = templateService;
_userIdKeyResolver = userIdKeyResolver;
_templateService = syncTemplateService;
}

protected override async Task<SyncAttempt<ITemplate>> ProcessDeleteAsync(Guid key, string alias, SerializerFlags flags)
Expand Down Expand Up @@ -103,7 +101,13 @@ protected override async Task<SyncAttempt<ITemplate>> DeserializeCoreAsync(XElem
userKey, key);

if (attempt.Success is false)
return SyncAttempt<ITemplate>.Fail(name, ChangeType.Import, "Failed to create template");
{
logger.LogWarning("Failed to create template {alias} {name} - {error} - {status}",
alias, name, attempt.Exception?.Message ?? "Unknown error", attempt.Status);

return SyncAttempt<ITemplate>.Fail(name, ChangeType.Import,
$"Failed to create template {alias} {name} - {attempt.Exception?.Message ?? "Unknown error"} - {attempt.Status}");
}

item = attempt.Result;
details.AddNew(alias, alias, "Template");
Expand All @@ -118,8 +122,8 @@ protected override async Task<SyncAttempt<ITemplate>> DeserializeCoreAsync(XElem
if (item is null)
{
// creating went wrong
logger.LogWarning("Failed to create template");
return SyncAttempt<ITemplate>.Fail(name, ChangeType.Import, "Failed to create template");
logger.LogWarning("Failed to create template - item is null after create process.");
return SyncAttempt<ITemplate>.Fail(name, ChangeType.Import, "Failed to create template - no new item created.");

Copilot AI Apr 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The if (item is null) block after the creation branch is effectively unreachable because the item is null case above returns on both success and failure paths. Keeping this block makes control flow confusing; consider removing it (or restructuring the earlier branch) so there’s a single, reachable null-handling path.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the unreachable if (item is null) block in cb1c7ca. The first if (item is null) block always exits via return on both the failure and success paths, so the second check was dead code.

}

if (item.Key != key)
Expand Down
16 changes: 16 additions & 0 deletions uSync.Core/Templates/ISyncTemplateService.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Services.OperationStatus;

namespace uSync.Core.Templates;

public interface ISyncTemplateService
{
Task<Attempt<ITemplate, TemplateOperationStatus>> CreateAsync(string name, string alias, string? content, Guid userKey, Guid key);
Task DeleteAsync(string alias, Guid userKey);
Task<ITemplate?> GetAsync(string alias);
Task<ITemplate?> GetAsync(Guid key);
Task<ITemplate?> GetAsync(int id);
Task<IEnumerable<ITemplate>> GetChildrenAsync(int templateId);
Task<Attempt<ITemplate, TemplateOperationStatus>> UpdateAsync(ITemplate template, Guid userKey);
}
104 changes: 104 additions & 0 deletions uSync.Core/Templates/SyncTemplateService.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
using Microsoft.Extensions.Configuration;

using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Persistence.Repositories;
using Umbraco.Cms.Core.Scoping;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Services.OperationStatus;
using Umbraco.Cms.Core.Strings;

using uSync.Core.Versions;

namespace uSync.Core.Templates;

/// <summary>
/// does creating of templates, espeically if we are in production mode.
/// </summary>
internal class SyncTemplateService : ISyncTemplateService
{
private readonly ITemplateService _templateService;
private readonly IConfiguration _configuration;
private readonly ICoreScopeProvider _scopeProvider;

private readonly ITemplateRepository _templateRepository;
private readonly IShortStringHelper _shortStringHelper;

public SyncTemplateService(ITemplateService templateService, IConfiguration configuration, ICoreScopeProvider scopeProvider, ITemplateRepository templateRepository, IShortStringHelper shortStringHelper)
{
_templateService = templateService;
_configuration = configuration;
_scopeProvider = scopeProvider;
_templateRepository = templateRepository;
_shortStringHelper = shortStringHelper;
}

/// <summary>
/// creates a template, but only if we are in production mode.
/// </summary>
/// <param name="name"></param>
/// <param name="alias"></param>
/// <param name="description"></param>
/// <returns></returns>
Comment thread
KevinJump marked this conversation as resolved.
public async Task<Attempt<ITemplate, TemplateOperationStatus>> CreateAsync(string name, string alias, string? content, Guid userKey, Guid key)
{
TemplateOperationStatus lastKnownStatus = TemplateOperationStatus.Success;

if (IsInProductionMode() is false)
{
var attempt = await _templateService.CreateAsync(name, alias, content, userKey, key);
if (attempt.Success)
return attempt;
}

// else - lets try the repository way (surely this is a hack?)
// https://github.com/umbraco/Umbraco-CMS/pull/21600#issuecomment-4205232583
return await CreateTemplateInternal(name, alias, content, userKey, key, lastKnownStatus);

Copilot AI Apr 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lastKnownStatus is initialized to TemplateOperationStatus.Success and never updated when _templateService.CreateAsync(...) fails. If the repository save then throws, this method will return a failed Attempt with a Success status, which is misleading for callers and logs. Capture the failed attempt’s status (and potentially return the original attempt when not in production) so failure status accurately reflects what happened.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 025f2de. CreateAsync now captures the failed attempt and returns it directly unless the status is NotAllowedInProductionMode — in that case it falls through to the repository path. The CreateTemplateInternal catch now also uses TemplateOperationStatus.NotAllowedInProductionMode as the failure status instead of Success.

Copilot AI Apr 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CreateAsync falls back to repository creation even when not in production mode if _templateService.CreateAsync(...) fails for any reason (e.g., validation/duplicate alias). That bypasses service-level validation/eventing and can create templates in cases the service correctly rejected. Consider only using the repository workaround when in production mode (or when the failure status is the specific “not allowed in production” case), otherwise return the original failed attempt.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 025f2de. CreateAsync now only falls back to the repository workaround when: (1) already in production mode, or (2) not in production mode but the service returned NotAllowedInProductionMode. Any other service failure (validation, duplicate alias, etc.) is returned directly to the caller.

}

private async Task<Attempt<ITemplate, TemplateOperationStatus>> CreateTemplateInternal(string name, string alias, string? content, Guid userKey, Guid key, TemplateOperationStatus lastKnownStatus)
{
var template = new Template(_shortStringHelper, name, alias)
{
Content = content,
Key = key,
};

try
{
using (var scope = _scopeProvider.CreateCoreScope(autoComplete: true))
{
_templateRepository.Save(template);
scope.Complete();

Copilot AI Apr 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scope usage is inconsistent: CreateCoreScope(autoComplete: true) implies the scope will complete on dispose, but the code also calls scope.Complete(). This is redundant at best and may be confusing for future changes; prefer either autoComplete: false + explicit Complete(), or keep autoComplete: true and remove the explicit Complete() call (matching other uSync code such as SyncDocumentUrlCleaner).

Suggested change
scope.Complete();

Copilot uses AI. Check for mistakes.

return Attempt.SucceedWithStatus<ITemplate, TemplateOperationStatus>(TemplateOperationStatus.Success, template);
}
}
catch (Exception ex)
{
return Attempt.FailWithStatus<ITemplate, TemplateOperationStatus>(lastKnownStatus, template, ex);

Copilot AI Apr 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CreateTemplateInternal is marked async but contains no await, which adds unnecessary state-machine overhead and can hide synchronous exceptions behind an async boundary. Either make it synchronous and return Task.FromResult(...)/Task directly, or introduce real async work (there currently isn’t any).

Suggested change
private async Task<Attempt<ITemplate, TemplateOperationStatus>> CreateTemplateInternal(string name, string alias, string? content, Guid userKey, Guid key, TemplateOperationStatus lastKnownStatus)
{
var template = new Template(_shortStringHelper, name, alias)
{
Content = content,
Key = key,
};
try
{
using (var scope = _scopeProvider.CreateCoreScope(autoComplete: true))
{
_templateRepository.Save(template);
scope.Complete();
return Attempt.SucceedWithStatus<ITemplate, TemplateOperationStatus>(TemplateOperationStatus.Success, template);
}
}
catch (Exception ex)
{
return Attempt.FailWithStatus<ITemplate, TemplateOperationStatus>(lastKnownStatus, template, ex);
private Task<Attempt<ITemplate, TemplateOperationStatus>> CreateTemplateInternal(string name, string alias, string? content, Guid userKey, Guid key, TemplateOperationStatus lastKnownStatus)
{
ITemplate? template = null;
try
{
template = new Template(_shortStringHelper, name, alias)
{
Content = content,
Key = key,
};
using (var scope = _scopeProvider.CreateCoreScope(autoComplete: true))
{
_templateRepository.Save(template);
scope.Complete();
return Task.FromResult(Attempt.SucceedWithStatus<ITemplate, TemplateOperationStatus>(TemplateOperationStatus.Success, template));
}
}
catch (Exception ex)
{
return Task.FromResult(Attempt.FailWithStatus<ITemplate, TemplateOperationStatus>(lastKnownStatus, template, ex));

Copilot uses AI. Check for mistakes.
}
}


public async Task<ITemplate?> GetAsync(Guid key)
=> await _templateService.GetAsync(key);

public async Task<ITemplate?> GetAsync(string alias)
=> await _templateService.GetAsync(alias);

public async Task<ITemplate?> GetAsync(int id)
=> await _templateService.GetAsync(id);

public async Task<Attempt<ITemplate, TemplateOperationStatus>> UpdateAsync(ITemplate template, Guid userKey)
=> await _templateService.UpdateAsync(template, userKey);

public async Task DeleteAsync(string alias, Guid userKey)
=> await _templateService.DeleteAsync(alias, userKey);

public async Task<IEnumerable<ITemplate>> GetChildrenAsync(int templateId)
=> await _templateService.GetChildrenAsync(templateId);

private bool IsInProductionMode()
=> _configuration.IsUmbracoRunningInProductionMode();
}
4 changes: 4 additions & 0 deletions uSync.Core/uSyncCoreBuilderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using uSync.Core.Mapping.Tracking;
using uSync.Core.Roots.Configs;
using uSync.Core.Serialization;
using uSync.Core.Templates;
using uSync.Core.Tracking;

namespace uSync.Core;
Expand Down Expand Up @@ -41,6 +42,9 @@ public static IUmbracoBuilder AdduSyncCore(this IUmbracoBuilder builder)
// cache for entity items, we use it to speed up lookups.
builder.Services.AddSingleton<SyncEntityCache>();

// templates have a wrapper service because we have to do work when in production mode
builder.Services.AddSingleton<ISyncTemplateService, SyncTemplateService>();

// register *all* ConfigurationSerializers except those marked [HideFromTypeFinder]
// has to happen before the DataTypeSerializer is loaded, because that is where
// they are used
Expand Down
Loading