diff --git a/src/JsonApiDotNetCore/Repositories/EntityFrameworkCoreRepository.cs b/src/JsonApiDotNetCore/Repositories/EntityFrameworkCoreRepository.cs index 24f835c6df..397eea4a5b 100644 --- a/src/JsonApiDotNetCore/Repositories/EntityFrameworkCoreRepository.cs +++ b/src/JsonApiDotNetCore/Repositories/EntityFrameworkCoreRepository.cs @@ -528,7 +528,7 @@ protected virtual async Task SaveChangesAsync(CancellationToken cancellationToke { await _dbContext.SaveChangesAsync(cancellationToken); } - catch (DbUpdateException exception) + catch (Exception exception) when (exception is DbUpdateException || exception is InvalidOperationException) { if (_dbContext.Database.CurrentTransaction != null) { diff --git a/src/JsonApiDotNetCore/Repositories/PlaceholderResourceCollector.cs b/src/JsonApiDotNetCore/Repositories/PlaceholderResourceCollector.cs index d28ab4a6c0..51efa96aae 100644 --- a/src/JsonApiDotNetCore/Repositories/PlaceholderResourceCollector.cs +++ b/src/JsonApiDotNetCore/Repositories/PlaceholderResourceCollector.cs @@ -70,7 +70,15 @@ private void Detach(IEnumerable resources) { foreach (object resource in resources) { - _dbContext.Entry(resource).State = EntityState.Detached; + try + { + _dbContext.Entry(resource).State = EntityState.Detached; + } + catch (InvalidOperationException) + { + // If SaveChanges() threw due to a foreign key constraint violation, its exception is rethrown here. + // We swallow this exception, to allow the originating error to propagate. + } } } } diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/AtomicOperations/OperationsDbContext.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/AtomicOperations/OperationsDbContext.cs index c7ab6d943c..ad45e3bb85 100644 --- a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/AtomicOperations/OperationsDbContext.cs +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/AtomicOperations/OperationsDbContext.cs @@ -33,7 +33,7 @@ protected override void OnModelCreating(ModelBuilder builder) builder.Entity() .HasOne(musicTrack => musicTrack.Lyric) .WithOne(lyric => lyric.Track) - .HasForeignKey(); + .HasForeignKey("LyricId"); } } } diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/EagerLoadingDbContext.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/EagerLoadingDbContext.cs index efce1b3014..62dd263f79 100644 --- a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/EagerLoadingDbContext.cs +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/EagerLoadingDbContext.cs @@ -22,13 +22,13 @@ protected override void OnModelCreating(ModelBuilder builder) builder.Entity() .HasOne(building => building.PrimaryDoor) .WithOne() - .HasForeignKey("PrimaryDoorKey") + .HasForeignKey("PrimaryDoorId") .IsRequired(); builder.Entity() .HasOne(building => building.SecondaryDoor) .WithOne() - .HasForeignKey("SecondaryDoorKey") + .HasForeignKey("SecondaryDoorId") .IsRequired(false); } } diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Links/LinksDbContext.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Links/LinksDbContext.cs index 1e0a63bfad..bad61d1e3a 100644 --- a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Links/LinksDbContext.cs +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Links/LinksDbContext.cs @@ -22,7 +22,7 @@ protected override void OnModelCreating(ModelBuilder builder) builder.Entity() .HasOne(photo => photo.Location) .WithOne(location => location.Photo) - .HasForeignKey("PhotoLocationKey"); + .HasForeignKey("LocationId"); } } } diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/ReadWrite/ReadWriteDbContext.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/ReadWrite/ReadWriteDbContext.cs index da22daa4d3..c5d589cbb8 100644 --- a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/ReadWrite/ReadWriteDbContext.cs +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/ReadWrite/ReadWriteDbContext.cs @@ -33,7 +33,7 @@ protected override void OnModelCreating(ModelBuilder builder) builder.Entity() .HasOne(workItemGroup => workItemGroup.Color) .WithOne(color => color.Group) - .HasForeignKey(); + .HasForeignKey("GroupId"); builder.Entity() .HasKey(workItemTag => new diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/RequiredRelationships/DefaultBehaviorDbContext.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/RequiredRelationships/DefaultBehaviorDbContext.cs index d8a617037b..2c0e3a88df 100644 --- a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/RequiredRelationships/DefaultBehaviorDbContext.cs +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/RequiredRelationships/DefaultBehaviorDbContext.cs @@ -24,10 +24,14 @@ protected override void OnModelCreating(ModelBuilder builder) .WithOne(order => order.Customer) .IsRequired(); + // By default, EF Core generates an identifying foreign key for a required 1-to-1 relationship. + // This means no foreign key column is generated, instead the primary keys point to each other directly. + // That mechanism does not make sense for JSON:API, because patching a relationship would result in + // also changing the identity of a resource. Naming the key explicitly forces to create a foreign key column. builder.Entity() .HasOne(order => order.Shipment) .WithOne(shipment => shipment.Order) - .HasForeignKey() + .HasForeignKey("OrderId") .IsRequired(); } } diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/RequiredRelationships/DefaultBehaviorTests.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/RequiredRelationships/DefaultBehaviorTests.cs index fb5e465329..df90cfcf93 100644 --- a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/RequiredRelationships/DefaultBehaviorTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/RequiredRelationships/DefaultBehaviorTests.cs @@ -5,6 +5,7 @@ using JsonApiDotNetCore.Configuration; using JsonApiDotNetCore.Serialization.Objects; using JsonApiDotNetCoreExampleTests.Startups; +using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.DependencyInjection; using TestBuildingBlocks; using Xunit; @@ -113,12 +114,12 @@ await _testContext.RunOnDatabaseAsync(async dbContext => string route = $"/customers/{existingOrder.Customer.Id}"; // Act - (HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecuteDeleteAsync(route); + (HttpResponseMessage httpResponse, string responseDocument) = await _testContext.ExecuteDeleteAsync(route); // Assert httpResponse.Should().HaveStatusCode(HttpStatusCode.NoContent); - responseDocument.Should().BeNull(); + responseDocument.Should().BeEmpty(); await _testContext.RunOnDatabaseAsync(async dbContext => { @@ -147,12 +148,12 @@ await _testContext.RunOnDatabaseAsync(async dbContext => string route = $"/orders/{existingOrder.Id}"; // Act - (HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecuteDeleteAsync(route); + (HttpResponseMessage httpResponse, string responseDocument) = await _testContext.ExecuteDeleteAsync(route); // Assert httpResponse.Should().HaveStatusCode(HttpStatusCode.NoContent); - responseDocument.Should().BeNull(); + responseDocument.Should().BeEmpty(); await _testContext.RunOnDatabaseAsync(async dbContext => { @@ -382,7 +383,7 @@ await _testContext.RunOnDatabaseAsync(async dbContext => } [Fact] - public async Task Cannot_reassign_dependent_side_of_OneToOne_relationship_with_identifying_foreign_key_through_primary_endpoint() + public async Task Can_reassign_dependent_side_of_ZeroOrOneToOne_relationship_through_primary_endpoint() { // Arrange Order orderWithShipment = _fakers.Orders.Generate(); @@ -421,21 +422,24 @@ await _testContext.RunOnDatabaseAsync(async dbContext => string route = $"/orders/{orderWithoutShipment.Id}"; // Act - (HttpResponseMessage httpResponse, ErrorDocument responseDocument) = await _testContext.ExecutePatchAsync(route, requestBody); + (HttpResponseMessage httpResponse, string responseDocument) = await _testContext.ExecutePatchAsync(route, requestBody); // Assert - httpResponse.Should().HaveStatusCode(HttpStatusCode.InternalServerError); + httpResponse.Should().HaveStatusCode(HttpStatusCode.NoContent); - responseDocument.Errors.Should().HaveCount(1); + responseDocument.Should().BeEmpty(); - Error error = responseDocument.Errors[0]; - error.StatusCode.Should().Be(HttpStatusCode.InternalServerError); - error.Title.Should().Be("An unhandled error occurred while processing this request."); - error.Detail.Should().StartWith("The property 'Id' on entity type 'Shipment' is part of a key and so cannot be modified or marked as modified."); + await _testContext.RunOnDatabaseAsync(async dbContext => + { + Shipment existingShipmentInDatabase = + await dbContext.Shipments.Include(shipment => shipment.Order).FirstWithIdOrDefaultAsync(orderWithShipment.Shipment.Id); + + existingShipmentInDatabase.Order.Id.Should().Be(orderWithoutShipment.Id); + }); } [Fact] - public async Task Cannot_reassign_dependent_side_of_OneToOne_relationship_with_identifying_foreign_key_through_relationship_endpoint() + public async Task Can_reassign_dependent_side_of_ZeroOrOneToOne_relationship_through_relationship_endpoint() { // Arrange Order orderWithShipment = _fakers.Orders.Generate(); @@ -463,17 +467,20 @@ await _testContext.RunOnDatabaseAsync(async dbContext => string route = $"/orders/{orderWithoutShipment.Id}/relationships/shipment"; // Act - (HttpResponseMessage httpResponse, ErrorDocument responseDocument) = await _testContext.ExecutePatchAsync(route, requestBody); + (HttpResponseMessage httpResponse, string responseDocument) = await _testContext.ExecutePatchAsync(route, requestBody); // Assert - httpResponse.Should().HaveStatusCode(HttpStatusCode.InternalServerError); + httpResponse.Should().HaveStatusCode(HttpStatusCode.NoContent); - responseDocument.Errors.Should().HaveCount(1); + responseDocument.Should().BeEmpty(); - Error error = responseDocument.Errors[0]; - error.StatusCode.Should().Be(HttpStatusCode.InternalServerError); - error.Title.Should().Be("An unhandled error occurred while processing this request."); - error.Detail.Should().StartWith("The property 'Id' on entity type 'Shipment' is part of a key and so cannot be modified or marked as modified."); + await _testContext.RunOnDatabaseAsync(async dbContext => + { + Shipment existingShipmentInDatabase = + await dbContext.Shipments.Include(shipment => shipment.Order).FirstWithIdOrDefaultAsync(orderWithShipment.Shipment.Id); + + existingShipmentInDatabase.Order.Id.Should().Be(orderWithoutShipment.Id); + }); } } } diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/ResourceHooks/HooksDbContext.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/ResourceHooks/HooksDbContext.cs index a20c9a4f8f..1c8dd2948d 100644 --- a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/ResourceHooks/HooksDbContext.cs +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/ResourceHooks/HooksDbContext.cs @@ -37,7 +37,7 @@ protected override void OnModelCreating(ModelBuilder builder) builder.Entity() .HasOne(passport => passport.Person) .WithOne(person => person.Passport) - .HasForeignKey("PassportKey") + .HasForeignKey("PassportId") .OnDelete(DeleteBehavior.SetNull); } } diff --git a/test/UnitTests/ResourceHooks/HooksDbContext.cs b/test/UnitTests/ResourceHooks/HooksDbContext.cs index 5b5a8e0dcc..2d06f15b33 100644 --- a/test/UnitTests/ResourceHooks/HooksDbContext.cs +++ b/test/UnitTests/ResourceHooks/HooksDbContext.cs @@ -54,13 +54,13 @@ protected override void OnModelCreating(ModelBuilder builder) builder.Entity() .HasOne(passport => passport.Person) .WithOne(person => person.Passport) - .HasForeignKey("PassportKey") + .HasForeignKey("PassportId") .OnDelete(DeleteBehavior.SetNull); builder.Entity() .HasOne(todoItem => todoItem.OneToOnePerson) .WithOne(person => person.OneToOneTodoItem) - .HasForeignKey("OneToOnePersonKey"); + .HasForeignKey("OneToOnePersonId"); } } }