Skip to content

Commit e36e8ca

Browse files
author
Bart Koelman
committed
Since EF Core 5, SaveChanges automatically creates a savepoint and rolls back to it on failure. This produces more correct error responses in operations, compared to rolling back the entire transaction. For example, when an operations request creates a resource and the next operation fails, the resource service may incorrectly conclude that the resource from the first operation does not exist.
1 parent 7350540 commit e36e8ca

File tree

2 files changed

+83
-10
lines changed

2 files changed

+83
-10
lines changed

src/JsonApiDotNetCore/Repositories/EntityFrameworkCoreRepository.cs

-7
Original file line numberDiff line numberDiff line change
@@ -571,13 +571,6 @@ protected virtual async Task SaveChangesAsync(CancellationToken cancellationToke
571571
}
572572
catch (Exception exception) when (exception is DbUpdateException or InvalidOperationException)
573573
{
574-
if (_dbContext.Database.CurrentTransaction != null)
575-
{
576-
// The ResourceService calling us needs to run additional SQL queries after an aborted transaction,
577-
// to determine error cause. This fails when a failed transaction is still in progress.
578-
await _dbContext.Database.CurrentTransaction.RollbackAsync(cancellationToken);
579-
}
580-
581574
_dbContext.ResetChangeTracker();
582575

583576
throw new DataStoreUpdateException(exception);

test/JsonApiDotNetCoreTests/IntegrationTests/AtomicOperations/Transactions/AtomicRollbackTests.cs

+83-3
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ await _testContext.RunOnDatabaseAsync(async dbContext =>
3636
await dbContext.ClearTablesAsync<Performer, MusicTrack>();
3737
});
3838

39-
string performerId = Unknown.StringId.For<Performer, int>();
39+
string unknownPerformerId = Unknown.StringId.For<Performer, int>();
4040

4141
var requestBody = new
4242
{
@@ -74,7 +74,7 @@ await _testContext.RunOnDatabaseAsync(async dbContext =>
7474
new
7575
{
7676
type = "performers",
77-
id = performerId
77+
id = unknownPerformerId
7878
}
7979
}
8080
}
@@ -97,7 +97,87 @@ await _testContext.RunOnDatabaseAsync(async dbContext =>
9797
ErrorObject error = responseDocument.Errors[0];
9898
error.StatusCode.Should().Be(HttpStatusCode.NotFound);
9999
error.Title.Should().Be("A related resource does not exist.");
100-
error.Detail.Should().Be($"Related resource of type 'performers' with ID '{performerId}' in relationship 'performers' does not exist.");
100+
error.Detail.Should().Be($"Related resource of type 'performers' with ID '{unknownPerformerId}' in relationship 'performers' does not exist.");
101+
error.Source.ShouldNotBeNull();
102+
error.Source.Pointer.Should().Be("/atomic:operations[1]");
103+
104+
await _testContext.RunOnDatabaseAsync(async dbContext =>
105+
{
106+
List<Performer> performersInDatabase = await dbContext.Performers.ToListAsync();
107+
performersInDatabase.Should().BeEmpty();
108+
109+
List<MusicTrack> tracksInDatabase = await dbContext.MusicTracks.ToListAsync();
110+
tracksInDatabase.Should().BeEmpty();
111+
});
112+
}
113+
114+
[Fact]
115+
public async Task Can_restore_to_previous_savepoint_on_error()
116+
{
117+
// Arrange
118+
string newTrackTitle = _fakers.MusicTrack.Generate().Title;
119+
120+
await _testContext.RunOnDatabaseAsync(async dbContext =>
121+
{
122+
await dbContext.ClearTablesAsync<Performer, MusicTrack>();
123+
});
124+
125+
const string trackLid = "track-1";
126+
127+
string unknownPerformerId = Unknown.StringId.For<Performer, int>();
128+
129+
var requestBody = new
130+
{
131+
atomic__operations = new object[]
132+
{
133+
new
134+
{
135+
op = "add",
136+
data = new
137+
{
138+
type = "musicTracks",
139+
lid = trackLid,
140+
attributes = new
141+
{
142+
title = newTrackTitle
143+
}
144+
}
145+
},
146+
new
147+
{
148+
op = "add",
149+
@ref = new
150+
{
151+
type = "musicTracks",
152+
lid = trackLid,
153+
relationship = "performers"
154+
},
155+
data = new[]
156+
{
157+
new
158+
{
159+
type = "performers",
160+
id = unknownPerformerId
161+
}
162+
}
163+
}
164+
}
165+
};
166+
167+
const string route = "/operations";
168+
169+
// Act
170+
(HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecutePostAtomicAsync<Document>(route, requestBody);
171+
172+
// Assert
173+
httpResponse.Should().HaveStatusCode(HttpStatusCode.NotFound);
174+
175+
responseDocument.Errors.ShouldHaveCount(1);
176+
177+
ErrorObject error = responseDocument.Errors[0];
178+
error.StatusCode.Should().Be(HttpStatusCode.NotFound);
179+
error.Title.Should().Be("A related resource does not exist.");
180+
error.Detail.Should().Be($"Related resource of type 'performers' with ID '{unknownPerformerId}' in relationship 'performers' does not exist.");
101181
error.Source.ShouldNotBeNull();
102182
error.Source.Pointer.Should().Be("/atomic:operations[1]");
103183

0 commit comments

Comments
 (0)