From 02d19b5ecf71b5cf9652425b87ff61ae659ef773 Mon Sep 17 00:00:00 2001 From: Bart Koelman <10324372+bkoelman@users.noreply.github.com> Date: Thu, 4 Aug 2022 00:22:05 +0200 Subject: [PATCH 1/2] Adapts marking many-to-many relationships as tracked so it no longer needs the Issue26779 EF Core back-compat switch. The switch is unavailable in EF 7, so this change unblocks us from upgrade. --- .../Configuration/JsonApiOptions.cs | 6 ---- .../EntityFrameworkCoreRepository.cs | 28 +++++++++++++++++++ 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/src/JsonApiDotNetCore/Configuration/JsonApiOptions.cs b/src/JsonApiDotNetCore/Configuration/JsonApiOptions.cs index eda6374acf..aa7ed0d434 100644 --- a/src/JsonApiDotNetCore/Configuration/JsonApiOptions.cs +++ b/src/JsonApiDotNetCore/Configuration/JsonApiOptions.cs @@ -102,12 +102,6 @@ public sealed class JsonApiOptions : IJsonApiOptions } }; - static JsonApiOptions() - { - // Bug workaround for https://github.com/dotnet/efcore/issues/27436 - AppContext.SetSwitch("Microsoft.EntityFrameworkCore.Issue26779", true); - } - public JsonApiOptions() { _lazySerializerReadOptions = new Lazy(() => new JsonSerializerOptions(SerializerOptions), LazyThreadSafetyMode.PublicationOnly); diff --git a/src/JsonApiDotNetCore/Repositories/EntityFrameworkCoreRepository.cs b/src/JsonApiDotNetCore/Repositories/EntityFrameworkCoreRepository.cs index 358f63a2cd..1b807fd24f 100644 --- a/src/JsonApiDotNetCore/Repositories/EntityFrameworkCoreRepository.cs +++ b/src/JsonApiDotNetCore/Repositories/EntityFrameworkCoreRepository.cs @@ -547,6 +547,34 @@ private void MarkRelationshipAsLoaded(TResource leftResource, RelationshipAttrib EntityEntry leftEntry = _dbContext.Entry(leftResource); CollectionEntry rightCollectionEntry = leftEntry.Collection(relationship.Property.Name); rightCollectionEntry.IsLoaded = true; + + if (rightCollectionEntry.Metadata is ISkipNavigation skipNavigation) + { + MarkManyToManyRelationshipAsLoaded(leftEntry, skipNavigation); + } + } + + private void MarkManyToManyRelationshipAsLoaded(EntityEntry leftEntry, ISkipNavigation skipNavigation) + { + string[] primaryKeyNames = skipNavigation.ForeignKey.PrincipalKey.Properties.Select(property => property.Name).ToArray(); + object?[] primaryKeyValues = GetCurrentKeyValues(leftEntry, primaryKeyNames); + + string[] foreignKeyNames = skipNavigation.ForeignKey.Properties.Select(property => property.Name).ToArray(); + + foreach (EntityEntry joinEntry in _dbContext.ChangeTracker.Entries().Where(entry => entry.Metadata == skipNavigation.JoinEntityType).ToList()) + { + object?[] foreignKeyValues = GetCurrentKeyValues(joinEntry, foreignKeyNames); + + if (primaryKeyValues.SequenceEqual(foreignKeyValues)) + { + joinEntry.State = EntityState.Unchanged; + } + } + } + + private static object?[] GetCurrentKeyValues(EntityEntry entry, IEnumerable keyNames) + { + return keyNames.Select(keyName => entry.Property(keyName).CurrentValue).ToArray(); } protected async Task UpdateRelationshipAsync(RelationshipAttribute relationship, TResource leftResource, object? valueToAssign, From 6825c8345a81ed8024e2968bd95f0264c22cf1ee Mon Sep 17 00:00:00 2001 From: Bart Koelman <10324372+bkoelman@users.noreply.github.com> Date: Sat, 6 Aug 2022 10:16:04 +0200 Subject: [PATCH 2/2] Added test for removing from many-to-many relationship with composite key --- .../IntegrationTests/CompositeKeys/Car.cs | 3 ++ .../CompositeKeys/CompositeDbContext.cs | 4 ++ .../CompositeKeys/CompositeKeyTests.cs | 48 +++++++++++++++++++ .../CompositeKeys/Dealership.cs | 3 ++ 4 files changed, 58 insertions(+) diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/CompositeKeys/Car.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/CompositeKeys/Car.cs index 29213f5e69..d61d378db7 100644 --- a/test/JsonApiDotNetCoreTests/IntegrationTests/CompositeKeys/Car.cs +++ b/test/JsonApiDotNetCoreTests/IntegrationTests/CompositeKeys/Car.cs @@ -47,4 +47,7 @@ public override string? Id [HasOne] public Dealership? Dealership { get; set; } + + [HasMany] + public ISet PreviousDealerships { get; set; } = new HashSet(); } diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/CompositeKeys/CompositeDbContext.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/CompositeKeys/CompositeDbContext.cs index 67213057b7..e9e12439e6 100644 --- a/test/JsonApiDotNetCoreTests/IntegrationTests/CompositeKeys/CompositeDbContext.cs +++ b/test/JsonApiDotNetCoreTests/IntegrationTests/CompositeKeys/CompositeDbContext.cs @@ -34,5 +34,9 @@ protected override void OnModelCreating(ModelBuilder builder) builder.Entity() .HasMany(dealership => dealership.Inventory) .WithOne(car => car.Dealership!); + + builder.Entity() + .HasMany(car => car.PreviousDealerships) + .WithMany(dealership => dealership.SoldCars); } } diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/CompositeKeys/CompositeKeyTests.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/CompositeKeys/CompositeKeyTests.cs index 185367930d..9e273be36e 100644 --- a/test/JsonApiDotNetCoreTests/IntegrationTests/CompositeKeys/CompositeKeyTests.cs +++ b/test/JsonApiDotNetCoreTests/IntegrationTests/CompositeKeys/CompositeKeyTests.cs @@ -508,4 +508,52 @@ await _testContext.RunOnDatabaseAsync(async dbContext => carInDatabase.Should().BeNull(); }); } + + [Fact] + public async Task Can_remove_from_ManyToMany_relationship() + { + // Arrange + Dealership existingDealership = _fakers.Dealership.Generate(); + existingDealership.SoldCars = _fakers.Car.Generate(2).ToHashSet(); + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + await dbContext.ClearTableAsync(); + dbContext.Dealerships.Add(existingDealership); + await dbContext.SaveChangesAsync(); + }); + + var requestBody = new + { + data = new[] + { + new + { + type = "cars", + id = existingDealership.SoldCars.ElementAt(1).StringId + } + } + }; + + string route = $"/dealerships/{existingDealership.StringId}/relationships/soldCars"; + + // Act + (HttpResponseMessage httpResponse, string responseDocument) = await _testContext.ExecuteDeleteAsync(route, requestBody); + + // Assert + httpResponse.ShouldHaveStatusCode(HttpStatusCode.NoContent); + + responseDocument.Should().BeEmpty(); + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + Dealership dealershipInDatabase = await dbContext.Dealerships.Include(dealership => dealership.SoldCars).FirstWithIdAsync(existingDealership.Id); + + dealershipInDatabase.SoldCars.ShouldHaveCount(1); + dealershipInDatabase.SoldCars.Single().Id.Should().Be(existingDealership.SoldCars.ElementAt(0).Id); + + List carsInDatabase = await dbContext.Cars.ToListAsync(); + carsInDatabase.ShouldHaveCount(2); + }); + } } diff --git a/test/JsonApiDotNetCoreTests/IntegrationTests/CompositeKeys/Dealership.cs b/test/JsonApiDotNetCoreTests/IntegrationTests/CompositeKeys/Dealership.cs index 14784cb438..091e7acbe1 100644 --- a/test/JsonApiDotNetCoreTests/IntegrationTests/CompositeKeys/Dealership.cs +++ b/test/JsonApiDotNetCoreTests/IntegrationTests/CompositeKeys/Dealership.cs @@ -13,4 +13,7 @@ public sealed class Dealership : Identifiable [HasMany] public ISet Inventory { get; set; } = new HashSet(); + + [HasMany] + public ISet SoldCars { get; set; } = new HashSet(); }