Fixed filter queries for numeric and date based filters (#15286)

Co-authored-by: Elitsa <elm@umbraco.dk>

src/Umbraco.Cms.Api.Delivery/Services/ApiContentQueryProvider.cs

@@ -1,16 +1,14 @@
 using Examine;
-using Examine.Lucene.Providers;
-using Examine.Lucene.Search;
 using Examine.Search;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
+using Umbraco.Cms.Api.Delivery.Services.QueryBuilders;
 using Umbraco.Cms.Core;
 using Umbraco.Cms.Core.Configuration.Models;
 using Umbraco.Cms.Core.DeliveryApi;
 using Umbraco.Cms.Core.Models;
 using Umbraco.Cms.Core.Models.DeliveryApi;
 using Umbraco.Cms.Infrastructure.Examine;
-using Umbraco.Extensions;
 
 namespace Umbraco.Cms.Api.Delivery.Services;
 
@@ -21,10 +19,10 @@ internal sealed class ApiContentQueryProvider : IApiContentQueryProvider
 {
     private const string ItemIdFieldName = "itemId";
     private readonly IExamineManager _examineManager;
-    private readonly DeliveryApiSettings _deliveryApiSettings;
     private readonly ILogger<ApiContentQueryProvider> _logger;
-    private readonly string _fallbackGuidValue;
-    private readonly Dictionary<string, FieldType> _fieldTypes;
+    private readonly ApiContentQuerySelectorBuilder _selectorBuilder;
+    private readonly ApiContentQueryFilterBuilder _filterBuilder;
+    private readonly ApiContentQuerySortBuilder _sortBuilder;
 
     public ApiContentQueryProvider(
         IExamineManager examineManager,
@@ -33,18 +31,20 @@ internal sealed class ApiContentQueryProvider : IApiContentQueryProvider
         ILogger<ApiContentQueryProvider> logger)
     {
         _examineManager = examineManager;
-        _deliveryApiSettings = deliveryApiSettings.Value;
         _logger = logger;
 
-        // A fallback value is needed for Examine queries in case we don't have a value - we can't pass null or empty string
-        // It is set to a random guid since this would be highly unlikely to yield any results
-        _fallbackGuidValue = Guid.NewGuid().ToString("D");
-
         // build a look-up dictionary of field types by field name
-        _fieldTypes = indexHandlers
+        var fieldTypes = indexHandlers
             .SelectMany(handler => handler.GetFields())
             .DistinctBy(field => field.FieldName)
             .ToDictionary(field => field.FieldName, field => field.FieldType, StringComparer.InvariantCultureIgnoreCase);
+
+        // for the time being we're going to keep these as internal implementation details.
+        // perhaps later on it will make sense to expose them through the DI.
+        _selectorBuilder = new ApiContentQuerySelectorBuilder(deliveryApiSettings.Value);
+        _filterBuilder = new ApiContentQueryFilterBuilder(fieldTypes, _logger);
+        _sortBuilder = new ApiContentQuerySortBuilder(fieldTypes, _logger);
+
     }
 
     [Obsolete($"Use the {nameof(ExecuteQuery)} method that accepts {nameof(ProtectedAccess)}. Will be removed in V14.")]
@@ -75,10 +75,9 @@ internal sealed class ApiContentQueryProvider : IApiContentQueryProvider
             return new PagedModel<Guid>();
         }
 
-        IBooleanOperation queryOperation = BuildSelectorOperation(selectorOption, index, culture, protectedAccess, preview);
-
-        ApplyFiltering(filterOptions, queryOperation);
-        ApplySorting(sortOptions, queryOperation);
+        IBooleanOperation queryOperation = _selectorBuilder.Build(selectorOption, index, culture, protectedAccess, preview);
+        _filterBuilder.Append(filterOptions, queryOperation);
+        _sortBuilder.Append(sortOptions, queryOperation);
 
         ISearchResults? results = queryOperation
             .SelectField(ItemIdFieldName)
@@ -102,162 +101,4 @@ internal sealed class ApiContentQueryProvider : IApiContentQueryProvider
     {
         FieldName = UmbracoExamineFieldNames.CategoryFieldName, Values = new[] { "content" }
     };
-
-    private IBooleanOperation BuildSelectorOperation(SelectorOption selectorOption, IIndex index, string culture, ProtectedAccess protectedAccess, bool preview)
-    {
-        // Needed for enabling leading wildcards searches
-        BaseLuceneSearcher searcher = index.Searcher as BaseLuceneSearcher ?? throw new InvalidOperationException($"Index searcher must be of type {nameof(BaseLuceneSearcher)}.");
-
-        IQuery query = searcher.CreateQuery(
-            IndexTypes.Content,
-            BooleanOperation.And,
-            searcher.LuceneAnalyzer,
-            new LuceneSearchOptions { AllowLeadingWildcard = true });
-
-        IBooleanOperation selectorOperation = selectorOption.Values.Length == 1
-            ? query.Field(selectorOption.FieldName, selectorOption.Values.First())
-            : query.GroupedOr(new[] { selectorOption.FieldName }, selectorOption.Values);
-
-        AddCultureQuery(culture, selectorOperation);
-
-        if (_deliveryApiSettings.MemberAuthorizationIsEnabled())
-        {
-            AddProtectedAccessQuery(protectedAccess, selectorOperation);
-        }
-
-        // when not fetching for preview, make sure the "published" field is "y"
-        if (preview is false)
-        {
-            selectorOperation.And().Field(UmbracoExamineFieldNames.DeliveryApiContentIndex.Published, "y");
-        }
-
-        return selectorOperation;
-    }
-
-    private void AddCultureQuery(string culture, IBooleanOperation selectorOperation) =>
-        selectorOperation
-            .And()
-            .GroupedOr(
-                // Item culture must be either the requested culture or "none"
-                new[] { UmbracoExamineFieldNames.DeliveryApiContentIndex.Culture },
-                culture.ToLowerInvariant().IfNullOrWhiteSpace(_fallbackGuidValue),
-                "none");
-
-    private void AddProtectedAccessQuery(ProtectedAccess protectedAccess, IBooleanOperation selectorOperation)
-    {
-        var protectedAccessValues = new List<string>();
-        if (protectedAccess.MemberKey is not null)
-        {
-            protectedAccessValues.Add($"u:{protectedAccess.MemberKey}");
-        }
-
-        if (protectedAccess.MemberRoles?.Any() is true)
-        {
-            protectedAccessValues.AddRange(protectedAccess.MemberRoles.Select(r => $"r:{r}"));
-        }
-
-        if (protectedAccessValues.Any())
-        {
-            selectorOperation.And(
-                inner => inner
-                    .Field(UmbracoExamineFieldNames.DeliveryApiContentIndex.Protected, "n")
-                    .Or(protectedAccessInner => protectedAccessInner
-                        .GroupedOr(
-                            new[] { UmbracoExamineFieldNames.DeliveryApiContentIndex.ProtectedAccess },
-                            protectedAccessValues.ToArray())),
-                BooleanOperation.Or);
-        }
-        else
-        {
-            selectorOperation.And().Field(UmbracoExamineFieldNames.DeliveryApiContentIndex.Protected, "n");
-        }
-    }
-
-    private void ApplyFiltering(IList<FilterOption> filterOptions, IBooleanOperation queryOperation)
-    {
-        void HandleExact(IQuery query, string fieldName, string[] values)
-        {
-            if (values.Length == 1)
-            {
-                query.Field(fieldName, values[0]);
-            }
-            else
-            {
-                query.GroupedOr(new[] { fieldName }, values);
-            }
-        }
-
-        void HandleContains(IQuery query, string fieldName, string[] values)
-        {
-            if (values.Length == 1)
-            {
-                // The trailing wildcard is added automatically
-                query.Field(fieldName, (IExamineValue)new ExamineValue(Examineness.ComplexWildcard, $"*{values[0]}"));
-            }
-            else
-            {
-                // The trailing wildcard is added automatically
-                IExamineValue[] examineValues = values
-                    .Select(value => (IExamineValue)new ExamineValue(Examineness.ComplexWildcard, $"*{value}"))
-                    .ToArray();
-                query.GroupedOr(new[] { fieldName }, examineValues);
-            }
-        }
-
-        foreach (FilterOption filterOption in filterOptions)
-        {
-            var values = filterOption.Values.Any()
-                ? filterOption.Values
-                : new[] { _fallbackGuidValue };
-
-            switch (filterOption.Operator)
-            {
-                case FilterOperation.Is:
-                    HandleExact(queryOperation.And(), filterOption.FieldName, values);
-                    break;
-                case FilterOperation.IsNot:
-                    HandleExact(queryOperation.Not(), filterOption.FieldName, values);
-                    break;
-                case FilterOperation.Contains:
-                    HandleContains(queryOperation.And(), filterOption.FieldName, values);
-                    break;
-                case FilterOperation.DoesNotContain:
-                    HandleContains(queryOperation.Not(), filterOption.FieldName, values);
-                    break;
-                default:
-                    continue;
-            }
-        }
-    }
-
-    private void ApplySorting(IList<SortOption> sortOptions, IOrdering ordering)
-    {
-        foreach (SortOption sort in sortOptions)
-        {
-            if (_fieldTypes.TryGetValue(sort.FieldName, out FieldType fieldType) is false)
-            {
-                _logger.LogWarning(
-                    "Sort implementation for field name {FieldName} does not match an index handler implementation, cannot resolve field type.",
-                    sort.FieldName);
-                continue;
-            }
-
-            SortType sortType = fieldType switch
-            {
-                FieldType.Number => SortType.Int,
-                FieldType.Date => SortType.Long,
-                FieldType.StringRaw => SortType.String,
-                FieldType.StringAnalyzed => SortType.String,
-                FieldType.StringSortable => SortType.String,
-                _ => throw new ArgumentOutOfRangeException(nameof(fieldType))
-            };
-
-            ordering = sort.Direction switch
-            {
-                Direction.Ascending => ordering.OrderBy(new SortableField(sort.FieldName, sortType)),
-                Direction.Descending => ordering.OrderByDescending(new SortableField(sort.FieldName, sortType)),
-                _ => ordering
-            };
-        }
-    }
 }

src/Umbraco.Cms.Api.Delivery/Services/QueryBuilders/ApiContentQueryFilterBuilder.cs

@@ -0,0 +1,177 @@
+using Examine.Search;
+using Microsoft.Extensions.Logging;
+using Umbraco.Cms.Core.DeliveryApi;
+
+namespace Umbraco.Cms.Api.Delivery.Services.QueryBuilders;
+
+internal sealed class ApiContentQueryFilterBuilder
+{
+    private readonly IDictionary<string, FieldType> _fieldTypes;
+    private readonly ILogger _logger;
+    private readonly string _fallbackGuidValue;
+
+    public ApiContentQueryFilterBuilder(IDictionary<string, FieldType> fieldTypes, ILogger logger)
+    {
+        _fieldTypes = fieldTypes;
+        _logger = logger;
+
+        // A fallback value is needed for Examine queries in case we don't have a value - we can't pass null or empty string
+        // It is set to a random guid since this would be highly unlikely to yield any results
+        _fallbackGuidValue = Guid.NewGuid().ToString("D");
+    }
+
+    public void Append(IList<FilterOption> filterOptions, IBooleanOperation queryOperation)
+    {
+        foreach (FilterOption filterOption in filterOptions)
+        {
+            if (_fieldTypes.TryGetValue(filterOption.FieldName, out FieldType fieldType) is false)
+            {
+                _logger.LogWarning(
+                    "Filter implementation for field name {FieldName} does not match an index handler implementation, cannot resolve field type.",
+                    filterOption.FieldName);
+                continue;
+            }
+
+            var values = filterOption.Values.Any()
+                ? filterOption.Values
+                : new[] { _fallbackGuidValue };
+
+            switch (filterOption.Operator)
+            {
+                case FilterOperation.Is:
+                    ApplyExactFilter(queryOperation.And(), filterOption.FieldName, values, fieldType);
+                    break;
+                case FilterOperation.IsNot:
+                    ApplyExactFilter(queryOperation.Not(), filterOption.FieldName, values, fieldType);
+                    break;
+                case FilterOperation.Contains:
+                    ApplyContainsFilter(queryOperation.And(), filterOption.FieldName, values);
+                    break;
+                case FilterOperation.DoesNotContain:
+                    ApplyContainsFilter(queryOperation.Not(), filterOption.FieldName, values);
+                    break;
+                default:
+                    continue;
+            }
+        }
+    }
+
+    private void ApplyExactFilter(IQuery query, string fieldName, string[] values, FieldType fieldType)
+    {
+        switch (fieldType)
+        {
+            case FieldType.Number:
+                ApplyExactNumberFilter(query, fieldName, values);
+                break;
+            case FieldType.Date:
+                ApplyExactDateFilter(query, fieldName, values);
+                break;
+            default:
+                ApplyExactStringFilter(query, fieldName, values);
+                break;
+        }
+    }
+
+    private void ApplyExactNumberFilter(IQuery query, string fieldName, string[] values)
+    {
+        if (values.Length == 1)
+        {
+            if (TryParseIntFilterValue(values.First(), out int intValue))
+            {
+                query.Field(fieldName, intValue);
+            }
+        }
+        else
+        {
+            int[] intValues = values
+                .Select(value => TryParseIntFilterValue(value, out int intValue) ? intValue : (int?)null)
+                .Where(intValue => intValue.HasValue)
+                .Select(intValue => intValue!.Value)
+                .ToArray();
+
+            AddGroupedOrFilter(query, fieldName, intValues);
+        }
+    }
+
+    private void ApplyExactDateFilter(IQuery query, string fieldName, string[] values)
+    {
+        if (values.Length == 1)
+        {
+            if (TryParseDateTimeFilterValue(values.First(), out DateTime dateValue))
+            {
+                query.Field(fieldName, dateValue);
+            }
+        }
+        else
+        {
+            DateTime[] dateValues = values
+                .Select(value => TryParseDateTimeFilterValue(value, out DateTime dateValue) ? dateValue : (DateTime?)null)
+                .Where(dateValue => dateValue.HasValue)
+                .Select(dateValue => dateValue!.Value)
+                .ToArray();
+
+            AddGroupedOrFilter(query, fieldName, dateValues);
+        }
+    }
+
+    private void ApplyExactStringFilter(IQuery query, string fieldName, string[] values)
+    {
+        if (values.Length == 1)
+        {
+            query.Field(fieldName, values.First());
+        }
+        else
+        {
+            AddGroupedOrFilter(query, fieldName, values);
+        }
+    }
+
+    private void ApplyContainsFilter(IQuery query, string fieldName, string[] values)
+    {
+        if (values.Length == 1)
+        {
+            // The trailing wildcard is added automatically
+            query.Field(fieldName, (IExamineValue)new ExamineValue(Examineness.ComplexWildcard, $"*{values[0]}"));
+        }
+        else
+        {
+            // The trailing wildcard is added automatically
+            IExamineValue[] examineValues = values
+                .Select(value => (IExamineValue)new ExamineValue(Examineness.ComplexWildcard, $"*{value}"))
+                .ToArray();
+            query.GroupedOr(new[] { fieldName }, examineValues);
+        }
+    }
+
+    private void AddGroupedOrFilter<T>(IQuery query, string fieldName, params T[] values)
+        where T : struct
+    {
+        if (values.Length == 0)
+        {
+            return;
+        }
+
+        var fields = new[] { fieldName };
+        query.Group(
+            nestedQuery =>
+            {
+                INestedBooleanOperation nestedQueryOperation =
+                    nestedQuery.RangeQuery<T>(fields, values[0], values[0]);
+                foreach (T value in values.Skip(1))
+                {
+                    nestedQueryOperation = nestedQueryOperation.Or().RangeQuery<T>(fields, value, value);
+                }
+
+                return nestedQueryOperation;
+            });
+    }
+
+    private void AddGroupedOrFilter(IQuery query, string fieldName, params string[] values)
+        => query.GroupedOr(new[] { fieldName }, values);
+
+    private bool TryParseIntFilterValue(string value, out int intValue)
+        => int.TryParse(value, out intValue);
+
+    private bool TryParseDateTimeFilterValue(string value, out DateTime dateValue)
+        => DateTime.TryParse(value, out dateValue);
+}

src/Umbraco.Cms.Api.Delivery/Services/QueryBuilders/ApiContentQuerySelectorBuilder.cs

@@ -0,0 +1,96 @@
+using Examine;
+using Examine.Lucene.Providers;
+using Examine.Lucene.Search;
+using Examine.Search;
+using Umbraco.Cms.Core.Configuration.Models;
+using Umbraco.Cms.Core.DeliveryApi;
+using Umbraco.Cms.Core.Models.DeliveryApi;
+using Umbraco.Cms.Infrastructure.Examine;
+using Umbraco.Extensions;
+
+namespace Umbraco.Cms.Api.Delivery.Services.QueryBuilders;
+
+internal sealed class ApiContentQuerySelectorBuilder
+{
+    private readonly DeliveryApiSettings _deliveryApiSettings;
+    private readonly string _fallbackGuidValue;
+
+    public ApiContentQuerySelectorBuilder(DeliveryApiSettings deliveryApiSettings)
+    {
+        _deliveryApiSettings = deliveryApiSettings;
+
+        // A fallback value is needed for Examine queries in case we don't have a value - we can't pass null or empty string
+        // It is set to a random guid since this would be highly unlikely to yield any results
+        _fallbackGuidValue = Guid.NewGuid().ToString("D");
+    }
+
+    public IBooleanOperation Build(SelectorOption selectorOption, IIndex index, string culture, ProtectedAccess protectedAccess, bool preview)
+    {
+        // Needed for enabling leading wildcards searches
+        BaseLuceneSearcher searcher = index.Searcher as BaseLuceneSearcher ?? throw new InvalidOperationException($"Index searcher must be of type {nameof(BaseLuceneSearcher)}.");
+
+        IQuery query = searcher.CreateQuery(
+            IndexTypes.Content,
+            BooleanOperation.And,
+            searcher.LuceneAnalyzer,
+            new LuceneSearchOptions { AllowLeadingWildcard = true });
+
+        IBooleanOperation selectorOperation = selectorOption.Values.Length == 1
+            ? query.Field(selectorOption.FieldName, selectorOption.Values.First())
+            : query.GroupedOr(new[] { selectorOption.FieldName }, selectorOption.Values);
+
+        AddCultureQuery(culture, selectorOperation);
+
+        if (_deliveryApiSettings.MemberAuthorizationIsEnabled())
+        {
+            AddProtectedAccessQuery(protectedAccess, selectorOperation);
+        }
+
+        // when not fetching for preview, make sure the "published" field is "y"
+        if (preview is false)
+        {
+            selectorOperation.And().Field(UmbracoExamineFieldNames.DeliveryApiContentIndex.Published, "y");
+        }
+
+        return selectorOperation;
+    }
+
+    private void AddCultureQuery(string culture, IBooleanOperation selectorOperation) =>
+        selectorOperation
+            .And()
+            .GroupedOr(
+                // Item culture must be either the requested culture or "none"
+                new[] { UmbracoExamineFieldNames.DeliveryApiContentIndex.Culture },
+                culture.ToLowerInvariant().IfNullOrWhiteSpace(_fallbackGuidValue),
+                "none");
+
+    private void AddProtectedAccessQuery(ProtectedAccess protectedAccess, IBooleanOperation selectorOperation)
+    {
+        var protectedAccessValues = new List<string>();
+        if (protectedAccess.MemberKey is not null)
+        {
+            protectedAccessValues.Add($"u:{protectedAccess.MemberKey}");
+        }
+
+        if (protectedAccess.MemberRoles?.Any() is true)
+        {
+            protectedAccessValues.AddRange(protectedAccess.MemberRoles.Select(r => $"r:{r}"));
+        }
+
+        if (protectedAccessValues.Any())
+        {
+            selectorOperation.And(
+                inner => inner
+                    .Field(UmbracoExamineFieldNames.DeliveryApiContentIndex.Protected, "n")
+                    .Or(protectedAccessInner => protectedAccessInner
+                        .GroupedOr(
+                            new[] { UmbracoExamineFieldNames.DeliveryApiContentIndex.ProtectedAccess },
+                            protectedAccessValues.ToArray())),
+                BooleanOperation.Or);
+        }
+        else
+        {
+            selectorOperation.And().Field(UmbracoExamineFieldNames.DeliveryApiContentIndex.Protected, "n");
+        }
+    }
+}

src/Umbraco.Cms.Api.Delivery/Services/QueryBuilders/ApiContentQuerySortBuilder.cs

@@ -0,0 +1,49 @@
+using Examine.Search;
+using Microsoft.Extensions.Logging;
+using Umbraco.Cms.Core;
+using Umbraco.Cms.Core.DeliveryApi;
+
+namespace Umbraco.Cms.Api.Delivery.Services.QueryBuilders;
+
+internal sealed class ApiContentQuerySortBuilder
+{
+    private readonly Dictionary<string, FieldType> _fieldTypes;
+    private readonly ILogger _logger;
+
+    public ApiContentQuerySortBuilder(Dictionary<string, FieldType> fieldTypes, ILogger logger)
+    {
+        _fieldTypes = fieldTypes;
+        _logger = logger;
+    }
+
+    public void Append(IList<SortOption> sortOptions, IOrdering queryOperation)
+    {
+        foreach (SortOption sort in sortOptions)
+        {
+            if (_fieldTypes.TryGetValue(sort.FieldName, out FieldType fieldType) is false)
+            {
+                _logger.LogWarning(
+                    "Sort implementation for field name {FieldName} does not match an index handler implementation, cannot resolve field type.",
+                    sort.FieldName);
+                continue;
+            }
+
+            SortType sortType = fieldType switch
+            {
+                FieldType.Number => SortType.Int,
+                FieldType.Date => SortType.Long,
+                FieldType.StringRaw => SortType.String,
+                FieldType.StringAnalyzed => SortType.String,
+                FieldType.StringSortable => SortType.String,
+                _ => throw new ArgumentOutOfRangeException(nameof(fieldType))
+            };
+
+            queryOperation = sort.Direction switch
+            {
+                Direction.Ascending => queryOperation.OrderBy(new SortableField(sort.FieldName, sortType)),
+                Direction.Descending => queryOperation.OrderByDescending(new SortableField(sort.FieldName, sortType)),
+                _ => queryOperation
+            };
+        }
+    }
+}