From 96c0509719492577d8a34d5baa52a3043d5b0cbb Mon Sep 17 00:00:00 2001 From: Kenn Jacobsen Date: Tue, 15 Apr 2025 16:18:42 +0200 Subject: [PATCH] Content search abstractions to facilitate new search in the backoffice (#19046) --- .../Services/IContentSearchService.cs | 7 + .../IContentSearchServiceOfTContent.cs | 14 ++ .../Services/IMediaSearchService.cs | 7 + .../UmbracoBuilder.Services.cs | 2 + .../Services/ContentListViewServiceBase.cs | 42 +----- .../Implement/ContentListViewService.cs | 30 +--- .../Implement/ContentSearchService.cs | 33 ++++ .../Implement/ContentSearchServiceBase.cs | 77 ++++++++++ .../Implement/MediaListViewService.cs | 27 +--- .../Services/Implement/MediaSearchService.cs | 33 ++++ .../Services/ContentSearchServiceTests.cs | 141 ++++++++++++++++++ 11 files changed, 325 insertions(+), 88 deletions(-) create mode 100644 src/Umbraco.Core/Services/IContentSearchService.cs create mode 100644 src/Umbraco.Core/Services/IContentSearchServiceOfTContent.cs create mode 100644 src/Umbraco.Core/Services/IMediaSearchService.cs create mode 100644 src/Umbraco.Infrastructure/Services/Implement/ContentSearchService.cs create mode 100644 src/Umbraco.Infrastructure/Services/Implement/ContentSearchServiceBase.cs create mode 100644 src/Umbraco.Infrastructure/Services/Implement/MediaSearchService.cs create mode 100644 tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentSearchServiceTests.cs diff --git a/src/Umbraco.Core/Services/IContentSearchService.cs b/src/Umbraco.Core/Services/IContentSearchService.cs new file mode 100644 index 0000000000..afebc9d871 --- /dev/null +++ b/src/Umbraco.Core/Services/IContentSearchService.cs @@ -0,0 +1,7 @@ +using Umbraco.Cms.Core.Models; + +namespace Umbraco.Cms.Core.Services; + +public interface IContentSearchService : IContentSearchService +{ +} diff --git a/src/Umbraco.Core/Services/IContentSearchServiceOfTContent.cs b/src/Umbraco.Core/Services/IContentSearchServiceOfTContent.cs new file mode 100644 index 0000000000..ca8b88d4bc --- /dev/null +++ b/src/Umbraco.Core/Services/IContentSearchServiceOfTContent.cs @@ -0,0 +1,14 @@ +using Umbraco.Cms.Core.Models; + +namespace Umbraco.Cms.Core.Services; + +public interface IContentSearchService + where TContent : class, IContentBase +{ + Task> SearchChildrenAsync( + string? query, + Guid? parentId, + Ordering? ordering, + int skip = 0, + int take = 100); +} diff --git a/src/Umbraco.Core/Services/IMediaSearchService.cs b/src/Umbraco.Core/Services/IMediaSearchService.cs new file mode 100644 index 0000000000..ce8262956f --- /dev/null +++ b/src/Umbraco.Core/Services/IMediaSearchService.cs @@ -0,0 +1,7 @@ +using Umbraco.Cms.Core.Models; + +namespace Umbraco.Cms.Core.Services; + +public interface IMediaSearchService : IContentSearchService +{ +} diff --git a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.Services.cs b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.Services.cs index 6f2beb4ef9..4d78be637b 100644 --- a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.Services.cs +++ b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.Services.cs @@ -81,6 +81,8 @@ public static partial class UmbracoBuilderExtensions builder.Services.AddUnique(); builder.Services.AddUnique(); builder.Services.TryAddTransient(); + builder.Services.AddUnique(); + builder.Services.AddUnique(); return builder; } diff --git a/src/Umbraco.Infrastructure/Services/ContentListViewServiceBase.cs b/src/Umbraco.Infrastructure/Services/ContentListViewServiceBase.cs index 2d7794d5aa..38459d3b91 100644 --- a/src/Umbraco.Infrastructure/Services/ContentListViewServiceBase.cs +++ b/src/Umbraco.Infrastructure/Services/ContentListViewServiceBase.cs @@ -1,11 +1,9 @@ using Umbraco.Cms.Core; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.Membership; -using Umbraco.Cms.Core.Persistence.Querying; using Umbraco.Cms.Core.PropertyEditors; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Services.OperationStatus; -using Umbraco.Cms.Infrastructure.Persistence; using Umbraco.Extensions; namespace Umbraco.Cms.Infrastructure.Services; @@ -17,24 +15,17 @@ internal abstract class ContentListViewServiceBase _contentSearchService; - protected ContentListViewServiceBase(TContentTypeService contentTypeService, IDataTypeService dataTypeService, ISqlContext sqlContext) + protected ContentListViewServiceBase(TContentTypeService contentTypeService, IDataTypeService dataTypeService, IContentSearchService contentSearchService) { _contentTypeService = contentTypeService; _dataTypeService = dataTypeService; - _sqlContext = sqlContext; + _contentSearchService = contentSearchService; } protected abstract Guid DefaultListViewKey { get; } - protected abstract Task> GetPagedChildrenAsync( - int id, - IQuery? filter, - Ordering? ordering, - int skip, - int take); - protected abstract Task HasAccessToListViewItemAsync(IUser user, Guid key); protected async Task?, ContentCollectionOperationStatus>> GetListViewResultAsync( @@ -62,7 +53,7 @@ internal abstract class ContentListViewServiceBase?, ContentCollectionOperationStatus>(orderingAttempt.Status, null); } - PagedModel items = await GetAllowedListViewItemsAsync(user, content?.Id ?? Constants.System.Root, filter, orderingAttempt.Result, skip, take); + PagedModel items = await GetAllowedListViewItemsAsync(user, content?.Key, filter, orderingAttempt.Result, skip, take); var result = new ListViewPagedModel { @@ -216,20 +207,13 @@ internal abstract class ContentListViewServiceBase> GetAllowedListViewItemsAsync(IUser user, int contentId, string? filter, Ordering? ordering, int skip, int take) + private async Task> GetAllowedListViewItemsAsync(IUser user, Guid? contentId, string? filter, Ordering? ordering, int skip, int take) { - var queryFilter = ParseQueryFilter(filter); - - var pagedChildren = await GetPagedChildrenAsync( - contentId, - queryFilter, - ordering, - skip, - take); + PagedModel pagedChildren = await _contentSearchService.SearchChildrenAsync(filter, contentId, ordering, skip, take); // Filtering out child nodes after getting a paged result is an active choice here, even though the pagination might get off. // This has been the case with this functionality in Umbraco for a long time. - var items = await FilterItemsBasedOnAccessAsync(user, pagedChildren.Items); + IEnumerable items = await FilterItemsBasedOnAccessAsync(user, pagedChildren.Items); var pagedResult = new PagedModel { @@ -240,18 +224,6 @@ internal abstract class ContentListViewServiceBase? ParseQueryFilter(string? filter) - { - // Adding multiple conditions - considering key (as Guid) & name as filter param - Guid.TryParse(filter, out Guid filterAsGuid); - - return filter.IsNullOrWhiteSpace() - ? null - : _sqlContext.Query() - .Where(c => (c.Name != null && c.Name.Contains(filter)) || - c.Key == filterAsGuid); - } - // TODO: Optimize the way we filter out only the nodes the user is allowed to see - instead of checking one by one private async Task> FilterItemsBasedOnAccessAsync(IUser user, IEnumerable items) { diff --git a/src/Umbraco.Infrastructure/Services/Implement/ContentListViewService.cs b/src/Umbraco.Infrastructure/Services/Implement/ContentListViewService.cs index e9d46ddaa8..4bab473d55 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/ContentListViewService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/ContentListViewService.cs @@ -21,9 +21,9 @@ internal sealed class ContentListViewService : ContentListViewServiceBase> GetPagedChildrenAsync( - int id, - IQuery? filter, - Ordering? ordering, - int skip, - int take) - { - PaginationHelper.ConvertSkipTakeToPaging(skip, take, out var pageNumber, out var pageSize); - - IEnumerable items = _contentService.GetPagedChildren( - id, - pageNumber, - pageSize, - out var total, - filter, - ordering); - - var pagedResult = new PagedModel - { - Items = items, - Total = total, - }; - - return Task.FromResult(pagedResult); - } - // We can use an authorizer here, as it already handles all the necessary checks for this filtering. // However, we cannot pass in all the items; we want only the ones that comply, as opposed to // a general response whether the user has access to all nodes. diff --git a/src/Umbraco.Infrastructure/Services/Implement/ContentSearchService.cs b/src/Umbraco.Infrastructure/Services/Implement/ContentSearchService.cs new file mode 100644 index 0000000000..0aea3ff871 --- /dev/null +++ b/src/Umbraco.Infrastructure/Services/Implement/ContentSearchService.cs @@ -0,0 +1,33 @@ +using Microsoft.Extensions.Logging; +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.Persistence.Querying; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Infrastructure.Persistence; + +namespace Umbraco.Cms.Infrastructure.Services.Implement; + +internal sealed class ContentSearchService : ContentSearchServiceBase, IContentSearchService +{ + private readonly IContentService _contentService; + + public ContentSearchService(ISqlContext sqlContext, IIdKeyMap idKeyMap, ILogger logger, IContentService contentService) + : base(sqlContext, idKeyMap, logger) + => _contentService = contentService; + + protected override UmbracoObjectTypes ObjectType => UmbracoObjectTypes.Document; + + protected override Task> SearchChildrenAsync( + IQuery? query, + int parentId, + Ordering? ordering, + long pageNumber, + int pageSize, + out long total) + => Task.FromResult(_contentService.GetPagedChildren( + parentId, + pageNumber, + pageSize, + out total, + query, + ordering)); +} diff --git a/src/Umbraco.Infrastructure/Services/Implement/ContentSearchServiceBase.cs b/src/Umbraco.Infrastructure/Services/Implement/ContentSearchServiceBase.cs new file mode 100644 index 0000000000..bf95880b8b --- /dev/null +++ b/src/Umbraco.Infrastructure/Services/Implement/ContentSearchServiceBase.cs @@ -0,0 +1,77 @@ +using Microsoft.Extensions.Logging; +using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.Persistence.Querying; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Infrastructure.Persistence; +using Umbraco.Extensions; + +namespace Umbraco.Cms.Infrastructure.Services.Implement; + +internal abstract class ContentSearchServiceBase : IContentSearchService + where TContent : class, IContentBase +{ + private readonly ISqlContext _sqlContext; + private readonly IIdKeyMap _idKeyMap; + private readonly ILogger> _logger; + + protected ContentSearchServiceBase(ISqlContext sqlContext, IIdKeyMap idKeyMap, ILogger> logger) + { + _sqlContext = sqlContext; + _idKeyMap = idKeyMap; + _logger = logger; + } + + protected abstract UmbracoObjectTypes ObjectType { get; } + + protected abstract Task> SearchChildrenAsync( + IQuery? query, + int parentId, + Ordering? ordering, + long pageNumber, + int pageSize, + out long total); + + public async Task> SearchChildrenAsync( + string? query, + Guid? parentId, + Ordering? ordering, + int skip = 0, + int take = 100) + { + var parentIdAsInt = Constants.System.Root; + if (parentId.HasValue) + { + Attempt keyToId = _idKeyMap.GetIdForKey(parentId.Value, ObjectType); + if (keyToId.Success is false) + { + _logger.LogWarning("Could not obtain an ID for parent key: {parentId} (object type: {contentType}", parentId, typeof(TContent).FullName); + return new PagedModel(0, []); + } + + parentIdAsInt = keyToId.Result; + } + + PaginationHelper.ConvertSkipTakeToPaging(skip, take, out var pageNumber, out var pageSize); + IQuery? contentQuery = ParseQuery(query); + + IEnumerable items = await SearchChildrenAsync(contentQuery, parentIdAsInt, ordering, pageNumber, pageSize, out var total); + return new PagedModel + { + Items = items, + Total = total, + }; + } + + private IQuery? ParseQuery(string? query) + { + // Adding multiple conditions - considering key (as Guid) & name as filter param + Guid.TryParse(query, out Guid filterAsGuid); + + return query.IsNullOrWhiteSpace() + ? null + : _sqlContext + .Query() + .Where(c => (c.Name != null && c.Name.Contains(query)) || c.Key == filterAsGuid); + } +} diff --git a/src/Umbraco.Infrastructure/Services/Implement/MediaListViewService.cs b/src/Umbraco.Infrastructure/Services/Implement/MediaListViewService.cs index 63d9298ec3..5537697f75 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/MediaListViewService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/MediaListViewService.cs @@ -1,11 +1,9 @@ using Umbraco.Cms.Core; using Umbraco.Cms.Core.Models; using Umbraco.Cms.Core.Models.Membership; -using Umbraco.Cms.Core.Persistence.Querying; using Umbraco.Cms.Core.Security.Authorization; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Services.OperationStatus; -using Umbraco.Cms.Infrastructure.Persistence; namespace Umbraco.Cms.Infrastructure.Services.Implement; @@ -20,9 +18,9 @@ internal sealed class MediaListViewService : ContentListViewServiceBase> GetPagedChildrenAsync(int id, IQuery? filter, Ordering? ordering, int skip, int take) - { - PaginationHelper.ConvertSkipTakeToPaging(skip, take, out var pageNumber, out var pageSize); - - IEnumerable items = _mediaService.GetPagedChildren( - id, - pageNumber, - pageSize, - out var total, - filter, - ordering); - - var pagedResult = new PagedModel - { - Items = items, - Total = total, - }; - - return Task.FromResult(pagedResult); - } - // We can use an authorizer here, as it already handles all the necessary checks for this filtering. // However, we cannot pass in all the items; we want only the ones that comply, as opposed to // a general response whether the user has access to all nodes. diff --git a/src/Umbraco.Infrastructure/Services/Implement/MediaSearchService.cs b/src/Umbraco.Infrastructure/Services/Implement/MediaSearchService.cs new file mode 100644 index 0000000000..17bc62e7a9 --- /dev/null +++ b/src/Umbraco.Infrastructure/Services/Implement/MediaSearchService.cs @@ -0,0 +1,33 @@ +using Microsoft.Extensions.Logging; +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.Persistence.Querying; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Infrastructure.Persistence; + +namespace Umbraco.Cms.Infrastructure.Services.Implement; + +internal sealed class MediaSearchService : ContentSearchServiceBase, IMediaSearchService +{ + private readonly IMediaService _mediaService; + + public MediaSearchService(ISqlContext sqlContext, IIdKeyMap idKeyMap, ILogger logger, IMediaService mediaService) + : base(sqlContext, idKeyMap, logger) + => _mediaService = mediaService; + + protected override UmbracoObjectTypes ObjectType => UmbracoObjectTypes.Media; + + protected override Task> SearchChildrenAsync( + IQuery? query, + int parentId, + Ordering? ordering, + long pageNumber, + int pageSize, + out long total) + => Task.FromResult(_mediaService.GetPagedChildren( + parentId, + pageNumber, + pageSize, + out total, + query, + ordering)); +} diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentSearchServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentSearchServiceTests.cs new file mode 100644 index 0000000000..035e1aa4e2 --- /dev/null +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ContentSearchServiceTests.cs @@ -0,0 +1,141 @@ +using NUnit.Framework; +using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Models; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Tests.Common.Builders; +using Umbraco.Cms.Tests.Common.Builders.Extensions; +using Umbraco.Cms.Tests.Common.Testing; +using Umbraco.Cms.Tests.Integration.Testing; + +namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services; + +// NOTE: this is not an exhaustive test suite for IContentSearchService, because the core implementation simply +// wraps IContentService to search children (IContentService.GetPagedChildren), and that's tested elsewhere. +// instead, these tests aim at testing the logic of the shared base between the core implementations of +// IContentSearchService and IMediaSearchService (which does the same thing using IMediaService). +[TestFixture] +[UmbracoTest( + Database = UmbracoTestOptions.Database.NewSchemaPerFixture, + PublishedRepositoryEvents = true, + WithApplication = true)] +public class ContentSearchServiceTests : UmbracoIntegrationTest +{ + private Dictionary _contentByName = new (); + + protected IContentSearchService ContentSearchService => GetRequiredService(); + + protected IContentService ContentService => GetRequiredService(); + + protected IContentTypeService ContentTypeService => GetRequiredService(); + + [Test] + public async Task Can_Search_Children_Of_System_Root() + { + var result = await ContentSearchService.SearchChildrenAsync(null, null, null, 0, 1000); + Assert.Multiple(() => + { + Assert.AreEqual(3, result.Total); + Assert.AreEqual(3, result.Items.Count()); + }); + + var resultKeys = result.Items.Select(item => item.Key).ToArray(); + var expectedKeys = new[] + { + _contentByName["Root 1"].Key, + _contentByName["Root 2"].Key, + _contentByName["Root 3"].Key + }; + CollectionAssert.AreEqual(expectedKeys, resultKeys); + } + + [Test] + public async Task Can_Search_Children_Of_Specified_Parent() + { + var result = await ContentSearchService.SearchChildrenAsync(null, _contentByName["Root 1"].Key, null, 0, 1000); + Assert.Multiple(() => + { + Assert.AreEqual(5, result.Total); + Assert.AreEqual(5, result.Items.Count()); + }); + + var resultKeys = result.Items.Select(item => item.Key).ToArray(); + var expectedKeys = new[] + { + _contentByName["Root 1/Child 1"].Key, + _contentByName["Root 1/Child 2"].Key, + _contentByName["Root 1/Child 3"].Key, + _contentByName["Root 1/Child 4"].Key, + _contentByName["Root 1/Child 5"].Key + }; + CollectionAssert.AreEqual(expectedKeys, resultKeys); + } + + [Test] + public async Task Can_Apply_Pagination_With_Skip_Take() + { + var result = await ContentSearchService.SearchChildrenAsync(null, _contentByName["Root 2"].Key, Ordering.By("name"), 2, 2); + Assert.Multiple(() => + { + Assert.AreEqual(5, result.Total); + Assert.AreEqual(2, result.Items.Count()); + }); + + var resultKeys = result.Items.Select(item => item.Key).ToArray(); + var expectedKeys = new[] + { + _contentByName["Root 2/Child 3"].Key, + _contentByName["Root 2/Child 4"].Key + }; + CollectionAssert.AreEqual(expectedKeys, resultKeys); + } + + [Test] + public async Task Can_Filter_By_Name() + { + var result = await ContentSearchService.SearchChildrenAsync("2", _contentByName["Root 3"].Key, null); + Assert.Multiple(() => + { + Assert.AreEqual(1, result.Total); + Assert.AreEqual(1, result.Items.Count()); + }); + + Assert.AreEqual(_contentByName["Root 3/Child 2"].Key, result.Items.First().Key); + } + + [SetUp] + public async Task SetUpTest() + { + if (_contentByName.Any()) + { + return; + } + + var contentType = new ContentTypeBuilder() + .WithAlias("theContentType") + .Build(); + contentType.AllowedAsRoot = true; + await ContentTypeService.CreateAsync(contentType, Constants.Security.SuperUserKey); + contentType.AllowedContentTypes = [new() { Alias = contentType.Alias, Key = contentType.Key }]; + await ContentTypeService.UpdateAsync(contentType, Constants.Security.SuperUserKey); + foreach (var rootNumber in Enumerable.Range(1, 3)) + { + var root = new ContentBuilder() + .WithContentType(contentType) + .WithName($"Root {rootNumber}") + .Build(); + ContentService.Save(root); + _contentByName[root.Name!] = root; + + foreach (var childNumber in Enumerable.Range(1, 5)) + { + var child = new ContentBuilder() + .WithContentType(contentType) + .WithParent(root) + .WithName($"Child {childNumber}") + .Build(); + ContentService.Save(child); + _contentByName[$"{root.Name!}/{child.Name!}"] = child; + } + } + } +}