From f52e1d35b01aae849b25f377507ee19c774c23b7 Mon Sep 17 00:00:00 2001 From: Bart Koelman <10324372+bkoelman@users.noreply.github.com> Date: Wed, 5 Oct 2022 03:27:53 +0200 Subject: [PATCH] WIP: Investigating fix for #1118 --- .../Repositories/DbContextExtensions.cs | 9 ++ .../EntityFrameworkCoreRepository.cs | 115 ++++++++++++++++++ .../IntegrationTests/Experiments/Customer.cs | 22 ++++ .../Experiments/ExperimentsDbContext.cs | 57 +++++++++ .../Experiments/ExperimentsFakers.cs | 29 +++++ .../Experiments/ExperimentsTests.cs | 73 +++++++++++ .../IntegrationTests/Experiments/Order.cs | 19 +++ .../Experiments/ShoppingBasket.cs | 16 +++ 8 files changed, 340 insertions(+) create mode 100644 test/JsonApiDotNetCoreTests/IntegrationTests/Experiments/Customer.cs create mode 100644 test/JsonApiDotNetCoreTests/IntegrationTests/Experiments/ExperimentsDbContext.cs create mode 100644 test/JsonApiDotNetCoreTests/IntegrationTests/Experiments/ExperimentsFakers.cs create mode 100644 test/JsonApiDotNetCoreTests/IntegrationTests/Experiments/ExperimentsTests.cs create mode 100644 test/JsonApiDotNetCoreTests/IntegrationTests/Experiments/Order.cs create mode 100644 test/JsonApiDotNetCoreTests/IntegrationTests/Experiments/ShoppingBasket.cs diff --git a/src/JsonApiDotNetCore/Repositories/DbContextExtensions.cs b/src/JsonApiDotNetCore/Repositories/DbContextExtensions.cs index f78841defc..7bd1265a2f 100644 --- a/src/JsonApiDotNetCore/Repositories/DbContextExtensions.cs +++ b/src/JsonApiDotNetCore/Repositories/DbContextExtensions.cs @@ -1,3 +1,4 @@ +using System.Reflection; using JetBrains.Annotations; using JsonApiDotNetCore.Resources; using Microsoft.EntityFrameworkCore; @@ -8,6 +9,8 @@ namespace JsonApiDotNetCore.Repositories; [PublicAPI] public static class DbContextExtensions { + private static readonly MethodInfo DbContextSetMethod = typeof(DbContext).GetMethod(nameof(DbContext.Set), Type.EmptyTypes)!; + /// /// If not already tracked, attaches the specified resource to the change tracker in state. /// @@ -57,4 +60,10 @@ public static void ResetChangeTracker(this DbContext dbContext) dbContext.ChangeTracker.Clear(); } + + internal static IQueryable Set(this DbContext context, Type entityType) + { + MethodInfo setMethod = DbContextSetMethod.MakeGenericMethod(entityType); + return (IQueryable)setMethod.Invoke(context, null)!; + } } diff --git a/src/JsonApiDotNetCore/Repositories/EntityFrameworkCoreRepository.cs b/src/JsonApiDotNetCore/Repositories/EntityFrameworkCoreRepository.cs index 653db6129a..59d6d7b8c7 100644 --- a/src/JsonApiDotNetCore/Repositories/EntityFrameworkCoreRepository.cs +++ b/src/JsonApiDotNetCore/Repositories/EntityFrameworkCoreRepository.cs @@ -325,6 +325,9 @@ public virtual async Task DeleteAsync(TResource? resourceFromDatabase, TId id, C var resourceTracked = (TResource)_dbContext.GetTrackedOrAttach(placeholderResource); + EnsureIncomingNavigationsAreTracked(resourceTracked); + + /* foreach (RelationshipAttribute relationship in _resourceGraph.GetResourceType().Relationships) { // Loads the data of the relationship, if in Entity Framework Core it is configured in such a way that loading @@ -335,6 +338,7 @@ public virtual async Task DeleteAsync(TResource? resourceFromDatabase, TId id, C await navigation.LoadAsync(cancellationToken); } } + */ _dbContext.Remove(resourceTracked); @@ -343,6 +347,117 @@ public virtual async Task DeleteAsync(TResource? resourceFromDatabase, TId id, C await _resourceDefinitionAccessor.OnWriteSucceededAsync(resourceTracked, WriteOperationKind.DeleteResource, cancellationToken); } + private void EnsureIncomingNavigationsAreTracked(TResource resourceTracked) + { + IEntityType[] entityTypes = _dbContext.Model.GetEntityTypes().ToArray(); + IEntityType thisEntityType = entityTypes.Single(entityType => entityType.ClrType == typeof(TResource)); + + HashSet navigationsToLoad = new(); + + foreach (INavigation navigation in entityTypes.SelectMany(entityType => entityType.GetNavigations())) + { + bool isIncomingNavigation = + navigation.IsOnDependent ? navigation.TargetEntityType == thisEntityType : navigation.DeclaringEntityType == thisEntityType; + + if (isIncomingNavigation && navigation.ForeignKey.DeleteBehavior == DeleteBehavior.ClientSetNull) + { + navigationsToLoad.Add(navigation); + } + } + + // {Navigation: Customer.FirstOrder (Order) ToPrincipal Order} + // var query = from _dbContext.Set().Where(customer => customer.FirstOrder == resourceTracked) // .Select(customer => customer.Id) + + // {Navigation: Customer.LastOrder (Order) ToPrincipal Order} + // var query = from _dbContext.Set().Where(customer => customer.LastOrder == resourceTracked) // .Select(customer => customer.Id) + + // {Navigation: Order.Parent (Order) ToPrincipal Order} + // var query = from _dbContext.Set().Where(order => order.Parent == resourceTracked) // .Select(order => order.Id) + + // {Navigation: ShoppingBasket.CurrentOrder (Order) ToPrincipal Order} + // var query = from _dbContext.Set().Where(shoppingBasket => shoppingBasket.CurrentOrder == resourceTracked) // .Select(shoppingBasket => shoppingBasket.Id) + + var nameFactory = new LambdaParameterNameFactory(); + var scopeFactory = new LambdaScopeFactory(nameFactory); + + foreach (INavigation navigation in navigationsToLoad) + { + if (!navigation.IsOnDependent && navigation.Inverse != null) + { + // TODO: Handle the case where there is no inverse. + continue; + } + + IQueryable source = _dbContext.Set(navigation.DeclaringEntityType.ClrType); + + using LambdaScope scope = scopeFactory.CreateScope(source.ElementType); + + Expression expression; + + if (navigation.IsCollection) + { + /* + {Navigation: WorkItem.Subscribers (ISet) Collection ToDependent UserAccount} + + var subscribers = dbContext.WorkItems + .Where(workItem => workItem == existingWorkItem) + .Include(workItem => workItem.Subscribers) + .Select(workItem => workItem.Subscribers); + */ + + Expression left = scope.Accessor; + Expression right = Expression.Constant(resourceTracked, resourceTracked.GetType()); + + Expression whereBody = Expression.Equal(left, right); + LambdaExpression wherePredicate = Expression.Lambda(whereBody, scope.Parameter); + Expression whereExpression = WhereExtensionMethodCall(source.Expression, scope, wherePredicate); + + // TODO: Use typed overload + Expression includeExpression = IncludeExtensionMethodCall(whereExpression, scope, navigation.Name); + + MemberExpression selectorBody = Expression.MakeMemberAccess(scope.Accessor, navigation.PropertyInfo); + LambdaExpression selectorLambda = Expression.Lambda(selectorBody, scope.Parameter); + + expression = SelectExtensionMethodCall(includeExpression, source.ElementType, navigation.PropertyInfo.PropertyType, selectorLambda); + } + else + { + MemberExpression left = Expression.MakeMemberAccess(scope.Parameter, navigation.PropertyInfo); + ConstantExpression right = Expression.Constant(resourceTracked, resourceTracked.GetType()); + + Expression body = Expression.Equal(left, right); + LambdaExpression selectorLambda = Expression.Lambda(body, scope.Parameter); + expression = WhereExtensionMethodCall(source.Expression, scope, selectorLambda); + } + + IQueryable queryable = source.Provider.CreateQuery(expression); + + // Executes the query and loads the returned entities in the change tracker. + // We can likely optimize this by only fetching ~IDs~ (primary/foreign keys) and creating placeholder resources for them. + // The reason we can't fetch by ID is because there's no interception possible (see CompositeKeyTests); there's no access + // to QueryExpressionRewriter, and even if there was, we need to handle unexpected relationships so can't rely on our query abstractions. + object[] results = queryable.Cast().ToArray(); + } + } + + private Expression WhereExtensionMethodCall(Expression source, LambdaScope lambdaScope, LambdaExpression predicate) + { + return Expression.Call(typeof(Queryable), "Where", lambdaScope.Parameter.Type.AsArray(), source, predicate); + } + + private Expression IncludeExtensionMethodCall(Expression source, LambdaScope lambdaScope, string navigationPropertyPath) + { + Expression navigationExpression = Expression.Constant(navigationPropertyPath); + + return Expression.Call(typeof(EntityFrameworkQueryableExtensions), "Include", lambdaScope.Parameter.Type.AsArray(), source, navigationExpression); + } + + private Expression SelectExtensionMethodCall(Expression source, Type elementType, Type bodyType, Expression selectorBody) + { + Type[] typeArguments = ArrayFactory.Create(elementType, bodyType); + return Expression.Call(typeof(Queryable), "Select", typeArguments, source, selectorBody); + } + private NavigationEntry GetNavigationEntry(TResource resource, RelationshipAttribute relationship) { EntityEntry entityEntry = _dbContext.Entry(resource); diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/Experiments/Customer.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/Experiments/Customer.cs new file mode 100644 index 0000000000..5bb7ae4d5e --- /dev/null +++ b/test/JsonApiDotNetCoreTests/IntegrationTests/Experiments/Customer.cs @@ -0,0 +1,22 @@ +using JetBrains.Annotations; +using JsonApiDotNetCore.Resources; +using JsonApiDotNetCore.Resources.Annotations; + +namespace JsonApiDotNetCoreTests.IntegrationTests.Experiments; + +[UsedImplicitly(ImplicitUseTargetFlags.Members)] +[Resource(ControllerNamespace = "JsonApiDotNetCoreTests.IntegrationTests.Experiments")] +public sealed class Customer : Identifiable +{ + [Attr] + public string Name { get; set; } = null!; + + [HasOne] + public Order? FirstOrder { get; set; } + + [HasOne] + public Order? LastOrder { get; set; } + + [HasMany] + public ISet Orders { get; set; } = new HashSet(); +} diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/Experiments/ExperimentsDbContext.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/Experiments/ExperimentsDbContext.cs new file mode 100644 index 0000000000..f2533336ec --- /dev/null +++ b/test/JsonApiDotNetCoreTests/IntegrationTests/Experiments/ExperimentsDbContext.cs @@ -0,0 +1,57 @@ +using JetBrains.Annotations; +using Microsoft.EntityFrameworkCore; + +// @formatter:wrap_chained_method_calls chop_always + +namespace JsonApiDotNetCoreTests.IntegrationTests.Experiments; + +[UsedImplicitly(ImplicitUseTargetFlags.Members)] +public sealed class ExperimentsDbContext : DbContext +{ + public DbSet Customers => Set(); + public DbSet Orders => Set(); + public DbSet ShoppingBaskets => Set(); + + public ExperimentsDbContext(DbContextOptions options) + : base(options) + { + } + + protected override void OnModelCreating(ModelBuilder builder) + { + // https://stackoverflow.com/questions/54326165/ef-core-why-clientsetnull-is-default-ondelete-behavior-for-optional-relations + // https://learn.microsoft.com/en-us/ef/core/saving/cascade-delete + + builder.Entity() + .HasMany(customer => customer.Orders) + .WithOne(order => order.Customer); + + builder.Entity() + .HasOne(customer => customer.FirstOrder) + .WithOne() + .HasForeignKey("FirstOrderId") + .OnDelete(DeleteBehavior.ClientSetNull); + //.OnDelete(DeleteBehavior.SetNull); + + builder.Entity() + .HasOne(customer => customer.LastOrder) + .WithOne() + .HasForeignKey("LastOrderId") + .OnDelete(DeleteBehavior.ClientSetNull); + //.OnDelete(DeleteBehavior.SetNull); + + builder.Entity() + .HasOne(order => order.Parent) + .WithOne() + .HasForeignKey("ParentOrderId") + .OnDelete(DeleteBehavior.ClientSetNull); + //.OnDelete(DeleteBehavior.SetNull); + + builder.Entity() + .HasOne(shoppingBasket => shoppingBasket.CurrentOrder) + .WithOne() + .HasForeignKey("CurrentOrderId") + .OnDelete(DeleteBehavior.ClientSetNull); + //.OnDelete(DeleteBehavior.SetNull); + } +} diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/Experiments/ExperimentsFakers.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/Experiments/ExperimentsFakers.cs new file mode 100644 index 0000000000..ff7dcc62dd --- /dev/null +++ b/test/JsonApiDotNetCoreTests/IntegrationTests/Experiments/ExperimentsFakers.cs @@ -0,0 +1,29 @@ +using Bogus; +using TestBuildingBlocks; + +// @formatter:wrap_chained_method_calls chop_always +// @formatter:keep_existing_linebreaks true + +namespace JsonApiDotNetCoreTests.IntegrationTests.Experiments; + +internal sealed class ExperimentsFakers : FakerContainer +{ + private readonly Lazy> _lazyCustomerFaker = new(() => + new Faker() + .UseSeed(GetFakerSeed()) + .RuleFor(customer => customer.Name, faker => faker.Person.FullName)); + + private readonly Lazy> _lazyOrderFaker = new(() => + new Faker() + .UseSeed(GetFakerSeed()) + .RuleFor(order => order.Amount, faker => faker.Finance.Amount())); + + private readonly Lazy> _lazyShoppingBasketFaker = new(() => + new Faker() + .UseSeed(GetFakerSeed()) + .RuleFor(shoppingBasket => shoppingBasket.ProductCount, faker => faker.Random.Int(0, 5))); + + public Faker Customer => _lazyCustomerFaker.Value; + public Faker Order => _lazyOrderFaker.Value; + public Faker ShoppingBasket => _lazyShoppingBasketFaker.Value; +} diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/Experiments/ExperimentsTests.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/Experiments/ExperimentsTests.cs new file mode 100644 index 0000000000..2e1b9fdd4a --- /dev/null +++ b/test/JsonApiDotNetCoreTests/IntegrationTests/Experiments/ExperimentsTests.cs @@ -0,0 +1,73 @@ +using System.Net; +using FluentAssertions; +using FluentAssertions.Common; +using FluentAssertions.Extensions; +using JsonApiDotNetCore.Configuration; +using JsonApiDotNetCoreTests.IntegrationTests.SoftDeletion; +using Microsoft.AspNetCore.Authentication; +using Microsoft.Extensions.DependencyInjection; +using TestBuildingBlocks; +using Xunit; + +namespace JsonApiDotNetCoreTests.IntegrationTests.Experiments; + +public sealed class ExperimentsTests : IClassFixture, ExperimentsDbContext>> +{ + private readonly IntegrationTestContext, ExperimentsDbContext> _testContext; + private readonly ExperimentsFakers _fakers = new(); + + public ExperimentsTests(IntegrationTestContext, ExperimentsDbContext> testContext) + { + _testContext = testContext; + + testContext.UseController(); + testContext.UseController(); + testContext.UseController(); + + testContext.ConfigureServicesAfterStartup(services => + { + services.AddSingleton(new FrozenSystemClock + { + UtcNow = 1.January(2005).ToDateTimeOffset() + }); + + services.AddResourceService>(); + services.AddResourceService>(); + }); + } + + [Fact] + public async Task Can_delete_resource() + { + // Arrange + Order existingOrder = _fakers.Order.Generate(); + existingOrder.Customer = _fakers.Customer.Generate(); + existingOrder.Parent = _fakers.Order.Generate(); + existingOrder.Parent.Customer = existingOrder.Customer; + + List existingBaskets = _fakers.ShoppingBasket.Generate(3); + existingBaskets[0].CurrentOrder = existingOrder; + existingBaskets[1].CurrentOrder = existingOrder; + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + dbContext.Orders.Add(existingOrder); + await dbContext.SaveChangesAsync(); + + existingOrder.Customer.FirstOrder = existingOrder; + existingOrder.Customer.LastOrder = existingOrder; + dbContext.ShoppingBaskets.AddRange(existingBaskets); + await dbContext.SaveChangesAsync(); + }); + + string route = $"/orders/{existingOrder.StringId}"; + + // Act + (HttpResponseMessage httpResponse, string responseDocument) = await _testContext.ExecuteDeleteAsync(route); + + // Assert + httpResponse.ShouldHaveStatusCode(HttpStatusCode.NoContent); + + responseDocument.Should().BeEmpty(); + } +} diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/Experiments/Order.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/Experiments/Order.cs new file mode 100644 index 0000000000..2d4efe3af4 --- /dev/null +++ b/test/JsonApiDotNetCoreTests/IntegrationTests/Experiments/Order.cs @@ -0,0 +1,19 @@ +using JetBrains.Annotations; +using JsonApiDotNetCore.Resources; +using JsonApiDotNetCore.Resources.Annotations; + +namespace JsonApiDotNetCoreTests.IntegrationTests.Experiments; + +[UsedImplicitly(ImplicitUseTargetFlags.Members)] +[Resource(ControllerNamespace = "JsonApiDotNetCoreTests.IntegrationTests.Experiments")] +public sealed class Order : Identifiable +{ + [Attr] + public decimal Amount { get; set; } + + [HasOne] + public Customer Customer { get; set; } = null!; + + [HasOne] + public Order? Parent { get; set; } +} diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/Experiments/ShoppingBasket.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/Experiments/ShoppingBasket.cs new file mode 100644 index 0000000000..db220c2538 --- /dev/null +++ b/test/JsonApiDotNetCoreTests/IntegrationTests/Experiments/ShoppingBasket.cs @@ -0,0 +1,16 @@ +using JetBrains.Annotations; +using JsonApiDotNetCore.Resources; +using JsonApiDotNetCore.Resources.Annotations; + +namespace JsonApiDotNetCoreTests.IntegrationTests.Experiments; + +[UsedImplicitly(ImplicitUseTargetFlags.Members)] +[Resource(ControllerNamespace = "JsonApiDotNetCoreTests.IntegrationTests.Experiments")] +public sealed class ShoppingBasket : Identifiable +{ + [Attr] + public int ProductCount { get; set; } + + [HasOne] + public Order? CurrentOrder { get; set; } +}