From 83230d377c114134166f6375d2d44305afce6d9c Mon Sep 17 00:00:00 2001 From: Elitsa Marinovska <21998037+elit0451@users.noreply.github.com> Date: Mon, 8 May 2023 06:52:02 +0200 Subject: [PATCH] Handle unhappy paths for the multiple items endpoint (#14199) --- .../Controllers/ContentApiControllerBase.cs | 24 +++++ .../Controllers/QueryContentApiController.cs | 15 +++- .../Services/ApiContentQueryService.cs | 90 +++++++++++-------- .../DeliveryApi/IApiContentQueryService.cs | 7 +- .../DeliveryApi/NoopApiContentQueryService.cs | 5 +- .../ApiContentQueryOperationStatus.cs | 10 +++ 6 files changed, 108 insertions(+), 43 deletions(-) create mode 100644 src/Umbraco.Core/Services/OperationStatus/ApiContentQueryOperationStatus.cs diff --git a/src/Umbraco.Cms.Api.Delivery/Controllers/ContentApiControllerBase.cs b/src/Umbraco.Cms.Api.Delivery/Controllers/ContentApiControllerBase.cs index ea85c5f544..1213def424 100644 --- a/src/Umbraco.Cms.Api.Delivery/Controllers/ContentApiControllerBase.cs +++ b/src/Umbraco.Cms.Api.Delivery/Controllers/ContentApiControllerBase.cs @@ -1,6 +1,9 @@ using Microsoft.AspNetCore.Mvc; +using Umbraco.Cms.Api.Common.Builders; using Umbraco.Cms.Api.Delivery.Routing; +using Umbraco.Cms.Core; using Umbraco.Cms.Core.DeliveryApi; +using Umbraco.Cms.Core.Services.OperationStatus; namespace Umbraco.Cms.Api.Delivery.Controllers; @@ -17,4 +20,25 @@ public abstract class ContentApiControllerBase : DeliveryApiControllerBase ApiPublishedContentCache = apiPublishedContentCache; ApiContentResponseBuilder = apiContentResponseBuilder; } + + protected IActionResult ApiContentQueryOperationStatusResult(ApiContentQueryOperationStatus status) => + status switch + { + ApiContentQueryOperationStatus.FilterOptionNotFound => BadRequest(new ProblemDetailsBuilder() + .WithTitle("Filter option not found") + .WithDetail("One of the attempted 'filter' options does not exist") + .Build()), + ApiContentQueryOperationStatus.IndexNotFound => BadRequest(new ProblemDetailsBuilder() + .WithTitle("Examine index not found") + .WithDetail($"No index found with name {Constants.UmbracoIndexes.DeliveryApiContentIndexName}") + .Build()), + ApiContentQueryOperationStatus.SelectorOptionNotFound => BadRequest(new ProblemDetailsBuilder() + .WithTitle("Selector option not found") + .WithDetail("The attempted 'fetch' option does not exist") + .Build()), + ApiContentQueryOperationStatus.SortOptionNotFound => BadRequest(new ProblemDetailsBuilder() + .WithTitle("Sort option not found") + .WithDetail("One of the attempted 'sort' options does not exist") + .Build()), + }; } diff --git a/src/Umbraco.Cms.Api.Delivery/Controllers/QueryContentApiController.cs b/src/Umbraco.Cms.Api.Delivery/Controllers/QueryContentApiController.cs index e220269310..c1a6d8e197 100644 --- a/src/Umbraco.Cms.Api.Delivery/Controllers/QueryContentApiController.cs +++ b/src/Umbraco.Cms.Api.Delivery/Controllers/QueryContentApiController.cs @@ -1,9 +1,11 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; using Umbraco.Cms.Api.Common.ViewModels.Pagination; +using Umbraco.Cms.Core; using Umbraco.Cms.Core.DeliveryApi; using Umbraco.Cms.Core.Models.DeliveryApi; using Umbraco.Cms.Core.Models.PublishedContent; +using Umbraco.Cms.Core.Services.OperationStatus; using Umbraco.New.Cms.Core.Models; namespace Umbraco.Cms.Api.Delivery.Controllers; @@ -31,15 +33,22 @@ public class QueryContentApiController : ContentApiControllerBase [HttpGet] [MapToApiVersion("1.0")] [ProducesResponseType(typeof(PagedViewModel), StatusCodes.Status200OK)] - [ProducesResponseType(StatusCodes.Status400BadRequest)] - public async Task>> Query( + [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status400BadRequest)] + public async Task Query( string? fetch, [FromQuery] string[] filter, [FromQuery] string[] sort, int skip = 0, int take = 10) { - PagedModel pagedResult = _apiContentQueryService.ExecuteQuery(fetch, filter, sort, skip, take); + Attempt, ApiContentQueryOperationStatus> queryAttempt = _apiContentQueryService.ExecuteQuery(fetch, filter, sort, skip, take); + + if (queryAttempt.Success is false) + { + return ApiContentQueryOperationStatusResult(queryAttempt.Status); + } + + PagedModel pagedResult = queryAttempt.Result; IEnumerable contentItems = ApiPublishedContentCache.GetByIds(pagedResult.Items); IApiContentResponse[] apiContentItems = contentItems.Select(ApiContentResponseBuilder.Build).ToArray(); diff --git a/src/Umbraco.Cms.Api.Delivery/Services/ApiContentQueryService.cs b/src/Umbraco.Cms.Api.Delivery/Services/ApiContentQueryService.cs index 7a6a0181e8..83d0f1652f 100644 --- a/src/Umbraco.Cms.Api.Delivery/Services/ApiContentQueryService.cs +++ b/src/Umbraco.Cms.Api.Delivery/Services/ApiContentQueryService.cs @@ -5,6 +5,7 @@ using Umbraco.Cms.Api.Delivery.Indexing.Sorts; using Umbraco.Cms.Core; using Umbraco.Cms.Core.DeliveryApi; using Umbraco.Cms.Core.Models.PublishedContent; +using Umbraco.Cms.Core.Services.OperationStatus; using Umbraco.Cms.Infrastructure.Examine; using Umbraco.New.Cms.Core.Models; @@ -39,13 +40,13 @@ internal sealed class ApiContentQueryService : IApiContentQueryService // Examin } /// - public PagedModel ExecuteQuery(string? fetch, IEnumerable filters, IEnumerable sorts, int skip, int take) + public Attempt, ApiContentQueryOperationStatus> ExecuteQuery(string? fetch, IEnumerable filters, IEnumerable sorts, int skip, int take) { var emptyResult = new PagedModel(); if (!_examineManager.TryGetIndex(Constants.UmbracoIndexes.DeliveryApiContentIndexName, out IIndex? apiIndex)) { - return emptyResult; + return Attempt.FailWithStatus(ApiContentQueryOperationStatus.IndexNotFound, emptyResult); } IQuery baseQuery = apiIndex.Searcher.CreateQuery(); @@ -56,14 +57,26 @@ internal sealed class ApiContentQueryService : IApiContentQueryService // Examin // If no Selector could be found, we return no results if (queryOperation is null) { - return emptyResult; + return Attempt.FailWithStatus(ApiContentQueryOperationStatus.SelectorOptionNotFound, emptyResult); } // Handle Filtering - HandleFiltering(filters, queryOperation); + var canApplyFiltering = CanHandleFiltering(filters, queryOperation); + + // If there is an invalid Filter option, we return no results + if (canApplyFiltering is false) + { + return Attempt.FailWithStatus(ApiContentQueryOperationStatus.FilterOptionNotFound, emptyResult); + } // Handle Sorting - IOrdering sortQuery = HandleSorting(sorts, queryOperation).SelectFields(_itemIdOnlyFieldSet); + IOrdering? sortQuery = HandleSorting(sorts, queryOperation)?.SelectFields(_itemIdOnlyFieldSet); + + // If there is an invalid Sort option, we return no results + if (sortQuery is null) + { + return Attempt.FailWithStatus(ApiContentQueryOperationStatus.SortOptionNotFound, emptyResult); + } ISearchResults? results = sortQuery .SelectFields(_itemIdOnlyFieldSet) @@ -71,14 +84,16 @@ internal sealed class ApiContentQueryService : IApiContentQueryService // Examin if (results is null) { - return emptyResult; + // The query yield no results + return Attempt.SucceedWithStatus(ApiContentQueryOperationStatus.Success, emptyResult); } Guid[] items = results .Where(r => r.Values.ContainsKey("itemId")) .Select(r => Guid.Parse(r.Values["itemId"])) .ToArray(); - return new PagedModel(results.TotalItemCount, items); + + return Attempt.SucceedWithStatus(ApiContentQueryOperationStatus.Success, new PagedModel(results.TotalItemCount, items)); } private IBooleanOperation? HandleSelector(string? fetch, IQuery baseQuery) @@ -121,45 +136,50 @@ internal sealed class ApiContentQueryService : IApiContentQueryService // Examin return baseQuery.Field(fieldName, fieldValue); } - private void HandleFiltering(IEnumerable filters, IBooleanOperation queryOperation) + private bool CanHandleFiltering(IEnumerable filters, IBooleanOperation queryOperation) { foreach (var filterValue in filters) { IFilterHandler? filterHandler = _filterHandlers.FirstOrDefault(h => h.CanHandle(filterValue)); FilterOption? filter = filterHandler?.BuildFilterOption(filterValue); - if (filter is not null) + if (filter is null) { - var value = string.IsNullOrWhiteSpace(filter.Value) == false - ? filter.Value - : _fallbackGuidValue; + return false; + } - switch (filter.Operator) - { - case FilterOperation.Is: - queryOperation.And().Field(filter.FieldName, - (IExamineValue)new ExamineValue(Examineness.Explicit, - value)); // TODO: doesn't work for explicit word(s) match - break; - case FilterOperation.IsNot: - queryOperation.Not().Field(filter.FieldName, - (IExamineValue)new ExamineValue(Examineness.Explicit, - value)); // TODO: doesn't work for explicit word(s) match - break; - // TODO: Fix - case FilterOperation.Contains: - break; - // TODO: Fix - case FilterOperation.DoesNotContain: - break; - default: - continue; - } + var value = string.IsNullOrWhiteSpace(filter.Value) == false + ? filter.Value + : _fallbackGuidValue; + + switch (filter.Operator) + { + case FilterOperation.Is: + queryOperation.And().Field(filter.FieldName, + (IExamineValue)new ExamineValue(Examineness.Explicit, + value)); // TODO: doesn't work for explicit word(s) match + break; + case FilterOperation.IsNot: + queryOperation.Not().Field(filter.FieldName, + (IExamineValue)new ExamineValue(Examineness.Explicit, + value)); // TODO: doesn't work for explicit word(s) match + break; + // TODO: Fix + case FilterOperation.Contains: + break; + // TODO: Fix + case FilterOperation.DoesNotContain: + break; + default: + continue; } } + + return true; } - private IOrdering HandleSorting(IEnumerable sorts, IBooleanOperation queryCriteria) + + private IOrdering? HandleSorting(IEnumerable sorts, IBooleanOperation queryCriteria) { IOrdering? orderingQuery = null; @@ -170,7 +190,7 @@ internal sealed class ApiContentQueryService : IApiContentQueryService // Examin if (sort is null) { - continue; + return null; } SortType sortType = sort.FieldType switch diff --git a/src/Umbraco.Core/DeliveryApi/IApiContentQueryService.cs b/src/Umbraco.Core/DeliveryApi/IApiContentQueryService.cs index 142cd4734f..e026264ba2 100644 --- a/src/Umbraco.Core/DeliveryApi/IApiContentQueryService.cs +++ b/src/Umbraco.Core/DeliveryApi/IApiContentQueryService.cs @@ -1,3 +1,4 @@ +using Umbraco.Cms.Core.Services.OperationStatus; using Umbraco.New.Cms.Core.Models; namespace Umbraco.Cms.Core.DeliveryApi; @@ -8,13 +9,13 @@ namespace Umbraco.Cms.Core.DeliveryApi; public interface IApiContentQueryService { /// - /// Returns a collection of item ids that passed the search criteria as a paged model. + /// Returns an attempt with a collection of item ids that passed the search criteria as a paged model. /// /// Optional fetch query parameter value. /// Optional filter query parameters values. /// Optional sort query parameters values. /// The amount of items to skip. /// The amount of items to take. - /// A paged model of item ids that are returned after applying the search queries. - PagedModel ExecuteQuery(string? fetch, IEnumerable filters, IEnumerable sorts, int skip, int take); + /// A paged model of item ids that are returned after applying the search queries in an attempt. + Attempt, ApiContentQueryOperationStatus> ExecuteQuery(string? fetch, IEnumerable filters, IEnumerable sorts, int skip, int take); } diff --git a/src/Umbraco.Core/DeliveryApi/NoopApiContentQueryService.cs b/src/Umbraco.Core/DeliveryApi/NoopApiContentQueryService.cs index f95aa8a06e..96c60eeae7 100644 --- a/src/Umbraco.Core/DeliveryApi/NoopApiContentQueryService.cs +++ b/src/Umbraco.Core/DeliveryApi/NoopApiContentQueryService.cs @@ -1,3 +1,4 @@ +using Umbraco.Cms.Core.Services.OperationStatus; using Umbraco.New.Cms.Core.Models; namespace Umbraco.Cms.Core.DeliveryApi; @@ -5,6 +6,6 @@ namespace Umbraco.Cms.Core.DeliveryApi; public sealed class NoopApiContentQueryService : IApiContentQueryService { /// - public PagedModel ExecuteQuery(string? fetch, IEnumerable filters, IEnumerable sorts, int skip, int take) - => new(); + public Attempt, ApiContentQueryOperationStatus> ExecuteQuery(string? fetch, IEnumerable filters, IEnumerable sorts, int skip, int take) + => Attempt.SucceedWithStatus(ApiContentQueryOperationStatus.Success, new PagedModel()); } diff --git a/src/Umbraco.Core/Services/OperationStatus/ApiContentQueryOperationStatus.cs b/src/Umbraco.Core/Services/OperationStatus/ApiContentQueryOperationStatus.cs new file mode 100644 index 0000000000..0c3c859070 --- /dev/null +++ b/src/Umbraco.Core/Services/OperationStatus/ApiContentQueryOperationStatus.cs @@ -0,0 +1,10 @@ +namespace Umbraco.Cms.Core.Services.OperationStatus; + +public enum ApiContentQueryOperationStatus +{ + Success, + FilterOptionNotFound, + IndexNotFound, + SelectorOptionNotFound, + SortOptionNotFound +}