Skip to content

Commit 9b2fd12

Browse files
AndyButlandCopilot
AndyButland
and
Copilot
committed
Optimize initialization of document URLs on start-up (#19498)
* Optimize initialization of document URLs on startup. * Apply suggestions from code review Co-authored-by: Copilot <[email protected]> --------- Co-authored-by: Copilot <[email protected]>
1 parent 024697f commit 9b2fd12

File tree

2 files changed

+172
-25
lines changed

2 files changed

+172
-25
lines changed

‎src/Umbraco.Core/Services/DocumentUrlService.cs

Lines changed: 45 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -44,26 +44,51 @@ public class DocumentUrlService : IDocumentUrlService
4444
/// <summary>
4545
/// Model used to cache a single published document along with all it's URL segments.
4646
/// </summary>
47-
private class PublishedDocumentUrlSegments
47+
/// <remarks>Internal for the purpose of unit and benchmark testing.</remarks>
48+
internal class PublishedDocumentUrlSegments
4849
{
50+
/// <summary>
51+
/// Gets or sets the document key.
52+
/// </summary>
4953
public required Guid DocumentKey { get; set; }
5054

55+
/// <summary>
56+
/// Gets or sets the language Id.
57+
/// </summary>
5158
public required int LanguageId { get; set; }
5259

60+
/// <summary>
61+
/// Gets or sets the collection of <see cref="UrlSegment"/> for the document, language and state.
62+
/// </summary>
5363
public required IList<UrlSegment> UrlSegments { get; set; }
5464

65+
/// <summary>
66+
/// Gets or sets a value indicating whether the document is a draft version or not.
67+
/// </summary>
5568
public required bool IsDraft { get; set; }
5669

70+
/// <summary>
71+
/// Model used to represent a URL segment for a document in the cache.
72+
/// </summary>
5773
public class UrlSegment
5874
{
75+
/// <summary>
76+
/// Initializes a new instance of the <see cref="UrlSegment"/> class.
77+
/// </summary>
5978
public UrlSegment(string segment, bool isPrimary)
6079
{
6180
Segment = segment;
6281
IsPrimary = isPrimary;
6382
}
6483

84+
/// <summary>
85+
/// Gets the URL segment string.
86+
/// </summary>
6587
public string Segment { get; }
6688

89+
/// <summary>
90+
/// Gets a value indicating whether this URL segment is the primary one for the document, language and state.
91+
/// </summary>
6792
public bool IsPrimary { get; }
6893
}
6994
}
@@ -168,45 +193,40 @@ public async Task RebuildAllUrlsAsync()
168193
scope.Complete();
169194
}
170195

171-
private static IEnumerable<PublishedDocumentUrlSegments> ConvertToCacheModel(IEnumerable<PublishedDocumentUrlSegment> publishedDocumentUrlSegments)
196+
/// <summary>
197+
/// Converts a collection of <see cref="PublishedDocumentUrlSegment"/> to a collection of <see cref="PublishedDocumentUrlSegments"/> for caching purposes.
198+
/// </summary>
199+
/// <param name="publishedDocumentUrlSegments">The collection of <see cref="PublishedDocumentUrlSegment"/> retrieved from the database on startup.</param>
200+
/// <returns>The collection of cache models.</returns>
201+
/// <remarks>Internal for the purpose of unit and benchmark testing.</remarks>
202+
internal static IEnumerable<PublishedDocumentUrlSegments> ConvertToCacheModel(IEnumerable<PublishedDocumentUrlSegment> publishedDocumentUrlSegments)
172203
{
173-
var cacheModels = new List<PublishedDocumentUrlSegments>();
204+
var cacheModels = new Dictionary<(Guid DocumentKey, int LanguageId, bool IsDraft), PublishedDocumentUrlSegments>();
205+
174206
foreach (PublishedDocumentUrlSegment model in publishedDocumentUrlSegments)
175207
{
176-
PublishedDocumentUrlSegments? existingCacheModel = GetModelFromCache(cacheModels, model);
177-
if (existingCacheModel is null)
208+
(Guid DocumentKey, int LanguageId, bool IsDraft) key = (model.DocumentKey, model.LanguageId, model.IsDraft);
209+
210+
if (!cacheModels.TryGetValue(key, out PublishedDocumentUrlSegments? existingCacheModel))
178211
{
179-
cacheModels.Add(new PublishedDocumentUrlSegments
212+
cacheModels[key] = new PublishedDocumentUrlSegments
180213
{
181214
DocumentKey = model.DocumentKey,
182215
LanguageId = model.LanguageId,
183216
UrlSegments = [new PublishedDocumentUrlSegments.UrlSegment(model.UrlSegment, model.IsPrimary)],
184217
IsDraft = model.IsDraft,
185-
});
218+
};
186219
}
187220
else
188221
{
189-
existingCacheModel.UrlSegments = GetUpdatedUrlSegments(existingCacheModel.UrlSegments, model.UrlSegment, model.IsPrimary);
222+
if (existingCacheModel.UrlSegments.Any(x => x.Segment == model.UrlSegment) is false)
223+
{
224+
existingCacheModel.UrlSegments.Add(new PublishedDocumentUrlSegments.UrlSegment(model.UrlSegment, model.IsPrimary));
225+
}
190226
}
191227
}
192228

193-
return cacheModels;
194-
}
195-
196-
[MethodImpl(MethodImplOptions.AggressiveInlining)]
197-
private static PublishedDocumentUrlSegments? GetModelFromCache(List<PublishedDocumentUrlSegments> cacheModels, PublishedDocumentUrlSegment model)
198-
=> cacheModels
199-
.SingleOrDefault(x => x.DocumentKey == model.DocumentKey && x.LanguageId == model.LanguageId && x.IsDraft == model.IsDraft);
200-
201-
[MethodImpl(MethodImplOptions.AggressiveInlining)]
202-
private static IList<PublishedDocumentUrlSegments.UrlSegment> GetUpdatedUrlSegments(IList<PublishedDocumentUrlSegments.UrlSegment> urlSegments, string segment, bool isPrimary)
203-
{
204-
if (urlSegments.FirstOrDefault(x => x.Segment == segment) is null)
205-
{
206-
urlSegments.Add(new PublishedDocumentUrlSegments.UrlSegment(segment, isPrimary));
207-
}
208-
209-
return urlSegments;
229+
return cacheModels.Values;
210230
}
211231

212232
private void RemoveFromCache(IScopeContext scopeContext, Guid documentKey, string isoCode, bool isDraft)

‎tests/Umbraco.Tests.UnitTests/Umbraco.Core/Services/DocumentUrlServiceTests.cs

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
using NUnit.Framework;
2+
using Umbraco.Cms.Core.Models;
3+
using Umbraco.Cms.Core.Services;
4+
5+
namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Services;
6+
7+
[TestFixture]
8+
public class DocumentUrlServiceTests
9+
{
10+
[Test]
11+
public void ConvertToCacheModel_Converts_Single_Document_With_Single_Segment_To_Expected_Cache_Model()
12+
{
13+
var segments = new List<PublishedDocumentUrlSegment>
14+
{
15+
new()
16+
{
17+
DocumentKey = Guid.NewGuid(),
18+
IsDraft = false,
19+
IsPrimary = true,
20+
LanguageId = 1,
21+
UrlSegment = "test-segment",
22+
},
23+
};
24+
var cacheModels = DocumentUrlService.ConvertToCacheModel(segments).ToList();
25+
26+
Assert.AreEqual(1, cacheModels.Count);
27+
Assert.AreEqual(segments[0].DocumentKey, cacheModels[0].DocumentKey);
28+
Assert.AreEqual(1, cacheModels[0].LanguageId);
29+
Assert.AreEqual(1, cacheModels[0].UrlSegments.Count);
30+
Assert.AreEqual("test-segment", cacheModels[0].UrlSegments[0].Segment);
31+
Assert.IsTrue(cacheModels[0].UrlSegments[0].IsPrimary);
32+
}
33+
34+
[Test]
35+
public void ConvertToCacheModel_Converts_Multiple_Documents_With_Single_Segment_To_Expected_Cache_Model()
36+
{
37+
var segments = new List<PublishedDocumentUrlSegment>
38+
{
39+
new()
40+
{
41+
DocumentKey = Guid.NewGuid(),
42+
IsDraft = false,
43+
IsPrimary = true,
44+
LanguageId = 1,
45+
UrlSegment = "test-segment",
46+
},
47+
new()
48+
{
49+
DocumentKey = Guid.NewGuid(),
50+
IsDraft = false,
51+
IsPrimary = true,
52+
LanguageId = 1,
53+
UrlSegment = "test-segment-2",
54+
},
55+
};
56+
var cacheModels = DocumentUrlService.ConvertToCacheModel(segments).ToList();
57+
58+
Assert.AreEqual(2, cacheModels.Count);
59+
Assert.AreEqual(segments[0].DocumentKey, cacheModels[0].DocumentKey);
60+
Assert.AreEqual(segments[1].DocumentKey, cacheModels[1].DocumentKey);
61+
Assert.AreEqual(1, cacheModels[0].LanguageId);
62+
Assert.AreEqual(1, cacheModels[1].LanguageId);
63+
Assert.AreEqual(1, cacheModels[0].UrlSegments.Count);
64+
Assert.AreEqual("test-segment", cacheModels[0].UrlSegments[0].Segment);
65+
Assert.AreEqual(1, cacheModels[1].UrlSegments.Count);
66+
Assert.AreEqual("test-segment-2", cacheModels[1].UrlSegments[0].Segment);
67+
Assert.IsTrue(cacheModels[0].UrlSegments[0].IsPrimary);
68+
Assert.IsTrue(cacheModels[1].UrlSegments[0].IsPrimary);
69+
}
70+
71+
[Test]
72+
public void ConvertToCacheModel_Converts_Single_Document_With_Multiple_Segments_To_Expected_Cache_Model()
73+
{
74+
var documentKey = Guid.NewGuid();
75+
var segments = new List<PublishedDocumentUrlSegment>
76+
{
77+
new()
78+
{
79+
DocumentKey = documentKey,
80+
IsDraft = false,
81+
IsPrimary = true,
82+
LanguageId = 1,
83+
UrlSegment = "test-segment",
84+
},
85+
new()
86+
{
87+
DocumentKey = documentKey,
88+
IsDraft = false,
89+
IsPrimary = false,
90+
LanguageId = 1,
91+
UrlSegment = "test-segment-2",
92+
},
93+
};
94+
var cacheModels = DocumentUrlService.ConvertToCacheModel(segments).ToList();
95+
96+
Assert.AreEqual(1, cacheModels.Count);
97+
Assert.AreEqual(documentKey, cacheModels[0].DocumentKey);
98+
Assert.AreEqual(1, cacheModels[0].LanguageId);
99+
Assert.AreEqual(2, cacheModels[0].UrlSegments.Count);
100+
Assert.AreEqual("test-segment", cacheModels[0].UrlSegments[0].Segment);
101+
Assert.AreEqual("test-segment-2", cacheModels[0].UrlSegments[1].Segment);
102+
Assert.IsTrue(cacheModels[0].UrlSegments[0].IsPrimary);
103+
Assert.IsFalse(cacheModels[0].UrlSegments[1].IsPrimary);
104+
}
105+
106+
[Test]
107+
public void ConvertToCacheModel_Performance_Test()
108+
{
109+
const int NumberOfSegments = 1;
110+
var segments = Enumerable.Range(0, NumberOfSegments)
111+
.Select((x, i) => new PublishedDocumentUrlSegment
112+
{
113+
DocumentKey = Guid.NewGuid(),
114+
IsDraft = false,
115+
IsPrimary = true,
116+
LanguageId = 1,
117+
UrlSegment = $"test-segment-{x + 1}",
118+
});
119+
var cacheModels = DocumentUrlService.ConvertToCacheModel(segments).ToList();
120+
121+
Assert.AreEqual(NumberOfSegments, cacheModels.Count);
122+
123+
// Benchmarking (for NumberOfSegments = 50000):
124+
// - Initial implementation (15.4): ~28s
125+
// - Current implementation: ~100ms
126+
}
127+
}

0 commit comments

Comments
 (0)