From 25b26925b429da57b8e196f90cd4f7534e32de02 Mon Sep 17 00:00:00 2001 From: Bart Koelman Date: Tue, 16 Nov 2021 15:08:07 +0100 Subject: [PATCH 01/29] Fixed inconsistencies in naming of resource type --- .../Queries/Expressions/SparseFieldTableExpression.cs | 4 ++-- src/JsonApiDotNetCore/Queries/IQueryLayerComposer.cs | 2 +- .../Queries/Internal/QueryLayerComposer.cs | 8 ++++---- .../Internal/SparseFieldSetQueryStringParameterReader.cs | 6 +++--- .../Serialization/Response/ResponseModelAdapter.cs | 6 +++--- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/JsonApiDotNetCore/Queries/Expressions/SparseFieldTableExpression.cs b/src/JsonApiDotNetCore/Queries/Expressions/SparseFieldTableExpression.cs index b17255fe3a..8543823199 100644 --- a/src/JsonApiDotNetCore/Queries/Expressions/SparseFieldTableExpression.cs +++ b/src/JsonApiDotNetCore/Queries/Expressions/SparseFieldTableExpression.cs @@ -30,14 +30,14 @@ public override string ToString() { var builder = new StringBuilder(); - foreach ((ResourceType resource, SparseFieldSetExpression fields) in Table) + foreach ((ResourceType resourceType, SparseFieldSetExpression fields) in Table) { if (builder.Length > 0) { builder.Append(','); } - builder.Append(resource.PublicName); + builder.Append(resourceType.PublicName); builder.Append('('); builder.Append(fields); builder.Append(')'); diff --git a/src/JsonApiDotNetCore/Queries/IQueryLayerComposer.cs b/src/JsonApiDotNetCore/Queries/IQueryLayerComposer.cs index 2fbee698ca..b81c9bacd4 100644 --- a/src/JsonApiDotNetCore/Queries/IQueryLayerComposer.cs +++ b/src/JsonApiDotNetCore/Queries/IQueryLayerComposer.cs @@ -46,7 +46,7 @@ QueryLayer WrapLayerForSecondaryEndpoint(QueryLayer secondaryLayer, Resourc /// Builds a query that retrieves the primary resource, including all of its attributes and all targeted relationships, during a create/update/delete /// request. /// - QueryLayer ComposeForUpdate(TId id, ResourceType primaryResource); + QueryLayer ComposeForUpdate(TId id, ResourceType primaryResourceType); /// /// Builds a query for each targeted relationship with a filter to match on its right resource IDs. diff --git a/src/JsonApiDotNetCore/Queries/Internal/QueryLayerComposer.cs b/src/JsonApiDotNetCore/Queries/Internal/QueryLayerComposer.cs index 2f8965070a..d1a55551de 100644 --- a/src/JsonApiDotNetCore/Queries/Internal/QueryLayerComposer.cs +++ b/src/JsonApiDotNetCore/Queries/Internal/QueryLayerComposer.cs @@ -382,16 +382,16 @@ private IncludeExpression RewriteIncludeForSecondaryEndpoint(IncludeExpression? } /// - public QueryLayer ComposeForUpdate(TId id, ResourceType primaryResource) + public QueryLayer ComposeForUpdate(TId id, ResourceType primaryResourceType) { - ArgumentGuard.NotNull(primaryResource, nameof(primaryResource)); + ArgumentGuard.NotNull(primaryResourceType, nameof(primaryResourceType)); IImmutableSet includeElements = _targetedFields.Relationships .Select(relationship => new IncludeElementExpression(relationship)).ToImmutableHashSet(); - AttrAttribute primaryIdAttribute = GetIdAttribute(primaryResource); + AttrAttribute primaryIdAttribute = GetIdAttribute(primaryResourceType); - QueryLayer primaryLayer = ComposeTopLayer(Array.Empty(), primaryResource); + QueryLayer primaryLayer = ComposeTopLayer(Array.Empty(), primaryResourceType); primaryLayer.Include = includeElements.Any() ? new IncludeExpression(includeElements) : IncludeExpression.Empty; primaryLayer.Sort = null; primaryLayer.Pagination = null; diff --git a/src/JsonApiDotNetCore/QueryStrings/Internal/SparseFieldSetQueryStringParameterReader.cs b/src/JsonApiDotNetCore/QueryStrings/Internal/SparseFieldSetQueryStringParameterReader.cs index 111f82b7c2..07ba665f18 100644 --- a/src/JsonApiDotNetCore/QueryStrings/Internal/SparseFieldSetQueryStringParameterReader.cs +++ b/src/JsonApiDotNetCore/QueryStrings/Internal/SparseFieldSetQueryStringParameterReader.cs @@ -69,10 +69,10 @@ public virtual void Read(string parameterName, StringValues parameterValue) try { - ResourceType targetResource = GetSparseFieldType(parameterName); - SparseFieldSetExpression sparseFieldSet = GetSparseFieldSet(parameterValue, targetResource); + ResourceType targetResourceType = GetSparseFieldType(parameterName); + SparseFieldSetExpression sparseFieldSet = GetSparseFieldSet(parameterValue, targetResourceType); - _sparseFieldTableBuilder[targetResource] = sparseFieldSet; + _sparseFieldTableBuilder[targetResourceType] = sparseFieldSet; } catch (QueryParseException exception) { diff --git a/src/JsonApiDotNetCore/Serialization/Response/ResponseModelAdapter.cs b/src/JsonApiDotNetCore/Serialization/Response/ResponseModelAdapter.cs index 40dd7c045c..b729d1ed52 100644 --- a/src/JsonApiDotNetCore/Serialization/Response/ResponseModelAdapter.cs +++ b/src/JsonApiDotNetCore/Serialization/Response/ResponseModelAdapter.cs @@ -211,12 +211,12 @@ protected virtual ResourceObject ConvertResource(IIdentifiable resource, Resourc return resourceObject; } - protected virtual IDictionary? ConvertAttributes(IIdentifiable resource, ResourceType resourceType, + protected virtual IDictionary? ConvertAttributes(IIdentifiable resource, ResourceType type, IImmutableSet fieldSet) { - var attrMap = new Dictionary(resourceType.Attributes.Count); + var attrMap = new Dictionary(type.Attributes.Count); - foreach (AttrAttribute attr in resourceType.Attributes) + foreach (AttrAttribute attr in type.Attributes) { if (!fieldSet.Contains(attr) || attr.Property.Name == nameof(Identifiable.Id)) { From 8d176667feca2f8e2079ef2ee0fe175943db06d9 Mon Sep 17 00:00:00 2001 From: Bart Koelman Date: Mon, 15 Nov 2021 13:04:18 +0100 Subject: [PATCH 02/29] Fixed: Query to determine initial state is sent to the read-only database on POST many-to-many and DELETE to-many requests --- src/JsonApiDotNetCore/Services/JsonApiResourceService.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/JsonApiDotNetCore/Services/JsonApiResourceService.cs b/src/JsonApiDotNetCore/Services/JsonApiResourceService.cs index 8ddfaf8bce..196772bf41 100644 --- a/src/JsonApiDotNetCore/Services/JsonApiResourceService.cs +++ b/src/JsonApiDotNetCore/Services/JsonApiResourceService.cs @@ -347,9 +347,7 @@ private async Task GetForHasManyUpdateAsync(HasManyAttribute hasManyR CancellationToken cancellationToken) { QueryLayer queryLayer = _queryLayerComposer.ComposeForHasMany(hasManyRelationship, leftId, rightResourceIds); - IReadOnlyCollection leftResources = await _repositoryAccessor.GetAsync(queryLayer, cancellationToken); - - TResource? leftResource = leftResources.FirstOrDefault(); + var leftResource = await _repositoryAccessor.GetForUpdateAsync(queryLayer, cancellationToken); AssertPrimaryResourceExists(leftResource); return leftResource; @@ -517,8 +515,8 @@ protected async Task GetPrimaryResourceForUpdateAsync(TId id, Cancell QueryLayer queryLayer = _queryLayerComposer.ComposeForUpdate(id, _request.PrimaryResourceType); var resource = await _repositoryAccessor.GetForUpdateAsync(queryLayer, cancellationToken); - AssertPrimaryResourceExists(resource); + return resource; } From f990010ca81e2f461cfdd59c8be9402c51591020 Mon Sep 17 00:00:00 2001 From: Bart Koelman Date: Tue, 9 Nov 2021 22:44:51 +0100 Subject: [PATCH 03/29] Make command/query controllers inherit from JsonApiController, passing null for missing services This produces HTTP 405 (Method Not Allowed) instead of 404 (Not Found), which better reveals the intent --- .../Controllers/JsonApiCommandController.cs | 59 ++----------------- .../Controllers/JsonApiQueryController.cs | 44 ++------------ 2 files changed, 9 insertions(+), 94 deletions(-) diff --git a/src/JsonApiDotNetCore/Controllers/JsonApiCommandController.cs b/src/JsonApiDotNetCore/Controllers/JsonApiCommandController.cs index 5c641804e5..bbfa52af89 100644 --- a/src/JsonApiDotNetCore/Controllers/JsonApiCommandController.cs +++ b/src/JsonApiDotNetCore/Controllers/JsonApiCommandController.cs @@ -1,18 +1,13 @@ -using System.Collections.Generic; -using System.Threading; -using System.Threading.Tasks; using JsonApiDotNetCore.Configuration; using JsonApiDotNetCore.Resources; using JsonApiDotNetCore.Services; -using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Logging; namespace JsonApiDotNetCore.Controllers { /// - /// The base class to derive resource-specific write-only controllers from. This class delegates all work to - /// but adds attributes for routing templates. If you want to provide routing templates yourself, - /// you should derive from BaseJsonApiController directly. + /// The base class to derive resource-specific write-only controllers from. Returns HTTP 405 on read-only endpoints. If you want to provide routing + /// templates yourself, you should derive from BaseJsonApiController directly. /// /// /// The resource type. @@ -20,7 +15,7 @@ namespace JsonApiDotNetCore.Controllers /// /// The resource identifier type. /// - public abstract class JsonApiCommandController : BaseJsonApiController + public abstract class JsonApiCommandController : JsonApiController where TResource : class, IIdentifiable { /// @@ -28,53 +23,9 @@ public abstract class JsonApiCommandController : BaseJsonApiCont /// protected JsonApiCommandController(IJsonApiOptions options, IResourceGraph resourceGraph, ILoggerFactory loggerFactory, IResourceCommandService commandService) - : base(options, resourceGraph, loggerFactory, null, commandService) + : base(options, resourceGraph, loggerFactory, null, null, null, null, commandService, commandService, commandService, commandService, + commandService, commandService) { } - - /// - [HttpPost] - public override async Task PostAsync([FromBody] TResource resource, CancellationToken cancellationToken) - { - return await base.PostAsync(resource, cancellationToken); - } - - /// - [HttpPost("{id}/relationships/{relationshipName}")] - public override async Task PostRelationshipAsync(TId id, string relationshipName, [FromBody] ISet rightResourceIds, - CancellationToken cancellationToken) - { - return await base.PostRelationshipAsync(id, relationshipName, rightResourceIds, cancellationToken); - } - - /// - [HttpPatch("{id}")] - public override async Task PatchAsync(TId id, [FromBody] TResource resource, CancellationToken cancellationToken) - { - return await base.PatchAsync(id, resource, cancellationToken); - } - - /// - [HttpPatch("{id}/relationships/{relationshipName}")] - public override async Task PatchRelationshipAsync(TId id, string relationshipName, [FromBody] object? rightValue, - CancellationToken cancellationToken) - { - return await base.PatchRelationshipAsync(id, relationshipName, rightValue, cancellationToken); - } - - /// - [HttpDelete("{id}")] - public override async Task DeleteAsync(TId id, CancellationToken cancellationToken) - { - return await base.DeleteAsync(id, cancellationToken); - } - - /// - [HttpDelete("{id}/relationships/{relationshipName}")] - public override async Task DeleteRelationshipAsync(TId id, string relationshipName, [FromBody] ISet rightResourceIds, - CancellationToken cancellationToken) - { - return await base.DeleteRelationshipAsync(id, relationshipName, rightResourceIds, cancellationToken); - } } } diff --git a/src/JsonApiDotNetCore/Controllers/JsonApiQueryController.cs b/src/JsonApiDotNetCore/Controllers/JsonApiQueryController.cs index ce4ef04718..d56eab8d60 100644 --- a/src/JsonApiDotNetCore/Controllers/JsonApiQueryController.cs +++ b/src/JsonApiDotNetCore/Controllers/JsonApiQueryController.cs @@ -1,17 +1,13 @@ -using System.Threading; -using System.Threading.Tasks; using JsonApiDotNetCore.Configuration; using JsonApiDotNetCore.Resources; using JsonApiDotNetCore.Services; -using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Logging; namespace JsonApiDotNetCore.Controllers { /// - /// The base class to derive resource-specific read-only controllers from. This class delegates all work to - /// but adds attributes for routing templates. If you want to provide routing templates yourself, - /// you should derive from BaseJsonApiController directly. + /// The base class to derive resource-specific read-only controllers from. Returns HTTP 405 on write-only endpoints. If you want to provide routing + /// templates yourself, you should derive from BaseJsonApiController directly. /// /// /// The resource type. @@ -19,7 +15,7 @@ namespace JsonApiDotNetCore.Controllers /// /// The resource identifier type. /// - public abstract class JsonApiQueryController : BaseJsonApiController + public abstract class JsonApiQueryController : JsonApiController where TResource : class, IIdentifiable { /// @@ -27,40 +23,8 @@ public abstract class JsonApiQueryController : BaseJsonApiContro /// protected JsonApiQueryController(IJsonApiOptions options, IResourceGraph resourceGraph, ILoggerFactory loggerFactory, IResourceQueryService queryService) - : base(options, resourceGraph, loggerFactory, queryService) + : base(options, resourceGraph, loggerFactory, queryService, queryService, queryService, queryService) { } - - /// - [HttpGet] - [HttpHead] - public override async Task GetAsync(CancellationToken cancellationToken) - { - return await base.GetAsync(cancellationToken); - } - - /// - [HttpGet("{id}")] - [HttpHead("{id}")] - public override async Task GetAsync(TId id, CancellationToken cancellationToken) - { - return await base.GetAsync(id, cancellationToken); - } - - /// - [HttpGet("{id}/{relationshipName}")] - [HttpHead("{id}/{relationshipName}")] - public override async Task GetSecondaryAsync(TId id, string relationshipName, CancellationToken cancellationToken) - { - return await base.GetSecondaryAsync(id, relationshipName, cancellationToken); - } - - /// - [HttpGet("{id}/relationships/{relationshipName}")] - [HttpHead("{id}/relationships/{relationshipName}")] - public override async Task GetRelationshipAsync(TId id, string relationshipName, CancellationToken cancellationToken) - { - return await base.GetRelationshipAsync(id, relationshipName, cancellationToken); - } } } From 61aa23eb478ab10aebbe41196d420d1f3ba19b9c Mon Sep 17 00:00:00 2001 From: Bart Koelman Date: Tue, 9 Nov 2021 22:56:04 +0100 Subject: [PATCH 04/29] Added overload on TypeExtensions.IsOrImplementsInterface to take a type parameter, used for non-generic or generic constructed interfaces --- src/JsonApiDotNetCore/CollectionConverter.cs | 2 +- .../Configuration/ResourceGraphBuilder.cs | 2 +- .../Configuration/TypeLocator.cs | 2 +- .../Middleware/JsonApiRoutingConvention.cs | 2 +- src/JsonApiDotNetCore/TypeExtensions.cs | 18 ++++++++++++++++-- test/UnitTests/Internal/TypeExtensionsTests.cs | 4 ++-- 6 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/JsonApiDotNetCore/CollectionConverter.cs b/src/JsonApiDotNetCore/CollectionConverter.cs index 5c5dcba845..1ab1768cb9 100644 --- a/src/JsonApiDotNetCore/CollectionConverter.cs +++ b/src/JsonApiDotNetCore/CollectionConverter.cs @@ -98,7 +98,7 @@ public ICollection ExtractResources(object? value) { if (type.IsGenericType && type.GenericTypeArguments.Length == 1) { - if (type.IsOrImplementsInterface(typeof(IEnumerable))) + if (type.IsOrImplementsInterface()) { return type.GenericTypeArguments[0]; } diff --git a/src/JsonApiDotNetCore/Configuration/ResourceGraphBuilder.cs b/src/JsonApiDotNetCore/Configuration/ResourceGraphBuilder.cs index 7d545a8490..ce3ac57e01 100644 --- a/src/JsonApiDotNetCore/Configuration/ResourceGraphBuilder.cs +++ b/src/JsonApiDotNetCore/Configuration/ResourceGraphBuilder.cs @@ -143,7 +143,7 @@ public ResourceGraphBuilder Add(Type resourceClrType, Type? idClrType = null, st return this; } - if (resourceClrType.IsOrImplementsInterface(typeof(IIdentifiable))) + if (resourceClrType.IsOrImplementsInterface()) { string effectivePublicName = publicName ?? FormatResourceName(resourceClrType); Type? effectiveIdType = idClrType ?? _typeLocator.LookupIdType(resourceClrType); diff --git a/src/JsonApiDotNetCore/Configuration/TypeLocator.cs b/src/JsonApiDotNetCore/Configuration/TypeLocator.cs index 6282ef05f4..9ef4f3590e 100644 --- a/src/JsonApiDotNetCore/Configuration/TypeLocator.cs +++ b/src/JsonApiDotNetCore/Configuration/TypeLocator.cs @@ -27,7 +27,7 @@ internal sealed class TypeLocator /// public ResourceDescriptor? ResolveResourceDescriptor(Type? type) { - if (type != null && type.IsOrImplementsInterface(typeof(IIdentifiable))) + if (type != null && type.IsOrImplementsInterface()) { Type? idType = LookupIdType(type); diff --git a/src/JsonApiDotNetCore/Middleware/JsonApiRoutingConvention.cs b/src/JsonApiDotNetCore/Middleware/JsonApiRoutingConvention.cs index 04ceb1c038..d304fc9d1c 100644 --- a/src/JsonApiDotNetCore/Middleware/JsonApiRoutingConvention.cs +++ b/src/JsonApiDotNetCore/Middleware/JsonApiRoutingConvention.cs @@ -162,7 +162,7 @@ private string TemplateFromController(ControllerModel model) if ((nextBaseType == aspNetControllerType || nextBaseType == coreControllerType) && currentType.IsGenericType) { Type? resourceClrType = currentType.GetGenericArguments() - .FirstOrDefault(typeArgument => typeArgument.IsOrImplementsInterface(typeof(IIdentifiable))); + .FirstOrDefault(typeArgument => typeArgument.IsOrImplementsInterface()); if (resourceClrType != null) { diff --git a/src/JsonApiDotNetCore/TypeExtensions.cs b/src/JsonApiDotNetCore/TypeExtensions.cs index 41450afdf7..18e6dc6967 100644 --- a/src/JsonApiDotNetCore/TypeExtensions.cs +++ b/src/JsonApiDotNetCore/TypeExtensions.cs @@ -8,7 +8,15 @@ internal static class TypeExtensions /// /// Whether the specified source type implements or equals the specified interface. /// - public static bool IsOrImplementsInterface(this Type? source, Type interfaceType) + public static bool IsOrImplementsInterface(this Type? source) + { + return IsOrImplementsInterface(source, typeof(TInterface)); + } + + /// + /// Whether the specified source type implements or equals the specified interface. This overload enables to test for an open generic interface. + /// + private static bool IsOrImplementsInterface(this Type? source, Type interfaceType) { ArgumentGuard.NotNull(interfaceType, nameof(interfaceType)); @@ -17,7 +25,13 @@ public static bool IsOrImplementsInterface(this Type? source, Type interfaceType return false; } - return source == interfaceType || source.GetInterfaces().Any(type => type == interfaceType); + return AreTypesEqual(interfaceType, source, interfaceType.IsGenericType) || + source.GetInterfaces().Any(type => AreTypesEqual(interfaceType, type, interfaceType.IsGenericType)); + } + + private static bool AreTypesEqual(Type left, Type right, bool isLeftGeneric) + { + return isLeftGeneric ? right.IsGenericType && right.GetGenericTypeDefinition() == left : left == right; } /// diff --git a/test/UnitTests/Internal/TypeExtensionsTests.cs b/test/UnitTests/Internal/TypeExtensionsTests.cs index 01e16dfffe..494e91510e 100644 --- a/test/UnitTests/Internal/TypeExtensionsTests.cs +++ b/test/UnitTests/Internal/TypeExtensionsTests.cs @@ -15,7 +15,7 @@ public void Implements_Returns_True_If_Type_Implements_Interface() Type type = typeof(Model); // Act - bool result = type.IsOrImplementsInterface(typeof(IIdentifiable)); + bool result = type.IsOrImplementsInterface(); // Assert result.Should().BeTrue(); @@ -28,7 +28,7 @@ public void Implements_Returns_False_If_Type_DoesNot_Implement_Interface() Type type = typeof(string); // Act - bool result = type.IsOrImplementsInterface(typeof(IIdentifiable)); + bool result = type.IsOrImplementsInterface(); // Assert result.Should().BeFalse(); From caa131e20609a99dc2011bcb943a77d322fadff4 Mon Sep 17 00:00:00 2001 From: Bart Koelman Date: Wed, 17 Nov 2021 17:07:02 +0100 Subject: [PATCH 05/29] Use ResourceType instead of public name in local-id tracker --- .../AtomicOperations/ILocalIdTracker.cs | 8 ++++--- .../AtomicOperations/LocalIdTracker.cs | 23 ++++++++++--------- .../AtomicOperations/LocalIdValidator.cs | 6 ++--- .../AtomicOperations/OperationsProcessor.cs | 4 ++-- .../Processors/CreateProcessor.cs | 5 +--- 5 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/JsonApiDotNetCore/AtomicOperations/ILocalIdTracker.cs b/src/JsonApiDotNetCore/AtomicOperations/ILocalIdTracker.cs index eb61e41371..1f6eda010d 100644 --- a/src/JsonApiDotNetCore/AtomicOperations/ILocalIdTracker.cs +++ b/src/JsonApiDotNetCore/AtomicOperations/ILocalIdTracker.cs @@ -1,3 +1,5 @@ +using JsonApiDotNetCore.Configuration; + namespace JsonApiDotNetCore.AtomicOperations { /// @@ -13,16 +15,16 @@ public interface ILocalIdTracker /// /// Declares a local ID without assigning a server-generated value. /// - void Declare(string localId, string resourceType); + void Declare(string localId, ResourceType resourceType); /// /// Assigns a server-generated ID value to a previously declared local ID. /// - void Assign(string localId, string resourceType, string stringId); + void Assign(string localId, ResourceType resourceType, string stringId); /// /// Gets the server-assigned ID for the specified local ID. /// - string GetValue(string localId, string resourceType); + string GetValue(string localId, ResourceType resourceType); } } diff --git a/src/JsonApiDotNetCore/AtomicOperations/LocalIdTracker.cs b/src/JsonApiDotNetCore/AtomicOperations/LocalIdTracker.cs index 408553529e..9b24ff4e18 100644 --- a/src/JsonApiDotNetCore/AtomicOperations/LocalIdTracker.cs +++ b/src/JsonApiDotNetCore/AtomicOperations/LocalIdTracker.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using JsonApiDotNetCore.Configuration; using JsonApiDotNetCore.Errors; namespace JsonApiDotNetCore.AtomicOperations @@ -16,10 +17,10 @@ public void Reset() } /// - public void Declare(string localId, string resourceType) + public void Declare(string localId, ResourceType resourceType) { ArgumentGuard.NotNullNorEmpty(localId, nameof(localId)); - ArgumentGuard.NotNullNorEmpty(resourceType, nameof(resourceType)); + ArgumentGuard.NotNull(resourceType, nameof(resourceType)); AssertIsNotDeclared(localId); @@ -35,10 +36,10 @@ private void AssertIsNotDeclared(string localId) } /// - public void Assign(string localId, string resourceType, string stringId) + public void Assign(string localId, ResourceType resourceType, string stringId) { ArgumentGuard.NotNullNorEmpty(localId, nameof(localId)); - ArgumentGuard.NotNullNorEmpty(resourceType, nameof(resourceType)); + ArgumentGuard.NotNull(resourceType, nameof(resourceType)); ArgumentGuard.NotNullNorEmpty(stringId, nameof(stringId)); AssertIsDeclared(localId); @@ -56,10 +57,10 @@ public void Assign(string localId, string resourceType, string stringId) } /// - public string GetValue(string localId, string resourceType) + public string GetValue(string localId, ResourceType resourceType) { ArgumentGuard.NotNullNorEmpty(localId, nameof(localId)); - ArgumentGuard.NotNullNorEmpty(resourceType, nameof(resourceType)); + ArgumentGuard.NotNull(resourceType, nameof(resourceType)); AssertIsDeclared(localId); @@ -83,20 +84,20 @@ private void AssertIsDeclared(string localId) } } - private static void AssertSameResourceType(string currentType, string declaredType, string localId) + private static void AssertSameResourceType(ResourceType currentType, ResourceType declaredType, string localId) { - if (declaredType != currentType) + if (!declaredType.Equals(currentType)) { - throw new IncompatibleLocalIdTypeException(localId, declaredType, currentType); + throw new IncompatibleLocalIdTypeException(localId, declaredType.PublicName, currentType.PublicName); } } private sealed class LocalIdState { - public string ResourceType { get; } + public ResourceType ResourceType { get; } public string? ServerId { get; set; } - public LocalIdState(string resourceType) + public LocalIdState(ResourceType resourceType) { ResourceType = resourceType; } diff --git a/src/JsonApiDotNetCore/AtomicOperations/LocalIdValidator.cs b/src/JsonApiDotNetCore/AtomicOperations/LocalIdValidator.cs index f08c1bc44c..670280e59b 100644 --- a/src/JsonApiDotNetCore/AtomicOperations/LocalIdValidator.cs +++ b/src/JsonApiDotNetCore/AtomicOperations/LocalIdValidator.cs @@ -81,7 +81,7 @@ private void DeclareLocalId(IIdentifiable resource, ResourceType resourceType) { if (resource.LocalId != null) { - _localIdTracker.Declare(resource.LocalId, resourceType.PublicName); + _localIdTracker.Declare(resource.LocalId, resourceType); } } @@ -89,7 +89,7 @@ private void AssignLocalId(OperationContainer operation, ResourceType resourceTy { if (operation.Resource.LocalId != null) { - _localIdTracker.Assign(operation.Resource.LocalId, resourceType.PublicName, "placeholder"); + _localIdTracker.Assign(operation.Resource.LocalId, resourceType, "placeholder"); } } @@ -98,7 +98,7 @@ private void AssertLocalIdIsAssigned(IIdentifiable resource) if (resource.LocalId != null) { ResourceType resourceType = _resourceGraph.GetResourceType(resource.GetType()); - _localIdTracker.GetValue(resource.LocalId, resourceType.PublicName); + _localIdTracker.GetValue(resource.LocalId, resourceType); } } } diff --git a/src/JsonApiDotNetCore/AtomicOperations/OperationsProcessor.cs b/src/JsonApiDotNetCore/AtomicOperations/OperationsProcessor.cs index b9cbe983e0..ceca03ccdf 100644 --- a/src/JsonApiDotNetCore/AtomicOperations/OperationsProcessor.cs +++ b/src/JsonApiDotNetCore/AtomicOperations/OperationsProcessor.cs @@ -136,7 +136,7 @@ private void DeclareLocalId(IIdentifiable resource, ResourceType resourceType) { if (resource.LocalId != null) { - _localIdTracker.Declare(resource.LocalId, resourceType.PublicName); + _localIdTracker.Declare(resource.LocalId, resourceType); } } @@ -145,7 +145,7 @@ private void AssignStringId(IIdentifiable resource) if (resource.LocalId != null) { ResourceType resourceType = _resourceGraph.GetResourceType(resource.GetType()); - resource.StringId = _localIdTracker.GetValue(resource.LocalId, resourceType.PublicName); + resource.StringId = _localIdTracker.GetValue(resource.LocalId, resourceType); } } } diff --git a/src/JsonApiDotNetCore/AtomicOperations/Processors/CreateProcessor.cs b/src/JsonApiDotNetCore/AtomicOperations/Processors/CreateProcessor.cs index 36cb8c573f..06d9ae485a 100644 --- a/src/JsonApiDotNetCore/AtomicOperations/Processors/CreateProcessor.cs +++ b/src/JsonApiDotNetCore/AtomicOperations/Processors/CreateProcessor.cs @@ -1,7 +1,6 @@ using System.Threading; using System.Threading.Tasks; using JetBrains.Annotations; -using JsonApiDotNetCore.Configuration; using JsonApiDotNetCore.Resources; using JsonApiDotNetCore.Services; @@ -34,9 +33,7 @@ public CreateProcessor(ICreateService service, ILocalIdTracker l if (operation.Resource.LocalId != null) { string serverId = newResource != null ? newResource.StringId! : operation.Resource.StringId!; - ResourceType resourceType = operation.Request.PrimaryResourceType!; - - _localIdTracker.Assign(operation.Resource.LocalId, resourceType.PublicName, serverId); + _localIdTracker.Assign(operation.Resource.LocalId, operation.Request.PrimaryResourceType!, serverId); } return newResource == null ? null : operation.WithResource(newResource); From 0328293226d91ff80de488f6960be18afd5014ad Mon Sep 17 00:00:00 2001 From: Bart Koelman Date: Thu, 18 Nov 2021 15:49:01 +0100 Subject: [PATCH 06/29] Change IQueryStringParameterReader.AllowEmptyValue into normal interface member --- .../QueryStrings/IQueryStringParameterReader.cs | 4 ++-- .../FilterQueryStringParameterReader.cs | 2 ++ .../IncludeQueryStringParameterReader.cs | 2 ++ .../PaginationQueryStringParameterReader.cs | 2 ++ ...esourceDefinitionQueryableParameterReader.cs | 2 ++ .../Internal/SortQueryStringParameterReader.cs | 2 ++ .../DisableQueryStringTests.cs | 17 ++++++++++++++++- .../SkipCacheQueryStringParameterReader.cs | 11 +++-------- 8 files changed, 31 insertions(+), 11 deletions(-) diff --git a/src/JsonApiDotNetCore/QueryStrings/IQueryStringParameterReader.cs b/src/JsonApiDotNetCore/QueryStrings/IQueryStringParameterReader.cs index 024ee564f2..ebfdac0976 100644 --- a/src/JsonApiDotNetCore/QueryStrings/IQueryStringParameterReader.cs +++ b/src/JsonApiDotNetCore/QueryStrings/IQueryStringParameterReader.cs @@ -9,9 +9,9 @@ namespace JsonApiDotNetCore.QueryStrings public interface IQueryStringParameterReader { /// - /// Indicates whether this reader supports empty query string parameter values. Defaults to false. + /// Indicates whether this reader supports empty query string parameter values. /// - bool AllowEmptyValue => false; + bool AllowEmptyValue { get; } /// /// Indicates whether usage of this query string parameter is blocked using on a controller. diff --git a/src/JsonApiDotNetCore/QueryStrings/Internal/FilterQueryStringParameterReader.cs b/src/JsonApiDotNetCore/QueryStrings/Internal/FilterQueryStringParameterReader.cs index 245aa7a787..30d1e5d904 100644 --- a/src/JsonApiDotNetCore/QueryStrings/Internal/FilterQueryStringParameterReader.cs +++ b/src/JsonApiDotNetCore/QueryStrings/Internal/FilterQueryStringParameterReader.cs @@ -29,6 +29,8 @@ public class FilterQueryStringParameterReader : QueryStringParameterReader, IFil private string? _lastParameterName; + public bool AllowEmptyValue => false; + public FilterQueryStringParameterReader(IJsonApiRequest request, IResourceGraph resourceGraph, IResourceFactory resourceFactory, IJsonApiOptions options) : base(request, resourceGraph) diff --git a/src/JsonApiDotNetCore/QueryStrings/Internal/IncludeQueryStringParameterReader.cs b/src/JsonApiDotNetCore/QueryStrings/Internal/IncludeQueryStringParameterReader.cs index 77a5bce864..f5e98b9a20 100644 --- a/src/JsonApiDotNetCore/QueryStrings/Internal/IncludeQueryStringParameterReader.cs +++ b/src/JsonApiDotNetCore/QueryStrings/Internal/IncludeQueryStringParameterReader.cs @@ -21,6 +21,8 @@ public class IncludeQueryStringParameterReader : QueryStringParameterReader, IIn private IncludeExpression? _includeExpression; private string? _lastParameterName; + public bool AllowEmptyValue => false; + public IncludeQueryStringParameterReader(IJsonApiRequest request, IResourceGraph resourceGraph, IJsonApiOptions options) : base(request, resourceGraph) { diff --git a/src/JsonApiDotNetCore/QueryStrings/Internal/PaginationQueryStringParameterReader.cs b/src/JsonApiDotNetCore/QueryStrings/Internal/PaginationQueryStringParameterReader.cs index 2dc722c621..023a4a67bd 100644 --- a/src/JsonApiDotNetCore/QueryStrings/Internal/PaginationQueryStringParameterReader.cs +++ b/src/JsonApiDotNetCore/QueryStrings/Internal/PaginationQueryStringParameterReader.cs @@ -25,6 +25,8 @@ public class PaginationQueryStringParameterReader : QueryStringParameterReader, private PaginationQueryStringValueExpression? _pageSizeConstraint; private PaginationQueryStringValueExpression? _pageNumberConstraint; + public bool AllowEmptyValue => false; + public PaginationQueryStringParameterReader(IJsonApiRequest request, IResourceGraph resourceGraph, IJsonApiOptions options) : base(request, resourceGraph) { diff --git a/src/JsonApiDotNetCore/QueryStrings/Internal/ResourceDefinitionQueryableParameterReader.cs b/src/JsonApiDotNetCore/QueryStrings/Internal/ResourceDefinitionQueryableParameterReader.cs index 125700b0a3..bc53a9ed4d 100644 --- a/src/JsonApiDotNetCore/QueryStrings/Internal/ResourceDefinitionQueryableParameterReader.cs +++ b/src/JsonApiDotNetCore/QueryStrings/Internal/ResourceDefinitionQueryableParameterReader.cs @@ -19,6 +19,8 @@ public class ResourceDefinitionQueryableParameterReader : IResourceDefinitionQue private readonly IResourceDefinitionAccessor _resourceDefinitionAccessor; private readonly List _constraints = new(); + public bool AllowEmptyValue => false; + public ResourceDefinitionQueryableParameterReader(IJsonApiRequest request, IResourceDefinitionAccessor resourceDefinitionAccessor) { ArgumentGuard.NotNull(request, nameof(request)); diff --git a/src/JsonApiDotNetCore/QueryStrings/Internal/SortQueryStringParameterReader.cs b/src/JsonApiDotNetCore/QueryStrings/Internal/SortQueryStringParameterReader.cs index 80f3510766..d231f176f5 100644 --- a/src/JsonApiDotNetCore/QueryStrings/Internal/SortQueryStringParameterReader.cs +++ b/src/JsonApiDotNetCore/QueryStrings/Internal/SortQueryStringParameterReader.cs @@ -21,6 +21,8 @@ public class SortQueryStringParameterReader : QueryStringParameterReader, ISortQ private readonly List _constraints = new(); private string? _lastParameterName; + public bool AllowEmptyValue => false; + public SortQueryStringParameterReader(IJsonApiRequest request, IResourceGraph resourceGraph) : base(request, resourceGraph) { diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/RestrictedControllers/DisableQueryStringTests.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/RestrictedControllers/DisableQueryStringTests.cs index 3c367218e9..3c98cb4e3e 100644 --- a/test/JsonApiDotNetCoreTests/IntegrationTests/RestrictedControllers/DisableQueryStringTests.cs +++ b/test/JsonApiDotNetCoreTests/IntegrationTests/RestrictedControllers/DisableQueryStringTests.cs @@ -72,11 +72,26 @@ public async Task Cannot_paginate_if_query_string_parameter_is_blocked_by_contro error.Source.Parameter.Should().Be("page[number]"); } + [Fact] + public async Task Can_use_custom_query_string_parameter() + { + // Arrange + const string route = "/sofas?skipCache"; + + // Act + (HttpResponseMessage httpResponse, string responseDocument) = await _testContext.ExecuteGetAsync(route); + + // Assert + httpResponse.Should().HaveStatusCode(HttpStatusCode.OK); + + responseDocument.ShouldNotBeEmpty(); + } + [Fact] public async Task Cannot_use_custom_query_string_parameter_if_blocked_by_controller() { // Arrange - const string route = "/beds?skipCache=true"; + const string route = "/beds?skipCache"; // Act (HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecuteGetAsync(route); diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/RestrictedControllers/SkipCacheQueryStringParameterReader.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/RestrictedControllers/SkipCacheQueryStringParameterReader.cs index 43fe12f42a..ec8e731983 100644 --- a/test/JsonApiDotNetCoreTests/IntegrationTests/RestrictedControllers/SkipCacheQueryStringParameterReader.cs +++ b/test/JsonApiDotNetCoreTests/IntegrationTests/RestrictedControllers/SkipCacheQueryStringParameterReader.cs @@ -1,6 +1,5 @@ using JetBrains.Annotations; using JsonApiDotNetCore.Controllers.Annotations; -using JsonApiDotNetCore.Errors; using JsonApiDotNetCore.QueryStrings; using Microsoft.Extensions.Primitives; @@ -13,6 +12,8 @@ public sealed class SkipCacheQueryStringParameterReader : IQueryStringParameterR [UsedImplicitly] public bool SkipCache { get; private set; } + public bool AllowEmptyValue => true; + public bool IsEnabled(DisableQueryStringAttribute disableQueryStringAttribute) { return !disableQueryStringAttribute.ParameterNames.Contains(SkipCacheParameterName); @@ -25,13 +26,7 @@ public bool CanRead(string parameterName) public void Read(string parameterName, StringValues parameterValue) { - if (!bool.TryParse(parameterValue, out bool skipCache)) - { - throw new InvalidQueryStringParameterException(parameterName, "Boolean value required.", - $"The value '{parameterValue}' is not a valid boolean."); - } - - SkipCache = skipCache; + SkipCache = true; } } } From d343c9d26ea405ca1c75b23844d94a1183e3dc1a Mon Sep 17 00:00:00 2001 From: Bart Koelman Date: Thu, 18 Nov 2021 15:59:49 +0100 Subject: [PATCH 07/29] Simplified startup in example project --- src/Examples/JsonApiDotNetCoreExample/Startup.cs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/Examples/JsonApiDotNetCoreExample/Startup.cs b/src/Examples/JsonApiDotNetCoreExample/Startup.cs index 84ebc30360..9e0b608f85 100644 --- a/src/Examples/JsonApiDotNetCoreExample/Startup.cs +++ b/src/Examples/JsonApiDotNetCoreExample/Startup.cs @@ -62,17 +62,13 @@ public void ConfigureServices(IServiceCollection services) } // This method gets called by the runtime. Use this method to configure the HTTP request pipeline. - public void Configure(IApplicationBuilder app, IWebHostEnvironment environment, ILoggerFactory loggerFactory) + public void Configure(IApplicationBuilder app, IWebHostEnvironment environment, ILoggerFactory loggerFactory, AppDbContext dbContext) { ILogger logger = loggerFactory.CreateLogger(); using (CodeTimingSessionManager.Current.Measure("Initialize other (startup)")) { - using (IServiceScope scope = app.ApplicationServices.CreateScope()) - { - var appDbContext = scope.ServiceProvider.GetRequiredService(); - appDbContext.Database.EnsureCreated(); - } + dbContext.Database.EnsureCreated(); app.UseRouting(); From 760df70d1dea6dd7fcc22a34fd53bda1383ecf3f Mon Sep 17 00:00:00 2001 From: Bart Koelman Date: Thu, 18 Nov 2021 17:09:26 +0100 Subject: [PATCH 08/29] Removed left-over overload with single type parameter in ResourceGraphBuilder --- .../DeserializationBenchmarkBase.cs | 2 +- .../QueryStringParserBenchmarks.cs | 3 ++- .../SerializationBenchmarkBase.cs | 2 +- .../NoEntityFrameworkExample/Startup.cs | 2 +- .../Configuration/ResourceGraphBuilder.cs | 16 ------------ .../ModelStateValidationTests.cs | 4 +-- .../QueryStringParameters/BaseParseTests.cs | 14 +++++----- .../ResourceGraphBuilderTests.cs | 26 +++++++++---------- .../Serialization/InputConversionTests.cs | 2 +- .../Response/ResponseModelAdapterTests.cs | 10 +++---- .../Middleware/JsonApiRequestTests.cs | 6 ++--- 11 files changed, 36 insertions(+), 51 deletions(-) diff --git a/benchmarks/Deserialization/DeserializationBenchmarkBase.cs b/benchmarks/Deserialization/DeserializationBenchmarkBase.cs index 8b2deb98b9..b21d7c85e7 100644 --- a/benchmarks/Deserialization/DeserializationBenchmarkBase.cs +++ b/benchmarks/Deserialization/DeserializationBenchmarkBase.cs @@ -21,7 +21,7 @@ public abstract class DeserializationBenchmarkBase protected DeserializationBenchmarkBase() { var options = new JsonApiOptions(); - IResourceGraph resourceGraph = new ResourceGraphBuilder(options, NullLoggerFactory.Instance).Add().Build(); + IResourceGraph resourceGraph = new ResourceGraphBuilder(options, NullLoggerFactory.Instance).Add().Build(); options.SerializerOptions.Converters.Add(new ResourceObjectConverter(resourceGraph)); SerializerReadOptions = ((IJsonApiOptions)options).SerializerReadOptions; diff --git a/benchmarks/QueryString/QueryStringParserBenchmarks.cs b/benchmarks/QueryString/QueryStringParserBenchmarks.cs index 6e576bc4a7..42d34f8ce4 100644 --- a/benchmarks/QueryString/QueryStringParserBenchmarks.cs +++ b/benchmarks/QueryString/QueryStringParserBenchmarks.cs @@ -29,7 +29,8 @@ public QueryStringParserBenchmarks() EnableLegacyFilterNotation = true }; - IResourceGraph resourceGraph = new ResourceGraphBuilder(options, NullLoggerFactory.Instance).Add("alt-resource-name").Build(); + IResourceGraph resourceGraph = + new ResourceGraphBuilder(options, NullLoggerFactory.Instance).Add("alt-resource-name").Build(); var request = new JsonApiRequest { diff --git a/benchmarks/Serialization/SerializationBenchmarkBase.cs b/benchmarks/Serialization/SerializationBenchmarkBase.cs index 2abde17e42..dc02951dfa 100644 --- a/benchmarks/Serialization/SerializationBenchmarkBase.cs +++ b/benchmarks/Serialization/SerializationBenchmarkBase.cs @@ -40,7 +40,7 @@ protected SerializationBenchmarkBase() } }; - ResourceGraph = new ResourceGraphBuilder(options, NullLoggerFactory.Instance).Add().Build(); + ResourceGraph = new ResourceGraphBuilder(options, NullLoggerFactory.Instance).Add().Build(); SerializerWriteOptions = ((IJsonApiOptions)options).SerializerWriteOptions; // ReSharper disable VirtualMemberCallInConstructor diff --git a/src/Examples/NoEntityFrameworkExample/Startup.cs b/src/Examples/NoEntityFrameworkExample/Startup.cs index 14642d408f..dc86192832 100644 --- a/src/Examples/NoEntityFrameworkExample/Startup.cs +++ b/src/Examples/NoEntityFrameworkExample/Startup.cs @@ -24,7 +24,7 @@ public Startup(IConfiguration configuration) // This method gets called by the runtime. Use this method to add services to the container. public void ConfigureServices(IServiceCollection services) { - services.AddJsonApi(options => options.Namespace = "api/v1", resources: builder => builder.Add("workItems")); + services.AddJsonApi(options => options.Namespace = "api/v1", resources: builder => builder.Add("workItems")); services.AddResourceService(); diff --git a/src/JsonApiDotNetCore/Configuration/ResourceGraphBuilder.cs b/src/JsonApiDotNetCore/Configuration/ResourceGraphBuilder.cs index ce3ac57e01..53e206cc0f 100644 --- a/src/JsonApiDotNetCore/Configuration/ResourceGraphBuilder.cs +++ b/src/JsonApiDotNetCore/Configuration/ResourceGraphBuilder.cs @@ -86,22 +86,6 @@ private static bool IsImplicitManyToManyJoinEntity(IEntityType entityType) #pragma warning restore EF1001 // Internal Entity Framework Core API usage. } - /// - /// Adds a JSON:API resource with int as the identifier CLR type. - /// - /// - /// The resource CLR type. - /// - /// - /// The name under which the resource is publicly exposed by the API. If nothing is specified, the naming convention is applied on the pluralized CLR - /// type name. - /// - public ResourceGraphBuilder Add(string? publicName = null) - where TResource : class, IIdentifiable - { - return Add(publicName); - } - /// /// Adds a JSON:API resource. /// diff --git a/test/JsonApiDotNetCoreTests/UnitTests/ModelStateValidation/ModelStateValidationTests.cs b/test/JsonApiDotNetCoreTests/UnitTests/ModelStateValidation/ModelStateValidationTests.cs index a6dafd3b35..efb63a856b 100644 --- a/test/JsonApiDotNetCoreTests/UnitTests/ModelStateValidation/ModelStateValidationTests.cs +++ b/test/JsonApiDotNetCoreTests/UnitTests/ModelStateValidation/ModelStateValidationTests.cs @@ -39,7 +39,7 @@ public void Renders_JSON_path_for_ModelState_key_in_resource_request(string mode { // Arrange var options = new JsonApiOptions(); - IResourceGraph resourceGraph = new ResourceGraphBuilder(options, NullLoggerFactory.Instance).Add().Add().Build(); + IResourceGraph resourceGraph = new ResourceGraphBuilder(options, NullLoggerFactory.Instance).Add().Add().Build(); var modelState = new ModelStateDictionary(); modelState.AddModelError(modelStateKey, "(ignored error message)"); @@ -87,7 +87,7 @@ public void Renders_JSON_path_for_ModelState_key_in_operations_request(string mo { // Arrange var options = new JsonApiOptions(); - IResourceGraph resourceGraph = new ResourceGraphBuilder(options, NullLoggerFactory.Instance).Add().Add().Build(); + IResourceGraph resourceGraph = new ResourceGraphBuilder(options, NullLoggerFactory.Instance).Add().Add().Build(); var modelState = new ModelStateDictionary(); modelState.AddModelError(modelStateKey, "(ignored error message)"); diff --git a/test/JsonApiDotNetCoreTests/UnitTests/QueryStringParameters/BaseParseTests.cs b/test/JsonApiDotNetCoreTests/UnitTests/QueryStringParameters/BaseParseTests.cs index 193a5c1ea9..c0929a162f 100644 --- a/test/JsonApiDotNetCoreTests/UnitTests/QueryStringParameters/BaseParseTests.cs +++ b/test/JsonApiDotNetCoreTests/UnitTests/QueryStringParameters/BaseParseTests.cs @@ -19,13 +19,13 @@ protected BaseParseTests() // @formatter:keep_existing_linebreaks true ResourceGraph = new ResourceGraphBuilder(Options, NullLoggerFactory.Instance) - .Add() - .Add() - .Add