Merge pull request #1628 from json-api-dotnet/fix-operations-inheritance

Fix resource inheritance with atomic operations
json-api-dotnet · Nov 10, 2024 · 43c1a4b · 43c1a4b
2 parents 661afb3 + aa763bf
commit 43c1a4b
Show file tree

Hide file tree

Showing 7 changed files with 229 additions and 9 deletions.
diff --git a/src/JsonApiDotNetCore/AtomicOperations/DefaultOperationFilter.cs b/src/JsonApiDotNetCore/AtomicOperations/DefaultOperationFilter.cs
@@ -7,13 +7,37 @@
 namespace JsonApiDotNetCore.AtomicOperations;
 
 /// <inheritdoc />
-internal sealed class DefaultOperationFilter : IAtomicOperationFilter
+public class DefaultOperationFilter : IAtomicOperationFilter
 {
     /// <inheritdoc />
-    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<ResourceAttribute>();
-        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
@@ -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<IntegrationTestContext<TestableStartup<TablePerHierarchyDbContext>, TablePerHierarchyDbContext>>
+{
+    private readonly IntegrationTestContext<TestableStartup<TablePerHierarchyDbContext>, TablePerHierarchyDbContext> _testContext;
+    private readonly ResourceInheritanceFakers _fakers = new();
+
+    public AtomicOperationTests(IntegrationTestContext<TestableStartup<TablePerHierarchyDbContext>, TablePerHierarchyDbContext> testContext)
+    {
+        _testContext = testContext;
+
+        testContext.UseController<OperationsController>();
+    }
+
+    [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<Document>(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/...tCoreTests/IntegrationTests/ResourceInheritance/ChangeTracking/ChangeTrackingDbContext.cs b/...tCoreTests/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<ChangeTrackingDbContext> options)
-    : ResourceInheritanceDbContext(options)
-{
-    public DbSet<AlwaysMovingTandem> AlwaysMovingTandems => Set<AlwaysMovingTandem>();
-}
+    : ResourceInheritanceDbContext(options);
diff --git a/.../JsonApiDotNetCoreTests/IntegrationTests/ResourceInheritance/Models/AlwaysMovingTandem.cs b/.../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
@@ -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/...onApiDotNetCoreTests/IntegrationTests/ResourceInheritance/ResourceInheritanceDbContext.cs b/...onApiDotNetCoreTests/IntegrationTests/ResourceInheritance/ResourceInheritanceDbContext.cs
@@ -12,6 +12,7 @@ public abstract class ResourceInheritanceDbContext(DbContextOptions options)
     public DbSet<Vehicle> Vehicles => Set<Vehicle>();
     public DbSet<Bike> Bikes => Set<Bike>();
     public DbSet<Tandem> Tandems => Set<Tandem>();
+    public DbSet<AlwaysMovingTandem> AlwaysMovingTandems => Set<AlwaysMovingTandem>();
     public DbSet<MotorVehicle> MotorVehicles => Set<MotorVehicle>();
     public DbSet<Car> Cars => Set<Car>();
     public DbSet<Truck> Trucks => Set<Truck>();

diff --git a/test/JsonApiDotNetCoreTests/UnitTests/Controllers/DefaultOperationFilterTests.cs 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<AbstractBaseType, long>()
+        .Add<ConcreteBaseType, long>()
+        .Add<ConcreteDerivedType, long>()
+        .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<AbstractBaseType>();
+        ResourceType concreteBaseType = ResourceGraph.GetResourceType<ConcreteBaseType>();
+        ResourceType concreteDerivedType = ResourceGraph.GetResourceType<ConcreteDerivedType>();
+
+        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<AbstractBaseType>();
+        ResourceType concreteBaseType = ResourceGraph.GetResourceType<ConcreteBaseType>();
+        ResourceType concreteDerivedType = ResourceGraph.GetResourceType<ConcreteDerivedType>();
+
+        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<ResourceType, bool> _isResourceTypeEnabled;
+
+        public FakeOperationFilter(Func<ResourceType, bool> isResourceTypeEnabled)
+        {
+            ArgumentGuard.NotNull(isResourceTypeEnabled);
+
+            _isResourceTypeEnabled = isResourceTypeEnabled;
+        }
+
+        protected override JsonApiEndpoints? GetJsonApiEndpoints(ResourceType resourceType)
+        {
+            return _isResourceTypeEnabled(resourceType) ? JsonApiEndpoints.All : JsonApiEndpoints.None;
+        }
+    }
+
+    private abstract class AbstractBaseType : Identifiable<long>;
+
+    private class ConcreteBaseType : AbstractBaseType;
+
+    [UsedImplicitly(ImplicitUseKindFlags.InstantiatedNoFixedConstructorSignature)]
+    private sealed class ConcreteDerivedType : ConcreteBaseType;
+}