Skip to content

Commit

Permalink
[ODS-6182] Detect Profile scenarios where resources (or any of its ch…
Browse files Browse the repository at this point in the history
…ildren) cannot be created due to excluded required members (#955)
  • Loading branch information
gmcelhanon authored Feb 20, 2024
1 parent cdd9786 commit b00b70d
Show file tree
Hide file tree
Showing 42 changed files with 12,420 additions and 822 deletions.
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

0 comments on commit b00b70d

Please sign in to comment.