From aa763bf68e77daff7f075244658a7a3497a13c4d Mon Sep 17 00:00:00 2001 From: Bart Koelman <10324372+bkoelman@users.noreply.github.com> Date: Sun, 10 Nov 2024 21:51:23 +0100 Subject: [PATCH] Fixed: If an operation is enabled on a base type, it should implicitly be enabled on all derived types --- .../DefaultOperationFilter.cs | 30 ++++- .../AtomicOperationTests.cs | 80 +++++++++++++ .../ChangeTracking/ChangeTrackingDbContext.cs | 6 +- .../Models/AlwaysMovingTandem.cs | 3 +- .../OperationsController.cs | 13 +++ .../ResourceInheritanceDbContext.cs | 1 + .../DefaultOperationFilterTests.cs | 105 ++++++++++++++++++ 7 files changed, 229 insertions(+), 9 deletions(-) create mode 100644 test/JsonApiDotNetCoreTests/IntegrationTests/ResourceInheritance/AtomicOperationTests.cs create mode 100644 test/JsonApiDotNetCoreTests/IntegrationTests/ResourceInheritance/OperationsController.cs create mode 100644 test/JsonApiDotNetCoreTests/UnitTests/Controllers/DefaultOperationFilterTests.cs diff --git a/src/JsonApiDotNetCore/AtomicOperations/DefaultOperationFilter.cs b/src/JsonApiDotNetCore/AtomicOperations/DefaultOperationFilter.cs index d1ec1bd65c..1407674150 100644 --- a/src/JsonApiDotNetCore/AtomicOperations/DefaultOperationFilter.cs +++ b/src/JsonApiDotNetCore/AtomicOperations/DefaultOperationFilter.cs @@ -7,13 +7,37 @@ namespace JsonApiDotNetCore.AtomicOperations; /// -internal sealed class DefaultOperationFilter : IAtomicOperationFilter +public class DefaultOperationFilter : IAtomicOperationFilter { /// - public bool IsEnabled(ResourceType resourceType, WriteOperationKind writeOperation) + public virtual bool IsEnabled(ResourceType resourceType, WriteOperationKind writeOperation) { + ArgumentGuard.NotNull(resourceType); + + // To match the behavior of non-operations controllers: + // If an operation is enabled on a base type, it is implicitly enabled on all derived types. + ResourceType currentResourceType = resourceType; + + while (true) + { + JsonApiEndpoints? endpoints = GetJsonApiEndpoints(currentResourceType); + bool isEnabled = endpoints != null && Contains(endpoints.Value, writeOperation); + + if (isEnabled || currentResourceType.BaseType == null) + { + return isEnabled; + } + + currentResourceType = currentResourceType.BaseType; + } + } + + protected virtual JsonApiEndpoints? GetJsonApiEndpoints(ResourceType resourceType) + { + ArgumentGuard.NotNull(resourceType); + var resourceAttribute = resourceType.ClrType.GetCustomAttribute(); - return resourceAttribute != null && Contains(resourceAttribute.GenerateControllerEndpoints, writeOperation); + return resourceAttribute?.GenerateControllerEndpoints; } private static bool Contains(JsonApiEndpoints endpoints, WriteOperationKind writeOperation) diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/ResourceInheritance/AtomicOperationTests.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/ResourceInheritance/AtomicOperationTests.cs new file mode 100644 index 0000000000..9ce92fbd6f --- /dev/null +++ b/test/JsonApiDotNetCoreTests/IntegrationTests/ResourceInheritance/AtomicOperationTests.cs @@ -0,0 +1,80 @@ +using System.Net; +using FluentAssertions; +using JsonApiDotNetCore.Serialization.Objects; +using JsonApiDotNetCoreTests.IntegrationTests.ResourceInheritance.Models; +using JsonApiDotNetCoreTests.IntegrationTests.ResourceInheritance.TablePerHierarchy; +using TestBuildingBlocks; +using Xunit; + +namespace JsonApiDotNetCoreTests.IntegrationTests.ResourceInheritance; + +public sealed class AtomicOperationTests : IClassFixture, TablePerHierarchyDbContext>> +{ + private readonly IntegrationTestContext, TablePerHierarchyDbContext> _testContext; + private readonly ResourceInheritanceFakers _fakers = new(); + + public AtomicOperationTests(IntegrationTestContext, TablePerHierarchyDbContext> testContext) + { + _testContext = testContext; + + testContext.UseController(); + } + + [Fact] + public async Task When_operation_is_enabled_on_base_type_it_is_implicitly_enabled_on_derived_types() + { + // Arrange + AlwaysMovingTandem newMovingTandem = _fakers.AlwaysMovingTandem.GenerateOne(); + + var requestBody = new + { + atomic__operations = new[] + { + new + { + op = "add", + data = new + { + type = "alwaysMovingTandems", + attributes = new + { + weight = newMovingTandem.Weight, + requiresDriverLicense = newMovingTandem.RequiresDriverLicense, + gearCount = newMovingTandem.GearCount + } + } + } + } + }; + + const string route = "/operations"; + + // Act + (HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecutePostAtomicAsync(route, requestBody); + + // Assert + httpResponse.ShouldHaveStatusCode(HttpStatusCode.OK); + + responseDocument.Results.ShouldHaveCount(1); + + responseDocument.Results[0].Data.SingleValue.ShouldNotBeNull().With(resource => + { + resource.Type.Should().Be("alwaysMovingTandems"); + resource.Attributes.ShouldContainKey("weight").With(value => value.Should().Be(newMovingTandem.Weight)); + resource.Attributes.ShouldContainKey("requiresDriverLicense").With(value => value.Should().Be(newMovingTandem.RequiresDriverLicense)); + resource.Attributes.ShouldContainKey("gearCount").With(value => value.Should().Be(newMovingTandem.GearCount)); + resource.Relationships.Should().BeNull(); + }); + + long newMovingTandemId = long.Parse(responseDocument.Results[0].Data.SingleValue!.Id.ShouldNotBeNull()); + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + AlwaysMovingTandem movingTandemInDatabase = await dbContext.AlwaysMovingTandems.FirstWithIdAsync(newMovingTandemId); + + movingTandemInDatabase.Weight.Should().Be(newMovingTandem.Weight); + movingTandemInDatabase.RequiresDriverLicense.Should().Be(newMovingTandem.RequiresDriverLicense); + movingTandemInDatabase.GearCount.Should().Be(newMovingTandem.GearCount); + }); + } +} diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/ResourceInheritance/ChangeTracking/ChangeTrackingDbContext.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/ResourceInheritance/ChangeTracking/ChangeTrackingDbContext.cs index b905450d85..7aea8f5af1 100644 --- a/test/JsonApiDotNetCoreTests/IntegrationTests/ResourceInheritance/ChangeTracking/ChangeTrackingDbContext.cs +++ b/test/JsonApiDotNetCoreTests/IntegrationTests/ResourceInheritance/ChangeTracking/ChangeTrackingDbContext.cs @@ -1,12 +1,8 @@ using JetBrains.Annotations; -using JsonApiDotNetCoreTests.IntegrationTests.ResourceInheritance.Models; using Microsoft.EntityFrameworkCore; namespace JsonApiDotNetCoreTests.IntegrationTests.ResourceInheritance.ChangeTracking; [UsedImplicitly(ImplicitUseTargetFlags.Members)] public sealed class ChangeTrackingDbContext(DbContextOptions options) - : ResourceInheritanceDbContext(options) -{ - public DbSet AlwaysMovingTandems => Set(); -} + : ResourceInheritanceDbContext(options); diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/ResourceInheritance/Models/AlwaysMovingTandem.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/ResourceInheritance/Models/AlwaysMovingTandem.cs index 5d5aa241e1..590e053339 100644 --- a/test/JsonApiDotNetCoreTests/IntegrationTests/ResourceInheritance/Models/AlwaysMovingTandem.cs +++ b/test/JsonApiDotNetCoreTests/IntegrationTests/ResourceInheritance/Models/AlwaysMovingTandem.cs @@ -1,11 +1,12 @@ using System.ComponentModel.DataAnnotations.Schema; using JetBrains.Annotations; +using JsonApiDotNetCore.Controllers; using JsonApiDotNetCore.Resources.Annotations; namespace JsonApiDotNetCoreTests.IntegrationTests.ResourceInheritance.Models; [UsedImplicitly(ImplicitUseTargetFlags.Members)] -[Resource(ControllerNamespace = "JsonApiDotNetCoreTests.IntegrationTests.ResourceInheritance")] +[Resource(ControllerNamespace = "JsonApiDotNetCoreTests.IntegrationTests.ResourceInheritance", GenerateControllerEndpoints = JsonApiEndpoints.None)] public sealed class AlwaysMovingTandem : Bike { [NotMapped] diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/ResourceInheritance/OperationsController.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/ResourceInheritance/OperationsController.cs new file mode 100644 index 0000000000..6b742c8243 --- /dev/null +++ b/test/JsonApiDotNetCoreTests/IntegrationTests/ResourceInheritance/OperationsController.cs @@ -0,0 +1,13 @@ +using JsonApiDotNetCore.AtomicOperations; +using JsonApiDotNetCore.Configuration; +using JsonApiDotNetCore.Controllers; +using JsonApiDotNetCore.Middleware; +using JsonApiDotNetCore.Resources; +using Microsoft.Extensions.Logging; + +namespace JsonApiDotNetCoreTests.IntegrationTests.ResourceInheritance; + +public sealed class OperationsController( + IJsonApiOptions options, IResourceGraph resourceGraph, ILoggerFactory loggerFactory, IOperationsProcessor processor, IJsonApiRequest request, + ITargetedFields targetedFields, IAtomicOperationFilter operationFilter) + : JsonApiOperationsController(options, resourceGraph, loggerFactory, processor, request, targetedFields, operationFilter); diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/ResourceInheritance/ResourceInheritanceDbContext.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/ResourceInheritance/ResourceInheritanceDbContext.cs index 6ead9fe296..b5db6cdf11 100644 --- a/test/JsonApiDotNetCoreTests/IntegrationTests/ResourceInheritance/ResourceInheritanceDbContext.cs +++ b/test/JsonApiDotNetCoreTests/IntegrationTests/ResourceInheritance/ResourceInheritanceDbContext.cs @@ -12,6 +12,7 @@ public abstract class ResourceInheritanceDbContext(DbContextOptions options) public DbSet Vehicles => Set(); public DbSet Bikes => Set(); public DbSet Tandems => Set(); + public DbSet AlwaysMovingTandems => Set(); public DbSet MotorVehicles => Set(); public DbSet Cars => Set(); public DbSet Trucks => Set(); diff --git a/test/JsonApiDotNetCoreTests/UnitTests/Controllers/DefaultOperationFilterTests.cs b/test/JsonApiDotNetCoreTests/UnitTests/Controllers/DefaultOperationFilterTests.cs new file mode 100644 index 0000000000..d977518940 --- /dev/null +++ b/test/JsonApiDotNetCoreTests/UnitTests/Controllers/DefaultOperationFilterTests.cs @@ -0,0 +1,105 @@ +using FluentAssertions; +using JetBrains.Annotations; +using JsonApiDotNetCore; +using JsonApiDotNetCore.AtomicOperations; +using JsonApiDotNetCore.Configuration; +using JsonApiDotNetCore.Controllers; +using JsonApiDotNetCore.Middleware; +using JsonApiDotNetCore.Resources; +using Microsoft.Extensions.Logging.Abstractions; +using Xunit; + +namespace JsonApiDotNetCoreTests.UnitTests.Controllers; + +public sealed class DefaultOperationFilterTests +{ + // @formatter:wrap_chained_method_calls chop_always + // @formatter:wrap_before_first_method_call true + + private static readonly IResourceGraph ResourceGraph = new ResourceGraphBuilder(new JsonApiOptions(), NullLoggerFactory.Instance) + .Add() + .Add() + .Add() + .Build(); + + // @formatter:wrap_before_first_method_call restore + // @formatter:wrap_chained_method_calls restore + + [Theory] + [InlineData(WriteOperationKind.CreateResource)] + [InlineData(WriteOperationKind.UpdateResource)] + [InlineData(WriteOperationKind.DeleteResource)] + [InlineData(WriteOperationKind.SetRelationship)] + [InlineData(WriteOperationKind.AddToRelationship)] + [InlineData(WriteOperationKind.RemoveFromRelationship)] + public void Operations_enabled_on_abstract_base_type_are_implicitly_enabled_on_derived_types(WriteOperationKind writeOperation) + { + // Arrange + ResourceType abstractBaseType = ResourceGraph.GetResourceType(); + ResourceType concreteBaseType = ResourceGraph.GetResourceType(); + ResourceType concreteDerivedType = ResourceGraph.GetResourceType(); + + var filter = new FakeOperationFilter(resourceType => resourceType.Equals(abstractBaseType)); + + // Act + bool abstractBaseIsEnabled = filter.IsEnabled(abstractBaseType, writeOperation); + bool concreteBaseIsEnabled = filter.IsEnabled(concreteBaseType, writeOperation); + bool concreteDerivedIsEnabled = filter.IsEnabled(concreteDerivedType, writeOperation); + + // Assert + abstractBaseIsEnabled.Should().BeTrue(); + concreteBaseIsEnabled.Should().BeTrue(); + concreteDerivedIsEnabled.Should().BeTrue(); + } + + [Theory] + [InlineData(WriteOperationKind.CreateResource)] + [InlineData(WriteOperationKind.UpdateResource)] + [InlineData(WriteOperationKind.DeleteResource)] + [InlineData(WriteOperationKind.SetRelationship)] + [InlineData(WriteOperationKind.AddToRelationship)] + [InlineData(WriteOperationKind.RemoveFromRelationship)] + public void Operations_enabled_on_concrete_base_type_are_implicitly_enabled_on_derived_types(WriteOperationKind writeOperation) + { + // Arrange + ResourceType abstractBaseType = ResourceGraph.GetResourceType(); + ResourceType concreteBaseType = ResourceGraph.GetResourceType(); + ResourceType concreteDerivedType = ResourceGraph.GetResourceType(); + + var filter = new FakeOperationFilter(resourceType => resourceType.Equals(concreteBaseType)); + + // Act + bool abstractBaseIsEnabled = filter.IsEnabled(abstractBaseType, writeOperation); + bool concreteBaseIsEnabled = filter.IsEnabled(concreteBaseType, writeOperation); + bool concreteDerivedIsEnabled = filter.IsEnabled(concreteDerivedType, writeOperation); + + // Assert + abstractBaseIsEnabled.Should().BeFalse(); + concreteBaseIsEnabled.Should().BeTrue(); + concreteDerivedIsEnabled.Should().BeTrue(); + } + + private sealed class FakeOperationFilter : DefaultOperationFilter + { + private readonly Func _isResourceTypeEnabled; + + public FakeOperationFilter(Func isResourceTypeEnabled) + { + ArgumentGuard.NotNull(isResourceTypeEnabled); + + _isResourceTypeEnabled = isResourceTypeEnabled; + } + + protected override JsonApiEndpoints? GetJsonApiEndpoints(ResourceType resourceType) + { + return _isResourceTypeEnabled(resourceType) ? JsonApiEndpoints.All : JsonApiEndpoints.None; + } + } + + private abstract class AbstractBaseType : Identifiable; + + private class ConcreteBaseType : AbstractBaseType; + + [UsedImplicitly(ImplicitUseKindFlags.InstantiatedNoFixedConstructorSignature)] + private sealed class ConcreteDerivedType : ConcreteBaseType; +}