Skip to content

Fix resource inheritance with atomic operations #1628

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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;
}
Loading