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
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public override PropertyCacheLevel GetPropertyCacheLevel(IPublishedPropertyType
var sourceString = source.ToString();

// ensures string is parsed for {localLink} and URLs are resolved correctly
sourceString = _linkParser.EnsureInternalLinks(sourceString!, preview);
sourceString = _linkParser.EnsureInternalLinks(sourceString!);
sourceString = _urlParser.EnsureUrls(sourceString);

return sourceString;
Expand Down
20 changes: 11 additions & 9 deletions src/Umbraco.Core/Templates/HtmlLocalLinkParser.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System.Globalization;

Check warning on line 1 in src/Umbraco.Core/Templates/HtmlLocalLinkParser.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ New issue: Primitive Obsession

In this module, 78.6% of all function arguments are primitive types, threshold = 30.0%. The functions in this file have too many primitive types (e.g. int, double, float) in their function argument lists. Using many primitive types lead to the code smell Primitive Obsession. Avoid adding more primitive arguments.

Check notice on line 1 in src/Umbraco.Core/Templates/HtmlLocalLinkParser.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

✅ Getting better: String Heavy Function Arguments

The ratio of strings in function arguments decreases from 66.67% to 64.29%, threshold = 39.0%. The functions in this file have a high ratio of strings as arguments. Avoid adding more.
using System.Text.RegularExpressions;
using Umbraco.Cms.Core.Models.PublishedContent;
using Umbraco.Cms.Core.Routing;

namespace Umbraco.Cms.Core.Templates;
Expand Down Expand Up @@ -45,26 +46,27 @@
/// <summary>
/// Parses the string looking for the {localLink} syntax and updates them to their correct links.
/// </summary>
/// <param name="text"></param>
/// <param name="preview"></param>
/// <returns></returns>
[Obsolete("This method overload is no longer used in Umbraco and delegates to the overload without the preview parameter. Scheduled for removal in Umbraco 18.")]
public string EnsureInternalLinks(string text, bool preview) => EnsureInternalLinks(text);

/// <summary>
/// Parses the string looking for the {localLink} syntax and updates them to their correct links.
/// </summary>
/// <param name="text"></param>
/// <returns></returns>
public string EnsureInternalLinks(string text)
public string EnsureInternalLinks(string text) => EnsureInternalLinks(text, UrlMode.Default);

/// <summary>
/// Parses the string looking for the {localLink} syntax and updates them to their correct links.
/// </summary>
public string EnsureInternalLinks(string text, UrlMode urlMode)
{
foreach (LocalLinkTag tagData in FindLocalLinkIds(text))
{
if (tagData.Udi is not null)
{
var newLink = tagData.Udi?.EntityType switch
{
Constants.UdiEntityType.Document => _publishedUrlProvider.GetUrl(tagData.Udi.Guid),
Constants.UdiEntityType.Media => _publishedUrlProvider.GetMediaUrl(tagData.Udi.Guid),
Constants.UdiEntityType.Document => _publishedUrlProvider.GetUrl(tagData.Udi.Guid, urlMode),
Constants.UdiEntityType.Media => _publishedUrlProvider.GetMediaUrl(tagData.Udi.Guid, urlMode),
_ => string.Empty,
};

Expand All @@ -73,7 +75,7 @@
}
else if (tagData.IntId.HasValue)
{
var newLink = _publishedUrlProvider.GetUrl(tagData.IntId.Value);
var newLink = _publishedUrlProvider.GetUrl(tagData.IntId.Value, urlMode);
text = text.Replace(tagData.TagHref, newLink);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public override PropertyCacheLevel GetPropertyCacheLevel(IPublishedPropertyType
var sourceString = source.ToString()!;

// ensures string is parsed for {localLink} and URLs are resolved correctly
sourceString = _localLinkParser.EnsureInternalLinks(sourceString, preview);
sourceString = _localLinkParser.EnsureInternalLinks(sourceString);
sourceString = _urlParser.EnsureUrls(sourceString);

return sourceString;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ public Type GetDeliveryApiPropertyValueType(IPublishedPropertyType propertyType)
var sourceString = intermediateValue.Markup;

// ensures string is parsed for {localLink} and URLs and media are resolved correctly
sourceString = _linkParser.EnsureInternalLinks(sourceString, preview);
sourceString = _linkParser.EnsureInternalLinks(sourceString);
sourceString = _urlParser.EnsureUrls(sourceString);
sourceString = _imageSourceParser.EnsureImageSources(sourceString);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) Umbraco.

Check warning on line 1 in tests/Umbraco.Tests.UnitTests/Umbraco.Core/Templates/HtmlLocalLinkParserTests.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ New issue: Code Duplication

The module contains 2 functions with similar structure: Returns_Udis_From_Legacy_LocalLinks,Returns_Udis_From_LocalLinks. Avoid duplicated, aka copy-pasted, code inside the module. More duplication lowers the code health.

Check notice on line 1 in tests/Umbraco.Tests.UnitTests/Umbraco.Core/Templates/HtmlLocalLinkParserTests.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

✅ No longer an issue: Large Method

ParseLocalLinks is no longer above the threshold for lines of code. Large functions with many lines of code are generally harder to understand and lower the code health. Avoid adding more lines to this function.
// See LICENSE for more details.

using Microsoft.Extensions.Options;
Expand All @@ -11,6 +11,7 @@
using Umbraco.Cms.Core.Routing;
using Umbraco.Cms.Core.Services.Navigation;
using Umbraco.Cms.Core.Templates;
using Umbraco.Cms.Core.Web;
using Umbraco.Cms.Tests.Common;
using Umbraco.Cms.Tests.UnitTests.TestHelpers.Objects;

Expand Down Expand Up @@ -216,48 +217,244 @@

var umbracoContextAccessor = new TestUmbracoContextAccessor();

var umbracoContextFactory = TestUmbracoContextFactory.Create(
umbracoContextAccessor: umbracoContextAccessor);

using (var reference = umbracoContextFactory.EnsureUmbracoContext())
{
var contentCache = Mock.Get(reference.UmbracoContext.Content);
contentCache.Setup(x => x.GetById(It.IsAny<int>())).Returns(publishedContent.Object);
contentCache.Setup(x => x.GetById(It.IsAny<Guid>())).Returns(publishedContent.Object);

var mediaCache = Mock.Get(reference.UmbracoContext.Media);
mediaCache.Setup(x => x.GetById(It.IsAny<int>())).Returns(media.Object);
mediaCache.Setup(x => x.GetById(It.IsAny<Guid>())).Returns(media.Object);

var publishedUrlProvider = CreatePublishedUrlProvider(
contentUrlProvider,
mediaUrlProvider,
umbracoContextAccessor);

var linkParser = new HtmlLocalLinkParser(publishedUrlProvider);

var output = linkParser.EnsureInternalLinks(input);

Assert.AreEqual(result, output);
}
}

[Test]
public void ParseLocalLinks_WithUrlMode_RespectsUrlMode()
{
// Arrange
var input = "hello href=\"{localLink:umb://document/9931BDE0AAC34BABB838909A7B47570E}\" world";

// Setup content URL provider that returns different URLs based on UrlMode
var contentUrlProvider = new Mock<IUrlProvider>();
contentUrlProvider
.Setup(x => x.GetUrl(
It.IsAny<IPublishedContent>(),
UrlMode.Relative,
It.IsAny<string>(),
It.IsAny<Uri>()))
.Returns(UrlInfo.Url("/relative-url"));
contentUrlProvider
.Setup(x => x.GetUrl(
It.IsAny<IPublishedContent>(),
UrlMode.Absolute,
It.IsAny<string>(),
It.IsAny<Uri>()))
.Returns(UrlInfo.Url("http://example.com/absolute-url"));

var contentType = new PublishedContentType(
Guid.NewGuid(),
666,
"alias",
PublishedItemType.Content,
Enumerable.Empty<string>(),
Enumerable.Empty<PublishedPropertyType>(),
ContentVariation.Nothing);
var publishedContent = new Mock<IPublishedContent>();
publishedContent.Setup(x => x.Id).Returns(1234);
publishedContent.Setup(x => x.ContentType).Returns(contentType);

var umbracoContextAccessor = new TestUmbracoContextAccessor();
var umbracoContextFactory = TestUmbracoContextFactory.Create(
umbracoContextAccessor: umbracoContextAccessor);

var webRoutingSettings = new WebRoutingSettings();

var navigationQueryService = new Mock<IDocumentNavigationQueryService>();
// Guid? parentKey = null;
// navigationQueryService.Setup(x => x.TryGetParentKey(It.IsAny<Guid>(), out parentKey)).Returns(true);
IEnumerable<Guid> ancestorKeys = [];
navigationQueryService.Setup(x => x.TryGetAncestorsKeys(It.IsAny<Guid>(), out ancestorKeys)).Returns(true);
var publishedUrlProvider = CreatePublishedUrlProvider(
contentUrlProvider,
new Mock<IMediaUrlProvider>(),
umbracoContextAccessor);

var publishedContentStatusFilteringService = new Mock<IPublishedContentStatusFilteringService>();
using (var reference = umbracoContextFactory.EnsureUmbracoContext())
{
var contentCache = Mock.Get(reference.UmbracoContext.Content);
contentCache.Setup(x => x.GetById(It.IsAny<Guid>())).Returns(publishedContent.Object);

var linkParser = new HtmlLocalLinkParser(publishedUrlProvider);

// Act
var relativeOutput = linkParser.EnsureInternalLinks(input, UrlMode.Relative);
var absoluteOutput = linkParser.EnsureInternalLinks(input, UrlMode.Absolute);

// Assert
Assert.AreEqual("hello href=\"/relative-url\" world", relativeOutput);
Assert.AreEqual("hello href=\"http://example.com/absolute-url\" world", absoluteOutput);
}
}

[TestCase(UrlMode.Default, "hello href=\"{localLink:1234}\" world ", "hello href=\"/relative-url\" world ")]
[TestCase(UrlMode.Relative, "hello href=\"{localLink:1234}\" world ", "hello href=\"/relative-url\" world ")]
[TestCase(UrlMode.Absolute, "hello href=\"{localLink:1234}\" world ", "hello href=\"https://example.com/absolute-url\" world ")]
[TestCase(UrlMode.Auto, "hello href=\"{localLink:1234}\" world ", "hello href=\"/relative-url\" world ")]
[TestCase(UrlMode.Default, "hello href=\"{localLink:umb://document/9931BDE0AAC34BABB838909A7B47570E}\" world ", "hello href=\"/relative-url\" world ")]
[TestCase(UrlMode.Relative, "hello href=\"{localLink:umb://document/9931BDE0AAC34BABB838909A7B47570E}\" world ", "hello href=\"/relative-url\" world ")]
[TestCase(UrlMode.Absolute, "hello href=\"{localLink:umb://document/9931BDE0AAC34BABB838909A7B47570E}\" world ", "hello href=\"https://example.com/absolute-url\" world ")]
[TestCase(UrlMode.Auto, "hello href=\"{localLink:umb://document/9931BDE0AAC34BABB838909A7B47570E}\" world ", "hello href=\"/relative-url\" world ")]
[TestCase(UrlMode.Default, "hello href=\"{localLink:umb://media/9931BDE0AAC34BABB838909A7B47570E}\" world ", "hello href=\"/media/relative/image.jpg\" world ")]
[TestCase(UrlMode.Relative, "hello href=\"{localLink:umb://media/9931BDE0AAC34BABB838909A7B47570E}\" world ", "hello href=\"/media/relative/image.jpg\" world ")]
[TestCase(UrlMode.Absolute, "hello href=\"{localLink:umb://media/9931BDE0AAC34BABB838909A7B47570E}\" world ", "hello href=\"https://example.com/media/absolute/image.jpg\" world ")]
[TestCase(UrlMode.Auto, "hello href=\"{localLink:umb://media/9931BDE0AAC34BABB838909A7B47570E}\" world ", "hello href=\"/media/relative/image.jpg\" world ")]
public void ParseLocalLinks_WithVariousUrlModes_ReturnsCorrectUrls(UrlMode urlMode, string input, string expectedResult)
{
// setup content URL provider that returns different URLs based on UrlMode
var contentUrlProvider = new Mock<IUrlProvider>();
contentUrlProvider
.Setup(x => x.GetUrl(
It.IsAny<IPublishedContent>(),
UrlMode.Default,
It.IsAny<string>(),
It.IsAny<Uri>()))
.Returns(UrlInfo.Url("/relative-url"));
contentUrlProvider
.Setup(x => x.GetUrl(
It.IsAny<IPublishedContent>(),
UrlMode.Relative,
It.IsAny<string>(),
It.IsAny<Uri>()))
.Returns(UrlInfo.Url("/relative-url"));
contentUrlProvider
.Setup(x => x.GetUrl(
It.IsAny<IPublishedContent>(),
UrlMode.Absolute,
It.IsAny<string>(),
It.IsAny<Uri>()))
.Returns(UrlInfo.Url("https://example.com/absolute-url"));
contentUrlProvider
.Setup(x => x.GetUrl(
It.IsAny<IPublishedContent>(),
UrlMode.Auto,
It.IsAny<string>(),
It.IsAny<Uri>()))
.Returns(UrlInfo.Url("/relative-url"));

var contentType = new PublishedContentType(
Guid.NewGuid(),
666,
"alias",
PublishedItemType.Content,
Enumerable.Empty<string>(),
Enumerable.Empty<PublishedPropertyType>(),
ContentVariation.Nothing);
var publishedContent = new Mock<IPublishedContent>();
publishedContent.Setup(x => x.Id).Returns(1234);
publishedContent.Setup(x => x.ContentType).Returns(contentType);

// setup media URL provider that returns different URLs based on UrlMode
var mediaUrlProvider = new Mock<IMediaUrlProvider>();
mediaUrlProvider.Setup(x => x.GetMediaUrl(
It.IsAny<IPublishedContent>(),
It.IsAny<string>(),
UrlMode.Default,
It.IsAny<string>(),
It.IsAny<Uri>()))
.Returns(UrlInfo.Url("/media/relative/image.jpg"));
mediaUrlProvider.Setup(x => x.GetMediaUrl(
It.IsAny<IPublishedContent>(),
It.IsAny<string>(),
UrlMode.Relative,
It.IsAny<string>(),
It.IsAny<Uri>()))
.Returns(UrlInfo.Url("/media/relative/image.jpg"));
mediaUrlProvider.Setup(x => x.GetMediaUrl(
It.IsAny<IPublishedContent>(),
It.IsAny<string>(),
UrlMode.Absolute,
It.IsAny<string>(),
It.IsAny<Uri>()))
.Returns(UrlInfo.Url("https://example.com/media/absolute/image.jpg"));
mediaUrlProvider.Setup(x => x.GetMediaUrl(
It.IsAny<IPublishedContent>(),
It.IsAny<string>(),
UrlMode.Auto,
It.IsAny<string>(),
It.IsAny<Uri>()))
.Returns(UrlInfo.Url("/media/relative/image.jpg"));

var mediaType = new PublishedContentType(
Guid.NewGuid(),
777,
"image",
PublishedItemType.Media,
Enumerable.Empty<string>(),
Enumerable.Empty<PublishedPropertyType>(),
ContentVariation.Nothing);
var media = new Mock<IPublishedContent>();
media.Setup(x => x.ContentType).Returns(mediaType);

var umbracoContextAccessor = new TestUmbracoContextAccessor();
var umbracoContextFactory = TestUmbracoContextFactory.Create(
umbracoContextAccessor: umbracoContextAccessor);

var webRoutingSettings = new WebRoutingSettings();

var publishedUrlProvider = CreatePublishedUrlProvider(
contentUrlProvider,
mediaUrlProvider,
umbracoContextAccessor);

using (var reference = umbracoContextFactory.EnsureUmbracoContext())
{
var contentCache = Mock.Get(reference.UmbracoContext.Content);
contentCache.Setup(x => x.GetById(It.IsAny<int>())).Returns(publishedContent.Object);
contentCache.Setup(x => x.GetById(It.IsAny<Guid>())).Returns(publishedContent.Object);

var mediaCache = Mock.Get(reference.UmbracoContext.Media);
mediaCache.Setup(x => x.GetById(It.IsAny<int>())).Returns(media.Object);
mediaCache.Setup(x => x.GetById(It.IsAny<Guid>())).Returns(media.Object);

var publishStatusQueryService = new Mock<IPublishStatusQueryService>();
publishStatusQueryService
.Setup(x => x.IsDocumentPublished(It.IsAny<Guid>(), It.IsAny<string>()))
.Returns(true);

var publishedUrlProvider = new UrlProvider(
umbracoContextAccessor,
Options.Create(webRoutingSettings),
new UrlProviderCollection(() => new[] { contentUrlProvider.Object }),
new MediaUrlProviderCollection(() => new[] { mediaUrlProvider.Object }),
Mock.Of<IVariationContextAccessor>(),
navigationQueryService.Object,
publishedContentStatusFilteringService.Object);

var linkParser = new HtmlLocalLinkParser(publishedUrlProvider);

var output = linkParser.EnsureInternalLinks(input);
var output = linkParser.EnsureInternalLinks(input, urlMode);

Assert.AreEqual(result, output);
Assert.AreEqual(expectedResult, output);

Check warning on line 433 in tests/Umbraco.Tests.UnitTests/Umbraco.Core/Templates/HtmlLocalLinkParserTests.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (main)

❌ New issue: Large Method

ParseLocalLinks_WithVariousUrlModes_ReturnsCorrectUrls has 103 lines, threshold = 70. Large functions with many lines of code are generally harder to understand and lower the code health. Avoid adding more lines to this function.
}
}

private static UrlProvider CreatePublishedUrlProvider(
Mock<IUrlProvider> contentUrlProvider,
Mock<IMediaUrlProvider> mediaUrlProvider,
TestUmbracoContextAccessor umbracoContextAccessor)
{
var navigationQueryService = new Mock<IDocumentNavigationQueryService>();
IEnumerable<Guid> ancestorKeys = [];
navigationQueryService.Setup(x => x.TryGetAncestorsKeys(It.IsAny<Guid>(), out ancestorKeys)).Returns(true);

var publishStatusQueryService = new Mock<IPublishStatusQueryService>();
publishStatusQueryService
.Setup(x => x.IsDocumentPublished(It.IsAny<Guid>(), It.IsAny<string>()))
.Returns(true);

return new UrlProvider(
umbracoContextAccessor,
Options.Create(new WebRoutingSettings()),
new UrlProviderCollection(() => new[] { contentUrlProvider.Object }),
new MediaUrlProviderCollection(() => new[] { mediaUrlProvider.Object }),
Mock.Of<IVariationContextAccessor>(),
navigationQueryService.Object,
new Mock<IPublishedContentStatusFilteringService>().Object);
}
}
Loading