[ODS-6444] Explore alternatives for ICriteria-based query building fo…

…r paged queries (#1121)
Ed-Fi-Alliance-OSS · Sep 2, 2024 · ecb8945 · ecb8945
1 parent bfdfc2f
commit ecb8945
Show file tree

Hide file tree

Showing 30 changed files with 1,019 additions and 895 deletions.
diff --git a/Application/EdFi.Ods.Api/Container/Modules/PersistenceModule.cs b/Application/EdFi.Ods.Api/Container/Modules/PersistenceModule.cs
@@ -105,10 +105,6 @@ protected override void Load(ContainerBuilder builder)
                 .As(typeof(IPagedAggregateIdsCriteriaProvider<>))
                 .SingleInstance();
 
-            builder.RegisterGeneric(typeof(TotalCountCriteriaProvider<>))
-                .As(typeof(ITotalCountCriteriaProvider<>))
-                .SingleInstance();
-
             builder.RegisterGeneric(typeof(CreateEntity<>))
                 .As(typeof(ICreateEntity<>))
                 .SingleInstance();

diff --git a/...egateRootCriteriaProviderDecoratorBase.cs → ...CriteriaProviderAuthorizationDecorator.cs b/...egateRootCriteriaProviderDecoratorBase.cs → ...CriteriaProviderAuthorizationDecorator.cs
@@ -6,32 +6,30 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
-using NHibernate;
-using NHibernate.Criterion;
 using EdFi.Ods.Common;
 using EdFi.Ods.Common.Providers.Criteria;
 using EdFi.Ods.Api.Security.Authorization.Filtering;
 using EdFi.Ods.Api.Security.AuthorizationStrategies.Relationships.Filters;
+using EdFi.Ods.Common.Database.Querying;
 using EdFi.Ods.Common.Infrastructure.Filtering;
 using EdFi.Ods.Common.Security.Authorization;
-using NHibernate.SqlCommand;
 
 namespace EdFi.Ods.Api.Security.Authorization.Repositories
 {
     /// <summary>
-    /// Provides an abstract implementation for applying authorization filters to <see cref="ICriteria"/> queries on aggregate roots.
+    /// Provides an abstract implementation for applying authorization filters to queries on aggregate roots built using the <see cref="QueryBuilder"/>.
     /// </summary>
     /// <typeparam name="TEntity">The type of the aggregate root entity being queried.</typeparam>
-    public abstract class AggregateRootCriteriaProviderAuthorizationDecoratorBase<TEntity>
-        : IAggregateRootCriteriaProvider<TEntity>
+    public class PagedAggregateIdsCriteriaProviderAuthorizationDecorator<TEntity>
+        : IPagedAggregateIdsCriteriaProvider<TEntity>
         where TEntity : class
     {
-        private readonly IAggregateRootCriteriaProvider<TEntity> _decoratedInstance;
+        private readonly IPagedAggregateIdsCriteriaProvider<TEntity> _decoratedInstance;
         private readonly IAuthorizationFilterContextProvider _authorizationFilterContextProvider;
         private readonly IAuthorizationFilterDefinitionProvider _authorizationFilterDefinitionProvider;
 
-        protected AggregateRootCriteriaProviderAuthorizationDecoratorBase(
-            IAggregateRootCriteriaProvider<TEntity> decoratedInstance,
+        public PagedAggregateIdsCriteriaProviderAuthorizationDecorator(
+            IPagedAggregateIdsCriteriaProvider<TEntity> decoratedInstance,
             IAuthorizationFilterContextProvider authorizationFilterContextProvider,
             IAuthorizationFilterDefinitionProvider authorizationFilterDefinitionProvider)
         {
@@ -46,29 +44,21 @@ protected AggregateRootCriteriaProviderAuthorizationDecoratorBase(
         /// <param name="specification">An instance of the entity representing the parameters to the query.</param>
         /// <param name="queryParameters">The parameter values to apply to the query.</param>
         /// <returns>The criteria created by the decorated instance.</returns>
-        public ICriteria GetCriteriaQuery(TEntity specification, IQueryParameters queryParameters)
+        public QueryBuilder GetQueryBuilder(TEntity specification, IQueryParameters queryParameters)
         {
-            var criteria = _decoratedInstance.GetCriteriaQuery(specification, queryParameters);
+            var queryBuilder = _decoratedInstance.GetQueryBuilder(specification, queryParameters);
 
             var authorizationFiltering = _authorizationFilterContextProvider.GetFilterContext();
 
             var unsupportedAuthorizationFilters = new HashSet<string>();
 
-            // Create the "AND" junction
-            var mainConjunction = new Conjunction();
-
-            // Create the "OR" junction
-            var mainDisjunction = new Disjunction();
-
             // If there are multiple relationship-based authorization strategies with views (that are combined with OR), we must use left outer joins and null/not null checks
             var relationshipBasedAuthViewJoinType = DetermineRelationshipBasedAuthViewJoinType();
 
-            bool conjunctionFiltersWereApplied = ApplyAuthorizationStrategiesCombinedWithAndLogic();
-            bool disjunctionFiltersWereApplied = ApplyAuthorizationStrategiesCombinedWithOrLogic();
-
-            ApplyJunctionsToCriteriaQuery();
+            ApplyAuthorizationStrategiesCombinedWithAndLogic();
+            ApplyAuthorizationStrategiesCombinedWithOrLogic();
 
-            return criteria;
+            return queryBuilder;
 
             JoinType DetermineRelationshipBasedAuthViewJoinType()
             {
@@ -94,57 +84,62 @@ JoinType DetermineRelationshipBasedAuthViewJoinType()
                     : JoinType.InnerJoin;
             }
 
-            bool ApplyAuthorizationStrategiesCombinedWithAndLogic()
+            void ApplyAuthorizationStrategiesCombinedWithAndLogic()
             {
                 var andStrategies = authorizationFiltering.Where(x => x.Operator == FilterOperator.And).ToArray();
 
                 // Combine 'AND' strategies
-                bool conjunctionFiltersApplied = false;
-
                 foreach (var andStrategy in andStrategies)
                 {
-                    if (!TryApplyFilters(mainConjunction, andStrategy.Filters, andStrategy.AuthorizationStrategy, JoinType.InnerJoin))
+                    if (!TryApplyFilters(queryBuilder, andStrategy.Filters, andStrategy.AuthorizationStrategy, JoinType.InnerJoin))
                     {
                         // All filters for AND strategies must be applied, and if not, this is an error condition
                         throw new Exception($"The following authorization filters are not recognized: {string.Join(" ", unsupportedAuthorizationFilters)}");
                     }
-
-                    conjunctionFiltersApplied = true;
                 }
-
-                return conjunctionFiltersApplied;
             }
 
-            bool ApplyAuthorizationStrategiesCombinedWithOrLogic()
+            void ApplyAuthorizationStrategiesCombinedWithOrLogic()
             {
                 var orStrategies = authorizationFiltering.Where(x => x.Operator == FilterOperator.Or).ToArray();
 
-                // Combine 'OR' strategies
                 bool disjunctionFiltersApplied = false;
 
-                foreach (var orStrategy in orStrategies)
-                {
-                    var filtersConjunction = new Conjunction(); // Combine filters with 'AND'
-
-                    if (TryApplyFilters(filtersConjunction, orStrategy.Filters, orStrategy.AuthorizationStrategy, relationshipBasedAuthViewJoinType))
+                // Combine 'OR' strategies
+                queryBuilder.Where(
+                    disjunctionBuilder =>
                     {
-                        mainDisjunction.Add(filtersConjunction);
-
-                        disjunctionFiltersApplied = true;
-                    }
-                }
+                        foreach (var orStrategy in orStrategies)
+                        {
+                            disjunctionBuilder.OrWhere(
+                                filtersConjunctionBuilder =>
+                                {
+                                    // Combine filters with 'AND'
+                                    if (TryApplyFilters(
+                                            filtersConjunctionBuilder,
+                                            orStrategy.Filters,
+                                            orStrategy.AuthorizationStrategy,
+                                            relationshipBasedAuthViewJoinType))
+                                    {
+                                        disjunctionFiltersApplied = true;
+                                    }
+
+                                    return filtersConjunctionBuilder;
+                                });
+                        }
+
+                        return disjunctionBuilder;
+                    });
 
                 // If we have some OR strategies with filters defined, but no filters were applied, this is an error condition
                 if (orStrategies.SelectMany(s => s.Filters).Any() && !disjunctionFiltersApplied)
                 {
                     throw new Exception($"The following authorization filters are not recognized: {string.Join(" ", unsupportedAuthorizationFilters)}");
                 }
-
-                return disjunctionFiltersApplied;
             }
 
             bool TryApplyFilters(
-                Conjunction conjunction,
+                QueryBuilder conjunctionQueryBuilder,
                 IReadOnlyList<AuthorizationFilterContext> filters,
                 IAuthorizationStrategy authorizationStrategy,
                 JoinType joinType)
@@ -184,33 +179,19 @@ bool TryApplyFilters(
                             };
 
                     // Apply the authorization strategy filter
-                    filterDefinition.CriteriaApplicator(criteria, conjunction, filterContext.SubjectEndpointNames, parameterValues, joinType, authorizationStrategy);
+                    filterDefinition.CriteriaApplicator(
+                        queryBuilder,
+                        conjunctionQueryBuilder,
+                        filterContext.SubjectEndpointNames,
+                        parameterValues,
+                        joinType,
+                        authorizationStrategy);
 
                     filtersApplied = true;
                 }
 
                 return filtersApplied;
             }
-
-            void ApplyJunctionsToCriteriaQuery()
-            {
-                if (disjunctionFiltersWereApplied)
-                {
-                    if (conjunctionFiltersWereApplied)
-                    {
-                        mainConjunction.Add(mainDisjunction);
-                    }
-                    else
-                    {
-                        criteria.Add(mainDisjunction);
-                    }
-                }
-
-                if (conjunctionFiltersWereApplied)
-                {
-                    criteria.Add(mainConjunction);
-                }
-            }
         }
     }
 }
diff --git a/...Ods.Api/Security/Authorization/Repositories/PagedAggregateIdsCriteriaProviderDecorator.cs b/...Ods.Api/Security/Authorization/Repositories/PagedAggregateIdsCriteriaProviderDecorator.cs
diff --git a/...n/EdFi.Ods.Api/Security/Authorization/Repositories/TotalCountCriteriaProviderDecorator.cs b/...n/EdFi.Ods.Api/Security/Authorization/Repositories/TotalCountCriteriaProviderDecorator.cs
diff --git a/...orizationStrategies/NamespaceBased/NamespaceBasedAuthorizationFilterDefinitionsFactory.cs b/...orizationStrategies/NamespaceBased/NamespaceBasedAuthorizationFilterDefinitionsFactory.cs
@@ -16,9 +16,6 @@
 using EdFi.Ods.Common.Models.Resource;
 using EdFi.Ods.Common.Security.Authorization;
 using EdFi.Ods.Common.Security.Claims;
-using NHibernate;
-using NHibernate.Criterion;
-using NHibernate.SqlCommand;
 
 namespace EdFi.Ods.Api.Security.AuthorizationStrategies.NamespaceBased;
 
@@ -66,8 +63,8 @@ public IReadOnlyList<AuthorizationFilterDefinition> CreatePredefinedAuthorizatio
     }
 
     private static void ApplyAuthorizationCriteria(
-        ICriteria criteria,
-        Junction @where,
+        QueryBuilder queryBuilder,
+        QueryBuilder whereBuilder,
         string[] subjectEndpointNames,
         IDictionary<string, object> parameters,
         JoinType joinType,
@@ -88,18 +85,20 @@ private static void ApplyAuthorizationCriteria(
         }
 
         // Ensure the Namespace parameter is represented as an object array
-        var namespacePrefixes = parameterValue as object[] ?? new[] { parameterValue };
-
-        // Combine the namespace filters using OR (only one must match to grant authorization)
-        var namespacesDisjunction = new Disjunction();
-
-        foreach (var namespacePrefix in namespacePrefixes)
-        {
-            namespacesDisjunction.Add(Restrictions.Like(subjectEndpointName, namespacePrefix));
-        }
+        var namespacePrefixes = parameterValue as object[] ?? [parameterValue];
 
         // Add the final namespaces criteria to the supplied WHERE clause (junction)
-        @where.Add(new AndExpression(Restrictions.IsNotNull(subjectEndpointName), namespacesDisjunction));
+        whereBuilder.WhereNotNull(subjectEndpointName)
+            .Where(
+                qb =>
+                {
+                    foreach (var namespacePrefix in namespacePrefixes)
+                    {
+                        qb.OrWhereLike(subjectEndpointName, namespacePrefix, MatchMode.Start);
+                    }
+
+                    return qb;
+                });
     }
 
     private static PropertyMapping[] GetContextDataPropertyMappings(string resourceFullName, IEnumerable<string> availablePropertyNames)

diff --git a/...orizationStrategies/OwnershipBased/OwnershipBasedAuthorizationFilterDefinitionsFactory.cs b/...orizationStrategies/OwnershipBased/OwnershipBasedAuthorizationFilterDefinitionsFactory.cs
@@ -13,9 +13,6 @@
 using EdFi.Ods.Common.Models.Resource;
 using EdFi.Ods.Common.Security.Authorization;
 using EdFi.Ods.Common.Security.Claims;
-using NHibernate;
-using NHibernate.Criterion;
-using NHibernate.SqlCommand;
 
 namespace EdFi.Ods.Api.Security.AuthorizationStrategies.OwnershipBased;
 
@@ -51,8 +48,8 @@ public IReadOnlyList<AuthorizationFilterDefinition> CreatePredefinedAuthorizatio
     }
 
     private static void ApplyAuthorizationCriteria(
-        ICriteria criteria,
-        Junction @where,
+        QueryBuilder queryBuilder,
+        QueryBuilder whereQueryBuilder,
         string[] subjectEndpointNames,
         IDictionary<string, object> parameters,
         JoinType joinType,
@@ -65,7 +62,7 @@ private static void ApplyAuthorizationCriteria(
         }
 
         // NOTE: subjectEndpointName is ignored here -- we don't expect or want any variation due to role names applied here.
-        @where.ApplyPropertyFilters(parameters, FilterPropertyName);
+        whereQueryBuilder.ApplyPropertyFilters(parameters, FilterPropertyName);
     }
 
     private void ApplyTrackedChangesAuthorizationCriteria(

diff --git a/...EdFi.Ods.Api/Security/AuthorizationStrategies/Relationships/Filters/JunctionExtensions.cs b/...EdFi.Ods.Api/Security/AuthorizationStrategies/Relationships/Filters/JunctionExtensions.cs