Optimize initialization of document URLs on start-up

src/Umbraco.Core/Services/DocumentUrlService.cs

@@ -1,4 +1,4 @@
 using System.Collections.Concurrent;
 using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
 using System.Runtime.CompilerServices;
@@ -44,26 +44,51 @@
     /// <summary>
     /// Model used to cache a single published document along with all it's URL segments.
     /// </summary>
-    private class PublishedDocumentUrlSegments
+    /// <remarks>Internal for the purpose of unit and benchmark testing.</remarks>
+    internal class PublishedDocumentUrlSegments
     {
+        /// <summary>
+        /// Gets or sets the document key.
+        /// </summary>
         public required Guid DocumentKey { get; set; }
 
+        /// <summary>
+        /// Gets or sets the language Id.
+        /// </summary>
         public required int LanguageId { get; set; }
 
+        /// <summary>
+        /// Gets or sets the collection of <see cref="UrlSegment"/> for the document, language and state.
+        /// </summary>
         public required IList<UrlSegment> UrlSegments { get; set; }
 
+        /// <summary>
+        /// Gets or sets a value indicating whether the document is a draft version or not.
+        /// </summary>
         public required bool IsDraft { get; set; }
 
+        /// <summary>
+        /// Model used to represent a URL segment for a document in the cache.
+        /// </summary>
         public class UrlSegment
         {
+            /// <summary>
+            /// Initializes a new instance of the <see cref="UrlSegment"/> class.
+            /// </summary>
             public UrlSegment(string segment, bool isPrimary)
             {
                 Segment = segment;
                 IsPrimary = isPrimary;
             }
 
+            /// <summary>
+            /// Gets the URL segment string.
+            /// </summary>
             public string Segment { get; }
 
+            /// <summary>
+            /// Gets a value indicating whether this URL segment is the primary one for the document, language and state.
+            /// </summary>
             public bool IsPrimary { get; }
         }
     }
@@ -168,45 +193,40 @@
         scope.Complete();
     }
 
-    private static IEnumerable<PublishedDocumentUrlSegments> ConvertToCacheModel(IEnumerable<PublishedDocumentUrlSegment> publishedDocumentUrlSegments)
+    /// <summary>
+    /// Converts a collection of <see cref="PublishedDocumentUrlSegment"/> to a collection of <see cref="PublishedDocumentUrlSegments"/> for caching purposes.
+    /// </summary>
+    /// <param name="publishedDocumentUrlSegments">The collection of <see cref="PublishedDocumentUrlSegment"/> retrieved from the database on startup.</param>
+    /// <returns>The collection of cache models.</returns>
+    /// <remarks>Internal for the purpose of unit and benchmark testing.</remarks>
+    internal static IEnumerable<PublishedDocumentUrlSegments> ConvertToCacheModel(IEnumerable<PublishedDocumentUrlSegment> publishedDocumentUrlSegments)
     {
-        var cacheModels = new List<PublishedDocumentUrlSegments>();
+        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<PublishedDocumentUrlSegments> cacheModels, PublishedDocumentUrlSegment model)
-        => cacheModels
-            .SingleOrDefault(x => x.DocumentKey == model.DocumentKey && x.LanguageId == model.LanguageId && x.IsDraft == model.IsDraft);
-
-    [MethodImpl(MethodImplOptions.AggressiveInlining)]
-    private static IList<PublishedDocumentUrlSegments.UrlSegment> GetUpdatedUrlSegments(IList<PublishedDocumentUrlSegments.UrlSegment> 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)

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<PublishedDocumentUrlSegment>
+        {
+            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<PublishedDocumentUrlSegment>
+        {
+            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<PublishedDocumentUrlSegment>
+        {
+            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
+    }
+}