diff --git a/Application/EdFi.Ods.Api/Container/Modules/PersistenceModule.cs b/Application/EdFi.Ods.Api/Container/Modules/PersistenceModule.cs index a2b8f067e1..07298c7ffe 100644 --- 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/Application/EdFi.Ods.Api/Security/Authorization/Repositories/AggregateRootCriteriaProviderDecoratorBase.cs b/Application/EdFi.Ods.Api/Security/Authorization/Repositories/PagedAggregateIdsCriteriaProviderAuthorizationDecorator.cs similarity index 69% rename from Application/EdFi.Ods.Api/Security/Authorization/Repositories/AggregateRootCriteriaProviderDecoratorBase.cs rename to Application/EdFi.Ods.Api/Security/Authorization/Repositories/PagedAggregateIdsCriteriaProviderAuthorizationDecorator.cs index dab53f3215..8f2e0fbd77 100644 --- a/Application/EdFi.Ods.Api/Security/Authorization/Repositories/AggregateRootCriteriaProviderDecoratorBase.cs +++ b/Application/EdFi.Ods.Api/Security/Authorization/Repositories/PagedAggregateIdsCriteriaProviderAuthorizationDecorator.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 { /// - /// Provides an abstract implementation for applying authorization filters to queries on aggregate roots. + /// Provides an abstract implementation for applying authorization filters to queries on aggregate roots built using the . /// /// The type of the aggregate root entity being queried. - public abstract class AggregateRootCriteriaProviderAuthorizationDecoratorBase - : IAggregateRootCriteriaProvider + public class PagedAggregateIdsCriteriaProviderAuthorizationDecorator + : IPagedAggregateIdsCriteriaProvider where TEntity : class { - private readonly IAggregateRootCriteriaProvider _decoratedInstance; + private readonly IPagedAggregateIdsCriteriaProvider _decoratedInstance; private readonly IAuthorizationFilterContextProvider _authorizationFilterContextProvider; private readonly IAuthorizationFilterDefinitionProvider _authorizationFilterDefinitionProvider; - protected AggregateRootCriteriaProviderAuthorizationDecoratorBase( - IAggregateRootCriteriaProvider decoratedInstance, + public PagedAggregateIdsCriteriaProviderAuthorizationDecorator( + IPagedAggregateIdsCriteriaProvider decoratedInstance, IAuthorizationFilterContextProvider authorizationFilterContextProvider, IAuthorizationFilterDefinitionProvider authorizationFilterDefinitionProvider) { @@ -46,29 +44,21 @@ protected AggregateRootCriteriaProviderAuthorizationDecoratorBase( /// An instance of the entity representing the parameters to the query. /// The parameter values to apply to the query. /// The criteria created by the decorated instance. - 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(); - // 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 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/Application/EdFi.Ods.Api/Security/Authorization/Repositories/PagedAggregateIdsCriteriaProviderDecorator.cs b/Application/EdFi.Ods.Api/Security/Authorization/Repositories/PagedAggregateIdsCriteriaProviderDecorator.cs deleted file mode 100644 index 40db6da9ff..0000000000 --- a/Application/EdFi.Ods.Api/Security/Authorization/Repositories/PagedAggregateIdsCriteriaProviderDecorator.cs +++ /dev/null @@ -1,29 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -// Licensed to the Ed-Fi Alliance under one or more agreements. -// The Ed-Fi Alliance licenses this file to you under the Apache License, Version 2.0. -// See the LICENSE and NOTICES files in the project root for more information. - -using EdFi.Ods.Api.Security.Authorization.Filtering; -using EdFi.Ods.Common.Infrastructure.Filtering; -using EdFi.Ods.Common.Providers.Criteria; - -namespace EdFi.Ods.Api.Security.Authorization.Repositories -{ - /// - /// Applies authorization filtering criteria to the paged queries. - /// - /// The type of the aggregate root entity being queried. - public class PagedAggregateIdsCriteriaProviderAuthorizationDecorator - : AggregateRootCriteriaProviderAuthorizationDecoratorBase, IPagedAggregateIdsCriteriaProvider - where TEntity : class - { - public PagedAggregateIdsCriteriaProviderAuthorizationDecorator( - IPagedAggregateIdsCriteriaProvider decoratedInstance, - IAuthorizationFilterContextProvider authorizationFilterContextProvider, - IAuthorizationFilterDefinitionProvider authorizationFilterDefinitionProvider) - : base( - decoratedInstance, - authorizationFilterContextProvider, - authorizationFilterDefinitionProvider) { } - } -} diff --git a/Application/EdFi.Ods.Api/Security/Authorization/Repositories/TotalCountCriteriaProviderDecorator.cs b/Application/EdFi.Ods.Api/Security/Authorization/Repositories/TotalCountCriteriaProviderDecorator.cs deleted file mode 100644 index f86c9ce2f2..0000000000 --- a/Application/EdFi.Ods.Api/Security/Authorization/Repositories/TotalCountCriteriaProviderDecorator.cs +++ /dev/null @@ -1,29 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -// Licensed to the Ed-Fi Alliance under one or more agreements. -// The Ed-Fi Alliance licenses this file to you under the Apache License, Version 2.0. -// See the LICENSE and NOTICES files in the project root for more information. - -using EdFi.Ods.Api.Security.Authorization.Filtering; -using EdFi.Ods.Common.Infrastructure.Filtering; -using EdFi.Ods.Common.Providers.Criteria; - -namespace EdFi.Ods.Api.Security.Authorization.Repositories -{ - /// - /// Applies authorization filtering criteria to the "total count" queries. - /// - /// The type of the aggregate root entity being queried. - public class TotalCountCriteriaProviderAuthorizationDecorator - : AggregateRootCriteriaProviderAuthorizationDecoratorBase, ITotalCountCriteriaProvider - where TEntity : class - { - public TotalCountCriteriaProviderAuthorizationDecorator( - ITotalCountCriteriaProvider decoratedInstance, - IAuthorizationFilterContextProvider authorizationFilterContextProvider, - IAuthorizationFilterDefinitionProvider authorizationFilterDefinitionProvider) - : base( - decoratedInstance, - authorizationFilterContextProvider, - authorizationFilterDefinitionProvider) { } - } -} diff --git a/Application/EdFi.Ods.Api/Security/AuthorizationStrategies/NamespaceBased/NamespaceBasedAuthorizationFilterDefinitionsFactory.cs b/Application/EdFi.Ods.Api/Security/AuthorizationStrategies/NamespaceBased/NamespaceBasedAuthorizationFilterDefinitionsFactory.cs index 4a2e77a41b..307aa25191 100644 --- a/Application/EdFi.Ods.Api/Security/AuthorizationStrategies/NamespaceBased/NamespaceBasedAuthorizationFilterDefinitionsFactory.cs +++ b/Application/EdFi.Ods.Api/Security/AuthorizationStrategies/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 CreatePredefinedAuthorizatio } private static void ApplyAuthorizationCriteria( - ICriteria criteria, - Junction @where, + QueryBuilder queryBuilder, + QueryBuilder whereBuilder, string[] subjectEndpointNames, IDictionary 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 availablePropertyNames) diff --git a/Application/EdFi.Ods.Api/Security/AuthorizationStrategies/OwnershipBased/OwnershipBasedAuthorizationFilterDefinitionsFactory.cs b/Application/EdFi.Ods.Api/Security/AuthorizationStrategies/OwnershipBased/OwnershipBasedAuthorizationFilterDefinitionsFactory.cs index ed5f92a9ea..20db23a9a5 100644 --- a/Application/EdFi.Ods.Api/Security/AuthorizationStrategies/OwnershipBased/OwnershipBasedAuthorizationFilterDefinitionsFactory.cs +++ b/Application/EdFi.Ods.Api/Security/AuthorizationStrategies/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 CreatePredefinedAuthorizatio } private static void ApplyAuthorizationCriteria( - ICriteria criteria, - Junction @where, + QueryBuilder queryBuilder, + QueryBuilder whereQueryBuilder, string[] subjectEndpointNames, IDictionary 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/Application/EdFi.Ods.Api/Security/AuthorizationStrategies/Relationships/Filters/JunctionExtensions.cs b/Application/EdFi.Ods.Api/Security/AuthorizationStrategies/Relationships/Filters/JunctionExtensions.cs deleted file mode 100644 index 0163ac90b3..0000000000 --- a/Application/EdFi.Ods.Api/Security/AuthorizationStrategies/Relationships/Filters/JunctionExtensions.cs +++ /dev/null @@ -1,36 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -// Licensed to the Ed-Fi Alliance under one or more agreements. -// The Ed-Fi Alliance licenses this file to you under the Apache License, Version 2.0. -// See the LICENSE and NOTICES files in the project root for more information. - -using System; -using System.Collections.Generic; -using System.Linq; -using NHibernate.Criterion; - -namespace EdFi.Ods.Api.Security.AuthorizationStrategies.Relationships.Filters -{ - public static class JunctionExtensions - { - /// - /// Applies property-level filter expressions to the criteria as either Equal or In expressions based on the supplied parameters. - /// - /// The container for adding WHERE clause criterion. - /// The named parameters to be processed into the criteria query. - /// The array of property names that can be used for filtering. - public static void ApplyPropertyFilters(this Junction whereJunction, IDictionary parameters, params string[] availableFilterProperties) - { - foreach (var nameAndValue in parameters.Where(x => availableFilterProperties.Contains(x.Key, StringComparer.OrdinalIgnoreCase))) - { - if (nameAndValue.Value is object[] arrayOfValues) - { - whereJunction.Add(Restrictions.In($"{nameAndValue.Key}", arrayOfValues)); - } - else - { - whereJunction.Add(Restrictions.Eq($"{nameAndValue.Key}", nameAndValue.Value)); - } - } - } - } -} diff --git a/Application/EdFi.Ods.Api/Security/AuthorizationStrategies/Relationships/Filters/ICriteriaExtensions.cs b/Application/EdFi.Ods.Api/Security/AuthorizationStrategies/Relationships/Filters/QueryBuilderExtensions.cs similarity index 57% rename from Application/EdFi.Ods.Api/Security/AuthorizationStrategies/Relationships/Filters/ICriteriaExtensions.cs rename to Application/EdFi.Ods.Api/Security/AuthorizationStrategies/Relationships/Filters/QueryBuilderExtensions.cs index 64e017d398..d055bee994 100644 --- a/Application/EdFi.Ods.Api/Security/AuthorizationStrategies/Relationships/Filters/ICriteriaExtensions.cs +++ b/Application/EdFi.Ods.Api/Security/AuthorizationStrategies/Relationships/Filters/QueryBuilderExtensions.cs @@ -5,26 +5,20 @@ using System; using System.Collections.Generic; -using EdFi.Ods.Api.Security.AuthorizationStrategies.CustomViewBased; +using System.Linq; using EdFi.Ods.Common; -using EdFi.Ods.Common.Conventions; -using EdFi.Ods.Common.Infrastructure.Activities; -using EdFi.Ods.Common.Models.Domain; +using EdFi.Ods.Common.Database.Querying; using EdFi.Ods.Common.Security.Authorization; using EdFi.Ods.Common.Security.CustomViewBased; -using NHibernate; -using NHibernate.Criterion; -using NHibernate.SqlCommand; namespace EdFi.Ods.Api.Security.AuthorizationStrategies.Relationships.Filters { - public static class ICriteriaExtensions + public static class QueryBuilderExtensions { /// /// Applies a join-based filter to the criteria for the specified authorization view. /// - /// The criteria to which filters should be applied. - /// The container for adding WHERE clause criterion. + /// /// The named parameters to be used to satisfy additional filtering requirements. /// The name of the view to be filtered. /// The name of the property to be joined for the entity being queried. @@ -33,9 +27,7 @@ public static class ICriteriaExtensions /// The to be used. /// The name of the property to be used for auth View Alias name. public static void ApplySingleColumnJoinFilter( - this ICriteria criteria, - IMultiValueRestrictions multiValueRestrictions, - Junction whereJunction, + this QueryBuilder queryBuilder, IDictionary parameters, string viewName, string subjectEndpointName, @@ -47,13 +39,23 @@ public static void ApplySingleColumnJoinFilter( authViewAlias = string.IsNullOrWhiteSpace(authViewAlias) ? $"authView{viewName}" : $"authView{authViewAlias}"; // Apply authorization join using ICriteria - criteria.CreateEntityAlias( - authViewAlias, - Restrictions.EqProperty($"aggregateRoot.{subjectEndpointName}", - $"{authViewAlias}.{viewTargetEndpointName}"), - joinType, - $"{viewName.GetAuthorizationViewClassName()}".GetFullNameForView()); - + if (joinType == JoinType.InnerJoin) + { + queryBuilder.Join( + $"auth.{viewName} AS {authViewAlias}", + j => j.On($"r.{subjectEndpointName}", $"{authViewAlias}.{viewTargetEndpointName}")); + } + else if (joinType == JoinType.LeftOuterJoin) + { + queryBuilder.LeftJoin( + $"auth.{viewName} AS {authViewAlias}", + j => j.On($"r.{subjectEndpointName}", $"{authViewAlias}.{viewTargetEndpointName}")); + } + else + { + throw new NotSupportedException("Unsupported authorization view join type."); + } + // Defensive check to ensure required parameter is present if (!parameters.TryGetValue(RelationshipAuthorizationConventions.ClaimsParameterName, out object value)) { @@ -64,30 +66,26 @@ public static void ApplySingleColumnJoinFilter( { if (joinType == JoinType.InnerJoin) { - whereJunction.Add(multiValueRestrictions.In($"{{{authViewAlias}}}.{viewSourceEndpointName}", arrayOfValues)); + queryBuilder.WhereIn($"{authViewAlias}.{viewSourceEndpointName}", arrayOfValues); } else { - var and = new AndExpression( - multiValueRestrictions.In($"{{{authViewAlias}}}.{viewSourceEndpointName}", arrayOfValues), - Restrictions.IsNotNull($"{authViewAlias}.{viewTargetEndpointName}")); - - whereJunction.Add(and); + queryBuilder.Where(qb => + qb.WhereIn($"{authViewAlias}.{viewSourceEndpointName}", arrayOfValues) + .WhereNotNull($"{authViewAlias}.{viewTargetEndpointName}")); } } else { if (joinType == JoinType.InnerJoin) { - whereJunction.Add(Restrictions.Eq($"{authViewAlias}.{viewSourceEndpointName}", value)); + queryBuilder.Where($"{authViewAlias}.{viewSourceEndpointName}", value); } else { - var and = new AndExpression( - Restrictions.Eq($"{authViewAlias}.{viewSourceEndpointName}", value), - Restrictions.IsNotNull($"{authViewAlias}.{viewTargetEndpointName}")); - - whereJunction.Add(and); + queryBuilder.Where(qb => + qb.Where($"{authViewAlias}.{viewSourceEndpointName}", value) + .WhereNotNull($"{authViewAlias}.{viewTargetEndpointName}")); } } } @@ -108,7 +106,7 @@ private static string GetFullNameForView(this string viewName) /// The name of the property to be used for auth View Alias name. /// The authorization strategy instance (which may be an instance providing additional context for authorization). public static void ApplyCustomViewJoinFilter( - this ICriteria criteria, + this QueryBuilder queryBuilder, string viewName, string[] subjectEndpointNames, string[] viewTargetEndpointNames, @@ -126,44 +124,46 @@ public static void ApplyCustomViewJoinFilter( if (subjectEndpointNames.Length == 1) { - // Apply authorization join using ICriteria - criteria.CreateEntityAlias( - authViewFullAlias, - Restrictions.EqProperty( - $"aggregateRoot.{subjectEndpointNames[0]}", - $"{authViewFullAlias}.{viewTargetEndpointNames[0]}"), - joinType, - customViewBasedAuthorizationStrategy.BasisEntity.EntityTypeFullName()); + // Apply authorization join using the query builder + queryBuilder.Join( + $"auth.{viewName} AS {authViewFullAlias}", + j => j.On($"r.{subjectEndpointNames[0]}", $"{authViewFullAlias}.{viewTargetEndpointNames[0]}"), + joinType); } else { // Apply authorization join using ICriteria - criteria.CreateEntityAlias( - authViewFullAlias, - BuildJoinCriterion(0), - joinType, - customViewBasedAuthorizationStrategy.BasisEntity.EntityTypeFullName()); - - ICriterion BuildJoinCriterion(int endPointIndex) - { - // If this is the last 2 names, build an And expression and terminate recursion - if (endPointIndex == subjectEndpointNames.Length - 2) + queryBuilder.Join( + $"auth.{viewName} AS {authViewFullAlias}", j => { - return new AndExpression( - Restrictions.EqProperty( - $"aggregateRoot.{subjectEndpointNames[endPointIndex]}", - $"{authViewFullAlias}.{viewTargetEndpointNames[endPointIndex]}"), - Restrictions.EqProperty( - $"aggregateRoot.{subjectEndpointNames[endPointIndex + 1]}", - $"{authViewFullAlias}.{viewTargetEndpointNames[endPointIndex + 1]}")); - } + for (int i = 0; i < subjectEndpointNames.Length; i++) + { + j.On($"r.{subjectEndpointNames[i]}", $"{authViewFullAlias}.{viewTargetEndpointNames[i]}"); + } - // Recursively build the and expression - return new AndExpression( - Restrictions.EqProperty( - $"aggregateRoot.{subjectEndpointNames[endPointIndex]}", - $"{authViewFullAlias}.{viewTargetEndpointNames[endPointIndex]}"), - BuildJoinCriterion(endPointIndex + 1)); + return j; + }, + joinType); + } + } + + /// + /// Applies property-level filter expressions to the query builder as either Equal or In expressions based on the supplied parameters. + /// + /// The for adding WHERE clause-only criteria. + /// The named parameters to be processed into the criteria query. + /// The array of property names that can be used for filtering. + public static void ApplyPropertyFilters(this QueryBuilder whereQueryBuilder, IDictionary parameters, params string[] availableFilterProperties) + { + foreach (var nameAndValue in parameters.Where(x => availableFilterProperties.Contains(x.Key, StringComparer.OrdinalIgnoreCase))) + { + if (nameAndValue.Value is object[] arrayOfValues) + { + whereQueryBuilder.WhereIn($"{nameAndValue.Key}", arrayOfValues); + } + else + { + whereQueryBuilder.Where($"{nameAndValue.Key}", nameAndValue.Value); } } } diff --git a/Application/EdFi.Ods.Api/Security/AuthorizationStrategies/Relationships/Filters/ViewBasedAuthorizationFilterDefinition.cs b/Application/EdFi.Ods.Api/Security/AuthorizationStrategies/Relationships/Filters/ViewBasedAuthorizationFilterDefinition.cs index 70c2dad705..7316c26735 100644 --- a/Application/EdFi.Ods.Api/Security/AuthorizationStrategies/Relationships/Filters/ViewBasedAuthorizationFilterDefinition.cs +++ b/Application/EdFi.Ods.Api/Security/AuthorizationStrategies/Relationships/Filters/ViewBasedAuthorizationFilterDefinition.cs @@ -16,6 +16,8 @@ namespace EdFi.Ods.Api.Security.AuthorizationStrategies.Relationships.Filters { public class ViewBasedAuthorizationFilterDefinition : AuthorizationFilterDefinition { + private AliasGenerator _aliasGenerator = new AliasGenerator("authvw"); + /// /// Initializes a new instance of the class using parameters for a /// relationship-based authorization view join using a single column. @@ -35,31 +37,28 @@ public ViewBasedAuthorizationFilterDefinition( string viewTargetEndpointName, Action trackedChangesCriteriaApplicator, Func authorizeInstance, - IViewBasedSingleItemAuthorizationQuerySupport viewBasedSingleItemAuthorizationQuerySupport, - IMultiValueRestrictions multiValueRestrictions) + IViewBasedSingleItemAuthorizationQuerySupport viewBasedSingleItemAuthorizationQuerySupport) : base( filterName, $@"{{currentAlias}}.{{subjectEndpointName}} IN ( SELECT {{newAlias1}}.{viewTargetEndpointName} FROM {GetFullNameForView($"auth_{viewName}")} {{newAlias1}} WHERE {{newAlias1}}.{viewSourceEndpointName} IN (:{RelationshipAuthorizationConventions.ClaimsParameterName}))", - (criteria, @where, subjectEndpointNames, parameters, joinType, authorizationStrategy) => + (queryBuilder, @where, subjectEndpointNames, parameters, joinType, authorizationStrategy) => { if (subjectEndpointNames?.Length != 1) { throw new ArgumentException("Exactly one authorization subject endpoint name is expected for single-column view-based authorization."); } - criteria.ApplySingleColumnJoinFilter( - multiValueRestrictions, - @where, + @where.ApplySingleColumnJoinFilter( parameters, viewName, subjectEndpointNames[0], viewSourceEndpointName, viewTargetEndpointName, joinType, - Guid.NewGuid().ToString("N")); + Guid.NewGuid().ToString("N").Substring(0, 6)); }, trackedChangesCriteriaApplicator, authorizeInstance) diff --git a/Application/EdFi.Ods.Api/Security/AuthorizationStrategies/Relationships/RelationshipsAuthorizationStrategyFilterDefinitionsFactory.cs b/Application/EdFi.Ods.Api/Security/AuthorizationStrategies/Relationships/RelationshipsAuthorizationStrategyFilterDefinitionsFactory.cs index 9f627448a3..e8a06c71da 100644 --- a/Application/EdFi.Ods.Api/Security/AuthorizationStrategies/Relationships/RelationshipsAuthorizationStrategyFilterDefinitionsFactory.cs +++ b/Application/EdFi.Ods.Api/Security/AuthorizationStrategies/Relationships/RelationshipsAuthorizationStrategyFilterDefinitionsFactory.cs @@ -79,9 +79,9 @@ protected IEnumerable CreateAllEducation usiName, ApplyTrackedChangesAuthorizationCriteria, AuthorizeInstance, - _viewBasedSingleItemAuthorizationQuerySupport, - _multiValueRestrictions - )); + _viewBasedSingleItemAuthorizationQuerySupport + ) + ); } protected IEnumerable CreateAllEducationOrganizationToEducationOrganizationFilters() @@ -99,8 +99,9 @@ protected IEnumerable CreateAllEducation EducationOrganizationAuthorizationViewConstants.TargetColumnName, ApplyTrackedChangesAuthorizationCriteria, AuthorizeInstance, - _viewBasedSingleItemAuthorizationQuerySupport, - _multiValueRestrictions)) + _viewBasedSingleItemAuthorizationQuerySupport + ) + ) // Add filter definitions for using the EdOrg hierarchy inverted .Concat(concreteEdOrgIdNames // Sort the edorg id names to ensure a determinate alias generation during filter definition @@ -113,8 +114,10 @@ protected IEnumerable CreateAllEducation EducationOrganizationAuthorizationViewConstants.SourceColumnName, ApplyTrackedChangesAuthorizationCriteria, AuthorizeInstance, - _viewBasedSingleItemAuthorizationQuerySupport, - _multiValueRestrictions))); + _viewBasedSingleItemAuthorizationQuerySupport + ) + ) + ); } private InstanceAuthorizationResult AuthorizeInstance( diff --git a/Application/EdFi.Ods.Api/Security/Container/Modules/SecurityPersistenceModule.cs b/Application/EdFi.Ods.Api/Security/Container/Modules/SecurityPersistenceModule.cs index d105d4b879..f5ca528227 100644 --- a/Application/EdFi.Ods.Api/Security/Container/Modules/SecurityPersistenceModule.cs +++ b/Application/EdFi.Ods.Api/Security/Container/Modules/SecurityPersistenceModule.cs @@ -27,7 +27,6 @@ public class SecurityPersistenceModule : Module {typeof(IGetEntityByKey<>), typeof(GetEntityByKeyAuthorizationDecorator<>)}, {typeof(IGetEntitiesBySpecification<>), typeof(GetEntitiesBySpecificationAuthorizationDecorator<>)}, {typeof(IPagedAggregateIdsCriteriaProvider<>), typeof(PagedAggregateIdsCriteriaProviderAuthorizationDecorator<>)}, - {typeof(ITotalCountCriteriaProvider<>), typeof(TotalCountCriteriaProviderAuthorizationDecorator<>)}, {typeof(IGetEntityById<>), typeof(GetEntityByIdAuthorizationDecorator<>)}, {typeof(IGetEntitiesByIds<>), typeof(GetEntitiesByIdsAuthorizationDecorator<>)}, {typeof(ICreateEntity<>), typeof(CreateEntityAuthorizationDecorator<>)}, diff --git a/Application/EdFi.Ods.Common/Database/Querying/Dialects/SqlServerDialect.cs b/Application/EdFi.Ods.Common/Database/Querying/Dialects/SqlServerDialect.cs index 6859783604..cbb1a3c1d1 100644 --- a/Application/EdFi.Ods.Common/Database/Querying/Dialects/SqlServerDialect.cs +++ b/Application/EdFi.Ods.Common/Database/Querying/Dialects/SqlServerDialect.cs @@ -3,6 +3,7 @@ // The Ed-Fi Alliance licenses this file to you under the Apache License, Version 2.0. // See the LICENSE and NOTICES files in the project root for more information. +using System; using System.Collections; using System.Collections.Generic; using Dapper; @@ -36,6 +37,12 @@ public override (string sql, object parameters) GetInClause(string columnName, s { var itemSystemType = values[0].GetType(); + // ODS does not support TVPs using shorts, so use int instead + if (itemSystemType == typeof(short)) + { + itemSystemType = typeof(int); + } + var tvp = SqlServerTableValuedParameterHelper.CreateIdDataTable(values, itemSystemType) .AsTableValuedParameter(SqlServerStructuredMappings.StructuredTypeNameBySystemType[itemSystemType]); diff --git a/Application/EdFi.Ods.Common/Database/Querying/QueryBuilder.cs b/Application/EdFi.Ods.Common/Database/Querying/QueryBuilder.cs index 46cdd4e5c8..47f0404a27 100644 --- a/Application/EdFi.Ods.Common/Database/Querying/QueryBuilder.cs +++ b/Application/EdFi.Ods.Common/Database/Querying/QueryBuilder.cs @@ -16,11 +16,10 @@ namespace EdFi.Ods.Common.Database.Querying { public class QueryBuilder { - private readonly List _ctes = new List(); private readonly Dialect _dialect; - private readonly ParameterIndexer _parameterIndexer = new ParameterIndexer(); - private readonly SqlBuilder _sqlBuilder = new SqlBuilder(); + private readonly ParameterIndexer _parameterIndexer = new(); + private readonly SqlBuilder _sqlBuilder = new(); public QueryBuilder(Dialect dialect) { @@ -110,6 +109,12 @@ public QueryBuilder Where(string column, string op, object value) parameterName = parameterNameDisposition ?? $"@p{_parameterIndexer.Increment()}"; parameters.AddDynamicParams(new[] { new KeyValuePair(parameterName, values) }); } + else if (value is DynamicParameters dynamicParameters) + { + // Just use the dynamic parameters, as provided (but force an exception if more than 1 parameter is present) + parameters = dynamicParameters; + parameterName = $"@{dynamicParameters.ParameterNames.Single()}"; + } else { // Inline parameter, automatically named @@ -144,8 +149,10 @@ public QueryBuilder Where(Func nestedWhereApplicator ? new DynamicParameters(childScope.Parameters) : null); - // Wrap the WHERE clause directly - _sqlBuilder.Where($"({template.RawSql.Replace("WHERE ", string.Empty)})", template.Parameters); + string whereCriteria = template.RawSql.Replace("WHERE ", string.Empty); + + // Since we're dealing with a child scope with only 1 WHERE clause, no need to wrap it + _sqlBuilder.Where($"{whereCriteria}", template.Parameters); } // Incorporate any JOINs added into this builder @@ -168,14 +175,14 @@ public QueryBuilder OrWhere(Func nestedWhereApplicat { return this; } - + var template = childScopeSqlBuilder.AddTemplate( "/**where**/", childScope.Parameters.Any() ? new DynamicParameters(childScope.Parameters) : null); - // Wrap the WHERE clause directly + // SqlBuilder warps 'OR' where clauses when building the template SQL _sqlBuilder.OrWhere($"({template.RawSql.Replace("WHERE ", string.Empty)})", template.Parameters); // Incorporate the JOINs into this builder @@ -184,6 +191,20 @@ public QueryBuilder OrWhere(Func nestedWhereApplicat return this; } + public QueryBuilder OrWhere(string column, object value) + { + return OrWhere(column, "=", value); + } + + public QueryBuilder OrWhere(string column, string op, object value) + { + (DynamicParameters parameters, string parameterName) = GetParametersFromObject(value); + + _sqlBuilder.OrWhere($"{column} {op} {parameterName}", parameters); + + return this; + } + public QueryBuilder OrWhereNull(string column) { _sqlBuilder.OrWhere($"{column} IS NULL"); @@ -191,24 +212,46 @@ public QueryBuilder OrWhereNull(string column) return this; } - public QueryBuilder WhereLike(string columnName, object value) + public QueryBuilder WhereLike(string columnName, object value, MatchMode matchMode = MatchMode.Exact) { - (DynamicParameters parameters, string parameterName) = GetParametersFromObject(value); + (DynamicParameters parameters, string parameterName) = GetParametersFromObject( + matchMode switch + { + // NOTE: May need dialect support for varying text match symbols + MatchMode.Start => $"%{value}", + MatchMode.Anywhere => $"%{value}%", + MatchMode.End => $"{value}%", + _ => value + }); _sqlBuilder.Where($"{columnName} LIKE {parameterName}", parameters); return this; } - public QueryBuilder OrWhereLike(string expression, object value) + public QueryBuilder OrWhereLike(string expression, object value, MatchMode matchMode = MatchMode.Exact) { - (DynamicParameters parameters, string parameterName) = GetParametersFromObject(value); - + (DynamicParameters parameters, string parameterName) = GetParametersFromObject(matchMode switch + { + // NOTE: May need dialect support for varying text match symbols + MatchMode.Start => $"%{value}", + MatchMode.Anywhere => $"%{value}%", + MatchMode.End => $"{value}%", + _ => value + }); + _sqlBuilder.OrWhere($"{expression} LIKE {parameterName}", parameters); return this; } + public QueryBuilder WhereRaw(string rawSql) + { + _sqlBuilder.Where(rawSql); + + return this; + } + public QueryBuilder WhereNull(string columnName) { _sqlBuilder.Where($"{columnName} IS NULL"); @@ -261,21 +304,24 @@ public QueryBuilder Join(string tableName, string thisExpression, string otherEx return this; } - public QueryBuilder Join(string table, Func joiner) + public QueryBuilder Join(string table, Func joiner, JoinType joinType = JoinType.InnerJoin) { var join = joiner(new Join(table)); - _sqlBuilder.InnerJoin( - $"{join.TableName} ON {string.Join(" AND ", join.Segments.Select(s => $"{s.first} {s.@operator} {s.second}"))}"); + if (joinType == JoinType.InnerJoin) + { + _sqlBuilder.InnerJoin( + $"{join.TableName} ON {string.Join(" AND ", join.Segments.Select(s => $"{s.first} {s.@operator} {s.second}"))}"); + } + else if (joinType == JoinType.LeftOuterJoin) + { + _sqlBuilder.LeftJoin( + $"{join.TableName} ON {string.Join(" AND ", join.Segments.Select(s => $"{s.first} {s.@operator} {s.second}"))}"); + } return this; } - // public QueryBuilder Join(QueryBuilder query, Func joiner, string type = "inner join") - // { - // return this; - // } - public QueryBuilder LeftJoin(string tableName, string thisExpression, string otherExpression) { _sqlBuilder.LeftJoin($"{tableName} ON {thisExpression} = {otherExpression}"); @@ -293,11 +339,6 @@ public QueryBuilder LeftJoin(string table, Func joiner) return this; } - // public QueryBuilder LeftJoin(QueryBuilder query, Func joiner, string type = "inner join") - // { - // return this; - // } - public QueryBuilder Select(params string[] columns) { columns.ForEach(c => _sqlBuilder.Select(c)); @@ -444,4 +485,21 @@ public Join On(string first, string second, string @operator = "=") return this; } } + + public enum MatchMode + { + Exact, + Start, + Anywhere, + End, + } + + public enum JoinType + { + InnerJoin = 0, + LeftOuterJoin = 1, + // RightOuterJoin = 2, + // FullJoin = 4, + // CrossJoin = 8 + } } diff --git a/Application/EdFi.Ods.Common/Database/Querying/SqlBuilder_customizations.cs b/Application/EdFi.Ods.Common/Database/Querying/SqlBuilder_customizations.cs index ff7e5404e1..2c79199c45 100644 --- a/Application/EdFi.Ods.Common/Database/Querying/SqlBuilder_customizations.cs +++ b/Application/EdFi.Ods.Common/Database/Querying/SqlBuilder_customizations.cs @@ -11,18 +11,22 @@ namespace EdFi.Ods.Common.Database.Querying { public partial class SqlBuilder { + private const string Space = " "; + /// /// Sets a clause that should only appear once within the resulting query (e.g. paging). /// /// /// /// + /// + /// /// - protected SqlBuilder SetClause(string name, string sql, object parameters) + private SqlBuilder SetClause(string name, string sql, object parameters, string prefix = "\n", string postfix = "\n") { if (!_data.TryGetValue(name, out var clauses)) { - clauses = new Clauses(string.Empty, "\n", "\n"); + clauses = new Clauses(string.Empty, prefix, postfix); _data[name] = clauses; } else @@ -37,7 +41,7 @@ protected SqlBuilder SetClause(string name, string sql, object parameters) } public SqlBuilder Distinct() => - SetClause("distinct", "DISTINCT", null); + SetClause("distinct", "DISTINCT", null, null, null); public SqlBuilder LimitOffset(string sql, dynamic parameters = null) => SetClause("paging", sql, parameters); @@ -148,7 +152,7 @@ public bool HasWhereClause() { return HasClause(ClauseKey.Where); } - + private bool HasClause(string clausekey) { return _data.Any(kvp => kvp.Key == clausekey); diff --git a/Application/EdFi.Ods.Common/Exceptions/NotFoundException.cs b/Application/EdFi.Ods.Common/Exceptions/NotFoundException.cs index f579eccbd0..825c4db614 100644 --- a/Application/EdFi.Ods.Common/Exceptions/NotFoundException.cs +++ b/Application/EdFi.Ods.Common/Exceptions/NotFoundException.cs @@ -27,6 +27,12 @@ public NotFoundException() : base(DefaultDetail, DefaultDetail) { } public NotFoundException(string detail) : base(detail, detail) { } + public NotFoundException(string detail, string[] errors) + : base(detail, detail) + { + this.SetErrors(errors); + } + public NotFoundException(string detail, string message) : base(detail, message) { } diff --git a/Application/EdFi.Ods.Common/Extensions/TypeExtensions.cs b/Application/EdFi.Ods.Common/Extensions/TypeExtensions.cs index e54fb0c761..0e898d0911 100644 --- a/Application/EdFi.Ods.Common/Extensions/TypeExtensions.cs +++ b/Application/EdFi.Ods.Common/Extensions/TypeExtensions.cs @@ -9,16 +9,15 @@ using System.Linq; using System.Reflection; using EdFi.Ods.Common.Attributes; +using EdFi.Ods.Common.Models.Domain; namespace EdFi.Ods.Common.Extensions { public static class TypeExtensions { - private static readonly ConcurrentDictionary - _defaultValuesByType = new ConcurrentDictionary(); - - private static readonly ConcurrentDictionary _itemTypesByGenericListType = - new ConcurrentDictionary(); + private static readonly ConcurrentDictionary _defaultValuesByType = new(); + private static readonly ConcurrentDictionary _itemTypesByGenericListType = new(); + private static readonly ConcurrentDictionary _fullNameByEntityType = new(); public static T GetDefaultValue() { @@ -86,6 +85,28 @@ public static IEnumerable GetSignatureProperties(this Type type) p => Attribute.IsDefined(p, typeof(DomainSignatureAttribute), true)); } + public static FullName GetApiModelFullName(this Type entityType) + { + return _fullNameByEntityType.GetOrAdd( + entityType, + static t => + { + var schema = t.GetCustomAttribute(false)?.Schema + ?? throw new Exception($"The '{nameof(SchemaAttribute)}' was not found on entity type '{t.FullName}'."); + + var fullName = new FullName(schema, t.Name); + + return fullName; + }); + } + + public static FullName GetApiModelFullName(this TEntity entity) + { + Type entityType = typeof(TEntity); + + return GetApiModelFullName(entityType); + } + public static bool IsScalar(this Type type) { return type.IsValueType || type == typeof(string); diff --git a/Application/EdFi.Ods.Common/Infrastructure/Configuration/NHibernateOdsConnectionProvider.cs b/Application/EdFi.Ods.Common/Infrastructure/Configuration/NHibernateOdsConnectionProvider.cs index cce85643b7..955f19d903 100644 --- a/Application/EdFi.Ods.Common/Infrastructure/Configuration/NHibernateOdsConnectionProvider.cs +++ b/Application/EdFi.Ods.Common/Infrastructure/Configuration/NHibernateOdsConnectionProvider.cs @@ -34,10 +34,18 @@ public override DbConnection GetConnection() connection.Open(); } + catch (EdFiProblemDetailsExceptionBase) + { + connection.Dispose(); + + // Throw the problem details exception, as-is + throw; + } catch (Exception ex) { connection.Dispose(); + // Wrap the underlying exception with a user-facing exception. throw new DatabaseConnectionException("Unable to open connection to the ODS database.", ex); } @@ -54,10 +62,18 @@ public override async Task GetConnectionAsync(CancellationToken ca await connection.OpenAsync(cancellationToken); } + catch (EdFiProblemDetailsExceptionBase) + { + connection.Dispose(); + + // Throw the problem details exception, as-is + throw; + } catch (Exception ex) { connection.Dispose(); + // Wrap the underlying exception with a user-facing exception. throw new DatabaseConnectionException("Unable to open connection to the ODS database.", ex); } diff --git a/Application/EdFi.Ods.Common/Infrastructure/Filtering/AuthorizationFilterDefinition.cs b/Application/EdFi.Ods.Common/Infrastructure/Filtering/AuthorizationFilterDefinition.cs index 1466e69b8a..2b755cf3db 100644 --- a/Application/EdFi.Ods.Common/Infrastructure/Filtering/AuthorizationFilterDefinition.cs +++ b/Application/EdFi.Ods.Common/Infrastructure/Filtering/AuthorizationFilterDefinition.cs @@ -5,15 +5,10 @@ using System; using System.Collections.Generic; -using System.Reflection; -using System.Text.RegularExpressions; using EdFi.Ods.Common.Database.Querying; 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.Common.Infrastructure.Filtering { @@ -42,7 +37,7 @@ public class AuthorizationFilterDefinition public AuthorizationFilterDefinition( string filterName, string friendlyHqlConditionFormat, - Action, JoinType, IAuthorizationStrategy> criteriaApplicator, + Action, JoinType, IAuthorizationStrategy> criteriaApplicator, Action trackedChangesCriteriaApplicator, Func authorizeInstance) { @@ -63,7 +58,7 @@ public AuthorizationFilterDefinition( /// /// Gets the function for applying the filter using NHibernate's API. /// - public Action, JoinType, IAuthorizationStrategy> CriteriaApplicator { get; protected set; } + public Action, JoinType, IAuthorizationStrategy> CriteriaApplicator { get; protected set; } /// /// Gets the function for applying the filter to the for tracked changes queries. diff --git a/Application/EdFi.Ods.Common/Infrastructure/Repositories/GetEntitiesBySpecification.cs b/Application/EdFi.Ods.Common/Infrastructure/Repositories/GetEntitiesBySpecification.cs index 53d0e8fa9d..66fdf3926f 100644 --- a/Application/EdFi.Ods.Common/Infrastructure/Repositories/GetEntitiesBySpecification.cs +++ b/Application/EdFi.Ods.Common/Infrastructure/Repositories/GetEntitiesBySpecification.cs @@ -8,12 +8,12 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; +using Dapper; +using EdFi.Ods.Common.Database.Querying; using EdFi.Ods.Common.Models.Domain; using EdFi.Ods.Common.Providers.Criteria; using EdFi.Ods.Common.Repositories; using NHibernate; -using NHibernate.Criterion; -using NHibernate.Multi; namespace EdFi.Ods.Common.Infrastructure.Repositories { @@ -26,19 +26,16 @@ public class GetEntitiesBySpecification protected static IList EmptyList = new List(); private readonly IPagedAggregateIdsCriteriaProvider _pagedAggregateIdsCriteriaProvider; - private readonly ITotalCountCriteriaProvider _totalCountCriteriaProvider; private readonly IGetEntitiesByIds _getEntitiesByIds; public GetEntitiesBySpecification( ISessionFactory sessionFactory, IGetEntitiesByIds getEntitiesByIds, - IPagedAggregateIdsCriteriaProvider pagedAggregateIdsCriteriaProvider, - ITotalCountCriteriaProvider totalCountCriteriaProvider) + IPagedAggregateIdsCriteriaProvider pagedAggregateIdsCriteriaProvider) : base(sessionFactory) { _getEntitiesByIds = getEntitiesByIds; _pagedAggregateIdsCriteriaProvider = pagedAggregateIdsCriteriaProvider; - _totalCountCriteriaProvider = totalCountCriteriaProvider; } public async Task> GetBySpecificationAsync(TEntity specification, @@ -83,61 +80,87 @@ async Task GetPagedAggregateIdsAsync() return new SpecificationResult { Ids = Array.Empty() }; } - var queryBatch = Session.CreateQueryBatch(); - // If any items requested, get the requested page of Ids + QueryBuilder idsQueryBuilder = null; + + SqlBuilder.Template idsTemplate = null; + if (ItemsRequested()) { - var idQueryCriteria = _pagedAggregateIdsCriteriaProvider.GetCriteriaQuery(specification, queryParameters); - SetChangeQueriesCriteria(idQueryCriteria); - - queryBatch.Add(idQueryCriteria); + idsQueryBuilder = GetIdsQueryBuilder(); + idsTemplate = idsQueryBuilder.BuildTemplate(); } + SqlBuilder.Template countTemplate = null; + // If requested, get a total count of available records if (CountRequested()) { - var countQueryCriteria = _totalCountCriteriaProvider.GetCriteriaQuery(specification, queryParameters); - SetChangeQueriesCriteria(countQueryCriteria); + countTemplate = (idsQueryBuilder ?? GetIdsQueryBuilder()).BuildCountTemplate(); + } + + if (idsTemplate != null && countTemplate != null) + { + // Combine the SQL queries + var combinedSql = $"{idsTemplate.RawSql}; {countTemplate.RawSql}"; + + var parameters = new DynamicParameters(); + parameters.AddDynamicParams(idsTemplate.Parameters); - queryBatch.Add(countQueryCriteria); + await using var multi = await Session.Connection.QueryMultipleAsync(combinedSql, parameters); + + var ids = (await multi.ReadAsync()).ToList(); + var totalCount = await multi.ReadSingleAsync(); + + return new SpecificationResult + { + Ids = ids, + TotalCount = totalCount + }; } - int resultIndex = 0; - - var ids = ItemsRequested() - ? (await queryBatch.GetResultAsync(resultIndex++, cancellationToken)) - .Select(r => (Guid) r[0]) - .ToArray() - : Array.Empty(); - - var totalCount = CountRequested() - ? (await queryBatch.GetResultAsync(resultIndex, cancellationToken)).First() - : 0; + if (idsTemplate != null) + { + var idsResults = await Session.Connection.QueryAsync(idsTemplate.RawSql, idsTemplate.Parameters); + return new SpecificationResult + { + Ids = idsResults.Select(d => d.Id).ToArray() + }; + } + + var countResult = await Session.Connection.QuerySingleAsync(countTemplate.RawSql, countTemplate.Parameters); return new SpecificationResult { - Ids = ids, - TotalCount = totalCount + Ids = Array.Empty(), + TotalCount = countResult }; + } + + bool ItemsRequested() => !(queryParameters.Limit == 0); + + bool CountRequested() => queryParameters.TotalCount; + + QueryBuilder GetIdsQueryBuilder() + { + var idsQueryBuilder = _pagedAggregateIdsCriteriaProvider.GetQueryBuilder(specification, queryParameters); + SetChangeQueriesCriteria(idsQueryBuilder); - void SetChangeQueriesCriteria(ICriteria criteria) + return idsQueryBuilder; + + void SetChangeQueriesCriteria(QueryBuilder queryBuilder) { if (queryParameters.MinChangeVersion.HasValue) { - criteria.Add(Restrictions.Ge(ChangeVersion, queryParameters.MinChangeVersion.Value)); + queryBuilder.Where(ChangeVersion, ">=", queryParameters.MinChangeVersion.Value); } if (queryParameters.MaxChangeVersion.HasValue) { - criteria.Add(Restrictions.Le(ChangeVersion, queryParameters.MaxChangeVersion.Value)); + queryBuilder.Where(ChangeVersion, "<=", queryParameters.MaxChangeVersion.Value); } } } - - bool ItemsRequested() => !(queryParameters.Limit == 0); - - bool CountRequested() => queryParameters.TotalCount; } private class SpecificationResult @@ -145,5 +168,10 @@ private class SpecificationResult public IList Ids { get; set; } public int TotalCount { get; set; } } + + private class IdOnly + { + public Guid Id { get; set; } + } } } diff --git a/Application/EdFi.Ods.Common/Models/Domain/EntityExtensions.cs b/Application/EdFi.Ods.Common/Models/Domain/EntityExtensions.cs index 57824defaa..4102a24e30 100644 --- a/Application/EdFi.Ods.Common/Models/Domain/EntityExtensions.cs +++ b/Application/EdFi.Ods.Common/Models/Domain/EntityExtensions.cs @@ -274,16 +274,16 @@ public static string ColumnName(this EntityProperty property, DatabaseEngine dat /// /// The entity for which to obtain the physical table name. /// The key representing the database engine. - /// Explicit name to use instead of the property if no + /// Explicit name to use instead of the property if no /// entry is found for the specified database engine. /// - public static string TableName(this Entity entity, DatabaseEngine databaseEngine, string explicitTableName = null) + public static string TableName(this Entity entity, DatabaseEngine databaseEngine, string overrideFallbackName = null) { return entity.TableNameByDatabaseEngine == null || entity.TableNameByDatabaseEngine.Count == 0 - ? explicitTableName ?? entity.Name + ? overrideFallbackName ?? entity.Name : entity.TableNameByDatabaseEngine.TryGetValue(databaseEngine, out string tableName) ? tableName - : explicitTableName ?? entity.Name; + : overrideFallbackName ?? entity.Name; } /// diff --git a/Application/EdFi.Ods.Common/Providers/Criteria/AggregateRootCriteriaProviderBase.cs b/Application/EdFi.Ods.Common/Providers/Criteria/AggregateRootCriteriaProviderBase.cs deleted file mode 100644 index 0f4e6cbaed..0000000000 --- a/Application/EdFi.Ods.Common/Providers/Criteria/AggregateRootCriteriaProviderBase.cs +++ /dev/null @@ -1,152 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -// Licensed to the Ed-Fi Alliance under one or more agreements. -// The Ed-Fi Alliance licenses this file to you under the Apache License, Version 2.0. -// See the LICENSE and NOTICES files in the project root for more information. - -using System; -using System.ComponentModel; -using System.Linq; -using EdFi.Ods.Common.Descriptors; -using EdFi.Ods.Common.Extensions; -using EdFi.Ods.Common.Infrastructure.Repositories; -using EdFi.Ods.Common.Models.Queries; -using EdFi.Ods.Common.Specifications; -using NHibernate; -using NHibernate.Criterion; - -namespace EdFi.Ods.Common.Providers.Criteria -{ - /// - /// Contains common functionality for incorporating entity specifications and query parameters supplied by the API client - /// into the for servicing the request. - /// - /// The of the entity being queried. - public abstract class AggregateRootCriteriaProviderBase : NHibernateRepositoryOperationBase - { - private readonly IDescriptorResolver _descriptorResolver; - private readonly IPersonEntitySpecification _personEntitySpecification; - private readonly IPersonTypesProvider _personTypesProvider; - - protected AggregateRootCriteriaProviderBase( - ISessionFactory sessionFactory, - IDescriptorResolver descriptorResolver, - IPersonEntitySpecification personEntitySpecification, - IPersonTypesProvider personTypesProvider) - : base(sessionFactory) - { - _descriptorResolver = descriptorResolver ?? throw new ArgumentNullException(nameof(descriptorResolver)); - _personEntitySpecification = personEntitySpecification; - _personTypesProvider = personTypesProvider; - } - - protected void ProcessSpecification(ICriteria queryCriteria, TEntity specification) - { - if (specification != null) - { - var propertyValuePairs = specification.ToDictionary( - (descriptor, o) => ShouldIncludeInQueryCriteria(descriptor, o, specification)); - - foreach (var key in propertyValuePairs.Keys) - { - IHasLookupColumnPropertyMap map = specification as IHasLookupColumnPropertyMap; - - if (map.IdPropertyByLookupProperty.TryGetValue(key, out LookupColumnDetails columnDetails)) - { - // Look up the corresponding lookup id value from the cache - var lookupId = _descriptorResolver.GetDescriptorId( - columnDetails.LookupTypeName, - Convert.ToString(propertyValuePairs[key])); - - // Add criteria for the lookup Id value, to avoid need to incorporate an INNER JOIN into the query - queryCriteria.Add( - propertyValuePairs[key] != null - ? Restrictions.Eq(columnDetails.PropertyName, lookupId) - : Restrictions.IsNull(key)); - } - else - { - // Add the property equality condition to the query criteria - queryCriteria.Add( - propertyValuePairs[key] != null - ? Restrictions.Eq(key, propertyValuePairs[key]) - : Restrictions.IsNull(key)); - } - } - } - } - - protected static void ProcessQueryParameters(ICriteria queryCriteria, IQueryParameters parameters) - { - foreach (IQueryCriteriaBase criteria in parameters.QueryCriteria) - { - TextCriteria textCriteria = criteria as TextCriteria; - - if (textCriteria != null) - { - MatchMode mode; - - switch (textCriteria.MatchMode) - { - case TextMatchMode.Anywhere: - mode = MatchMode.Anywhere; - - break; - - case TextMatchMode.Start: - mode = MatchMode.Start; - - break; - - case TextMatchMode.End: - mode = MatchMode.End; - - break; - - //case TextMatchMode.Exact: - default: - mode = MatchMode.Exact; - - break; - } - - queryCriteria.Add(Restrictions.Like(textCriteria.PropertyName, textCriteria.Value, mode)); - } - } - } - - private bool ShouldIncludeInQueryCriteria(PropertyDescriptor property, object value, TEntity entity) - { - // Null values and underscore-prefixed properties are ignored for specification purposes - if (value == null || property.Name.StartsWith("_") || "|Url|".Contains((string)property.Name)) - { - // TODO: Come up with better way to exclude non-data properties - return false; - } - - Type valueType = value.GetType(); - - // Only use value types (or strings), and non-default values (i.e. ignore 0's) - var result = (valueType.IsValueType || valueType == typeof(string)) - && (!value.Equals(valueType.GetDefaultValue()) - || (UniqueIdConventions.IsUSI(property.Name) - && GetPropertyValue(entity, UniqueIdConventions.GetUniqueIdPropertyName(property.Name)) != null)); - - // Don't include properties that are explicitly to be ignored - result = result && !AggregateRootCriteriaProviderHelpers.PropertiesToIgnore.Contains(property.Name); - - // Don't include UniqueId properties when they appear on a Person entity - result = result - && (!AggregateRootCriteriaProviderHelpers.GetUniqueIdProperties(_personTypesProvider).Contains(property.Name) - || _personEntitySpecification.IsPersonEntity(entity.GetType())); - - return result; - } - - private object GetPropertyValue(TEntity entity, string propertyName) - { - var properties = entity.ToDictionary(); - - return properties.Where(p => p.Key == propertyName).Select(p => p.Value).SingleOrDefault(); - } - } -} diff --git a/Application/EdFi.Ods.Common/Providers/Criteria/IAggregateRootCriteriaProvider.cs b/Application/EdFi.Ods.Common/Providers/Criteria/IAggregateRootCriteriaProvider.cs deleted file mode 100644 index 2ca59e309e..0000000000 --- a/Application/EdFi.Ods.Common/Providers/Criteria/IAggregateRootCriteriaProvider.cs +++ /dev/null @@ -1,26 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -// Licensed to the Ed-Fi Alliance under one or more agreements. -// The Ed-Fi Alliance licenses this file to you under the Apache License, Version 2.0. -// See the LICENSE and NOTICES files in the project root for more information. - -using NHibernate; - -namespace EdFi.Ods.Common.Providers.Criteria -{ - /// - /// Provides an abstraction for aggregate queries using the API in NHibernate, primarily to - /// provide a seam for authorization filtering by the AggregateRootCriteriaProviderDecorator class. - /// - /// The type of the entity to which criteria is being applied. - public interface IAggregateRootCriteriaProvider - where TEntity : class - { - /// - /// Gets the ICriteria for an NHibernate query. - /// - /// An instance of the entity containing parameters to be added to the query. - /// The query parameters to be applied to the filtering. - /// The NHibernate instance representing the query. - ICriteria GetCriteriaQuery(TEntity specification, IQueryParameters queryParameters); - } -} \ No newline at end of file diff --git a/Application/EdFi.Ods.Common/Providers/Criteria/IPagedAggregateIdsCriteriaProvider.cs b/Application/EdFi.Ods.Common/Providers/Criteria/IPagedAggregateIdsCriteriaProvider.cs index d39c4e2aa0..0ac4505c40 100644 --- a/Application/EdFi.Ods.Common/Providers/Criteria/IPagedAggregateIdsCriteriaProvider.cs +++ b/Application/EdFi.Ods.Common/Providers/Criteria/IPagedAggregateIdsCriteriaProvider.cs @@ -3,12 +3,23 @@ // The Ed-Fi Alliance licenses this file to you under the Apache License, Version 2.0. // See the LICENSE and NOTICES files in the project root for more information. +using EdFi.Ods.Common.Database.Querying; + namespace EdFi.Ods.Common.Providers.Criteria { /// /// Defines a method for building a query that retrieves the Ids for the next page of data. /// /// The type of the entity to which criteria is being applied. - public interface IPagedAggregateIdsCriteriaProvider : IAggregateRootCriteriaProvider - where TEntity : class { } -} \ No newline at end of file + public interface IPagedAggregateIdsCriteriaProvider + where TEntity : class + { + /// + /// Gets the instance containing the query for getting the requested paged of data. + /// + /// An instance of the entity containing parameters to be added to the query. + /// The query parameters to be applied to the filtering. + /// The instance representing the query. + QueryBuilder GetQueryBuilder(TEntity specification, IQueryParameters queryParameters); + } +} diff --git a/Application/EdFi.Ods.Common/Providers/Criteria/ITotalCountCriteriaProvider.cs b/Application/EdFi.Ods.Common/Providers/Criteria/ITotalCountCriteriaProvider.cs deleted file mode 100644 index 60a8e352c8..0000000000 --- a/Application/EdFi.Ods.Common/Providers/Criteria/ITotalCountCriteriaProvider.cs +++ /dev/null @@ -1,14 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -// Licensed to the Ed-Fi Alliance under one or more agreements. -// The Ed-Fi Alliance licenses this file to you under the Apache License, Version 2.0. -// See the LICENSE and NOTICES files in the project root for more information. - -namespace EdFi.Ods.Common.Providers.Criteria -{ - /// - /// Defines a method for building a query that retrieves the total count of resource items available to the current caller. - /// - /// The type of the entity to which criteria is being applied. - public interface ITotalCountCriteriaProvider : IAggregateRootCriteriaProvider - where TEntity : class { } -} \ No newline at end of file diff --git a/Application/EdFi.Ods.Common/Providers/Criteria/PagedAggregateIdsCriteriaProvider.cs b/Application/EdFi.Ods.Common/Providers/Criteria/PagedAggregateIdsCriteriaProvider.cs index f06a910102..b2158f8d3c 100644 --- a/Application/EdFi.Ods.Common/Providers/Criteria/PagedAggregateIdsCriteriaProvider.cs +++ b/Application/EdFi.Ods.Common/Providers/Criteria/PagedAggregateIdsCriteriaProvider.cs @@ -4,13 +4,23 @@ // See the LICENSE and NOTICES files in the project root for more information. using System; -using EdFi.Ods.Common.Caching; +using System.Collections.Generic; +using System.ComponentModel; +using System.Data; +using System.Linq; +using Dapper; +using EdFi.Common.Configuration; using EdFi.Ods.Common.Configuration; +using EdFi.Ods.Common.Database.Querying; +using EdFi.Ods.Common.Database.Querying.Dialects; using EdFi.Ods.Common.Descriptors; +using EdFi.Ods.Common.Extensions; +using EdFi.Ods.Common.Infrastructure.Repositories; +using EdFi.Ods.Common.Models; +using EdFi.Ods.Common.Models.Domain; +using EdFi.Ods.Common.Models.Queries; using EdFi.Ods.Common.Specifications; using NHibernate; -using NHibernate.Criterion; -using NHibernate.Persister.Entity; namespace EdFi.Ods.Common.Providers.Criteria { @@ -18,81 +28,287 @@ namespace EdFi.Ods.Common.Providers.Criteria /// Builds a query that retrieves the Ids for the next page of data. /// /// The type of entity for which to build the query. - public class PagedAggregateIdsCriteriaProvider : AggregateRootCriteriaProviderBase, IPagedAggregateIdsCriteriaProvider + public class PagedAggregateIdsCriteriaProvider : NHibernateRepositoryOperationBase, IPagedAggregateIdsCriteriaProvider where TEntity : class { + private readonly IDescriptorResolver _descriptorResolver; + private readonly IPersonEntitySpecification _personEntitySpecification; + private readonly IDomainModelProvider _domainModelProvider; + private readonly Dialect _dialect; + private readonly DatabaseEngine _databaseEngine; + private readonly IPersonTypesProvider _personTypesProvider; + public PagedAggregateIdsCriteriaProvider( ISessionFactory sessionFactory, IDescriptorResolver descriptorResolver, IDefaultPageSizeLimitProvider defaultPageSizeLimitProvider, IPersonEntitySpecification personEntitySpecification, - IPersonTypesProvider personTypesProvider) - : base(sessionFactory, descriptorResolver, personEntitySpecification, personTypesProvider) + IPersonTypesProvider personTypesProvider, + IDomainModelProvider domainModelProvider, + Dialect dialect, + DatabaseEngine databaseEngine) + : base(sessionFactory) { - _identifierColumnNames = new Lazy( - () => - { - var persister = (AbstractEntityPersister) SessionFactory.GetClassMetadata(typeof(TEntity)); - - if (persister.IdentifierColumnNames != null && persister.IdentifierColumnNames.Length > 0) - { - return persister.IdentifierColumnNames; - } - - return new[] { "Id" }; - }); + _descriptorResolver = descriptorResolver; + _personEntitySpecification = personEntitySpecification; + _domainModelProvider = domainModelProvider; + _dialect = dialect; + _databaseEngine = databaseEngine; + _personTypesProvider = personTypesProvider; _defaultPageLimitSize = defaultPageSizeLimitProvider.GetDefaultPageSizeLimit(); } - private readonly Lazy _identifierColumnNames; private readonly int _defaultPageLimitSize; /// - /// Get a query that retrieves the Ids for the next page of data. + /// Get a containing the query that retrieves the Ids for the next page of data. /// /// An instance of the entity containing parameters to be added to the query. /// The query parameters to be applied to the filtering. - /// The NHibernate instance representing the query. - public ICriteria GetCriteriaQuery(TEntity specification, IQueryParameters queryParameters) + /// The instance representing the query. + public QueryBuilder GetQueryBuilder(TEntity specification, IQueryParameters queryParameters) { - var idQueryCriteria = Session.CreateCriteria("aggregateRoot") - .SetProjection(Projections.Distinct(GetColumnProjectionsForDistinctWithOrderBy())) - .SetFirstResult(queryParameters.Offset ?? 0) - .SetMaxResults(queryParameters.Limit ?? _defaultPageLimitSize); + var idQueryBuilder = new QueryBuilder(_dialect); + + var entityFullName = specification.GetApiModelFullName(); + + if (!_domainModelProvider.GetDomainModel().EntityByFullName.TryGetValue(entityFullName, out var entity)) + { + throw new Exception($"Unable to find API model entity for '{entityFullName}'."); + } + + // Get the fully qualified physical table name + var schemaTableName = $"{entity.Schema}.{entity.TableName(_databaseEngine)}"; + + string[] selectColumns = GetColumnProjectionsForDistinctWithOrderBy(entity).ToArray(); + + idQueryBuilder.From(schemaTableName.Alias("r")) + .Distinct() + .Select(selectColumns) + .LimitOffset(queryParameters.Limit ?? _defaultPageLimitSize, queryParameters.Offset ?? 0); + + // TODO: ODS-6444 - In order for query caching to work, limit/offset must be parameterized in query (not embedded as literal values) + + // Add the join to the base type + if (entity.IsDerived) + { + idQueryBuilder.Join( + $"{entity.BaseEntity.Schema}.{entity.BaseEntity.TableName(_databaseEngine)} AS b", + j => + { + foreach (var propertyMapping in entity.BaseAssociation.PropertyMappings) + { + j.On( + $"r.{propertyMapping.ThisProperty.ColumnNameByDatabaseEngine[_databaseEngine]}", + $"b.{propertyMapping.OtherProperty.ColumnNameByDatabaseEngine[_databaseEngine]}"); + } + + return j; + }); + } + + AddDefaultOrdering(idQueryBuilder, entity); - AddDefaultOrdering(idQueryCriteria); + // TODO: ODS-6444 - Consider cloning the querybuilder at this point and introducing a seam for applying the additional parameters so that authorization strategies can also be incorporated and cloned // Add specification-based criteria - ProcessSpecification(idQueryCriteria, specification); + ProcessSpecification(idQueryBuilder, specification, entity); // Add special query fields - ProcessQueryParameters(idQueryCriteria, queryParameters); + ProcessQueryParameters(idQueryBuilder, queryParameters); - return idQueryCriteria; + return idQueryBuilder; - IProjection GetColumnProjectionsForDistinctWithOrderBy() + IEnumerable GetColumnProjectionsForDistinctWithOrderBy(Entity entity) { - var projections = Projections.ProjectionList(); - // Add the resource identifier (this is the value we need for the secondary "page" query) - projections.Add(Projections.Property("Id")); + yield return "Id"; // Add the order by (primary key) columns (required when using DISTINCT with ORDER BY) - foreach (var identifierColumnName in _identifierColumnNames.Value) + foreach (var identifierProperty in (entity.BaseEntity ?? entity).Identifier.Properties) { - projections.Add(Projections.Property(identifierColumnName)); + string identifierColumnName = identifierProperty.ColumnName(_databaseEngine, identifierProperty.PropertyName); + + if (entity.IsDerived) + { + yield return $"b.{identifierColumnName}"; + } + else + { + yield return $"r.{identifierColumnName}"; + } } + } - return projections; + void AddDefaultOrdering(QueryBuilder queryBuilder, Entity entity) + { + foreach (var identifierProperty in (entity.BaseEntity ?? entity).Identifier.Properties) + { + string identifierColumnName = identifierProperty.ColumnName(_databaseEngine, identifierProperty.PropertyName); + + if (entity.IsDerived) + { + queryBuilder.OrderBy($"b.{identifierColumnName}"); + } + else + { + queryBuilder.OrderBy($"r.{identifierColumnName}"); + } + } + } + + static void ProcessQueryParameters(QueryBuilder queryBuilder, IQueryParameters parameters) + { + foreach (IQueryCriteriaBase criteria in parameters.QueryCriteria) + { + if (criteria is TextCriteria textCriteria) + { + MatchMode mode; + + switch (textCriteria.MatchMode) + { + case TextMatchMode.Anywhere: + mode = MatchMode.Anywhere; + + break; + + case TextMatchMode.Start: + mode = MatchMode.Start; + + break; + + case TextMatchMode.End: + mode = MatchMode.End; + + break; + + //case TextMatchMode.Exact: + default: + mode = MatchMode.Exact; + + break; + } + + queryBuilder.WhereLike(textCriteria.PropertyName, textCriteria.Value, mode); + } + } } } - private void AddDefaultOrdering(ICriteria queryCriteria) + private void ProcessSpecification(QueryBuilder queryBuilder, TEntity specification, Entity entity) { - foreach (var identifierColumnName in _identifierColumnNames.Value) + if (specification != null) + { + var propertyValuePairs = specification.ToDictionary( + (descriptor, o) => ShouldIncludeInQueryCriteria(descriptor, o, specification)); + + foreach (var key in propertyValuePairs.Keys) + { + IHasLookupColumnPropertyMap map = specification as IHasLookupColumnPropertyMap; + + if (map.IdPropertyByLookupProperty.TryGetValue(key, out LookupColumnDetails columnDetails)) + { + string alias = (!entity.IsDerived || entity.PropertyByName.ContainsKey(columnDetails.PropertyName)) + ? "r" + : "b"; + + // Look up the corresponding lookup id value from the cache + var lookupId = _descriptorResolver.GetDescriptorId( + columnDetails.LookupTypeName, + Convert.ToString(propertyValuePairs[key])); + + // Add criteria for the lookup Id value, to avoid need to incorporate an INNER JOIN into the query + if (lookupId != 0) + { + queryBuilder.Where($"{alias}.{columnDetails.PropertyName}", lookupId); + } + else + { + // Descriptor did not match any value -- criteria should exclude all entries + queryBuilder.WhereRaw("1 = 0"); + } + } + else + { + string alias; + + if (!entity.PropertyByName.TryGetValue(key, out var entityProperty)) + { + if (!entity.IsDerived || !entity.BaseEntity.PropertyByName.TryGetValue(key, out entityProperty)) + { + throw new ArgumentException($"Property '{key}' was not found."); + } + + alias = "b"; + } + else + { + alias = "r"; + } + + // Add the property equality condition to the query criteria + if (propertyValuePairs[key] != null) + { + // Special handling required for money data types due to PostgreSQL + if (entityProperty.PropertyType.DbType == DbType.Currency) + { + DynamicParameters parameter = new(); + parameter.Add($"@{key}", Convert.ToDecimal(propertyValuePairs[key]), DbType.Currency); + queryBuilder.Where($"{alias}.{key}", parameter); + } + else + { + queryBuilder.Where($"{alias}.{key}", propertyValuePairs[key]); + } + } + else + { + queryBuilder.WhereNull($"{alias}.{key}"); + } + } + } + } + + bool ShouldIncludeInQueryCriteria(PropertyDescriptor property, object value, TEntity entity) { - queryCriteria.AddOrder(Order.Asc(identifierColumnName)); + // Null values and underscore-prefixed properties are ignored for specification purposes + if (value == null || property.Name.StartsWith("_") || "|Url|".Contains((string)property.Name)) + { + // TODO: Come up with better way to exclude non-data properties + return false; + } + + if (property.Name.EndsWith("DescriptorId")) + { + // DescriptorIds are not used directly from the specification because they might not be set if the value is invalid (rather, the Descriptor lookup is used) + return false; + } + + Type valueType = value.GetType(); + + // Only use value types (or strings), and non-default values (i.e. ignore 0's) + var result = (valueType.IsValueType || valueType == typeof(string)) + && (!value.Equals(valueType.GetDefaultValue()) + || (UniqueIdConventions.IsUSI(property.Name) + && GetPropertyValue(entity, UniqueIdConventions.GetUniqueIdPropertyName(property.Name)) != null)); + + // Don't include properties that are explicitly to be ignored + result = result && !AggregateRootCriteriaProviderHelpers.PropertiesToIgnore.Contains(property.Name); + + // Don't include UniqueId properties when they appear on a Person entity + result = result + && (!AggregateRootCriteriaProviderHelpers.GetUniqueIdProperties(_personTypesProvider).Contains(property.Name) + || _personEntitySpecification.IsPersonEntity(entity.GetType())); + + return result; + + object GetPropertyValue(TEntity entity, string propertyName) + { + var properties = entity.ToDictionary(); + + return properties.Where(p => p.Key == propertyName).Select(p => p.Value).SingleOrDefault(); + } } } } diff --git a/Application/EdFi.Ods.Common/Providers/Criteria/TotalCountCriteriaProvider.cs b/Application/EdFi.Ods.Common/Providers/Criteria/TotalCountCriteriaProvider.cs deleted file mode 100644 index 0fe10b5008..0000000000 --- a/Application/EdFi.Ods.Common/Providers/Criteria/TotalCountCriteriaProvider.cs +++ /dev/null @@ -1,50 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -// Licensed to the Ed-Fi Alliance under one or more agreements. -// The Ed-Fi Alliance licenses this file to you under the Apache License, Version 2.0. -// See the LICENSE and NOTICES files in the project root for more information. - -using EdFi.Ods.Common.Caching; -using EdFi.Ods.Common.Descriptors; -using EdFi.Ods.Common.Models.Domain; -using EdFi.Ods.Common.Specifications; -using NHibernate; -using NHibernate.Criterion; - -namespace EdFi.Ods.Common.Providers.Criteria -{ - /// - /// Builds a query that retrieves the total count of resource items available to the current caller. - /// - /// The type of the entity to which criteria is being applied. - public class TotalCountCriteriaProvider : AggregateRootCriteriaProviderBase, ITotalCountCriteriaProvider - where TEntity : AggregateRootWithCompositeKey - { - public TotalCountCriteriaProvider( - ISessionFactory sessionFactory, - IDescriptorResolver descriptorResolver, - IPersonEntitySpecification personEntitySpecification, - IPersonTypesProvider personTypesProvider) - : base(sessionFactory, descriptorResolver, personEntitySpecification, personTypesProvider) { } - - /// - /// Get a query that retrieves the total count of resource items available to the current caller. - /// - /// An instance of the entity containing parameters to be added to the query. - /// The query parameters to be applied to the filtering. - /// The NHibernate instance representing the query. - public ICriteria GetCriteriaQuery(TEntity specification, IQueryParameters queryParameters) - { - var countQueryCriteria = Session - .CreateCriteria("aggregateRoot") - .SetProjection(Projections.CountDistinct(x => x.Id)); - - // Add specification-based criteria - ProcessSpecification(countQueryCriteria, specification); - - // Add special query fields - ProcessQueryParameters(countQueryCriteria, queryParameters); - - return countQueryCriteria; - } - } -} \ No newline at end of file diff --git a/Application/EdFi.Ods.Features/ChangeQueries/Providers/SnapshotOdsDatabaseConnectionStringProviderDecorator.cs b/Application/EdFi.Ods.Features/ChangeQueries/Providers/SnapshotOdsDatabaseConnectionStringProviderDecorator.cs index 71c2b85c91..fb7bfa9272 100644 --- a/Application/EdFi.Ods.Features/ChangeQueries/Providers/SnapshotOdsDatabaseConnectionStringProviderDecorator.cs +++ b/Application/EdFi.Ods.Features/ChangeQueries/Providers/SnapshotOdsDatabaseConnectionStringProviderDecorator.cs @@ -66,9 +66,11 @@ private string GetSnapshotConnectionString() if (!odsInstanceContext.ConnectionStringByDerivativeType.TryGetValue( DerivativeType.Snapshot, out string snapshotConnectionString)) { - throw new NotFoundException("Snapshots are not configured."); + throw new NotFoundException( + "Snapshot not found.", + ["The snapshot connection for the ODS has not been configured."]); } - + return snapshotConnectionString; } } diff --git a/Application/EdFi.Ods.Repositories.NHibernate.Tests/Modules/RepositoryFilterProvidersModule.cs b/Application/EdFi.Ods.Repositories.NHibernate.Tests/Modules/RepositoryFilterProvidersModule.cs index 2f28843314..cb61ab5e7e 100644 --- a/Application/EdFi.Ods.Repositories.NHibernate.Tests/Modules/RepositoryFilterProvidersModule.cs +++ b/Application/EdFi.Ods.Repositories.NHibernate.Tests/Modules/RepositoryFilterProvidersModule.cs @@ -15,10 +15,6 @@ protected override void Load(ContainerBuilder builder) builder.RegisterGeneric(typeof(PagedAggregateIdsCriteriaProvider<>)) .As(typeof(IPagedAggregateIdsCriteriaProvider<>)) .SingleInstance(); - - builder.RegisterGeneric(typeof(TotalCountCriteriaProvider<>)) - .As(typeof(ITotalCountCriteriaProvider<>)) - .SingleInstance(); } } -} \ No newline at end of file +} diff --git a/Application/EdFi.Ods.Tests/EdFi.Ods.Common/Database/Querying/QueryBuilderTests.cs b/Application/EdFi.Ods.Tests/EdFi.Ods.Common/Database/Querying/QueryBuilderTests.cs index a08dca932a..3b2325b6bd 100644 --- a/Application/EdFi.Ods.Tests/EdFi.Ods.Common/Database/Querying/QueryBuilderTests.cs +++ b/Application/EdFi.Ods.Tests/EdFi.Ods.Common/Database/Querying/QueryBuilderTests.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; +using System.Data; using System.Linq; using Microsoft.Data.SqlClient; using Dapper; @@ -39,6 +40,27 @@ public void Should_select_columns(DatabaseEngine databaseEngine) var clonedQueryResult = q.Clone().BuildTemplate(); template.RawSql.ShouldBe(clonedQueryResult.RawSql); } + + [TestCase(DatabaseEngine.MsSql)] + [TestCase(DatabaseEngine.PgSql)] + public void Should_select_distinct_columns(DatabaseEngine databaseEngine) + { + var q = new QueryBuilder(GetDialectFor(databaseEngine)) + .From("edfi.EducationOrganization") + .Select("NameOfInstitution", "WebSite") + .Distinct(); + + var template = q.BuildTemplate(); + + template.RawSql.NormalizeSql() + .ShouldBe("SELECT DISTINCT NameOfInstitution, WebSite FROM edfi.EducationOrganization"); + + ExecuteQueryAndWriteResults(databaseEngine, template); + + // Check the cloned query results + var clonedQueryResult = q.Clone().BuildTemplate(); + template.RawSql.ShouldBe(clonedQueryResult.RawSql); + } public class With_group_by { @@ -110,7 +132,8 @@ public void Should_apply_where_with_null(DatabaseEngine databaseEngine) .From("edfi.Student") .Select("FirstName", "LastSurname") .Where("BirthCity", "Chicago") - .WhereNull("MiddleName"); + .WhereNull("MiddleName") + .WhereRaw("1 = 0"); var template = q.BuildTemplate(); @@ -120,7 +143,8 @@ public void Should_apply_where_with_null(DatabaseEngine databaseEngine) SELECT FirstName, LastSurname FROM edfi.Student WHERE BirthCity = @p0 - AND MiddleName IS NULL".NormalizeSql())); + AND MiddleName IS NULL + AND 1 = 0".NormalizeSql())); ExecuteQueryAndWriteResults(databaseEngine, template); @@ -129,6 +153,37 @@ FROM edfi.Student template.RawSql.ShouldBe(clonedQueryResult.RawSql); } + [TestCase(DatabaseEngine.MsSql)] + [TestCase(DatabaseEngine.PgSql)] + public void Should_apply_where_with_nested_conditions_wrapped_in_parenthesis(DatabaseEngine databaseEngine) + { + var q = new QueryBuilder(GetDialectFor(databaseEngine)) + .From("edfi.Student") + .Select("FirstName", "LastSurname") + .OrWhere(qb => qb + .Where("BirthCity", "Chicago") + .WhereNull("MiddleName") + ) + .OrWhere("Name", "Bob") + .OrWhere("Age", ">", 42); + + var template = q.BuildTemplate(); + + template.ShouldSatisfyAllConditions( + () => template.RawSql.NormalizeSql() + .ShouldBe(@" + SELECT FirstName, LastSurname + FROM edfi.Student + WHERE ((BirthCity = @p0 + AND MiddleName IS NULL) OR Name = @p1 OR Age > @p2)".NormalizeSql())); + + ExecuteQueryAndWriteResults(databaseEngine, template); + + // Check the cloned query results + var clonedQueryResult = q.Clone().BuildTemplate(); + template.RawSql.ShouldBe(clonedQueryResult.RawSql); + } + [TestCase(DatabaseEngine.MsSql)] [TestCase(DatabaseEngine.PgSql)] public void Should_apply_where_with_not_null(DatabaseEngine databaseEngine) @@ -314,6 +369,34 @@ FROM edfi.StudentSchoolAssociation template.RawSql.ShouldBe(clonedQueryResult.RawSql); } + [TestCase(DatabaseEngine.MsSql, "(SELECT Id FROM @p1)")] + [TestCase(DatabaseEngine.PgSql, "(VALUES (@p1_0), (@p1_1), (@p1_2), (@p1_3))")] + public void Should_apply_OR_where_with_IN(DatabaseEngine databaseEngine, string expectedInClause) + { + var q = new QueryBuilder(GetDialectFor(databaseEngine)) + .From("edfi.StudentSchoolAssociation") + .Select("StudentUSI", "SchoolId", "EntryDate") + .OrWhere("SchoolId", 1234) + .OrWhereIn("StudentUSI", new [] { 12, 34, 56, 78 }); + + var template = q.BuildTemplate(); + + var actualParameters = template.Parameters as DynamicParameters; + + actualParameters.ShouldSatisfyAllConditions( + () => template.RawSql.NormalizeSql().ShouldBe(@$" + SELECT StudentUSI, SchoolId, EntryDate + FROM edfi.StudentSchoolAssociation + WHERE (SchoolId = @p0 OR StudentUSI IN {expectedInClause})".NormalizeSql()) + ); + + ExecuteQueryAndWriteResults(databaseEngine, template); + + // Check the cloned query results + var clonedQueryResult = q.Clone().BuildTemplate(); + template.RawSql.ShouldBe(clonedQueryResult.RawSql); + } + [TestCase(DatabaseEngine.MsSql)] [TestCase(DatabaseEngine.PgSql)] public void Should_apply_where_with_explicit_comparison_operators(DatabaseEngine databaseEngine) @@ -323,7 +406,9 @@ public void Should_apply_where_with_explicit_comparison_operators(DatabaseEngine .Select("FirstName", "LastSurname") .Select("ChangeVersion") .Where("ChangeVersion", ">", 10000) - .Where("ChangeVersion", "<", 11000); + .Where("ChangeVersion", "<", 11000) + .OrWhere("ChangeVersion", "<", 0) + .OrWhere("ChangeVersion", ">", 2000000); var template = q.BuildTemplate(); @@ -334,7 +419,8 @@ public void Should_apply_where_with_explicit_comparison_operators(DatabaseEngine SELECT FirstName, LastSurname, ChangeVersion FROM edfi.Student WHERE ChangeVersion > @p0 - AND ChangeVersion < @p1".NormalizeSql()), + AND ChangeVersion < @p1 + AND (ChangeVersion < @p2 OR ChangeVersion > @p3)".NormalizeSql()), () => actualParameters.ShouldNotBeNull(), () => actualParameters.ParameterNames.ShouldContain("p0"), () => actualParameters.Get("@p0").ShouldBe(10000), @@ -356,9 +442,12 @@ public void Should_apply_nested_conjunction_combined_with_a_disjunction(Database var q = new QueryBuilder(GetDialectFor(databaseEngine)) .From("edfi.Student") .Select("FirstName", "LastSurname", "ChangeVersion") - .Where( - q => q.OrWhere(q2 => q2.Where("ChangeVersion", ">=", 10000).Where("ChangeVersion", "<=", 11000)) - .OrWhereNull("ChangeVersion")); + .Where(q => + q.OrWhere(q2 => + q2.Where("ChangeVersion", ">=", 10000) + .Where("ChangeVersion", "<=", 11000) + ) + .OrWhereNull("ChangeVersion")); var template = q.BuildTemplate(); @@ -368,8 +457,8 @@ public void Should_apply_nested_conjunction_combined_with_a_disjunction(Database () => template.RawSql.NormalizeSql().ShouldBe(@" SELECT FirstName, LastSurname, ChangeVersion FROM edfi.Student - WHERE (((ChangeVersion >= @p0 AND ChangeVersion <= @p1) - OR ChangeVersion IS NULL))".NormalizeSql()), + WHERE ((ChangeVersion >= @p0 AND ChangeVersion <= @p1) + OR ChangeVersion IS NULL)".NormalizeSql()), () => actualParameters.ShouldNotBeNull(), () => actualParameters.ParameterNames.ShouldContain("p0"), () => actualParameters.Get("@p0").ShouldBe(10000), @@ -463,8 +552,8 @@ public void Should_apply_nested_conjunction_combined_with_a_disjunction_with_aut () => template.RawSql.NormalizeSql().ShouldBe(@" SELECT FirstName, LastSurname, ChangeVersion FROM edfi.Student - WHERE (((ChangeVersion >= @p0 AND ChangeVersion <= @p1) - OR ChangeVersion IS NULL))".NormalizeSql()), + WHERE ((ChangeVersion >= @p0 AND ChangeVersion <= @p1) + OR ChangeVersion IS NULL)".NormalizeSql()), () => actualParameters.ShouldNotBeNull(), () => actualParameters.ParameterNames.ShouldContain("p0"), () => actualParameters.Get("@p0").ShouldBe(10000), @@ -486,10 +575,11 @@ public void Should_apply_nested_conjunction_combined_with_a_disjunction_with_exp var q = new QueryBuilder(GetDialectFor(databaseEngine)) .From("edfi.Student") .Select("FirstName", "LastSurname", "ChangeVersion") - .Where( - q => q.OrWhere(q2 => - q2.Where("ChangeVersion", ">=", new Parameter("@MinChangeVersion", 10000)) - .Where("ChangeVersion", "<=", new Parameter( "@MaxChangeVersion", 11000))) + .Where(q => q + .OrWhere(q2 => q2 + .Where("ChangeVersion", ">=", new Parameter("@MinChangeVersion", 10000)) + .Where("ChangeVersion", "<=", new Parameter( "@MaxChangeVersion", 11000)) + ) .OrWhereNull("ChangeVersion")); var template = q.BuildTemplate(); @@ -500,8 +590,8 @@ public void Should_apply_nested_conjunction_combined_with_a_disjunction_with_exp () => template.RawSql.NormalizeSql().ShouldBe(@" SELECT FirstName, LastSurname, ChangeVersion FROM edfi.Student - WHERE (((ChangeVersion >= @MinChangeVersion AND ChangeVersion <= @MaxChangeVersion) - OR ChangeVersion IS NULL))".NormalizeSql()), + WHERE ((ChangeVersion >= @MinChangeVersion AND ChangeVersion <= @MaxChangeVersion) + OR ChangeVersion IS NULL)".NormalizeSql()), () => actualParameters.ShouldNotBeNull(), () => actualParameters.ParameterNames.ShouldContain("MinChangeVersion"), () => actualParameters.Get("@MinChangeVersion").ShouldBe(10000), @@ -525,7 +615,8 @@ public void Should_apply_nested_disjunction_introducing_joins_combined_by_a_conj .From("edfi.Student AS s") .Select("FirstName", "LastSurname", "ChangeVersion") .Where( - q => q.OrWhere( // <-- OrWhere with nested builder is the behavior being tested + q => q + .OrWhere( // <-- OrWhere with nested builder is the behavior being tested q2 => q2 .Join("auth.EducationOrganizationIdToStudentUSI AS rba0", "s.StudentUSI", "rba0.StudentUSI") .WhereIn($"rba0.SourceEducationOrganizationId", new[] {255901001, 255901044})) @@ -546,8 +637,8 @@ INNER JOIN auth.EducationOrganizationIdToStudentUSI AS rba0 ON s.StudentUSI = rba0.StudentUSI INNER JOIN auth.EducationOrganizationIdToStudentUSIThroughResponsibility AS rba1 ON s.StudentUSI = rba1.StudentUSI - WHERE (((rba0.SourceEducationOrganizationId IN {expectedInClause1}) - OR (rba1.SourceEducationOrganizationId IN {expectedInClause2}))) + WHERE ((rba0.SourceEducationOrganizationId IN {expectedInClause1}) + OR (rba1.SourceEducationOrganizationId IN {expectedInClause2})) ".NormalizeSql()), () => actualParameters.ShouldNotBeNull(), () => actualParameters.ParameterNames.ShouldBe(expectedParameterNames) @@ -568,15 +659,17 @@ public void Should_apply_nested_conjunction_introducing_joins_combined_by_a_conj var q = new QueryBuilder(GetDialectFor(databaseEngine)) .From("edfi.Student AS s") .Select("FirstName", "LastSurname", "ChangeVersion") - .Where( - q => q.Where( // <-- Where with nested builder is the behavior being tested - q2 => q2 - .Join("auth.EducationOrganizationIdToStudentUSI AS rba0", "s.StudentUSI", "rba0.StudentUSI") - .WhereIn($"rba0.SourceEducationOrganizationId", new[] {255901001, 255901044})) - .Where( // <-- Where with nested builder is the behavior being tested - q2 => q2 - .Join("auth.EducationOrganizationIdToStudentUSIThroughResponsibility AS rba1", "s.StudentUSI", "rba1.StudentUSI" ) - .WhereIn("rba1.SourceEducationOrganizationId", new[] {255901001, 255901044}))); + .Where(q => q + .Where( // <-- Where with nested builder is the behavior being tested + q2 => q2 + .Join("auth.EducationOrganizationIdToStudentUSI AS rba0", "s.StudentUSI", "rba0.StudentUSI") + .WhereIn($"rba0.SourceEducationOrganizationId", new[] {255901001, 255901044}) + ) + .Where( // <-- Where with nested builder is the behavior being tested + q2 => q2 + .Join("auth.EducationOrganizationIdToStudentUSIThroughResponsibility AS rba1", "s.StudentUSI", "rba1.StudentUSI" ) + .WhereIn("rba1.SourceEducationOrganizationId", new[] {255901001, 255901044})) + ); var template = q.BuildTemplate(); @@ -590,8 +683,8 @@ INNER JOIN auth.EducationOrganizationIdToStudentUSI AS rba0 ON s.StudentUSI = rba0.StudentUSI INNER JOIN auth.EducationOrganizationIdToStudentUSIThroughResponsibility AS rba1 ON s.StudentUSI = rba1.StudentUSI - WHERE ((rba0.SourceEducationOrganizationId IN {expectedInClause1}) - AND (rba1.SourceEducationOrganizationId IN {expectedInClause2})) + WHERE rba0.SourceEducationOrganizationId IN {expectedInClause1} + AND rba1.SourceEducationOrganizationId IN {expectedInClause2} ".NormalizeSql()), () => actualParameters.ShouldNotBeNull(), () => actualParameters.ParameterNames.ShouldBe(expectedParameterNames) @@ -603,6 +696,40 @@ INNER JOIN auth.EducationOrganizationIdToStudentUSIThroughResponsibility AS rba1 var clonedQueryResult = q.Clone().BuildTemplate(); template.RawSql.ShouldBe(clonedQueryResult.RawSql); } + + [TestCase(DatabaseEngine.MsSql)] + [TestCase(DatabaseEngine.PgSql)] + public void Should_apply_where_with_explicit_parameter_type(DatabaseEngine databaseEngine) + { + DynamicParameters parameter = new(); + parameter.Add($"@Cost", Convert.ToDecimal(100000), DbType.Currency); + + var q = new QueryBuilder(GetDialectFor(databaseEngine)) + .From("edfi.BusRoute") + .Select("RouteNumber", "RouteName") + .Select("OperationalCost") + .Where("OperationalCost", ">", parameter); + + var template = q.BuildTemplate(); + + var actualParameters = template.Parameters as DynamicParameters; + + actualParameters.ShouldSatisfyAllConditions( + () => template.RawSql.NormalizeSql().ShouldBe(@" + SELECT RouteNumber, RouteName, OperationalCost + FROM edfi.BusRoute + WHERE OperationalCost > @Cost".NormalizeSql()), + () => actualParameters.ShouldNotBeNull(), + () => actualParameters.ParameterNames.ShouldContain("Cost"), + () => actualParameters.Get("Cost").ShouldBe(100000) + ); + + ExecuteQueryAndWriteResults(databaseEngine, template); + + // Check the cloned query results + var clonedQueryResult = q.Clone().BuildTemplate(); + template.RawSql.ShouldBe(clonedQueryResult.RawSql); + } } public class With_paging @@ -662,6 +789,36 @@ ORDER BY s.LastSurname template.RawSql.ShouldBe(clonedQueryResult.RawSql); } + [TestCase(DatabaseEngine.MsSql, "OFFSET 25 ROWS FETCH NEXT 5 ROWS ONLY")] + [TestCase(DatabaseEngine.PgSql, "LIMIT 5 OFFSET 25")] + public void Should_apply_single_column_left_joins(DatabaseEngine databaseEngine, string pagingSql) + { + var q = new QueryBuilder(GetDialectFor(databaseEngine)) + .From("edfi.StudentSchoolAssociation AS ssa") + .Select("s.FirstName", "s.LastSurname", "edOrg.NameOfInstitution", "ssa.EntryDate") + .LeftJoin("edfi.Student s", "ssa.StudentUSI", "s.StudentUSI") + .LeftJoin("edfi.EducationOrganization edOrg", "ssa.SchoolId", "edOrg.EducationOrganizationId") + .OrderBy("s.LastSurname") + .LimitOffset(5, 25); + + var template = q.BuildTemplate(); + + template.RawSql.NormalizeSql() + .ShouldBe(@$" + SELECT s.FirstName, s.LastSurname, edOrg.NameOfInstitution, ssa.EntryDate + FROM edfi.StudentSchoolAssociation AS ssa + LEFT JOIN edfi.Student s ON ssa.StudentUSI = s.StudentUSI + LEFT JOIN edfi.EducationOrganization edOrg ON ssa.SchoolId = edOrg.EducationOrganizationId + ORDER BY s.LastSurname + {pagingSql}".NormalizeSql()); + + ExecuteQueryAndWriteResults(databaseEngine, template); + + // Check the cloned query results + var clonedQueryResult = q.Clone().BuildTemplate(); + template.RawSql.ShouldBe(clonedQueryResult.RawSql); + } + [TestCase(DatabaseEngine.MsSql)] [TestCase(DatabaseEngine.PgSql)] public void Should_apply_multiple_column_left_join(DatabaseEngine databaseEngine) @@ -693,6 +850,40 @@ LEFT JOIN edfi.CalendarDate src var clonedQueryResult = q.Clone().BuildTemplate(); template.RawSql.ShouldBe(clonedQueryResult.RawSql); } + + [TestCase(DatabaseEngine.MsSql, JoinType.InnerJoin, "INNER")] + [TestCase(DatabaseEngine.MsSql, JoinType.LeftOuterJoin, "LEFT")] + [TestCase(DatabaseEngine.PgSql, JoinType.InnerJoin, "INNER")] + [TestCase(DatabaseEngine.PgSql, JoinType.LeftOuterJoin, "LEFT")] + public void Should_apply_multiple_column_join_using_enumerated_join_type(DatabaseEngine databaseEngine, JoinType joinType, string expectedJoinModifier) + { + var q = new QueryBuilder(GetDialectFor(databaseEngine)) + .From("tracked_changes_edfi.CalendarDate AS c") + .Select("c.Id", "c.ChangeVersion") + .Join("edfi.CalendarDate src", joiner => + joiner.On("c.OldCalendarCode", "src.CalendarCode") + .On("c.OldDate", "src.Date") + .On("c.OldSchoolId", "src.SchoolId") + .On("c.OldSchoolYear", "src.SchoolYear"), joinType); + + var template = q.BuildTemplate(); + + template.RawSql.NormalizeSql() + .ShouldBe(@$" + SELECT c.Id, c.ChangeVersion + FROM tracked_changes_edfi.CalendarDate AS c + {expectedJoinModifier} JOIN edfi.CalendarDate src + ON c.OldCalendarCode = src.CalendarCode + AND c.OldDate = src.Date + AND c.OldSchoolId = src.SchoolId + AND c.OldSchoolYear = src.SchoolYear".NormalizeSql()); + + ExecuteQueryAndWriteResults(databaseEngine, template); + + // Check the cloned query results + var clonedQueryResult = q.Clone().BuildTemplate(); + template.RawSql.ShouldBe(clonedQueryResult.RawSql); + } } public class With_CTEs diff --git a/Postman Test Suite/Ed-Fi ODS-API Integration Test Suite ResponseTests.postman_collection.json b/Postman Test Suite/Ed-Fi ODS-API Integration Test Suite ResponseTests.postman_collection.json index 5ecbeb6816..7e1dcf8e2d 100644 --- a/Postman Test Suite/Ed-Fi ODS-API Integration Test Suite ResponseTests.postman_collection.json +++ b/Postman Test Suite/Ed-Fi ODS-API Integration Test Suite ResponseTests.postman_collection.json @@ -1,6 +1,6 @@ { "info": { - "_postman_id": "aecc2c8a-5e04-4d78-9855-a49424bd1e3e", + "_postman_id": "d3d5da8b-42ff-4fcb-bb76-d215b5cebd98", "name": "Ed-Fi ODS/API Integration Test Suite ResponseTests", "description": "Localhost integration testing using Postman Scripts", "schema": "https://schema.getpostman.com/json/collection/v2.1.0/collection.json", @@ -1482,9 +1482,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -1543,9 +1542,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -1620,9 +1618,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -1681,9 +1678,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -1757,9 +1753,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -1818,9 +1813,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -1879,9 +1873,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "url": { @@ -2713,8 +2706,7 @@ "value": "invalid" } ] - }, - "description": "" + } }, "response": [] }, @@ -2782,8 +2774,7 @@ "value": "invalid" } ] - }, - "description": "" + } }, "response": [] } @@ -2847,9 +2838,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -2908,9 +2898,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -3058,9 +3047,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -3119,9 +3107,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -3399,14 +3386,17 @@ " pm.expect(pm.response.code).to.equal(404);", "});", "", - "pm.test(\"Should return a message indicating that the snapshot could not be found.\", () => {", - " const problemDetails = pm.response.json();", + "const problemDetails = pm.response.json();", "", + "pm.test(\"Should return a message indicating that the snapshot could not be found.\", () => {", " pm.expect(pm.response.code).equal(problemDetails.status);", " pm.expect(problemDetails.type).to.equal(\"urn:ed-fi:api:not-found\");", " pm.expect(problemDetails.detail).to.equal(\"Snapshot not found.\");", " pm.expect(problemDetails.title).to.equal(\"Not Found\");", + "});", "", + "pm.test(\"Should provide an error indicating that the snapshot connection has not been configured.\", () => {", + " pm.expect(problemDetails.errors).to.match(/The snapshot connection for the ODS has not been configured./);", "});" ], "type": "text/javascript", @@ -3422,8 +3412,7 @@ "header": [ { "key": "Use-Snapshot", - "value": "true", - "uuid": "272fc061-d817-41f8-b7a6-50409377a517" + "value": "true" } ], "body": { @@ -3505,9 +3494,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -3566,9 +3554,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -3685,9 +3672,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" }, { "key": "If-None-Match", @@ -3789,14 +3775,19 @@ " pm.expect(pm.response.code).to.equal(404);\r", "});\r", "\r", - "pm.test(\"Should return a message indicating that the specified data could not be found.\", () => {\r", - " const problemDetails = pm.response.json();\r", + "const problemDetails = pm.response.json();\r", "\r", + "pm.test(\"Should return a message indicating that the snapshot could not be found.\", () => {\r", " pm.expect(pm.response.code).equal(problemDetails.status);\r", " pm.expect(problemDetails.type).to.equal(\"urn:ed-fi:api:not-found\");\r", - " pm.expect(problemDetails.detail).to.equal(\"Snapshots are not configured.\");\r", + " pm.expect(problemDetails.detail).to.equal(\"Snapshot not found.\");\r", + " pm.expect(problemDetails.title).to.equal(\"Not Found\");\r", + "});\r", "\r", - "});" + "pm.test(\"Should provide an error indicating that the snapshot connection has not been configured.\", () => {\r", + " pm.expect(problemDetails.errors).to.match(/The snapshot connection for the ODS has not been configured./);\r", + "});\r", + "" ], "type": "text/javascript", "packages": {} @@ -3848,13 +3839,17 @@ " pm.expect(pm.response.code).to.equal(404);\r", "});\r", "\r", - "pm.test(\"Should return a message indicating that the specified data could not be found.\", () => {\r", - " const problemDetails = pm.response.json();\r", + "const problemDetails = pm.response.json();\r", "\r", + "pm.test(\"Should return a message indicating that the snapshot could not be found.\", () => {\r", " pm.expect(pm.response.code).equal(problemDetails.status);\r", " pm.expect(problemDetails.type).to.equal(\"urn:ed-fi:api:not-found\");\r", - " pm.expect(problemDetails.detail).to.equal(\"Snapshots are not configured.\");\r", + " pm.expect(problemDetails.detail).to.equal(\"Snapshot not found.\");\r", + " pm.expect(problemDetails.title).to.equal(\"Not Found\");\r", + "});\r", "\r", + "pm.test(\"Should provide an error indicating that the snapshot connection has not been configured.\", () => {\r", + " pm.expect(problemDetails.errors).to.match(/The snapshot connection for the ODS has not been configured./);\r", "});" ], "type": "text/javascript", @@ -4115,9 +4110,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -4176,9 +4170,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -4352,9 +4345,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -4442,9 +4434,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -4526,9 +4517,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -4600,9 +4590,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -4692,9 +4681,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -4754,9 +4742,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -4873,9 +4860,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -5017,9 +5003,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -5085,9 +5070,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -5148,9 +5132,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -5203,9 +5186,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -5267,9 +5249,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -5322,9 +5303,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -5385,9 +5365,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -5440,9 +5419,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -5504,9 +5482,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -5596,7 +5573,6 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", "value": "application/json", "type": "text" } @@ -5705,7 +5681,6 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", "value": "application/json", "type": "text" } @@ -5801,7 +5776,6 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", "value": "application/json", "type": "text" } @@ -5867,9 +5841,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "url": { @@ -6017,9 +5990,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -6082,13 +6054,11 @@ "header": [ { "key": "Content-Type", - "value": "application/json", - "uuid": "0eac8b7f-cc98-4c71-b47a-010cb1a4d0a0" + "value": "application/json" }, { "key": "Use-Snapshot", - "value": "true", - "uuid": "5b0fbcb3-766b-4f99-98f4-81936bae82f7" + "value": "true" } ], "body": { @@ -6803,9 +6773,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -6864,9 +6833,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -6985,9 +6953,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -7072,9 +7039,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -7133,9 +7099,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -7259,9 +7224,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -7346,9 +7310,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -7407,9 +7370,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -7527,9 +7489,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" }, { "key": "If-Match", @@ -7619,9 +7580,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -7680,9 +7640,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -7801,9 +7760,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" }, { "key": "If-Match", @@ -7893,9 +7851,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -7954,9 +7911,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -8076,9 +8032,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" }, { "key": "If-Match", @@ -8168,9 +8123,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -8229,9 +8183,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -8345,9 +8298,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -8432,9 +8384,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -8493,9 +8444,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -8614,9 +8564,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -8681,9 +8630,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "url": { @@ -8744,9 +8692,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "url": { @@ -8857,13 +8804,11 @@ "header": [ { "key": "Content-Type", - "value": "application/json", - "uuid": "6266b300-ec98-48b6-8d75-6df6f14d87df" + "value": "application/json" }, { "key": "Use-Snapshot", - "value": "true", - "uuid": "6e50d725-6315-4b13-93ec-cb4e80845091" + "value": "true" } ], "body": { @@ -8955,7 +8900,6 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", "value": "application/json", "type": "text" } @@ -9067,9 +9011,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -9125,9 +9068,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -9160,7 +9102,7 @@ "pm.environment.set('supplied:stateEducationAgencyId', pm.variables.replaceIn(\"{{$randomInt}}{{$randomInt}}\"));" ], "type": "text/javascript" - } + } }, { "listen": "test", @@ -9199,8 +9141,8 @@ "v3", "ed-fi", "stateEducationAgencies" - ] - }, + ] + }, "description": "This api post method adds new academicWeeks for particular school .\nThis test method will throw WeekIdentifier is required error when WeekIdentifier is not passed" }, "response": [] @@ -9509,9 +9451,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -9824,9 +9765,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -9878,9 +9818,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" }, { "key": "If-Match", @@ -9968,9 +9907,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -10029,9 +9967,8 @@ "header": [ { "key": "Content-Type", - "name": "Content-Type", - "type": "text", - "value": "application/json" + "value": "application/json", + "type": "text" } ], "body": { @@ -10210,13 +10147,11 @@ "header": [ { "key": "Content-Type", - "value": "application/json", - "uuid": "d75d4f07-9576-4838-8a6e-e91d079d2fc1" + "value": "application/json" }, { "key": "Use-Snapshot", - "value": "true", - "uuid": "2d5e379c-0e2d-4073-904c-5e8dbd148b78" + "value": "true" } ], "body": {