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-6182] Detect Profile scenarios where resources (or any of its children) cannot be created due to excluded required members #955

Merged
merged 19 commits into from
Feb 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
d85db1b
Semantic Model Enhancement: Minor changes after adding and reverting …
gmcelhanon Jan 10, 2024
80c5859
Add Profile-based create entity decorator to prevent resource creatio…
gmcelhanon Jan 10, 2024
8580b6a
Fixing compilation error in unit test.
gmcelhanon Jan 11, 2024
7efde5e
Added support for detecting when a Profile will prevent the creation …
gmcelhanon Jan 11, 2024
d4c92bf
Updated approval tests.
gmcelhanon Jan 11, 2024
ec2968b
Refined the DataPolicyException constructor for the specific supporte…
gmcelhanon Jan 11, 2024
ef9b06c
Added support for detecting uncreatable EmbeddedObjects.
gmcelhanon Jan 11, 2024
b688657
Added a test profile definition for an uncreatable AssessmentContentS…
gmcelhanon Jan 11, 2024
5cabf9f
Updated approval tests.
gmcelhanon Jan 11, 2024
33bc9f9
Added Profiles for testing scenarios of Data Policy enforcement based…
gmcelhanon Feb 8, 2024
eb052b4
Made member inclusion/exclusion filters use case-insensitive comparis…
gmcelhanon Feb 10, 2024
3242a16
Added support for detecting Data Profile exceptions during the mappin…
gmcelhanon Feb 10, 2024
3aa3e0b
Fixed issue with capturing Data Policy exception context everywhere i…
gmcelhanon Feb 10, 2024
741fea8
Updated approval tests.
gmcelhanon Feb 10, 2024
89446cb
Modified code gen to reduce code gen output from 8 lines to 1 line fo…
gmcelhanon Feb 13, 2024
8cd2919
Reduced quantity of lines of generated code with reduction of classes…
gmcelhanon Feb 14, 2024
c9a259c
Revert inadvertent committed change to launchSettings.json.
gmcelhanon Feb 14, 2024
d9050c7
Remove unused namespaces.
gmcelhanon Feb 16, 2024
43bfe8c
Empty-commit
gmcelhanon Feb 20, 2024
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
2 changes: 2 additions & 0 deletions Application/EdFi.Ods.Api/Startup/OdsStartupBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
using EdFi.Ods.Common.Database;
using EdFi.Ods.Common.Dependencies;
using EdFi.Ods.Common.Descriptors;
using EdFi.Ods.Common.Exceptions;
using EdFi.Ods.Common.Infrastructure.Configuration;
using EdFi.Ods.Common.Infrastructure.Extensibility;
using EdFi.Ods.Common.Models;
Expand Down Expand Up @@ -413,6 +414,7 @@ void SetStaticResolvers()
GeneratedArtifactStaticDependencies.Resolvers.Set(() => Container.Resolve<IContextProvider<UsiLookupsByUniqueIdContext>>());
GeneratedArtifactStaticDependencies.Resolvers.Set(() => (StringComparer) Container.Resolve<IDatabaseEngineSpecificEqualityComparerProvider<string>>().GetEqualityComparer());
GeneratedArtifactStaticDependencies.Resolvers.Set(() => Container.Resolve<IDescriptorResolver>());
GeneratedArtifactStaticDependencies.Resolvers.Set(() => Container.Resolve<IContextProvider<DataPolicyException>>());

// netcore has removed the claims principal from the thread, to be on the controller.
// as a workaround for our current application we can resolve the IHttpContextAccessor.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,10 @@
using System;
using EdFi.Ods.Common.Caching;
using EdFi.Ods.Common.Context;
using EdFi.Ods.Common.Database;
using EdFi.Ods.Common.Descriptors;
using EdFi.Ods.Common.Exceptions;
using EdFi.Ods.Common.Models;
using EdFi.Ods.Common.Profiles;
using EdFi.Ods.Common.Security;
using EdFi.Ods.Common.Security.Claims;

namespace EdFi.Ods.Common.Dependencies
Expand All @@ -29,6 +28,7 @@ public static class GeneratedArtifactStaticDependencies
private static Lazy<IContextProvider<ProfileContentTypeContext>> _profileContentTypeContextProvider;
private static Lazy<IContextProvider<UniqueIdLookupsByUsiContext>> _uniqueIdLookupsContextProvider;
private static Lazy<IContextProvider<UsiLookupsByUniqueIdContext>> _usiLookupsContextProvider;
private static Lazy<IContextProvider<DataPolicyException>> _dataPolicyExceptionContextProvider;
private static Lazy<StringComparer> _databaseEngineSpecificStringComparer;
private static Lazy<IDescriptorResolver> _descriptorResolver;

Expand All @@ -41,6 +41,7 @@ public static class GeneratedArtifactStaticDependencies
public static IContextProvider<ProfileContentTypeContext> ProfileContentTypeContextProvider => _profileContentTypeContextProvider?.Value;
public static IContextProvider<UniqueIdLookupsByUsiContext> UniqueIdLookupsByUsiContextProvider => _uniqueIdLookupsContextProvider?.Value;
public static IContextProvider<UsiLookupsByUniqueIdContext> UsiLookupsByUniqueIdContextProvider => _usiLookupsContextProvider?.Value;
public static IContextProvider<DataPolicyException> DataPolicyExceptionContextProvider => _dataPolicyExceptionContextProvider?.Value;
public static StringComparer DatabaseEngineSpecificStringComparer => _databaseEngineSpecificStringComparer?.Value;
public static IDescriptorResolver DescriptorResolver => _descriptorResolver?.Value;

Expand Down Expand Up @@ -95,11 +96,16 @@ public static void Set(Func<IContextProvider<UsiLookupsByUniqueIdContext>> resol
_usiLookupsContextProvider = new Lazy<IContextProvider<UsiLookupsByUniqueIdContext>>(resolver);
}

public static void Set(Func<IContextProvider<DataPolicyException>> resolver)
{
_dataPolicyExceptionContextProvider = new Lazy<IContextProvider<DataPolicyException>>(resolver);
}

public static void Set(Func<StringComparer> resolver)
{
_databaseEngineSpecificStringComparer = new Lazy<StringComparer>(resolver);
}

public static void Set(Func<IDescriptorResolver> resolver)
{
_descriptorResolver = new Lazy<IDescriptorResolver>(resolver);
Expand Down
54 changes: 54 additions & 0 deletions Application/EdFi.Ods.Common/Exceptions/DataPolicyException.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// 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;

namespace EdFi.Ods.Common.Exceptions;

/// <summary>
/// An exception that indicates that a Profile (Data Policy) has prevented the request data from being processed.
/// </summary>
public class DataPolicyException : BadRequestDataException
{
// Fields containing override values for Problem Details
private const string TypePart = "policy";
private const string TitleText = "Data Policy Enforced";
private const string DefaultDetail =
"The resource cannot be saved because a data policy has been applied to the request that prevents it.";

private const string ResourceMessageFormat =
"The Profile definition for '{0}' excludes (or does not include) one or more required data elements needed to create the resource.";

private const string ResourceChildMessageFormat =
"The Profile definition for '{0}' excludes (or does not include) one or more required data elements needed to create a child item of type '{1}' in the resource.";

public DataPolicyException(string profileName)
: base(DefaultDetail)
{
((IEdFiProblemDetails)this).Errors = new[] { string.Format(ResourceMessageFormat, profileName) };
}

public DataPolicyException(string profileName, string childTypeName)
: base(DefaultDetail)
{
((IEdFiProblemDetails)this).Errors = new[] { string.Format(ResourceChildMessageFormat, profileName, childTypeName) };
}

// ---------------------------
// Boilerplate for overrides
// ---------------------------
public override string Title { get => TitleText; }

protected override IEnumerable<string> GetTypeParts()
{
foreach (var part in base.GetTypeParts())
{
yield return part;
}

yield return TypePart;
}
// ---------------------------
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,11 @@ namespace EdFi.Ods.Common.Exceptions;
│ │ └─────────────────────────┘
│ │ △
│ │ │ ┌────────────────────────────────┐
│ │ └──┤ KeyChangeNotSupportedException |
│ │ └────────────────────────────────┘
│ │ ├──┤ KeyChangeNotSupportedException |
│ │ │ └────────────────────────────────┘
│ │ │ ┌─────────────────────┐
│ │ └──┤ DataPolicyException |
│ │ └─────────────────────┘
│ │ ┌──────────────────────────────┐
│ └──┤ BadRequestParameterException |
│ └──────────────────────────────┘
Expand Down
37 changes: 31 additions & 6 deletions Application/EdFi.Ods.Common/Extensions/CollectionExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,20 @@
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Linq;
using EdFi.Ods.Common.Dependencies;
using EdFi.Ods.Common.Exceptions;

namespace EdFi.Ods.Common.Extensions
{
public static class CollectionExtensions
{
private static readonly ConcurrentDictionary<Type, Type> _itemTypeByUnderlyingListType =
new ConcurrentDictionary<Type, Type>();
private static readonly ConcurrentDictionary<Type, Type> _itemTypeByUnderlyingListType = new();

public static bool SynchronizeCollectionTo<T>(
this ICollection<T> sourceList,
ICollection<T> targetList,
Action<T> onChildAdded,
bool itemCreatable,
Func<T, bool> includeItem = null)
where T : ISynchronizable //<T>
{
Expand Down Expand Up @@ -67,13 +69,23 @@ public static bool SynchronizeCollectionTo<T>(
.Except(targetList.Where(i => includeItem == null || includeItem(i)))
.ToList();

foreach (var item in itemsToAdd)
if (itemsToAdd.Any())
{
targetList.Add(item);
if (!itemCreatable)
{
string profileName = GeneratedArtifactStaticDependencies.ProfileContentTypeContextProvider.Get().ProfileName;

onChildAdded?.Invoke(item);
throw new DataPolicyException(profileName, itemsToAdd.First().GetType().Name);
}

foreach (var item in itemsToAdd)
{
targetList.Add(item);

isModified = true;
onChildAdded?.Invoke(item);

isModified = true;
}
}

return isModified;
Expand All @@ -82,6 +94,7 @@ public static bool SynchronizeCollectionTo<T>(
public static void MapCollectionTo<TSource, TTarget>(
this ICollection<TSource> sourceList,
ICollection<TTarget> targetList,
bool itemCreatable = true,
object parent = null,
Func<TSource, bool> isItemIncluded = null)
where TSource : IMappable
Expand All @@ -101,6 +114,18 @@ public static void MapCollectionTo<TSource, TTarget>(

foreach (var sourceItem in sourceList.Where(i => isItemIncluded == null || isItemIncluded(i)))
{
if (!itemCreatable)
{
// Use context provider to note the potential Data Policy Exception here (which applies only if the resource
// is being created, otherwise the SynchronizeCollectionTo method to the existing entity will handle any
// data policy violations).
if (GeneratedArtifactStaticDependencies.DataPolicyExceptionContextProvider.Get() == null)
{
string profileName = GeneratedArtifactStaticDependencies.ProfileContentTypeContextProvider.Get().ProfileName;
GeneratedArtifactStaticDependencies.DataPolicyExceptionContextProvider.Set(new DataPolicyException(profileName, itemType.Name));
}
}

var targetItem = (TTarget) Activator.CreateInstance(itemType);

if (parent != null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public class UpsertEntity<TEntity> : NHibernateRepositoryOperationBase, IUpsertE
private readonly IUpdateEntity<TEntity> _updateEntity;
private readonly IContextProvider<UniqueIdLookupsByUsiContext> _lookupContextProvider;
private readonly IPersonUniqueIdResolver _personUniqueIdResolver;
private readonly IContextProvider<DataPolicyException> _dataPolicyExceptionContextProvider;

public UpsertEntity(
ISessionFactory sessionFactory,
Expand All @@ -32,7 +33,8 @@ public UpsertEntity(
ICreateEntity<TEntity> createEntity,
IUpdateEntity<TEntity> updateEntity,
IContextProvider<UniqueIdLookupsByUsiContext> lookupContextProvider,
IPersonUniqueIdResolver personUniqueIdResolver)
IPersonUniqueIdResolver personUniqueIdResolver,
IContextProvider<DataPolicyException> dataPolicyExceptionContextProvider)
: base(sessionFactory)
{
_getEntityById = getEntityById;
Expand All @@ -41,6 +43,7 @@ public UpsertEntity(
_updateEntity = updateEntity;
_lookupContextProvider = lookupContextProvider;
_personUniqueIdResolver = personUniqueIdResolver;
_dataPolicyExceptionContextProvider = dataPolicyExceptionContextProvider;
}

public async Task<UpsertEntityResult<TEntity>> UpsertAsync(TEntity entity, bool enforceOptimisticLock, CancellationToken cancellationToken)
Expand Down Expand Up @@ -76,6 +79,15 @@ public async Task<UpsertEntityResult<TEntity>> UpsertAsync(TEntity entity, bool
// If there is no existing entity...
if (persistedEntity == null)
{
// Check for a data policy exception detected during mapping processing from resource model to entity model
var dataPolicyExceptionFromMapping = _dataPolicyExceptionContextProvider.Get();

// If there was a data policy violation detected, throw the exception now that we know we're creating the aggregate
if (dataPolicyExceptionFromMapping != null)
{
throw dataPolicyExceptionFromMapping;
}

// Create the entity
await _createEntity.CreateAsync(entity, enforceOptimisticLock, cancellationToken);
persistedEntity = entity;
Expand Down
2 changes: 2 additions & 0 deletions Application/EdFi.Ods.Common/Models/IMappingContract.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,6 @@ public interface IMappingContract
/// <param name="memberName">The name of member to be mapped/synchronized.</param>
/// <returns><b>true</b> if the member is supported; otherwise <b>false</b>.</returns>
bool IsMemberSupported(string memberName);

bool IsItemCreatable(string memberName);
}
44 changes: 44 additions & 0 deletions Application/EdFi.Ods.Common/Models/MappingContractProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,50 @@ private IMappingContract GetOrCreateMappingContract(MappingContractKey mappingCo
return null;
}

// Handle collections
if (parameterInfo.Name.EndsWith("ItemCreatable"))
{
string memberName = parameterInfo.Name.Substring(
2,
parameterInfo.Name.Length - "ItemCreatable".Length - 2);

if (key.ContentTypeUsage == ContentTypeUsage.Readable)
{
// Use of the readable content type implies outbound mapping is in play, which should always be supported
return true;
}

string collectionName = CompositeTermInflector.MakePlural(memberName);

if (profileResourceClass.CollectionByName.TryGetValue(collectionName, out var collection))
{
return contentTypes.CanCreateResourceClass(collection.ItemType.FullName);
}

return false;
}

// Handle embedded objects
if (parameterInfo.Name.EndsWith("Creatable"))
{
string memberName = parameterInfo.Name.Substring(
2,
parameterInfo.Name.Length - "Creatable".Length - 2);

if (key.ContentTypeUsage == ContentTypeUsage.Readable)
{
// Use of the readable content type implies outbound mapping is in play, which should always be supported
return true;
}

if (profileResourceClass.EmbeddedObjectByName.TryGetValue(memberName, out var embeddedObject))
{
return contentTypes.CanCreateResourceClass(embeddedObject.ObjectType.FullName);
}

return false;
}

throw new Exception(
$"Constructor argument '{parameterInfo.Name}' of '{mappingContractType.FullName}' did not conform to expected naming convention of isXxxxSupported or isXxxxIncluded.");
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.Linq;

namespace EdFi.Ods.Common.Models.Resource
Expand All @@ -20,12 +21,12 @@ public ExcludeOnlyMemberFilter(string[] excludedMemberNames, string[] excludedEx

public bool ShouldInclude(string memberName)
{
return memberName == "Id" || !_excludedMemberNames.Contains(memberName);
return memberName == "Id" || !_excludedMemberNames.Contains(memberName, StringComparer.OrdinalIgnoreCase);
}

public bool ShouldIncludeExtension(string extensionName)
{
return !_excludedExtensionNames.Contains(extensionName);
return !_excludedExtensionNames.Contains(extensionName, StringComparer.OrdinalIgnoreCase);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.Linq;

namespace EdFi.Ods.Common.Models.Resource
Expand All @@ -20,12 +21,12 @@ public IncludeOnlyMemberFilter(string[] includedMemberNames, string[] includedEx

public bool ShouldInclude(string memberName)
{
return memberName == "Id" || _includedMemberNames.Contains(memberName);
return memberName == "Id" || _includedMemberNames.Contains(memberName, StringComparer.OrdinalIgnoreCase);
}

public bool ShouldIncludeExtension(string extensionName)
{
return _includedExtensionNames.Contains(extensionName);
return _includedExtensionNames.Contains(extensionName, StringComparer.OrdinalIgnoreCase);
}
}
}
Loading
Loading