Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ODS-6512] Modify batched page queries in NHibernate to use AggregateId instead of Id for page-level inclusion criteria #1165

Merged
merged 4 commits into from
Oct 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,10 @@ protected override void Load(ContainerBuilder builder)
.As(typeof(IGetEntitiesByIds<>))
.SingleInstance();

builder.RegisterGeneric(typeof(GetEntitiesByAggregateIds<>))
.As(typeof(IGetEntitiesByAggregateIds<>))
.SingleInstance();

builder.RegisterGeneric(typeof(GetEntitiesBySpecification<>))
.As(typeof(IGetEntitiesBySpecification<>))
.WithAttributeFiltering()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// 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.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;
using EdFi.Ods.Common;
using EdFi.Ods.Common.Repositories;

namespace EdFi.Ods.Api.Security.Authorization.Repositories;

/// <summary>
/// Authorizes calls to the "GetByAggregateIds" repository method.
/// </summary>
/// <typeparam name="T">The Type of entity being queried.</typeparam>
public class GetEntitiesByAggregateIdsAuthorizationDecorator<T> : IGetEntitiesByAggregateIds<T>
where T : class, IHasIdentifier, IDateVersionedEntity
{
private readonly IGetEntitiesByAggregateIds<T> _next;

/// <summary>
/// Initializes a new instance of the <see cref="GetEntitiesByAggregateIdsAuthorizationDecorator{T}"/> class.
/// </summary>
/// <param name="next">The decorated instance for which authorization is being performed.</param>
public GetEntitiesByAggregateIdsAuthorizationDecorator(IGetEntitiesByAggregateIds<T> next)
{
_next = next;
}

/// <summary>
/// Authorizes a call to get multiple records by their record identifiers.
/// </summary>
/// <param name="aggregateIds">The values of the record identifiers to be retrieved.</param>
/// <returns>The specified entity if found; otherwise null.</returns>
public async Task<IList<T>> GetByAggregateIdsAsync(IList<int> aggregateIds, CancellationToken cancellationToken)
{
return await _next.GetByAggregateIdsAsync(aggregateIds, cancellationToken);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,40 +27,41 @@ public class SecurityPersistenceModule : Module
private readonly IDictionary<Type, Type> _genericServiceByAuthorizationDecorator = new Dictionary<Type, Type>
{
// NHibernate authorization decorators
{typeof(IGetEntityByKey<>), typeof(GetEntityByKeyAuthorizationDecorator<>)},
{typeof(IGetEntitiesBySpecification<>), typeof(GetEntitiesBySpecificationAuthorizationDecorator<>)},
{typeof(IGetEntityById<>), typeof(GetEntityByIdAuthorizationDecorator<>)},
{typeof(IGetEntitiesByIds<>), typeof(GetEntitiesByIdsAuthorizationDecorator<>)},
{typeof(ICreateEntity<>), typeof(CreateEntityAuthorizationDecorator<>)},
{typeof(IDeleteEntityById<>), typeof(DeleteEntityByIdAuthorizationDecorator<>)},
{typeof(IUpdateEntity<>), typeof(UpdateEntityAuthorizationDecorator<>)},
{typeof(IUpsertEntity<>), typeof(UpsertEntityAuthorizationDecorator<>)},
{typeof(GetEntityByKeyAuthorizationDecorator<>), typeof(IGetEntityByKey<>)},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Key/values were inverted in the dictionary here because the key should be the implementation, not the common interface (wasn't accounting for the possibility of multiple decorators implementing the same interface).

{typeof(GetEntitiesBySpecificationAuthorizationDecorator<>), typeof(IGetEntitiesBySpecification<>)},
{typeof(GetEntityByIdAuthorizationDecorator<>), typeof(IGetEntityById<>)},
{typeof(GetEntitiesByIdsAuthorizationDecorator<>), typeof(IGetEntitiesByIds<>)},
{typeof(GetEntitiesByAggregateIdsAuthorizationDecorator<>), typeof(IGetEntitiesByAggregateIds<>)},
{typeof(CreateEntityAuthorizationDecorator<>), typeof(ICreateEntity<>)},
{typeof(DeleteEntityByIdAuthorizationDecorator<>), typeof(IDeleteEntityById<>)},
{typeof(UpdateEntityAuthorizationDecorator<>), typeof(IUpdateEntity<>)},
{typeof(UpsertEntityAuthorizationDecorator<>), typeof(IUpsertEntity<>)},
};

private readonly IDictionary<Type, Type> _serviceByAuthorizationDecorator = new Dictionary<Type, Type>
{
// pipeline steps authorization decorators
{typeof(IGetPipelineStepsProvider), typeof(AuthorizationContextGetPipelineStepsProviderDecorator)},
{typeof(AuthorizationContextGetPipelineStepsProviderDecorator), typeof(IGetPipelineStepsProvider)},
{
typeof(IGetBySpecificationPipelineStepsProvider),
typeof(AuthorizationContextGetBySpecificationPipelineStepsProviderDecorator)
typeof(AuthorizationContextGetBySpecificationPipelineStepsProviderDecorator),
typeof(IGetBySpecificationPipelineStepsProvider)
},
{typeof(IUpsertPipelineStepsProvider), typeof(AuthorizationContextUpsertPipelineStepsProviderDecorator)},
{typeof(IDeletePipelineStepsProvider), typeof(AuthorizationContextDeletePipelineStepsProviderDecorator)},
{typeof(AuthorizationContextUpsertPipelineStepsProviderDecorator), typeof(IUpsertPipelineStepsProvider)},
{typeof(AuthorizationContextDeletePipelineStepsProviderDecorator), typeof(IDeletePipelineStepsProvider)},

{typeof(IAggregateRootQueryBuilderProvider), typeof(AggregateRootQueryBuilderProviderAuthorizationDecorator)},
{typeof(AggregateRootQueryBuilderProviderAuthorizationDecorator), typeof(IAggregateRootQueryBuilderProvider)},
};

protected override void Load(ContainerBuilder builder)
{
foreach (var decoratorRegistration in _genericServiceByAuthorizationDecorator)
{
builder.RegisterGenericDecorator(decoratorRegistration.Value, decoratorRegistration.Key);
builder.RegisterGenericDecorator(decoratorRegistration.Key, decoratorRegistration.Value);
}

foreach (var decoratorRegistration in _serviceByAuthorizationDecorator)
{
builder.RegisterDecorator(decoratorRegistration.Value, decoratorRegistration.Key);
builder.RegisterDecorator(decoratorRegistration.Key, decoratorRegistration.Value);
}

builder.RegisterType<ClientAppRepo>()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// 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.Collections.Generic;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using EdFi.Common;
using EdFi.Ods.Common.Context;
using EdFi.Ods.Common.Infrastructure.Activities;
using EdFi.Ods.Common.Models;
using EdFi.Ods.Common.Models.Domain;
using EdFi.Ods.Common.Repositories;
using EdFi.Ods.Common.Security.Claims;
using NHibernate;

namespace EdFi.Ods.Common.Infrastructure.Repositories;

public class GetEntitiesByAggregateIds<TEntity> : GetEntitiesBase<TEntity>, IGetEntitiesByAggregateIds<TEntity>
where TEntity : DomainObjectBase, IHasIdentifier, IDateVersionedEntity
{
private readonly IParameterListSetter _parameterListSetter;

public GetEntitiesByAggregateIds(
ISessionFactory sessionFactory,
IDomainModelProvider domainModelProvider,
IParameterListSetter parameterListSetter,
IContextProvider<DataManagementResourceContext> dataManagementResourceContextProvider)
: base(sessionFactory, domainModelProvider, dataManagementResourceContextProvider)
{
_parameterListSetter = Preconditions.ThrowIfNull(parameterListSetter, nameof(parameterListSetter));
}

public async Task<IList<TEntity>> GetByAggregateIdsAsync(IList<int> aggregateIds, CancellationToken cancellationToken)
{
using (new SessionScope(SessionFactory))
{
IEnumerable<TEntity> results;

if (aggregateIds.Count == 1)
{
results = await GetAggregateResultsAsync(
"where a.AggregateId = :id",
q => q.SetParameter("id", aggregateIds[0]), cancellationToken);
}
else
{
results = await GetAggregateResultsAsync(
"where a.AggregateId IN (:ids)",
q => _parameterListSetter.SetParameterList(q ,"ids", aggregateIds),
cancellationToken,
"order by a.AggregateId");
}

// Process multiple results in the first-level cache to a list of complete aggregates
return results.ToList();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,17 @@ public class GetEntitiesBySpecification<TEntity>

private readonly IAggregateRootQueryBuilderProvider _pagedAggregateIdsCriteriaProvider;
private readonly IDomainModelProvider _domainModelProvider;
private readonly IGetEntitiesByIds<TEntity> _getEntitiesByIds;
private readonly IGetEntitiesByAggregateIds<TEntity> _getEntitiesByAggregateIds;

public GetEntitiesBySpecification(
ISessionFactory sessionFactory,
IGetEntitiesByIds<TEntity> getEntitiesByIds,
IGetEntitiesByAggregateIds<TEntity> getEntitiesByAggregateIds,
[FromKeyedServices(PagedAggregateIdsQueryBuilderProvider.RegistrationKey)]
IAggregateRootQueryBuilderProvider pagedAggregateIdsCriteriaProvider,
IDomainModelProvider domainModelProvider)
: base(sessionFactory)
{
_getEntitiesByIds = getEntitiesByIds;
_getEntitiesByAggregateIds = getEntitiesByAggregateIds;
_pagedAggregateIdsCriteriaProvider = pagedAggregateIdsCriteriaProvider;
_domainModelProvider = domainModelProvider;
}
Expand Down Expand Up @@ -80,14 +80,9 @@ public async Task<GetBySpecificationResult<TEntity>> GetBySpecificationAsync(
}

// Get the full results
var ids = specificationResult.Ids.Select(x => x.Id).ToList();
var aggregateIds = specificationResult.Ids.Select(x => x.AggregateId).ToList();

var result = await _getEntitiesByIds.GetByIdsAsync(ids, cancellationToken);

// Restore original order of the result rows (GetByIds sorts by Id)
var resultWithOriginalOrder = ids
.Join(result, id => id, r => r.Id, (id, r) => r)
.ToList();
var result = await _getEntitiesByAggregateIds.GetByAggregateIdsAsync(aggregateIds, cancellationToken);

string nextPageToken = null;

Expand All @@ -100,7 +95,7 @@ public async Task<GetBySpecificationResult<TEntity>> GetBySpecificationAsync(

return new GetBySpecificationResult<TEntity>
{
Results = resultWithOriginalOrder,
Results = result,
ResultMetadata = new ResultMetadata
{
TotalCount = specificationResult.TotalCount,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ private QueryBuilder GetQueryBuilder(Entity aggregateRootEntity, PagingParameter

idQueryBuilder
.From(schemaTableName.Alias("r"))
.Select($"{rootTableAlias}.Id")
.Select($"{rootTableAlias}.AggregateId");

// NOTE: Optimization opportunity - th ederived entity may not be needed unless there is criteria to be applied that uses the derived type.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// 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.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;

namespace EdFi.Ods.Common.Repositories;

/// <summary>
/// Defines a method for retrieving a list of entities by their record identifiers.
/// </summary>
/// <typeparam name="TEntity">The Type of the entities to be retrieved.</typeparam>
public interface IGetEntitiesByAggregateIds<TEntity>
where TEntity : IHasIdentifier, IDateVersionedEntity
{
/// <summary>
/// Get a list of entities by their record identifiers.
/// </summary>
/// <param name="aggregateIds">The list of aggregate identifiers.</param>
/// <returns>The list of matching entities.</returns>
Task<IList<TEntity>> GetByAggregateIdsAsync(IList<int> aggregateIds, CancellationToken cancellationToken);
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ protected override void Load(ContainerBuilder builder)
.As(typeof(IGetEntitiesByIds<>))
.SingleInstance();

builder.RegisterGeneric(typeof(GetEntitiesByAggregateIds<>))
.As(typeof(IGetEntitiesByAggregateIds<>))
.SingleInstance();

builder.RegisterGeneric(typeof(GetEntitiesBySpecification<>))
.As(typeof(IGetEntitiesBySpecification<>))
.WithAttributeFiltering()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@

<!-- Unique Guid-based identifier for aggregate root -->
<property name="Id" column="Id" type="guid" not-null="true" />
<property name="AggregateId" column="AggregateId" type="int" not-null="true" insert="false" update="false" />

<!-- Properties -->

Expand Down Expand Up @@ -77,6 +78,7 @@

<!-- Unique Guid-based identifier for aggregate root -->
<property name="Id" column="Id" type="guid" not-null="true" />
<property name="AggregateId" column="AggregateId" type="int" not-null="true" insert="false" update="false" />

<!-- Properties -->

Expand Down Expand Up @@ -194,6 +196,7 @@

<!-- Unique Guid-based identifier for aggregate root -->
<property name="Id" column="Id" type="guid" not-null="true" />
<property name="AggregateId" column="AggregateId" type="int" not-null="true" insert="false" update="false" />

<!-- Properties -->
<property name="SchoolYear" column="SchoolYear" type="string" length="20" />
Expand Down Expand Up @@ -270,6 +273,7 @@

<!-- Unique Guid-based identifier for aggregate root -->
<property name="Id" column="Id" type="guid" not-null="true" />
<property name="AggregateId" column="AggregateId" type="int" not-null="true" insert="false" update="false" />

<!-- Properties -->

Expand Down Expand Up @@ -314,6 +318,7 @@

<!-- Unique Guid-based identifier for aggregate root -->
<property name="Id" column="Id" type="guid" not-null="true" />
<property name="AggregateId" column="AggregateId" type="int" not-null="true" insert="false" update="false" />

<!-- Properties -->

Expand Down Expand Up @@ -435,6 +440,7 @@

<!-- Unique Guid-based identifier for aggregate root -->
<property name="Id" column="Id" type="guid" not-null="true" />
<property name="AggregateId" column="AggregateId" type="int" not-null="true" insert="false" update="false" />

<!-- Properties -->
<property name="SchoolYear" column="SchoolYear" type="string" length="20" not-null="true" />
Expand Down Expand Up @@ -523,6 +529,7 @@

<!-- Unique Guid-based identifier for aggregate root -->
<property name="Id" column="Id" type="guid" not-null="true" />
<property name="AggregateId" column="AggregateId" type="int" not-null="true" insert="false" update="false" />

<!-- Properties -->

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@

<!-- Unique Guid-based identifier for aggregate root -->
<property name="Id" column="Id" type="guid" not-null="true" />
<property name="AggregateId" column="AggregateId" type="int" not-null="true" insert="false" update="false" />

<!-- Properties -->

Expand Down Expand Up @@ -77,6 +78,7 @@

<!-- Unique Guid-based identifier for aggregate root -->
<property name="Id" column="Id" type="guid" not-null="true" />
<property name="AggregateId" column="AggregateId" type="int" not-null="true" insert="false" update="false" />

<!-- Properties -->

Expand Down Expand Up @@ -194,6 +196,7 @@

<!-- Unique Guid-based identifier for aggregate root -->
<property name="Id" column="Id" type="guid" not-null="true" />
<property name="AggregateId" column="AggregateId" type="int" not-null="true" insert="false" update="false" />

<!-- Properties -->
<property name="SchoolYear" column="SchoolYear" type="string" length="20" />
Expand Down Expand Up @@ -270,6 +273,7 @@

<!-- Unique Guid-based identifier for aggregate root -->
<property name="Id" column="Id" type="guid" not-null="true" />
<property name="AggregateId" column="AggregateId" type="int" not-null="true" insert="false" update="false" />

<!-- Properties -->

Expand Down Expand Up @@ -314,6 +318,7 @@

<!-- Unique Guid-based identifier for aggregate root -->
<property name="Id" column="Id" type="guid" not-null="true" />
<property name="AggregateId" column="AggregateId" type="int" not-null="true" insert="false" update="false" />

<!-- Properties -->

Expand Down Expand Up @@ -435,6 +440,7 @@

<!-- Unique Guid-based identifier for aggregate root -->
<property name="Id" column="Id" type="guid" not-null="true" />
<property name="AggregateId" column="AggregateId" type="int" not-null="true" insert="false" update="false" />

<!-- Properties -->
<property name="SchoolYear" column="SchoolYear" type="string" length="20" not-null="true" />
Expand Down Expand Up @@ -523,6 +529,7 @@

<!-- Unique Guid-based identifier for aggregate root -->
<property name="Id" column="Id" type="guid" not-null="true" />
<property name="AggregateId" column="AggregateId" type="int" not-null="true" insert="false" update="false" />

<!-- Properties -->

Expand Down
Loading
Loading