Skip to content

Commit

Permalink
Process review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
maurei committed Nov 18, 2021
1 parent e07903c commit c6e8542
Show file tree
Hide file tree
Showing 21 changed files with 434 additions and 213 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using JsonApiDotNetCore.Configuration;
using JsonApiDotNetCore.Middleware;
using JsonApiDotNetCore.OpenApi.JsonApiMetadata;
using Microsoft.AspNetCore.Mvc;
Expand All @@ -26,15 +25,14 @@ internal sealed class JsonApiActionDescriptorCollectionProvider : IActionDescrip

public ActionDescriptorCollection ActionDescriptors => GetActionDescriptors();

public JsonApiActionDescriptorCollectionProvider(IResourceGraph resourceGraph, IControllerResourceMapping controllerResourceMapping,
public JsonApiActionDescriptorCollectionProvider(IControllerResourceMapping controllerResourceMapping,
IActionDescriptorCollectionProvider defaultProvider)
{
ArgumentGuard.NotNull(resourceGraph, nameof(resourceGraph));
ArgumentGuard.NotNull(controllerResourceMapping, nameof(controllerResourceMapping));
ArgumentGuard.NotNull(defaultProvider, nameof(defaultProvider));

_defaultProvider = defaultProvider;
_jsonApiEndpointMetadataProvider = new JsonApiEndpointMetadataProvider(resourceGraph, controllerResourceMapping);
_jsonApiEndpointMetadataProvider = new JsonApiEndpointMetadataProvider(controllerResourceMapping);
}

private ActionDescriptorCollection GetActionDescriptors()
Expand Down Expand Up @@ -107,7 +105,6 @@ private static void UpdateProducesResponseTypeAttribute(ActionDescriptor endpoin
if (producesResponse != null)
{
producesResponse.Type = responseTypeToSet;

return;
}
}
Expand All @@ -134,7 +131,7 @@ private static IList<ActionDescriptor> Expand(ActionDescriptor genericEndpoint,

if (expandedEndpoint.AttributeRouteInfo == null)
{
throw new NotSupportedException("Only attribute based routing is supported for JsonApiDotNetCore endpoints");
throw new UnreachableCodeException();
}

ExpandTemplate(expandedEndpoint.AttributeRouteInfo, relationshipName);
Expand Down Expand Up @@ -172,7 +169,12 @@ private static ActionDescriptor Clone(ActionDescriptor descriptor)
{
var clonedDescriptor = (ActionDescriptor)descriptor.MemberwiseClone();

clonedDescriptor.AttributeRouteInfo = (AttributeRouteInfo)descriptor.AttributeRouteInfo!.MemberwiseClone();
if (descriptor.AttributeRouteInfo == null)
{
throw new NotSupportedException("Only attribute routing is supported for JsonApiDotNetCore endpoints.");
}

clonedDescriptor.AttributeRouteInfo = (AttributeRouteInfo)descriptor.AttributeRouteInfo.MemberwiseClone();

clonedDescriptor.FilterDescriptors = new List<FilterDescriptor>();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,13 @@ namespace JsonApiDotNetCore.OpenApi.JsonApiMetadata
/// </summary>
internal sealed class JsonApiEndpointMetadataProvider
{
private readonly IResourceGraph _resourceGraph;
private readonly IControllerResourceMapping _controllerResourceMapping;
private readonly EndpointResolver _endpointResolver = new();

public JsonApiEndpointMetadataProvider(IResourceGraph resourceGraph, IControllerResourceMapping controllerResourceMapping)
public JsonApiEndpointMetadataProvider(IControllerResourceMapping controllerResourceMapping)
{
ArgumentGuard.NotNull(resourceGraph, nameof(resourceGraph));
ArgumentGuard.NotNull(controllerResourceMapping, nameof(controllerResourceMapping));

_resourceGraph = resourceGraph;
_controllerResourceMapping = controllerResourceMapping;
}

Expand All @@ -47,28 +44,28 @@ public JsonApiEndpointMetadataContainer Get(MethodInfo controllerAction)
throw new UnreachableCodeException();
}

IJsonApiRequestMetadata? requestMetadata = GetRequestMetadata(endpoint.Value, primaryResourceType.ClrType);
IJsonApiResponseMetadata? responseMetadata = GetResponseMetadata(endpoint.Value, primaryResourceType.ClrType);
IJsonApiRequestMetadata? requestMetadata = GetRequestMetadata(endpoint.Value, primaryResourceType);
IJsonApiResponseMetadata? responseMetadata = GetResponseMetadata(endpoint.Value, primaryResourceType);
return new JsonApiEndpointMetadataContainer(requestMetadata, responseMetadata);
}

private IJsonApiRequestMetadata? GetRequestMetadata(JsonApiEndpoint endpoint, Type primaryResourceType)
private static IJsonApiRequestMetadata? GetRequestMetadata(JsonApiEndpoint endpoint, ResourceType primaryResourceType)
{
switch (endpoint)
{
case JsonApiEndpoint.Post:
{
return GetPostRequestMetadata(primaryResourceType);
return GetPostRequestMetadata(primaryResourceType.ClrType);
}
case JsonApiEndpoint.Patch:
{
return GetPatchRequestMetadata(primaryResourceType);
return GetPatchRequestMetadata(primaryResourceType.ClrType);
}
case JsonApiEndpoint.PostRelationship:
case JsonApiEndpoint.PatchRelationship:
case JsonApiEndpoint.DeleteRelationship:
{
return GetRelationshipRequestMetadata(primaryResourceType, endpoint != JsonApiEndpoint.PatchRelationship);
return GetRelationshipRequestMetadata(primaryResourceType.Relationships, endpoint != JsonApiEndpoint.PatchRelationship);
}
default:
{
Expand All @@ -77,38 +74,45 @@ public JsonApiEndpointMetadataContainer Get(MethodInfo controllerAction)
}
}

private static PrimaryRequestMetadata GetPostRequestMetadata(Type primaryResourceType)
private static PrimaryRequestMetadata GetPostRequestMetadata(Type resourceClrType)
{
Type documentType = typeof(ResourcePostRequestDocument<>).MakeGenericType(primaryResourceType);
Type documentType = typeof(ResourcePostRequestDocument<>).MakeGenericType(resourceClrType);

return new PrimaryRequestMetadata(documentType);
}

private static PrimaryRequestMetadata GetPatchRequestMetadata(Type primaryResourceType)
private static PrimaryRequestMetadata GetPatchRequestMetadata(Type resourceClrType)
{
Type documentType = typeof(ResourcePatchRequestDocument<>).MakeGenericType(primaryResourceType);
Type documentType = typeof(ResourcePatchRequestDocument<>).MakeGenericType(resourceClrType);

return new PrimaryRequestMetadata(documentType);
}

private RelationshipRequestMetadata GetRelationshipRequestMetadata(Type primaryResourceType, bool ignoreHasOneRelationships)
private static RelationshipRequestMetadata GetRelationshipRequestMetadata(IEnumerable<RelationshipAttribute> relationships,
bool ignoreHasOneRelationships)
{
IEnumerable<RelationshipAttribute> relationships = _resourceGraph.GetResourceType(primaryResourceType).Relationships;
IEnumerable<RelationshipAttribute> relationshipInRequest = ignoreHasOneRelationships ? relationships.OfType<HasManyAttribute>() : relationships;

if (ignoreHasOneRelationships)
{
relationships = relationships.OfType<HasManyAttribute>();
}
IDictionary<string, Type> resourceTypesByRelationshipName = relationshipInRequest.ToDictionary(relationship => relationship.PublicName,
relationship =>
{
// @formatter:nested_ternary_style expanded

IDictionary<string, Type> resourceTypesByRelationshipName = relationships.ToDictionary(relationship => relationship.PublicName,
relationship => relationship is HasManyAttribute
? typeof(ToManyRelationshipRequestData<>).MakeGenericType(relationship.RightType.ClrType)
: typeof(ToOneRelationshipRequestData<>).MakeGenericType(relationship.RightType.ClrType));
Type requestDataType = relationship is HasManyAttribute
? typeof(ToManyRelationshipRequestData<>)
: relationship.IsNullable()
? typeof(NullableToOneRelationshipRequestData<>)
: typeof(ToOneRelationshipRequestData<>);

// @formatter:nested_ternary_style restore

return requestDataType.MakeGenericType(relationship.RightType.ClrType);
});

return new RelationshipRequestMetadata(resourceTypesByRelationshipName);
}

private IJsonApiResponseMetadata? GetResponseMetadata(JsonApiEndpoint endpoint, Type primaryResourceType)
private static IJsonApiResponseMetadata? GetResponseMetadata(JsonApiEndpoint endpoint, ResourceType primaryResourceType)
{
switch (endpoint)
{
Expand All @@ -117,15 +121,15 @@ private RelationshipRequestMetadata GetRelationshipRequestMetadata(Type primaryR
case JsonApiEndpoint.Post:
case JsonApiEndpoint.Patch:
{
return GetPrimaryResponseMetadata(primaryResourceType, endpoint == JsonApiEndpoint.GetCollection);
return GetPrimaryResponseMetadata(primaryResourceType.ClrType, endpoint == JsonApiEndpoint.GetCollection);
}
case JsonApiEndpoint.GetSecondary:
{
return GetSecondaryResponseMetadata(primaryResourceType);
return GetSecondaryResponseMetadata(primaryResourceType.Relationships);
}
case JsonApiEndpoint.GetRelationship:
{
return GetRelationshipResponseMetadata(primaryResourceType);
return GetRelationshipResponseMetadata(primaryResourceType.Relationships);
}
default:
{
Expand All @@ -134,17 +138,17 @@ private RelationshipRequestMetadata GetRelationshipRequestMetadata(Type primaryR
}
}

private static PrimaryResponseMetadata GetPrimaryResponseMetadata(Type primaryResourceType, bool endpointReturnsCollection)
private static PrimaryResponseMetadata GetPrimaryResponseMetadata(Type resourceClrType, bool endpointReturnsCollection)
{
Type documentOpenType = endpointReturnsCollection ? typeof(ResourceCollectionResponseDocument<>) : typeof(PrimaryResourceResponseDocument<>);
Type documentType = documentOpenType.MakeGenericType(primaryResourceType);
Type documentType = documentOpenType.MakeGenericType(resourceClrType);

return new PrimaryResponseMetadata(documentType);
}

private SecondaryResponseMetadata GetSecondaryResponseMetadata(Type primaryResourceType)
private static SecondaryResponseMetadata GetSecondaryResponseMetadata(IEnumerable<RelationshipAttribute> relationships)
{
IDictionary<string, Type> responseTypesByRelationshipName = GetMetadataByRelationshipName(primaryResourceType, relationship =>
IDictionary<string, Type> responseTypesByRelationshipName = relationships.ToDictionary(relationship => relationship.PublicName, relationship =>
{
Type documentType = relationship is HasManyAttribute
? typeof(ResourceCollectionResponseDocument<>)
Expand All @@ -156,17 +160,9 @@ private SecondaryResponseMetadata GetSecondaryResponseMetadata(Type primaryResou
return new SecondaryResponseMetadata(responseTypesByRelationshipName);
}

private IDictionary<string, Type> GetMetadataByRelationshipName(Type primaryResourceType,
Func<RelationshipAttribute, Type> extractRelationshipMetadataCallback)
{
IReadOnlyCollection<RelationshipAttribute> relationships = _resourceGraph.GetResourceType(primaryResourceType).Relationships;

return relationships.ToDictionary(relationship => relationship.PublicName, extractRelationshipMetadataCallback);
}

private RelationshipResponseMetadata GetRelationshipResponseMetadata(Type primaryResourceType)
private static RelationshipResponseMetadata GetRelationshipResponseMetadata(IEnumerable<RelationshipAttribute> relationships)
{
IDictionary<string, Type> responseTypesByRelationshipName = GetMetadataByRelationshipName(primaryResourceType,
IDictionary<string, Type> responseTypesByRelationshipName = relationships.ToDictionary(relationship => relationship.PublicName,
relationship => relationship is HasManyAttribute
? typeof(ResourceIdentifierCollectionResponseDocument<>).MakeGenericType(relationship.RightType.ClrType)
: typeof(ResourceIdentifierResponseDocument<>).MakeGenericType(relationship.RightType.ClrType));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@

namespace JsonApiDotNetCore.OpenApi.JsonApiObjects.Documents
{
/// Types in the <see cref="JsonApiDotNetCore.OpenApi.JsonApiObjects"/> namespace are never touched by ASP.NET ModelState validation, therefore using a
/// non-nullable reference type for a property does not imply this property is required. Instead, we rely on explicitly setting <see cref="RequiredAttribute"/>
/// which is how Swashbuckle is instructed to mark properties as required.
[UsedImplicitly(ImplicitUseTargetFlags.Members)]
internal sealed class PrimaryResourceResponseDocument<TResource> : SingleData<ResourceResponseObject<TResource>>
where TResource : IIdentifiable
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
using JetBrains.Annotations;
using JsonApiDotNetCore.OpenApi.JsonApiObjects.ResourceObjects;
using JsonApiDotNetCore.Resources;

namespace JsonApiDotNetCore.OpenApi.JsonApiObjects.RelationshipData
{
[UsedImplicitly(ImplicitUseTargetFlags.Members)]
internal sealed class NullableToOneRelationshipRequestData<TResource> : SingleData<ResourceIdentifierObject<TResource>?>
where TResource : IIdentifiable
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using JetBrains.Annotations;
using JsonApiDotNetCore.OpenApi.JsonApiObjects.Links;
using JsonApiDotNetCore.OpenApi.JsonApiObjects.ResourceObjects;
using JsonApiDotNetCore.Resources;

namespace JsonApiDotNetCore.OpenApi.JsonApiObjects.RelationshipData
{
[UsedImplicitly(ImplicitUseTargetFlags.Members)]
internal sealed class NullableToOneRelationshipResponseData<TResource> : SingleData<ResourceIdentifierObject<TResource>?>
where TResource : IIdentifiable
{
[Required]
public LinksInRelationshipObject Links { get; set; } = null!;

public IDictionary<string, object> Meta { get; set; } = null!;
}
}
2 changes: 1 addition & 1 deletion src/JsonApiDotNetCore.OpenApi/JsonApiObjects/SingleData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ namespace JsonApiDotNetCore.OpenApi.JsonApiObjects
{
[UsedImplicitly(ImplicitUseTargetFlags.Members)]
internal abstract class SingleData<TData>
where TData : ResourceIdentifierObject
where TData : ResourceIdentifierObject?
{
[Required]
public TData Data { get; set; } = null!;
Expand Down
13 changes: 7 additions & 6 deletions src/JsonApiDotNetCore.OpenApi/JsonApiOperationIdSelector.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ internal sealed class JsonApiOperationIdSelector
[typeof(ResourceIdentifierCollectionResponseDocument<>)] = RelationshipOperationIdTemplate,
[typeof(ResourceIdentifierResponseDocument<>)] = RelationshipOperationIdTemplate,
[typeof(ToOneRelationshipRequestData<>)] = RelationshipOperationIdTemplate,
[typeof(NullableToOneRelationshipRequestData<>)] = RelationshipOperationIdTemplate,
[typeof(ToManyRelationshipRequestData<>)] = RelationshipOperationIdTemplate
};

Expand Down Expand Up @@ -64,14 +65,14 @@ public string GetOperationId(ApiDescription endpoint)
return ApplyTemplate(template, primaryResourceType.ClrType, endpoint);
}

private static string GetTemplate(Type primaryResourceType, ApiDescription endpoint)
private static string GetTemplate(Type resourceClrType, ApiDescription endpoint)
{
Type requestDocumentType = GetDocumentType(primaryResourceType, endpoint);
Type requestDocumentType = GetDocumentType(resourceClrType, endpoint);

return DocumentOpenTypeToOperationIdTemplateMap[requestDocumentType];
}

private static Type GetDocumentType(Type primaryResourceType, ApiDescription endpoint)
private static Type GetDocumentType(Type resourceClrType, ApiDescription endpoint)
{
var producesResponseTypeAttribute = endpoint.ActionDescriptor.GetFilterMetadata<ProducesResponseTypeAttribute>();

Expand All @@ -89,7 +90,7 @@ private static Type GetDocumentType(Type primaryResourceType, ApiDescription end
{
Type documentResourceType = producesResponseTypeAttribute.Type.GetGenericArguments()[0];

if (documentResourceType != primaryResourceType)
if (documentResourceType != resourceClrType)
{
documentType = typeof(SecondaryResourceResponseDocument<>);
}
Expand All @@ -103,10 +104,10 @@ private static Type GetDocumentType(Type primaryResourceType, ApiDescription end
return type.IsConstructedGenericType ? type.GetGenericTypeDefinition() : null;
}

private string ApplyTemplate(string operationIdTemplate, Type primaryResourceType, ApiDescription endpoint)
private string ApplyTemplate(string operationIdTemplate, Type resourceClrType, ApiDescription endpoint)
{
string method = endpoint.HttpMethod!.ToLowerInvariant();
string primaryResourceName = _formatter.FormatResourceName(primaryResourceType).Singularize();
string primaryResourceName = _formatter.FormatResourceName(resourceClrType).Singularize();
string relationshipName = operationIdTemplate.Contains("[RelationshipName]") ? endpoint.RelativePath.Split("/").Last() : string.Empty;

// @formatter:wrap_chained_method_calls chop_always
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ internal sealed class JsonApiRequestFormatMetadataProvider : IInputFormatter, IA
{
typeof(ToManyRelationshipRequestData<>),
typeof(ToOneRelationshipRequestData<>),
typeof(NullableToOneRelationshipRequestData<>),
typeof(ResourcePostRequestDocument<>),
typeof(ResourcePatchRequestDocument<>)
};
Expand Down
2 changes: 2 additions & 0 deletions src/JsonApiDotNetCore.OpenApi/JsonApiSchemaIdSelector.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,15 @@ internal sealed class JsonApiSchemaIdSelector
[typeof(ResourcePostRequestObject<>)] = "###-data-in-post-request",
[typeof(ResourcePatchRequestObject<>)] = "###-data-in-patch-request",
[typeof(ToOneRelationshipRequestData<>)] = "to-one-###-request-data",
[typeof(NullableToOneRelationshipRequestData<>)] = "nullable-to-one-###-request-data",
[typeof(ToManyRelationshipRequestData<>)] = "to-many-###-request-data",
[typeof(PrimaryResourceResponseDocument<>)] = "###-primary-response-document",
[typeof(SecondaryResourceResponseDocument<>)] = "###-secondary-response-document",
[typeof(ResourceCollectionResponseDocument<>)] = "###-collection-response-document",
[typeof(ResourceIdentifierResponseDocument<>)] = "###-identifier-response-document",
[typeof(ResourceIdentifierCollectionResponseDocument<>)] = "###-identifier-collection-response-document",
[typeof(ToOneRelationshipResponseData<>)] = "to-one-###-response-data",
[typeof(NullableToOneRelationshipResponseData<>)] = "nullable-to-one-###-response-data",
[typeof(ToManyRelationshipResponseData<>)] = "to-many-###-response-data",
[typeof(ResourceResponseObject<>)] = "###-data-in-response",
[typeof(ResourceIdentifierObject<>)] = "###-identifier"
Expand Down
Loading

0 comments on commit c6e8542

Please sign in to comment.