From 46acd51759c4d02493b6bbb5a68e77c1324742c3 Mon Sep 17 00:00:00 2001 From: Sven Geusens Date: Tue, 2 Jul 2024 14:22:19 +0200 Subject: [PATCH] [V14] Make the backend work with the new localLinks format (#16661) * Support new localLink format in core link parsing * Updated devliery api to work with the new locallinks format Added tests for old and new format handling. * Fix error regarding type attribute not always being present (for example old format or non local links) --- .../Templates/HtmlLocalLinkParser.cs | 33 +++ .../DeliveryApi/ApiRichTextElementParser.cs | 6 + .../DeliveryApi/ApiRichTextMarkupParser.cs | 3 +- .../DeliveryApi/ApiRichTextParserBase.cs | 83 +++++++- .../Templates/HtmlLocalLinkParserTests.cs | 80 ++++++- .../ApiRichTextMarkupParserTests.cs | 200 ++++++++++++++++++ 6 files changed, 392 insertions(+), 13 deletions(-) create mode 100644 tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/DeliveryApi/ApiRichTextMarkupParserTests.cs diff --git a/src/Umbraco.Core/Templates/HtmlLocalLinkParser.cs b/src/Umbraco.Core/Templates/HtmlLocalLinkParser.cs index 059f5f9cef..74105da511 100644 --- a/src/Umbraco.Core/Templates/HtmlLocalLinkParser.cs +++ b/src/Umbraco.Core/Templates/HtmlLocalLinkParser.cs @@ -10,6 +10,13 @@ namespace Umbraco.Cms.Core.Templates; /// public sealed class HtmlLocalLinkParser { + // needs to support media and document links, order of attributes should not matter nor should other attributes mess with things + // media + // other page + internal static readonly Regex LocalLinkTagPattern = new( + @"document|media)['""].*?(?href=[""']/{localLink:(?[a-fA-F0-9-]+)})[""'])|((?href=[""']/{localLink:(?[a-fA-F0-9-]+)})[""'].*?type=(['""])(?document|media)(?:['""])))|(?:(?:type=['""](?document|media)['""])|(?:(?href=[""']/{localLink:[a-fA-F0-9-]+})[""'])))[^>]*>", + RegexOptions.IgnoreCase | RegexOptions.IgnorePatternWhitespace); + internal static readonly Regex LocalLinkPattern = new( @"href=""[/]?(?:\{|\%7B)localLink:([a-zA-Z0-9-://]+)(?:\}|\%7D)", RegexOptions.IgnoreCase | RegexOptions.IgnorePatternWhitespace); @@ -105,6 +112,32 @@ public sealed class HtmlLocalLinkParser } private IEnumerable<(int? intId, GuidUdi? udi, string tagValue)> FindLocalLinkIds(string text) + { + MatchCollection localLinkTagMatches = LocalLinkTagPattern.Matches(text); + foreach (Match linkTag in localLinkTagMatches) + { + if (linkTag.Groups.Count < 1) + { + continue; + } + + if (Guid.TryParse(linkTag.Groups["guid"].Value, out Guid guid) is false) + { + continue; + } + + yield return (null, new GuidUdi(linkTag.Groups["type"].Value, guid), linkTag.Groups["locallink"].Value); + } + + // also return legacy results for values that have not been migrated + foreach ((int? intId, GuidUdi? udi, string tagValue) legacyResult in FindLegacyLocalLinkIds(text)) + { + yield return legacyResult; + } + } + + // todo remove at some point? + private IEnumerable<(int? intId, GuidUdi? udi, string tagValue)> FindLegacyLocalLinkIds(string text) { // Parse internal links MatchCollection tags = LocalLinkPattern.Matches(text); diff --git a/src/Umbraco.Infrastructure/DeliveryApi/ApiRichTextElementParser.cs b/src/Umbraco.Infrastructure/DeliveryApi/ApiRichTextElementParser.cs index eed3b848eb..61b6418ae7 100644 --- a/src/Umbraco.Infrastructure/DeliveryApi/ApiRichTextElementParser.cs +++ b/src/Umbraco.Infrastructure/DeliveryApi/ApiRichTextElementParser.cs @@ -132,9 +132,15 @@ internal sealed class ApiRichTextElementParser : ApiRichTextParserBase, IApiRich return; } + if (attributes.ContainsKey("type") is false || attributes["type"] is not string type) + { + type = "unknown"; + } + ReplaceLocalLinks( publishedSnapshot, href, + type, route => { attributes["route"] = route; diff --git a/src/Umbraco.Infrastructure/DeliveryApi/ApiRichTextMarkupParser.cs b/src/Umbraco.Infrastructure/DeliveryApi/ApiRichTextMarkupParser.cs index 42c8829868..338966dc39 100644 --- a/src/Umbraco.Infrastructure/DeliveryApi/ApiRichTextMarkupParser.cs +++ b/src/Umbraco.Infrastructure/DeliveryApi/ApiRichTextMarkupParser.cs @@ -52,8 +52,9 @@ internal sealed class ApiRichTextMarkupParser : ApiRichTextParserBase, IApiRichT foreach (HtmlNode link in links) { ReplaceLocalLinks( - publishedSnapshot, + publishedSnapshot, link.GetAttributeValue("href", string.Empty), + link.GetAttributeValue("type", "unknown"), route => { link.SetAttributeValue("href", route.Path); diff --git a/src/Umbraco.Infrastructure/DeliveryApi/ApiRichTextParserBase.cs b/src/Umbraco.Infrastructure/DeliveryApi/ApiRichTextParserBase.cs index 7723fc835c..407bc1a022 100644 --- a/src/Umbraco.Infrastructure/DeliveryApi/ApiRichTextParserBase.cs +++ b/src/Umbraco.Infrastructure/DeliveryApi/ApiRichTextParserBase.cs @@ -4,6 +4,7 @@ using Umbraco.Cms.Core.DeliveryApi; using Umbraco.Cms.Core.Models.DeliveryApi; using Umbraco.Cms.Core.Models.PublishedContent; using Umbraco.Cms.Core.PublishedCache; +using Umbraco.Cms.Core.Templates; namespace Umbraco.Cms.Infrastructure.DeliveryApi; @@ -18,20 +19,35 @@ internal abstract partial class ApiRichTextParserBase _apiMediaUrlProvider = apiMediaUrlProvider; } - protected void ReplaceLocalLinks(IPublishedSnapshot publishedSnapshot, string href, Action handleContentRoute, Action handleMediaUrl, Action handleInvalidLink) + protected void ReplaceLocalLinks(IPublishedSnapshot publishedSnapshot, string href, string type, Action handleContentRoute, Action handleMediaUrl, Action handleInvalidLink) + { + ReplaceStatus replaceAttempt = ReplaceLocalLink(publishedSnapshot, href, type, handleContentRoute, handleMediaUrl); + if (replaceAttempt == ReplaceStatus.Success) + { + return; + } + + if (replaceAttempt == ReplaceStatus.InvalidEntityType || ReplaceLegacyLocalLink(publishedSnapshot, href, handleContentRoute, handleMediaUrl) == ReplaceStatus.InvalidEntityType) + { + handleInvalidLink(); + } + } + + private ReplaceStatus ReplaceLocalLink(IPublishedSnapshot publishedSnapshot, string href, string type, Action handleContentRoute, Action handleMediaUrl) { Match match = LocalLinkRegex().Match(href); if (match.Success is false) { - return; + return ReplaceStatus.NoMatch; } - if (UdiParser.TryParse(match.Groups["udi"].Value, out Udi? udi) is false) + if (Guid.TryParse(match.Groups["guid"].Value, out Guid guid) is false) { - return; + return ReplaceStatus.NoMatch; } - bool handled = false; + var udi = new GuidUdi(type, guid); + switch (udi.EntityType) { case Constants.UdiEntityType.Document: @@ -41,8 +57,8 @@ internal abstract partial class ApiRichTextParserBase : null; if (route != null) { - handled = true; handleContentRoute(route); + return ReplaceStatus.Success; } break; @@ -50,17 +66,56 @@ internal abstract partial class ApiRichTextParserBase IPublishedContent? media = publishedSnapshot.Media?.GetById(udi); if (media != null) { - handled = true; handleMediaUrl(_apiMediaUrlProvider.GetUrl(media)); + return ReplaceStatus.Success; } break; } - if(handled is false) + return ReplaceStatus.InvalidEntityType; + } + + private ReplaceStatus ReplaceLegacyLocalLink(IPublishedSnapshot publishedSnapshot, string href, Action handleContentRoute, Action handleMediaUrl) + { + Match match = LegacyLocalLinkRegex().Match(href); + if (match.Success is false) { - handleInvalidLink(); + return ReplaceStatus.NoMatch; } + + if (UdiParser.TryParse(match.Groups["udi"].Value, out Udi? udi) is false) + { + return ReplaceStatus.NoMatch; + } + + + switch (udi.EntityType) + { + case Constants.UdiEntityType.Document: + IPublishedContent? content = publishedSnapshot.Content?.GetById(udi); + IApiContentRoute? route = content != null + ? _apiContentRouteBuilder.Build(content) + : null; + if (route != null) + { + handleContentRoute(route); + return ReplaceStatus.Success; + } + + break; + case Constants.UdiEntityType.Media: + IPublishedContent? media = publishedSnapshot.Media?.GetById(udi); + if (media != null) + { + handleMediaUrl(_apiMediaUrlProvider.GetUrl(media)); + return ReplaceStatus.Success; + } + + break; + } + + return ReplaceStatus.InvalidEntityType; } protected void ReplaceLocalImages(IPublishedSnapshot publishedSnapshot, string udi, Action handleMediaUrl) @@ -80,5 +135,15 @@ internal abstract partial class ApiRichTextParserBase } [GeneratedRegex("{localLink:(?umb:.+)}")] + private static partial Regex LegacyLocalLinkRegex(); + + [GeneratedRegex("{localLink:(?.+)}")] private static partial Regex LocalLinkRegex(); + + private enum ReplaceStatus + { + NoMatch, + Success, + InvalidEntityType + } } diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Templates/HtmlLocalLinkParserTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Templates/HtmlLocalLinkParserTests.cs index c9d3f3ab9b..93b0d5bba3 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Templates/HtmlLocalLinkParserTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Templates/HtmlLocalLinkParserTests.cs @@ -13,6 +13,7 @@ using Umbraco.Cms.Core.Routing; using Umbraco.Cms.Core.Templates; using Umbraco.Cms.Tests.Common; using Umbraco.Cms.Tests.UnitTests.TestHelpers.Objects; +using Umbraco.Extensions; namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Templates; @@ -21,6 +22,32 @@ public class HtmlLocalLinkParserTests { [Test] public void Returns_Udis_From_LocalLinks() + { + var input = @"

+

+ + other page +
+

+media +

"; + + var umbracoContextAccessor = new TestUmbracoContextAccessor(); + var parser = new HtmlLocalLinkParser(umbracoContextAccessor, Mock.Of()); + + var result = parser.FindUdisFromLocalLinks(input).ToList(); + + Assert.Multiple(() => + { + Assert.AreEqual(2, result.Count); + Assert.Contains(UdiParser.Parse("umb://document/eed5fc6b-96fd-45a5-a0f1-b1adfb483c2f"), result); + Assert.Contains(UdiParser.Parse("umb://media/7e21a725-b905-4c5f-86dc-8c41ec116e39"), result); + }); + } + + // todo remove at some point and the implementation. + [Test] + public void Returns_Udis_From_Legacy_LocalLinks() { var input = @"

@@ -36,12 +63,59 @@ public class HtmlLocalLinkParserTests var result = parser.FindUdisFromLocalLinks(input).ToList(); - Assert.AreEqual(2, result.Count); - Assert.AreEqual(UdiParser.Parse("umb://document/C093961595094900AAF9170DDE6AD442"), result[0]); - Assert.AreEqual(UdiParser.Parse("umb://document-type/2D692FCB070B4CDA92FB6883FDBFD6E2"), result[1]); + Assert.Multiple(() => + { + Assert.AreEqual(2, result.Count); + Assert.Contains(UdiParser.Parse("umb://document/C093961595094900AAF9170DDE6AD442"), result); + Assert.Contains(UdiParser.Parse("umb://document-type/2D692FCB070B4CDA92FB6883FDBFD6E2"), result); + }); + } + + // todo remove at some point and the implementation. + [Test] + public void Returns_Udis_From_Legacy_And_Current_LocalLinks() + { + var input = @"

+

+ + hello +
+

+hello +

+

+

+ + other page +
+

+media +

"; + + var umbracoContextAccessor = new TestUmbracoContextAccessor(); + var parser = new HtmlLocalLinkParser(umbracoContextAccessor, Mock.Of()); + + var result = parser.FindUdisFromLocalLinks(input).ToList(); + + Assert.Multiple(() => + { + Assert.AreEqual(4, result.Count); + Assert.Contains(UdiParser.Parse("umb://document/eed5fc6b-96fd-45a5-a0f1-b1adfb483c2f"), result); + Assert.Contains(UdiParser.Parse("umb://media/7e21a725-b905-4c5f-86dc-8c41ec116e39"), result); + Assert.Contains(UdiParser.Parse("umb://document/C093961595094900AAF9170DDE6AD442"), result); + Assert.Contains(UdiParser.Parse("umb://document-type/2D692FCB070B4CDA92FB6883FDBFD6E2"), result); + }); } [TestCase("", "")] + // current + [TestCase( + "world", + "world")] + [TestCase( + "world", + "world")] + // legacy [TestCase( "hello href=\"{localLink:1234}\" world ", "hello href=\"/my-test-url\" world ")] diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/DeliveryApi/ApiRichTextMarkupParserTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/DeliveryApi/ApiRichTextMarkupParserTests.cs new file mode 100644 index 0000000000..43ec2136f7 --- /dev/null +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/DeliveryApi/ApiRichTextMarkupParserTests.cs @@ -0,0 +1,200 @@ +using Microsoft.Extensions.Logging; +using Moq; +using NUnit.Framework; +using Umbraco.Cms.Core; +using Umbraco.Cms.Core.DeliveryApi; +using Umbraco.Cms.Core.Models.DeliveryApi; +using Umbraco.Cms.Core.Models.PublishedContent; +using Umbraco.Cms.Core.PublishedCache; +using Umbraco.Cms.Infrastructure.DeliveryApi; + +namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.DeliveryApi; + +[TestFixture] +public class ApiRichTextMarkupParserTests +{ + private Mock _apiContentRouteBuilder; + private Mock _apiMediaUrlProvider; + private Mock _publishedSnapshotAccessor; + + [Test] + public void Can_Parse_Legacy_LocalLinks() + { + var key1 = Guid.Parse("a1c5d649977f4ea59b1cb26055f3eed3"); + var data1 = new MockData() + .WithKey(key1) + .WithRoutePath("/inline/") + .WithRouteStartPath("inline"); + + var mockData = new Dictionary + { + { key1, data1 }, + }; + + var parser = BuildDefaultSut(mockData); + + var legacyHtml = + "

link to another page

"; + + var expectedOutput = + "

link to another page

"; + + var parsedHtml = parser.Parse(legacyHtml); + + Assert.AreEqual(expectedOutput, parsedHtml); + } + + [Test] + public void Can_Parse_LocalLinks() + { + var key1 = Guid.Parse("eed5fc6b-96fd-45a5-a0f1-b1adfb483c2f"); + var data1 = new MockData() + .WithKey(key1) + .WithRoutePath("/self/") + .WithRouteStartPath("self"); + + var key2 = Guid.Parse("cc143afe-4cbf-46e5-b399-c9f451384373"); + var data2 = new MockData() + .WithKey(key2) + .WithRoutePath("/other/") + .WithRouteStartPath("other"); + + var mockData = new Dictionary + { + { key1, data1 }, + { key2, data2 }, + }; + + var parser = BuildDefaultSut(mockData); + + var html = + @"

Rich text outside of the blocks with a link to itself

+

and to the other page

"; + + var expectedOutput = + @"

Rich text outside of the blocks with a link to itself

+

and to the other page

"; + + var parsedHtml = parser.Parse(html); + + Assert.AreEqual(expectedOutput, parsedHtml); + } + + [Test] + public void Can_Parse_Legacy_LocalImages() + { + var key1 = Guid.Parse("395bdc0e8f4d4ad4af7f3a3f6265651e"); + var data1 = new MockData() + .WithKey(key1) + .WithMediaUrl("https://localhost:44331/media/bdofwokn/77gtp8fbrxmgkefatp10aw.webp"); + + var mockData = new Dictionary + { + { key1, data1 }, + }; + var parser = BuildDefaultSut(mockData); + + var legacyHtml = + @"

An image

\n

"; + + var expectedOutput = + @"

An image

\n

"; + + var parsedHtml = parser.Parse(legacyHtml); + + Assert.AreEqual(expectedOutput, parsedHtml); + } + + private ApiRichTextMarkupParser BuildDefaultSut(Dictionary mockData) + { + var contentCacheMock = new Mock(); + + contentCacheMock.Setup(cc => cc.GetById(It.IsAny(), It.IsAny())) + .Returns((preview, key) => mockData[key].PublishedContent); + contentCacheMock.Setup(cc => cc.GetById(It.IsAny())) + .Returns(key => mockData[key].PublishedContent); + contentCacheMock.Setup(cc => cc.GetById(It.IsAny(), It.IsAny())) + .Returns((preview, udi) => mockData[((GuidUdi)udi).Guid].PublishedContent); + contentCacheMock.Setup(cc => cc.GetById(It.IsAny())) + .Returns(udi => mockData[((GuidUdi)udi).Guid].PublishedContent); + + var mediaCacheMock = new Mock(); + mediaCacheMock.Setup(cc => cc.GetById(It.IsAny(), It.IsAny())) + .Returns((preview, key) => mockData[key].PublishedContent); + mediaCacheMock.Setup(cc => cc.GetById(It.IsAny())) + .Returns(key => mockData[key].PublishedContent); + mediaCacheMock.Setup(cc => cc.GetById(It.IsAny(), It.IsAny())) + .Returns((preview, udi) => mockData[((GuidUdi)udi).Guid].PublishedContent); + mediaCacheMock.Setup(cc => cc.GetById(It.IsAny())) + .Returns(udi => mockData[((GuidUdi)udi).Guid].PublishedContent); + + var snapshotMock = new Mock(); + snapshotMock.SetupGet(ss => ss.Content) + .Returns(contentCacheMock.Object); + snapshotMock.SetupGet(ss => ss.Media) + .Returns(mediaCacheMock.Object); + + var snapShot = snapshotMock.Object; + + _publishedSnapshotAccessor = new Mock(); + _publishedSnapshotAccessor.Setup(psa => psa.TryGetPublishedSnapshot(out snapShot)) + .Returns(true); + + _apiMediaUrlProvider = new Mock(); + _apiMediaUrlProvider.Setup(mup => mup.GetUrl(It.IsAny())) + .Returns(ipc => mockData[ipc.Key].MediaUrl); + + _apiContentRouteBuilder = new Mock(); + _apiContentRouteBuilder.Setup(acrb => acrb.Build(It.IsAny(), It.IsAny())) + .Returns((content, culture) => mockData[content.Key].ApiContentRoute); + + return new ApiRichTextMarkupParser( + _apiContentRouteBuilder.Object, + _apiMediaUrlProvider.Object, + _publishedSnapshotAccessor.Object, + Mock.Of>()); + } + + private class MockData + { + private Mock _publishedContentMock = new Mock(); + private Mock _apiContentRouteMock = new Mock(); + private Mock _apiContentStartItem = new Mock(); + + public IPublishedContent PublishedContent => _publishedContentMock.Object; + + public IApiContentRoute ApiContentRoute => _apiContentRouteMock.Object; + + public string MediaUrl { get; set; } = string.Empty; + + public MockData() + { + _apiContentRouteMock.SetupGet(r => r.StartItem).Returns(_apiContentStartItem.Object); + } + + public MockData WithKey(Guid key) + { + _publishedContentMock.SetupGet(i => i.Key).Returns(key); + _apiContentStartItem.SetupGet(rsi => rsi.Id).Returns(key); + return this; + } + + public MockData WithRoutePath(string path) + { + _apiContentRouteMock.SetupGet(r => r.Path).Returns(path); + return this; + } + + public MockData WithRouteStartPath(string path) + { + _apiContentStartItem.SetupGet(rsi => rsi.Path).Returns(path); + return this; + } + + public MockData WithMediaUrl(string url) + { + MediaUrl = url; + return this; + } + } +}