Skip to content

Commit

Permalink
[ODS-6063] Unclear error response when required references are not fu…
Browse files Browse the repository at this point in the history
…lly formed (#861)

Co-authored-by: semalaiappan <[email protected]>
  • Loading branch information
gmcelhanon and semalaiappan authored Nov 2, 2023
1 parent 6f9846d commit c971c5d
Show file tree
Hide file tree
Showing 26 changed files with 8,994 additions and 6,576 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,12 @@ public bool TryGetCachedObject(TKey key, out object value)
if (_logger.IsDebugEnabled)
{
_misses++;
_logger.Debug($"{nameof(ExpiringConcurrentDictionaryCacheProvider<TKey>)} cache '{_description}' MISS ({_hits} hits, {_misses} misses).");

if (_misses < 50 || _misses % 100 == 0)
{
_logger.Debug(
$"{nameof(ExpiringConcurrentDictionaryCacheProvider<TKey>)} cache '{_description}' MISS ({_hits} hits, {_misses} misses).");
}
}
}

Expand Down
1 change: 1 addition & 0 deletions Application/EdFi.Ods.Api/Startup/OdsStartupBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,7 @@ void SetStaticResolvers()
{
// Make this dependency available to generated artifacts
GeneratedArtifactStaticDependencies.Resolvers.Set(() => Container.Resolve<IResourceModelProvider>());
GeneratedArtifactStaticDependencies.Resolvers.Set(() => Container.Resolve<IProfileResourceModelProvider>());
GeneratedArtifactStaticDependencies.Resolvers.Set(() => Container.Resolve<IDomainModelProvider>());
GeneratedArtifactStaticDependencies.Resolvers.Set(() => Container.Resolve<IAuthorizationContextProvider>());
GeneratedArtifactStaticDependencies.Resolvers.Set(() => Container.Resolve<IETagProvider>());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// 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.ComponentModel.DataAnnotations;

namespace EdFi.Ods.Common.Attributes;

/// <summary>
/// Validates that a resource reference is fully formed.
/// </summary>
public class FullyDefinedReferenceAttribute : ValidationAttribute
{
protected override ValidationResult IsValid(object value, ValidationContext validationContext)
{
// Ensures the supplied reference is either null, or is fully formed.
if (value == null)
{
return ValidationResult.Success;
}

if (value is IResourceReference reference)
{
if (!reference.IsReferenceFullyDefined())
{
return new ValidationResult($"{validationContext.MemberName} is missing the following properties: {string.Join(", ", reference.GetUndefinedProperties())}");
}
}

return ValidationResult.Success;
}
}
18 changes: 18 additions & 0 deletions Application/EdFi.Ods.Common/Attributes/IResourceReference.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// 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.Attributes;

/// <summary>
/// Defines methods for determining whether (and how) a resource reference is partially formed.
/// </summary>
public interface IResourceReference
{
bool IsReferenceFullyDefined();

IEnumerable<string> GetUndefinedProperties();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
// 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.Concurrent;
using System.ComponentModel.DataAnnotations;
using EdFi.Common.Extensions;
using EdFi.Ods.Common.Dependencies;
using EdFi.Ods.Common.Models.Domain;

namespace EdFi.Ods.Common.Attributes;

public class RequiredReferenceAttribute : ValidationAttribute
{
private readonly bool _isIdentifying;
private readonly string _resourceSchema;
private readonly string _resourceName;

public RequiredReferenceAttribute(bool isIdentifying)
{
_isIdentifying = isIdentifying;
}

public RequiredReferenceAttribute(string resourceSchema, string resourceName)
{
_resourceSchema = resourceSchema;
_resourceName = resourceName;
}

private ConcurrentDictionary<string, string> _itemTypeNameByRequestTypeName = new();

protected override ValidationResult IsValid(object value, ValidationContext validationContext)
{
// Identifying references are forcibly included in Profiles, so we only need to check for Profile in force for non-identifying references.
if (!_isIdentifying)
{
string profileName = GeneratedArtifactStaticDependencies.ProfileContentTypeContextProvider.Get()?.ProfileName;

// Is there a Profile applied to this request?
if (profileName != null)
{
// Determine if this reference has been excluded
if (GeneratedArtifactStaticDependencies.ProfileResourceModelProvider.GetProfileResourceModel(profileName)
.ResourceByName.TryGetValue(new FullName(_resourceSchema, _resourceName), out var profileContentTypes))
{
// Maintain a dictionary of item type names rather than creating new strings that need to be GC'd for every reference check
string itemTypeName = _itemTypeNameByRequestTypeName.GetOrAdd(
validationContext.ObjectType.Name,
requestTypeName => requestTypeName.TrimSuffix("Post").TrimSuffix("Put"));

if (!profileContentTypes.Writable.ContainedItemTypeByName.TryGetValue(itemTypeName, out var resourceClass))
{
return ValidationResult.Success;
}

if (!resourceClass.ReferenceByName.TryGetValue(validationContext.MemberName, out var ignored))
{
return ValidationResult.Success;
}
}
}
}

// Perform required check
if (value != null)
{
return ValidationResult.Success;
}

return new ValidationResult($"{validationContext.MemberName} is required.", new[] { validationContext.MemberName });
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ public static class GeneratedArtifactStaticDependencies
{
private static Lazy<IAuthorizationContextProvider> _authorizationContextProvider;
private static Lazy<IResourceModelProvider> _resourceModelProvider;
private static Lazy<IProfileResourceModelProvider> _profileResourceModelProvider;
private static Lazy<IDomainModelProvider> _domainModelProvider;
private static Lazy<IETagProvider> _etagProvider;
private static Lazy<IMappingContractProvider> _mappingContractProvider;
Expand All @@ -33,6 +34,7 @@ public static class GeneratedArtifactStaticDependencies

public static IAuthorizationContextProvider AuthorizationContextProvider => _authorizationContextProvider?.Value;
public static IResourceModelProvider ResourceModelProvider => _resourceModelProvider?.Value;
public static IProfileResourceModelProvider ProfileResourceModelProvider => _profileResourceModelProvider?.Value;
public static IDomainModelProvider DomainModelProvider => _domainModelProvider?.Value;
public static IETagProvider ETagProvider => _etagProvider?.Value;
public static IMappingContractProvider MappingContractProvider => _mappingContractProvider?.Value;
Expand All @@ -58,6 +60,11 @@ public static void Set(Func<IResourceModelProvider> resolver)
_resourceModelProvider = new Lazy<IResourceModelProvider>(resolver);
}

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

public static void Set(Func<IDomainModelProvider> resolver)
{
_domainModelProvider = new Lazy<IDomainModelProvider>(resolver);
Expand Down
20 changes: 19 additions & 1 deletion Application/EdFi.Ods.Common/Models/Resource/Resource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,23 @@ namespace EdFi.Ods.Common.Models.Resource
public class Resource : ResourceClassBase
{
private readonly Lazy<IReadOnlyDictionary<string, LinkedCollection>> _linkedCollectionByName;
private readonly Lazy<IReadOnlyDictionary<string, ResourceClassBase>> _containedItemTypeByName;
private readonly Lazy<IReadOnlyList<LinkedCollection>> _linkedCollections;

internal Resource(IResourceModel resourceModel, Entity entity)
: base(resourceModel, entity)
{
_linkedCollections = LazyInitializeLinkedCollections(entity);
_linkedCollectionByName = LazyInitializeLinkedCollectionByName();
_containedItemTypeByName = LazyInitializeContainedItemTypeByName();
}

internal Resource(IResourceModel resourceModel, Entity entity, FilterContext filterContext)
: base(resourceModel, entity, filterContext)
{
_linkedCollections = LazyInitializeLinkedCollections(entity);
_linkedCollectionByName = LazyInitializeLinkedCollectionByName();
_containedItemTypeByName = LazyInitializeContainedItemTypeByName();
}

internal Resource(Resource[] resources)
Expand All @@ -41,6 +44,7 @@ internal Resource(Resource[] resources)

_linkedCollections = LazyInitializeLinkedCollections(entity);
_linkedCollectionByName = LazyInitializeLinkedCollectionByName();
_containedItemTypeByName = LazyInitializeContainedItemTypeByName();
}

/// <summary>
Expand All @@ -58,6 +62,8 @@ public Resource(string name)
_linkedCollectionByName = new Lazy<IReadOnlyDictionary<string, LinkedCollection>>(
() =>
new Dictionary<string, LinkedCollection>());

_containedItemTypeByName = LazyInitializeContainedItemTypeByName();
}

public bool IsEdFiCore { get; set; }
Expand Down Expand Up @@ -94,6 +100,11 @@ public Resource(string name)
public IReadOnlyList<ResourceClassBase> AllContainedItemTypesOrSelf
=> new ResourceClassBase[] { this }.Concat(ContainedItemTypes).ToArray();

/// <summary>
/// Gets a dictionary containing all the resource classes contained by the Resource, keyed by class name.
/// </summary>
public IReadOnlyDictionary<string, ResourceClassBase> ContainedItemTypeByName => _containedItemTypeByName.Value;

/// <summary>
/// Returns all the references contained within the resource.
/// </summary>
Expand All @@ -103,7 +114,14 @@ private Lazy<IReadOnlyDictionary<string, LinkedCollection>> LazyInitializeLinked
{
return new Lazy<IReadOnlyDictionary<string, LinkedCollection>>(
() =>
LinkedCollections.ToDictionary(x => x.Name, x => x, StringComparer.InvariantCultureIgnoreCase));
LinkedCollections.ToDictionary(x => x.Name, x => x, StringComparer.OrdinalIgnoreCase));
}

private Lazy<IReadOnlyDictionary<string, ResourceClassBase>> LazyInitializeContainedItemTypeByName()
{
return new Lazy<IReadOnlyDictionary<string, ResourceClassBase>>(
() =>
AllContainedItemTypesOrSelf.ToDictionary(x => x.Name, x => x, StringComparer.OrdinalIgnoreCase));
}

private Lazy<IReadOnlyList<LinkedCollection>> LazyInitializeLinkedCollections(Entity entity)
Expand Down
8 changes: 8 additions & 0 deletions Application/EdFi.Ods.Profiles.Test/Profiles.xml
Original file line number Diff line number Diff line change
Expand Up @@ -880,4 +880,12 @@
</ReadContentType>
</Resource>
</Profile>

<Profile name="Resource-with-excluded-required-reference">
<Resource name="CourseOffering">
<WriteContentType memberSelection="ExcludeOnly">
<Property name="CourseReference" />
</WriteContentType>
</Resource>
</Profile>
</Profiles>
Loading

0 comments on commit c971c5d

Please sign in to comment.