diff --git a/uSync.BackOffice/SyncHandlers/Handlers/TemplateHandler.cs b/uSync.BackOffice/SyncHandlers/Handlers/TemplateHandler.cs index 86a45463..ac9a6ab3 100644 --- a/uSync.BackOffice/SyncHandlers/Handlers/TemplateHandler.cs +++ b/uSync.BackOffice/SyncHandlers/Handlers/TemplateHandler.cs @@ -107,7 +107,8 @@ private async Task GetTemplateContentAsync(string alias) { if (_viewFileSystem is null) return string.Empty; - var templateFileName = _viewFileSystem.GetRelativePath(alias.Replace(" ", "") + ".cshtml"); + // Umbraco names the view file from the alias verbatim, so we have to do the same. + var templateFileName = _viewFileSystem.GetRelativePath(alias + ".cshtml"); if (templateFileName is null) return string.Empty; if (_viewFileSystem.FileExists(templateFileName) is false) return string.Empty; diff --git a/uSync.Core/Serialization/Serializers/TemplateSerializer.cs b/uSync.Core/Serialization/Serializers/TemplateSerializer.cs index 2cfa340c..c1299890 100644 --- a/uSync.Core/Serialization/Serializers/TemplateSerializer.cs +++ b/uSync.Core/Serialization/Serializers/TemplateSerializer.cs @@ -1,6 +1,4 @@ -using Lucene.Net.Queries.Function.ValueSources; - -using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; @@ -21,7 +19,6 @@ namespace uSync.Core.Serialization.Serializers; [SyncSerializer("D0E0769D-CCAE-47B4-AD34-4182C587B08A", "Template Serializer", uSyncConstants.Serialization.Template)] public class TemplateSerializer : SyncSerializerBase, ISyncSerializer { - private readonly IShortStringHelper _shortStringHelper; private readonly IFileSystem? _viewFileSystem; private readonly ITemplateService _templateService; @@ -34,6 +31,7 @@ public class TemplateSerializer : SyncSerializerBase, ISyncSerializer public TemplateSerializer( IEntityService entityService, ILogger logger, + // shortStringHelper is no longer used, but is kept so we don't break the constructor signature. IShortStringHelper shortStringHelper, FileSystems fileSystems, IConfiguration configuration, @@ -42,8 +40,6 @@ public TemplateSerializer( IUserIdKeyResolver userIdKeyResolver) : base(entityService, logger) { - _shortStringHelper = shortStringHelper; - _viewFileSystem = fileSystems.MvcViewsFileSystem; _configuration = configuration; _capabilityChecker = capabilityChecker; @@ -87,14 +83,16 @@ protected override async Task> DeserializeCoreAsync(XElem var alias = node.GetAlias(); var name = node.Element("Name").ValueOrDefault(string.Empty); - var contentAttempt = GetContentForTemplate(node, options); - if (!contentAttempt) return SyncAttempt.Fail(name, ChangeType.Import, contentAttempt.Exception?.Message ?? "Failed to get content"); - var details = new List(); var item = await FindTemplateFromNodeAsync(node); if (item is null) { + // we only need the content when we are creating the template, when we are updating + // it either comes from the node (below) or it is already on disk. + var contentAttempt = await GetContentForTemplateAsync(node, options); + if (!contentAttempt) return SyncAttempt.Fail(name, ChangeType.Import, contentAttempt.Exception?.Message ?? "Failed to get content"); + var userKey = await _userIdKeyResolver.GetAsync(options.UserId); var attempt = await _templateService.CreateAsync( name, @@ -152,8 +150,9 @@ protected override async Task> DeserializeCoreAsync(XElem return SyncAttempt.Succeed(item.Name, item, ChangeType.Import, details); } - private Attempt GetContentForTemplate(XElement node, SyncSerializerOptions options) + private async Task> GetContentForTemplateAsync(XElement node, SyncSerializerOptions options) { + // if the setup is configured this way, we get template content from the xml file directly. if (ShouldGetContentFromNode(node, options)) { if (logger.IsEnabled(LogLevel.Debug)) @@ -162,6 +161,32 @@ protected override async Task> DeserializeCoreAsync(XElem return Attempt.Succeed(GetContentFromConfig(node)); } + // if not, try and fetch the content the way umbraco does (via the template service) + var templateFileName = node.GetAlias(); + if (templateFileName.EndsWith(".cshtml", StringComparison.InvariantCultureIgnoreCase) is false) + templateFileName = $"{templateFileName}.cshtml"; + + // note: the template service returns Stream.Null (not null) when the file is missing, + // and an empty file tells us nothing - so both mean 'look somewhere else'. + var stream = await _templateService.GetFileContentStreamAsync(templateFileName); + if (stream is not null && stream != Stream.Null) + { + await using (stream) + { + using var sr = new StreamReader(stream); + var fileContent = await sr.ReadToEndAsync(); + + if (string.IsNullOrWhiteSpace(fileContent) is false) + { + if (logger.IsEnabled(LogLevel.Debug)) + logger.LogDebug("Reading {path} contents from template service", templateFileName); + + return Attempt.Succeed(fileContent); + } + } + } + + // if not - then old-school, attempt to get content from the viewFileSystem var templatePath = ViewPath(node.GetAlias()); if (templatePath is not null && _viewFileSystem?.FileExists(templatePath) is true) { @@ -171,7 +196,8 @@ protected override async Task> DeserializeCoreAsync(XElem return Attempt.Succeed(GetContentFromFile(templatePath)); } - // isn't on disk, but might be compiled. --> + // if we get here, we've failed to get the content from anywhere, so it might be missing or its + // compiled into the site, and in some dll (although the template service should fetch this?) if (ViewsAreCompiled(options) is true) { @@ -179,8 +205,14 @@ protected override async Task> DeserializeCoreAsync(XElem // if this finds the view it tells us that the view is somewhere else ? if (logger.IsEnabled(LogLevel.Debug)) logger.LogDebug("Failed to find content, but UsingRazorViews so if Umbraco creates anyway, we will then delete the file"); - - return Attempt.Succeed($""); + + // internally Umbraco parses the content for the master (from the Layout value) so we + // need to fake that. + var master = node.Element("Parent")?.ValueOrDefault(string.Empty); + var layout = string.IsNullOrWhiteSpace(master) ? "null" : $"\"{master}.cshtml\""; + + return Attempt.Succeed($"@{{\n Layout = {layout};\n}}\n" + + $""); } // template is missing and the views are not compiled , then we can't create. @@ -197,37 +229,28 @@ protected override async Task> DeserializeCoreAsync(XElem /// private bool ShouldGetContentFromNode(XElement node, SyncSerializerOptions options) { - if (_capabilityChecker != null - && _configuration != null - && _capabilityChecker.HasRuntimeMode) - { - if (node.Element("Contents") != null) - { - if (ViewsAreCompiled(options)) - { - if (logger.IsEnabled(LogLevel.Debug)) - logger.LogDebug("Template contents will not be imported because site is running in Production mode"); - - return false; - } + // no content in the file, so there is nothing to take from it. + if (node.Element("Contents") is null) return false; - // else (we have content - not running in Production) - return true; - } + // on a version of Umbraco that has runtime modes, we don't import the content + // in Production, because the views will be compiled into the site. + if (_capabilityChecker.HasRuntimeMode && ViewsAreCompiled(options)) + { + if (logger.IsEnabled(LogLevel.Debug)) + logger.LogDebug("Template contents will not be imported because site is running in Production mode"); - // else (we don't have content it doesn't matter) return false; } - // default - return node.Element("Contents") != null; - // && options.GetSetting(uSyncConstants.Conventions.IncludeContent, false); + // note: we don't check the IncludeContent setting here - that setting controls + // whether we *export* the content, if it's in the file we will import it. + return true; } public static string GetContentFromConfig(XElement node) => node.Element("Contents").ValueOrDefault(string.Empty); - public string GetContentFromFile(string templatePath) + private string GetContentFromFile(string templatePath) { try { @@ -238,29 +261,18 @@ public string GetContentFromFile(string templatePath) return System.IO.File.ReadAllText(templateFilePath); } } - catch(Exception ex) + catch (Exception ex) { + // any failure here is not fatal, we fall back to the filesystem provider below. logger.LogWarning(ex, "Error reading template, will read from filesystem provider instead"); } - - // via the file system, which does work, but occasionaly it locks. - var content = ""; - using (var stream = _viewFileSystem?.OpenFile(templatePath)) - { - if (stream is null) return content; - using (var sr = new StreamReader(stream)) - { - content = sr.ReadToEnd(); - sr.Close(); - sr.Dispose(); - } - - stream.Close(); - stream.Dispose(); - } + // via the file system, which does work, but occasionaly it locks. + using var stream = _viewFileSystem?.OpenFile(templatePath); + if (stream is null) return string.Empty; - return content; + using var sr = new StreamReader(stream); + return sr.ReadToEnd(); } public override async Task> DeserializeSecondPassAsync(ITemplate item, XElement node, SyncSerializerOptions options) @@ -390,19 +402,10 @@ public override async Task DeleteItemAsync(ITemplate item) public override string ItemAlias(ITemplate item) => item.Alias; - /// - /// we clean the content out of the template, - /// We don't care if the content has changed during a normal serialization - /// - protected override XElement CleanseNode(XElement node) - { - node.Element("Content")?.Remove(); - return base.CleanseNode(node); - } - - + // Umbraco names the view file from the alias verbatim (see TemplateRepository.SetVirtualPath) + // so we have to do the same, or we look for/delete the wrong file for aliases with spaces. private string? ViewPath(string alias) - => _viewFileSystem?.GetRelativePath(alias.Replace(" ", "") + ".cshtml"); + => _viewFileSystem?.GetRelativePath(alias + ".cshtml"); private bool ViewsAreCompiled(SyncSerializerOptions options) => _configuration.IsUmbracoRunningInProductionMode() diff --git a/uSync.Tests/Serializers/TemplateSerializerTests.cs b/uSync.Tests/Serializers/TemplateSerializerTests.cs index 2cb803f3..e8397d30 100644 --- a/uSync.Tests/Serializers/TemplateSerializerTests.cs +++ b/uSync.Tests/Serializers/TemplateSerializerTests.cs @@ -1,4 +1,6 @@ using System; +using System.Collections.Generic; +using System.IO; using System.Threading.Tasks; using System.Xml.Linq; @@ -29,9 +31,8 @@ namespace uSync.Tests.Serializers; /// -/// guards against #1044 - a failed template create used to be reported as a -/// hardcoded "Failed to create template" with no exception/status, hiding -/// why the import failed (e.g. only reproducible on IIS/Production). +/// tests for importing templates - how we report failures, and how we get +/// the content for a template when there is no .cshtml file on disk. /// [TestFixture] public class TemplateSerializerTests @@ -80,6 +81,12 @@ private static FileSystems BuildFileSystems() ioHelper.Setup(x => x.ResolveUrl(It.IsAny())).Returns(path => "/" + path.TrimStart('~', '/')); #pragma warning restore CS0618 + // PhysicalFileSystem uses this to check a path is inside its root - without it every + // path looks like it's outside the root, and FileExists throws instead of returning false. + ioHelper.Setup(x => x.PathStartsWith(It.IsAny(), It.IsAny(), It.IsAny())) + .Returns((string path, string root, char[] separators) + => path.StartsWith(root, StringComparison.OrdinalIgnoreCase)); + return new FileSystems( Mock.Of(), ioHelper.Object, @@ -87,13 +94,29 @@ private static FileSystems BuildFileSystems() hostingEnvironment.Object); } - private static XElement BuildTemplateNode(Guid key, string alias, string name) - => new XElement("Template", + /// + /// build a template node as it would appear in a .config file. Contents is optional - + /// it's only in the file when the IncludeContent setting is on. + /// + private static XElement BuildTemplateNode(Guid key, string alias, string name, string parent = null, string contents = null) + { + var node = new XElement("Template", new XAttribute(uSyncConstants.Xml.Key, key), new XAttribute(uSyncConstants.Xml.Alias, alias), new XElement("Name", name), - new XElement("Contents", new XCData("@{ Layout = null; }"))); + new XElement("Parent", parent ?? string.Empty)); + if (contents is not null) + node.Add(new XElement("Contents", new XCData(contents))); + + return node; + } + + /// + /// guards against #1044 - a failed template create used to be reported as a + /// hardcoded "Failed to create template" with no exception/status, hiding + /// why the import failed (e.g. only reproducible on IIS/Production). + /// [Test] public async Task Create_WhenTemplateServiceFails_ReturnsStatusAndException() { @@ -105,7 +128,7 @@ public async Task Create_WhenTemplateServiceFails_ReturnsStatusAndException() .Setup(x => x.CreateAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) .ReturnsAsync(Attempt.Fail(TemplateOperationStatus.DuplicateAlias)); - var node = BuildTemplateNode(Guid.NewGuid(), "linkTreeNew", "LinkTreeNew"); + var node = BuildTemplateNode(Guid.NewGuid(), "linkTreeNew", "LinkTreeNew", contents: "@{ Layout = null; }"); // act var result = await _serializer.DeserializeAsync(node, new SyncSerializerOptions()); @@ -132,10 +155,146 @@ public async Task Create_WhenTemplateServiceSucceeds_ReturnsSucceededAttempt() .Setup(x => x.CreateAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) .ReturnsAsync(Attempt.Succeed(TemplateOperationStatus.Success, created)); - var node = BuildTemplateNode(Guid.NewGuid(), "linkTreeNew", "LinkTreeNew"); + var node = BuildTemplateNode(Guid.NewGuid(), "linkTreeNew", "LinkTreeNew", contents: "@{ Layout = null; }"); var result = await _serializer.DeserializeAsync(node, new SyncSerializerOptions()); Assert.That(result.Success, Is.True); } + + #region No template file on disk + + /// + /// when the views are compiled into the site, there is no .cshtml on disk to read, so we + /// hand Umbraco a placeholder. Umbraco works the parent out by parsing the Layout value + /// out of that content, so the placeholder has to be something its parser understands. + /// + [Test] + public async Task Create_WhenNoFileOnDiskAndTemplateHasParent_PlaceholderContentSetsTheMaster() + { + // arrange: no files on disk, and the views are compiled (razor views mode). + var created = SetupTemplateServiceWithNoFilesOnDisk(); + var options = RazorViewOptions(); + + // act: the parent has to exist before the child can reference it. + var parentResult = await _serializer.DeserializeAsync( + BuildTemplateNode(Guid.NewGuid(), "master", "Master"), options); + + var childResult = await _serializer.DeserializeAsync( + BuildTemplateNode(Guid.NewGuid(), "childOfMaster", "Child Of Master", parent: "master"), options); + + // assert: both imported, and the content we sent for the child parses back to the parent + // alias using Umbraco's own parser - which is how the master template actually gets set. + var childContent = created["childOfMaster"]; + var parsedMaster = new TemplateContentParserService().MasterTemplateAlias(childContent); + + Assert.Multiple(() => + { + Assert.That(parentResult.Success, Is.True); + Assert.That(childResult.Success, Is.True); + Assert.That(parsedMaster, Is.EqualTo("master")); + + // and it's marked, so the second pass knows to delete the file Umbraco writes out. + Assert.That(childContent, Does.Contain($"[uSyncMarker:{_serializer.Id}]")); + }); + } + + /// + /// same path, but for a root template - the placeholder should tell Umbraco there is + /// no master, rather than pointing it at a template called "". + /// + [Test] + public async Task Create_WhenNoFileOnDiskAndTemplateHasNoParent_PlaceholderContentSetsNoMaster() + { + var created = SetupTemplateServiceWithNoFilesOnDisk(); + + var result = await _serializer.DeserializeAsync( + BuildTemplateNode(Guid.NewGuid(), "master", "Master"), RazorViewOptions()); + + var content = created["master"]; + var parsedMaster = new TemplateContentParserService().MasterTemplateAlias(content); + + Assert.Multiple(() => + { + Assert.That(result.Success, Is.True); + Assert.That(parsedMaster, Is.Null); + Assert.That(content, Does.Contain($"[uSyncMarker:{_serializer.Id}]")); + }); + } + + /// + /// when the views are not compiled a missing file is a genuine error - we can't create a + /// template from nothing. Umbraco's template service hands back Stream.Null (not null) for + /// a file that isn't there, so we have to spot that or we silently create empty templates. + /// + [Test] + public async Task Create_WhenNoFileOnDiskAndViewsAreNotCompiled_FailsWithMissingFile() + { + SetupTemplateServiceWithNoFilesOnDisk(); + + // note: no UsingRazorViews setting this time. + var result = await _serializer.DeserializeAsync( + BuildTemplateNode(Guid.NewGuid(), "orphan", "Orphan"), new SyncSerializerOptions()); + + Assert.Multiple(() => + { + Assert.That(result.Success, Is.False); + Assert.That(result.Message, Does.Contain("missing")); + }); + } + + /// + /// stands in for a site with no .cshtml files on disk (they are compiled into the site), + /// and remembers what we create so a child import can find its parent. + /// + /// the content we passed to the template service, keyed by alias. + private Dictionary SetupTemplateServiceWithNoFilesOnDisk() + { + var templates = new Dictionary(StringComparer.InvariantCultureIgnoreCase); + var content = new Dictionary(StringComparer.InvariantCultureIgnoreCase); + + // this is what Umbraco's TemplateRepository returns when the file isn't there. + _templateServiceMock.Setup(x => x.GetFileContentStreamAsync(It.IsAny())) + .ReturnsAsync(Stream.Null); + + _templateServiceMock.Setup(x => x.GetAsync(It.IsAny())) + .ReturnsAsync((string alias) => templates.TryGetValue(alias, out var found) ? found : null); + + _templateServiceMock.Setup(x => x.GetAsync(It.IsAny())) + .ReturnsAsync((Guid key) => + { + foreach (var template in templates.Values) + { + if (template.Key == key) return template; + } + return null; + }); + + _templateServiceMock + .Setup(x => x.CreateAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync((string name, string alias, string templateContent, Guid userKey, Guid? key) => + { + var template = new Template(Mock.Of(), name, alias) + { + Content = templateContent + }; + + if (key is not null) template.Key = key.Value; + + templates[alias] = template; + content[alias] = templateContent; + + return Attempt.Succeed(TemplateOperationStatus.Success, template); + }); + + return content; + } + + private static SyncSerializerOptions RazorViewOptions() + => new SyncSerializerOptions(new Dictionary + { + [uSyncConstants.DefaultSettings.UsingRazorViews] = true + }); + + #endregion }