diff --git a/src/Umbraco.Core/Services/DocumentUrlService.cs b/src/Umbraco.Core/Services/DocumentUrlService.cs index c73b2c41e6..c9bc338906 100644 --- a/src/Umbraco.Core/Services/DocumentUrlService.cs +++ b/src/Umbraco.Core/Services/DocumentUrlService.cs @@ -44,26 +44,51 @@ public class DocumentUrlService : IDocumentUrlService /// /// Model used to cache a single published document along with all it's URL segments. /// - private class PublishedDocumentUrlSegments + /// Internal for the purpose of unit and benchmark testing. + internal class PublishedDocumentUrlSegments { + /// + /// Gets or sets the document key. + /// public required Guid DocumentKey { get; set; } + /// + /// Gets or sets the language Id. + /// public required int LanguageId { get; set; } + /// + /// Gets or sets the collection of for the document, language and state. + /// public required IList UrlSegments { get; set; } + /// + /// Gets or sets a value indicating whether the document is a draft version or not. + /// public required bool IsDraft { get; set; } + /// + /// Model used to represent a URL segment for a document in the cache. + /// public class UrlSegment { + /// + /// Initializes a new instance of the class. + /// public UrlSegment(string segment, bool isPrimary) { Segment = segment; IsPrimary = isPrimary; } + /// + /// Gets the URL segment string. + /// public string Segment { get; } + /// + /// Gets a value indicating whether this URL segment is the primary one for the document, language and state. + /// public bool IsPrimary { get; } } } @@ -168,45 +193,40 @@ public class DocumentUrlService : IDocumentUrlService scope.Complete(); } - private static IEnumerable ConvertToCacheModel(IEnumerable publishedDocumentUrlSegments) + /// + /// Converts a collection of to a collection of for caching purposes. + /// + /// The collection of retrieved from the database on startup. + /// The collection of cache models. + /// Internal for the purpose of unit and benchmark testing. + internal static IEnumerable ConvertToCacheModel(IEnumerable publishedDocumentUrlSegments) { - var cacheModels = new List(); + var cacheModels = new Dictionary<(Guid DocumentKey, int LanguageId, bool IsDraft), PublishedDocumentUrlSegments>(); + foreach (PublishedDocumentUrlSegment model in publishedDocumentUrlSegments) { - PublishedDocumentUrlSegments? existingCacheModel = GetModelFromCache(cacheModels, model); - if (existingCacheModel is null) + (Guid DocumentKey, int LanguageId, bool IsDraft) key = (model.DocumentKey, model.LanguageId, model.IsDraft); + + if (!cacheModels.TryGetValue(key, out PublishedDocumentUrlSegments? existingCacheModel)) { - cacheModels.Add(new PublishedDocumentUrlSegments + cacheModels[key] = new PublishedDocumentUrlSegments { DocumentKey = model.DocumentKey, LanguageId = model.LanguageId, UrlSegments = [new PublishedDocumentUrlSegments.UrlSegment(model.UrlSegment, model.IsPrimary)], IsDraft = model.IsDraft, - }); + }; } else { - existingCacheModel.UrlSegments = GetUpdatedUrlSegments(existingCacheModel.UrlSegments, model.UrlSegment, model.IsPrimary); + if (existingCacheModel.UrlSegments.Any(x => x.Segment == model.UrlSegment) is false) + { + existingCacheModel.UrlSegments.Add(new PublishedDocumentUrlSegments.UrlSegment(model.UrlSegment, model.IsPrimary)); + } } } - return cacheModels; - } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static PublishedDocumentUrlSegments? GetModelFromCache(List cacheModels, PublishedDocumentUrlSegment model) - => cacheModels - .SingleOrDefault(x => x.DocumentKey == model.DocumentKey && x.LanguageId == model.LanguageId && x.IsDraft == model.IsDraft); - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static IList GetUpdatedUrlSegments(IList urlSegments, string segment, bool isPrimary) - { - if (urlSegments.FirstOrDefault(x => x.Segment == segment) is null) - { - urlSegments.Add(new PublishedDocumentUrlSegments.UrlSegment(segment, isPrimary)); - } - - return urlSegments; + return cacheModels.Values; } private void RemoveFromCache(IScopeContext scopeContext, Guid documentKey, string isoCode, bool isDraft) diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/DocumentUrlServiceTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/DocumentUrlServiceTests.cs new file mode 100644 index 0000000000..da7f433b6c --- /dev/null +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/DocumentUrlServiceTests.cs @@ -0,0 +1,127 @@ +using NUnit.Framework; +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.Services; + +namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Services; + +[TestFixture] +public class DocumentUrlServiceTests +{ + [Test] + public void ConvertToCacheModel_Converts_Single_Document_With_Single_Segment_To_Expected_Cache_Model() + { + var segments = new List + { + new() + { + DocumentKey = Guid.NewGuid(), + IsDraft = false, + IsPrimary = true, + LanguageId = 1, + UrlSegment = "test-segment", + }, + }; + var cacheModels = DocumentUrlService.ConvertToCacheModel(segments).ToList(); + + Assert.AreEqual(1, cacheModels.Count); + Assert.AreEqual(segments[0].DocumentKey, cacheModels[0].DocumentKey); + Assert.AreEqual(1, cacheModels[0].LanguageId); + Assert.AreEqual(1, cacheModels[0].UrlSegments.Count); + Assert.AreEqual("test-segment", cacheModels[0].UrlSegments[0].Segment); + Assert.IsTrue(cacheModels[0].UrlSegments[0].IsPrimary); + } + + [Test] + public void ConvertToCacheModel_Converts_Multiple_Documents_With_Single_Segment_To_Expected_Cache_Model() + { + var segments = new List + { + new() + { + DocumentKey = Guid.NewGuid(), + IsDraft = false, + IsPrimary = true, + LanguageId = 1, + UrlSegment = "test-segment", + }, + new() + { + DocumentKey = Guid.NewGuid(), + IsDraft = false, + IsPrimary = true, + LanguageId = 1, + UrlSegment = "test-segment-2", + }, + }; + var cacheModels = DocumentUrlService.ConvertToCacheModel(segments).ToList(); + + Assert.AreEqual(2, cacheModels.Count); + Assert.AreEqual(segments[0].DocumentKey, cacheModels[0].DocumentKey); + Assert.AreEqual(segments[1].DocumentKey, cacheModels[1].DocumentKey); + Assert.AreEqual(1, cacheModels[0].LanguageId); + Assert.AreEqual(1, cacheModels[1].LanguageId); + Assert.AreEqual(1, cacheModels[0].UrlSegments.Count); + Assert.AreEqual("test-segment", cacheModels[0].UrlSegments[0].Segment); + Assert.AreEqual(1, cacheModels[1].UrlSegments.Count); + Assert.AreEqual("test-segment-2", cacheModels[1].UrlSegments[0].Segment); + Assert.IsTrue(cacheModels[0].UrlSegments[0].IsPrimary); + Assert.IsTrue(cacheModels[1].UrlSegments[0].IsPrimary); + } + + [Test] + public void ConvertToCacheModel_Converts_Single_Document_With_Multiple_Segments_To_Expected_Cache_Model() + { + var documentKey = Guid.NewGuid(); + var segments = new List + { + new() + { + DocumentKey = documentKey, + IsDraft = false, + IsPrimary = true, + LanguageId = 1, + UrlSegment = "test-segment", + }, + new() + { + DocumentKey = documentKey, + IsDraft = false, + IsPrimary = false, + LanguageId = 1, + UrlSegment = "test-segment-2", + }, + }; + var cacheModels = DocumentUrlService.ConvertToCacheModel(segments).ToList(); + + Assert.AreEqual(1, cacheModels.Count); + Assert.AreEqual(documentKey, cacheModels[0].DocumentKey); + Assert.AreEqual(1, cacheModels[0].LanguageId); + Assert.AreEqual(2, cacheModels[0].UrlSegments.Count); + Assert.AreEqual("test-segment", cacheModels[0].UrlSegments[0].Segment); + Assert.AreEqual("test-segment-2", cacheModels[0].UrlSegments[1].Segment); + Assert.IsTrue(cacheModels[0].UrlSegments[0].IsPrimary); + Assert.IsFalse(cacheModels[0].UrlSegments[1].IsPrimary); + } + + [Test] + public void ConvertToCacheModel_Performance_Test() + { + const int NumberOfSegments = 1; + var segments = Enumerable.Range(0, NumberOfSegments) + .Select((x, i) => new PublishedDocumentUrlSegment + { + DocumentKey = Guid.NewGuid(), + IsDraft = false, + IsPrimary = true, + LanguageId = 1, + UrlSegment = $"test-segment-{x + 1}", + }); + var cacheModels = DocumentUrlService.ConvertToCacheModel(segments).ToList(); + + Assert.AreEqual(NumberOfSegments, cacheModels.Count); + + // Benchmarking (for NumberOfSegments = 50000): + // - Initial implementation (15.4): ~28s + // - Current implementation: ~100ms + } +}