From 17af016c33545c696cf02b7d5b7bdfee0765f1bb Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Fri, 25 Oct 2024 22:10:01 +0200 Subject: [PATCH] Fixes slow legacy routes and added async overloads for delivery api --- .../Content/ByIdContentApiController.cs | 7 ++- .../Content/ByIdsContentApiController.cs | 2 +- .../DeliveryApi/ApiPublishedContentCache.cs | 52 +++++++++++++++++- .../DeliveryApi/IApiPublishedContentCache.cs | 4 ++ .../Services/DocumentUrlService.cs | 54 ++++++++++++------- 5 files changed, 93 insertions(+), 26 deletions(-) diff --git a/src/Umbraco.Cms.Api.Delivery/Controllers/Content/ByIdContentApiController.cs b/src/Umbraco.Cms.Api.Delivery/Controllers/Content/ByIdContentApiController.cs index 4239b15f6066..523c4969564c 100644 --- a/src/Umbraco.Cms.Api.Delivery/Controllers/Content/ByIdContentApiController.cs +++ b/src/Umbraco.Cms.Api.Delivery/Controllers/Content/ByIdContentApiController.cs @@ -1,3 +1,4 @@ +using System.Diagnostics; using Asp.Versioning; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; @@ -46,19 +47,17 @@ public async Task ByIdV20(Guid id) private async Task HandleRequest(Guid id) { - IPublishedContent? contentItem = ApiPublishedContentCache.GetById(id); + IPublishedContent? contentItem = await ApiPublishedContentCache.GetByIdAsync(id).ConfigureAwait(false); if (contentItem is null) { return NotFound(); } - - IActionResult? deniedAccessResult = await HandleMemberAccessAsync(contentItem, _requestMemberAccessService); + IActionResult? deniedAccessResult = await HandleMemberAccessAsync(contentItem, _requestMemberAccessService).ConfigureAwait(false); if (deniedAccessResult is not null) { return deniedAccessResult; } - IApiContentResponse? apiContentResponse = ApiContentResponseBuilder.Build(contentItem); if (apiContentResponse is null) { diff --git a/src/Umbraco.Cms.Api.Delivery/Controllers/Content/ByIdsContentApiController.cs b/src/Umbraco.Cms.Api.Delivery/Controllers/Content/ByIdsContentApiController.cs index 61309079c6f3..0771c357581d 100644 --- a/src/Umbraco.Cms.Api.Delivery/Controllers/Content/ByIdsContentApiController.cs +++ b/src/Umbraco.Cms.Api.Delivery/Controllers/Content/ByIdsContentApiController.cs @@ -45,7 +45,7 @@ public async Task ItemsV20([FromQuery(Name = "id")] HashSet private async Task HandleRequest(HashSet ids) { - IPublishedContent[] contentItems = ApiPublishedContentCache.GetByIds(ids).ToArray(); + IPublishedContent[] contentItems = (await ApiPublishedContentCache.GetByIdsAsync(ids).ConfigureAwait(false)).ToArray(); IActionResult? deniedAccessResult = await HandleMemberAccessAsync(contentItems, _requestMemberAccessService); if (deniedAccessResult is not null) diff --git a/src/Umbraco.Core/DeliveryApi/ApiPublishedContentCache.cs b/src/Umbraco.Core/DeliveryApi/ApiPublishedContentCache.cs index 8d4cf4026a12..1c153b0143ac 100644 --- a/src/Umbraco.Core/DeliveryApi/ApiPublishedContentCache.cs +++ b/src/Umbraco.Core/DeliveryApi/ApiPublishedContentCache.cs @@ -32,6 +32,35 @@ public ApiPublishedContentCache( deliveryApiSettings.OnChange(settings => _deliveryApiSettings = settings); } + public async Task GetByRouteAsync(string route) + { + var isPreviewMode = _requestPreviewService.IsPreview(); + + // Handle the nasty logic with domain document ids in front of paths. + int? documentStartNodeId = null; + if (route.StartsWith("/") is false) + { + var index = route.IndexOf('/'); + + if (index > -1 && int.TryParse(route.Substring(0, index), out var nodeId)) + { + documentStartNodeId = nodeId; + route = route.Substring(index); + } + } + + Guid? documentKey = _documentUrlService.GetDocumentKeyByRoute( + route, + _requestCultureService.GetRequestedCulture(), + documentStartNodeId, + _requestPreviewService.IsPreview() + ); + IPublishedContent? content = documentKey.HasValue + ? await _publishedContentCache.GetByIdAsync(documentKey.Value, isPreviewMode) + : null; + + return ContentOrNullIfDisallowed(content); + } public IPublishedContent? GetByRoute(string route) { @@ -64,16 +93,37 @@ public ApiPublishedContentCache( return ContentOrNullIfDisallowed(content); } + public async Task GetByIdAsync(Guid contentId) + { + IPublishedContent? content = await _publishedContentCache.GetByIdAsync(contentId, _requestPreviewService.IsPreview()).ConfigureAwait(false); + return ContentOrNullIfDisallowed(content); + } + public IPublishedContent? GetById(Guid contentId) { IPublishedContent? content = _publishedContentCache.GetById(_requestPreviewService.IsPreview(), contentId); return ContentOrNullIfDisallowed(content); } + public async Task> GetByIdsAsync(IEnumerable contentIds) + { + var isPreviewMode = _requestPreviewService.IsPreview(); + + IEnumerable> tasks = contentIds + .Select(contentId => _publishedContentCache.GetByIdAsync(contentId, isPreviewMode)); + + IPublishedContent?[] allContent = await Task.WhenAll(tasks); + + return allContent + .WhereNotNull() + .Where(IsAllowedContentType) + .ToArray(); + } public IEnumerable GetByIds(IEnumerable contentIds) { + var isPreviewMode = _requestPreviewService.IsPreview(); return contentIds - .Select(contentId => _publishedContentCache.GetById(_requestPreviewService.IsPreview(), contentId)) + .Select(contentId => _publishedContentCache.GetById(isPreviewMode, contentId)) .WhereNotNull() .Where(IsAllowedContentType) .ToArray(); diff --git a/src/Umbraco.Core/DeliveryApi/IApiPublishedContentCache.cs b/src/Umbraco.Core/DeliveryApi/IApiPublishedContentCache.cs index e24e43474cf1..d72eacd1c1cf 100644 --- a/src/Umbraco.Core/DeliveryApi/IApiPublishedContentCache.cs +++ b/src/Umbraco.Core/DeliveryApi/IApiPublishedContentCache.cs @@ -9,4 +9,8 @@ public interface IApiPublishedContentCache IPublishedContent? GetById(Guid contentId); IEnumerable GetByIds(IEnumerable contentIds); + + Task GetByIdAsync(Guid contentId) => Task.FromResult(GetById(contentId)); + Task GetByRouteAsync(string route) => Task.FromResult(GetByRoute(route)); + Task> GetByIdsAsync(IEnumerable contentIds) => Task.FromResult(GetByIds(contentIds)); } diff --git a/src/Umbraco.Core/Services/DocumentUrlService.cs b/src/Umbraco.Core/Services/DocumentUrlService.cs index 0d6fb168fa89..7a125fdc9661 100644 --- a/src/Umbraco.Core/Services/DocumentUrlService.cs +++ b/src/Umbraco.Core/Services/DocumentUrlService.cs @@ -6,6 +6,7 @@ using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Persistence.Repositories; +using Umbraco.Cms.Core.PublishedCache; using Umbraco.Cms.Core.Routing; using Umbraco.Cms.Core.Scoping; using Umbraco.Cms.Core.Services.Navigation; @@ -30,8 +31,8 @@ public class DocumentUrlService : IDocumentUrlService private readonly IKeyValueService _keyValueService; private readonly IIdKeyMap _idKeyMap; private readonly IDocumentNavigationQueryService _documentNavigationQueryService; - private readonly IDomainService _domainService; private readonly IPublishStatusQueryService _publishStatusQueryService; + private readonly IDomainCacheService _domainCacheService; private readonly ConcurrentDictionary _cache = new(); private bool _isInitialized; @@ -49,8 +50,8 @@ public DocumentUrlService( IKeyValueService keyValueService, IIdKeyMap idKeyMap, IDocumentNavigationQueryService documentNavigationQueryService, - IDomainService domainService, - IPublishStatusQueryService publishStatusQueryService) + IPublishStatusQueryService publishStatusQueryService, + IDomainCacheService domainCacheService) { _logger = logger; _documentUrlRepository = documentUrlRepository; @@ -64,8 +65,8 @@ public DocumentUrlService( _keyValueService = keyValueService; _idKeyMap = idKeyMap; _documentNavigationQueryService = documentNavigationQueryService; - _domainService = domainService; _publishStatusQueryService = publishStatusQueryService; + _domainCacheService = domainCacheService; } public async Task InitAsync(bool forceEmpty, CancellationToken cancellationToken) @@ -443,19 +444,26 @@ public string GetLegacyRouteFormat(Guid docuemntKey, string? culture, bool isDra var cultureOrDefault = culture ?? _languageService.GetDefaultIsoCodeAsync().GetAwaiter().GetResult(); Guid[] ancestorsOrSelfKeysArray = ancestorsOrSelfKeys as Guid[] ?? ancestorsOrSelfKeys.ToArray(); - IDictionary ancestorOrSelfKeyToDomains = ancestorsOrSelfKeysArray.ToDictionary(x => x, ancestorKey => + IDictionary ancestorOrSelfKeyToDomains = ancestorsOrSelfKeysArray.ToDictionary(x => x, ancestorKey => { - IEnumerable domains = _domainService.GetAssignedDomainsAsync(ancestorKey, false).GetAwaiter().GetResult(); - return domains.FirstOrDefault(x=>x.LanguageIsoCode == cultureOrDefault); + Attempt idAttempt = _idKeyMap.GetIdForKey(ancestorKey, UmbracoObjectTypes.Document); + + if(idAttempt.Success is false) + { + return null; + } + + IEnumerable domains = _domainCacheService.GetAssigned(idAttempt.Result, false); + return domains.FirstOrDefault(x=>x.Culture == cultureOrDefault); }); var urlSegments = new List(); - IDomain? foundDomain = null; + Domain? foundDomain = null; foreach (Guid ancestorOrSelfKey in ancestorsOrSelfKeysArray) { - if (ancestorOrSelfKeyToDomains.TryGetValue(ancestorOrSelfKey, out IDomain? domain)) + if (ancestorOrSelfKeyToDomains.TryGetValue(ancestorOrSelfKey, out Domain? domain)) { if (domain is not null) { @@ -478,7 +486,7 @@ public string GetLegacyRouteFormat(Guid docuemntKey, string? culture, bool isDra if (foundDomain is not null) { //we found a domain, and not to construct the route in the funny legacy way - return foundDomain.RootContentId + "/" + string.Join("/", urlSegments); + return foundDomain.ContentId + "/" + string.Join("/", urlSegments); } var isRootFirstItem = GetTopMostRootKey(isDraft, cultureOrDefault) == ancestorsOrSelfKeysArray.Last(); @@ -510,24 +518,30 @@ public async Task> ListUrlsAsync(Guid contentKey) var cultures = languages.ToDictionary(x=>x.IsoCode); Guid[] ancestorsOrSelfKeysArray = ancestorsOrSelfKeys as Guid[] ?? ancestorsOrSelfKeys.ToArray(); - Dictionary>> ancestorOrSelfKeyToDomains = ancestorsOrSelfKeysArray.ToDictionary(x => x, async ancestorKey => + Dictionary>> ancestorOrSelfKeyToDomains = ancestorsOrSelfKeysArray.ToDictionary(x => x, async ancestorKey => { - IEnumerable domains = await _domainService.GetAssignedDomainsAsync(ancestorKey, false); - return domains.ToDictionary(x => x.LanguageIsoCode!); - }); + Attempt idAttempt = _idKeyMap.GetIdForKey(ancestorKey, UmbracoObjectTypes.Document); + + if(idAttempt.Success is false) + { + return null; + } + IEnumerable domains = _domainCacheService.GetAssigned(idAttempt.Result, false); + return domains.ToDictionary(x => x.Culture!); + })!; foreach ((string culture, ILanguage language) in cultures) { var urlSegments = new List(); - IDomain? foundDomain = null; + Domain? foundDomain = null; var hasUrlInCulture = true; foreach (Guid ancestorOrSelfKey in ancestorsOrSelfKeysArray) { - if (ancestorOrSelfKeyToDomains.TryGetValue(ancestorOrSelfKey, out Task>? domainDictionaryTask)) + if (ancestorOrSelfKeyToDomains.TryGetValue(ancestorOrSelfKey, out Task>? domainDictionaryTask)) { - Dictionary domainDictionary = await domainDictionaryTask; - if (domainDictionary.TryGetValue(culture, out IDomain? domain)) + Dictionary domainDictionary = await domainDictionaryTask; + if (domainDictionary.TryGetValue(culture, out Domain? domain)) { foundDomain = domain; break; @@ -562,14 +576,14 @@ public async Task> ListUrlsAsync(Guid contentKey) return result; } - private string GetFullUrl(bool isRootFirstItem, List reversedUrlSegments, IDomain? foundDomain) + private string GetFullUrl(bool isRootFirstItem, List reversedUrlSegments, Domain? foundDomain) { var urlSegments = new List(reversedUrlSegments); urlSegments.Reverse(); if (foundDomain is not null) { - return foundDomain.DomainName.EnsureEndsWith("/") + string.Join('/', urlSegments); + return foundDomain.Name.EnsureEndsWith("/") + string.Join('/', urlSegments); } return '/' + string.Join('/', urlSegments.Skip(_globalSettings.HideTopLevelNodeFromPath && isRootFirstItem ? 1 : 0));