Skip to content

Commit

Permalink
Fixed: If an operation is enabled on a base type, it should implicitl…
Browse files Browse the repository at this point in the history
…y be enabled on all derived types
  • Loading branch information
bkoelman committed Nov 10, 2024
1 parent 661afb3 commit aa763bf
Show file tree
Hide file tree
Showing 7 changed files with 229 additions and 9 deletions.
30 changes: 27 additions & 3 deletions src/JsonApiDotNetCore/AtomicOperations/DefaultOperationFilter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
});
}
}
Original file line number Diff line number Diff line change
@@ -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);
Original file line number Diff line number Diff line change
@@ -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]
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
Original file line number Diff line number Diff line change
Expand Up @@ -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>();
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}

0 comments on commit aa763bf

Please sign in to comment.