Skip to content

Commit acfe864

Browse files
koenbeukkoenbeuk
committed
handle recursive failures and retries
1 parent 946bb26 commit acfe864

3 files changed

Lines changed: 196 additions & 19 deletions

File tree

‎.github/workflows/release.yml‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,14 @@ env:
1414
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
1515
# Official NuGet Feed settings
1616
NUGET_FEED: https://api.nuget.org/v3/index.json
17-
NUGET_KEY: ${{ secrets.NUGET_KEY }}
1817

1918
jobs:
2019
deploy:
2120
runs-on: ubuntu-latest
21+
permissions:
22+
contents: read
23+
packages: write
24+
id-token: write
2225
steps:
2326
- uses: actions/checkout@v4
2427
- name: Setup .NET Core
@@ -39,9 +42,14 @@ jobs:
3942
do
4043
dotnet nuget push $f --source $GITHUB_FEED --api-key ${{github.token}} --skip-duplicate --no-symbols
4144
done
45+
- name: NuGet login
46+
uses: NuGet/login@v1
47+
id: nuget-login
48+
with:
49+
user: Koenbeuk
4250
- name: Push to NuGet Feed
4351
run: |
4452
for f in ./nupkg/*.nupkg
4553
do
46-
dotnet nuget push $f --source $NUGET_FEED --skip-duplicate --api-key $NUGET_KEY
54+
dotnet nuget push $f --source $NUGET_FEED --skip-duplicate --api-key ${{ steps.nuget-login.outputs.NUGET_API_KEY }}
4755
done

‎src/EntityFrameworkCore.Triggered/Internal/TriggerSessionSaveChangesInterceptor.cs‎

Lines changed: 75 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,11 @@ public class TriggerSessionSaveChangesInterceptor : ISaveChangesInterceptor, IRe
1717

1818
ITriggerSession? _triggerSession;
1919
int _parallelSaveChangesCount;
20+
int _afterSaveFailedTriggersRaisedDepth;
21+
bool _subscribedToSaveChangesFailed;
22+
23+
// Triggers can call SaveChanges on the same context, so this tracks the nesting depth whose AfterSaveFailed triggers already ran
24+
private bool AfterSaveFailedTriggersRaised => _parallelSaveChangesCount > 0 && _afterSaveFailedTriggersRaisedDepth == _parallelSaveChangesCount;
2025

2126
private void EnlistTriggerSession(DbContextEventData eventData)
2227
{
@@ -50,17 +55,28 @@ private void EnlistTriggerSession(DbContextEventData eventData)
5055
}
5156
}
5257

58+
if (!_subscribedToSaveChangesFailed)
59+
{
60+
eventData.Context!.SaveChangesFailed += OnSaveChangesFailed;
61+
_subscribedToSaveChangesFailed = true;
62+
}
63+
5364
_parallelSaveChangesCount += 1;
5465
}
5566

56-
private void DelistTriggerSession(DbContextEventData eventData)
67+
private void DelistTriggerSession(DbContext? context)
5768
{
5869
Debug.Assert(_triggerSession != null);
5970

6071
#if DEBUG
61-
Debug.Assert(_capturedDbContext == eventData.Context);
72+
Debug.Assert(_capturedDbContext == context);
6273
#endif
6374

75+
if (AfterSaveFailedTriggersRaised)
76+
{
77+
_afterSaveFailedTriggersRaisedDepth = 0;
78+
}
79+
6480
_parallelSaveChangesCount -= 1;
6581

6682
if (_parallelSaveChangesCount == 0)
@@ -90,7 +106,7 @@ public InterceptionResult<int> SavingChanges(DbContextEventData eventData, Inter
90106
catch
91107
{
92108
// We're aborting the SaveChanges call, delist the trigger session now
93-
DelistTriggerSession(eventData);
109+
DelistTriggerSession(eventData.Context);
94110
throw;
95111
}
96112
finally
@@ -126,7 +142,7 @@ public async ValueTask<InterceptionResult<int>> SavingChangesAsync(DbContextEven
126142
catch
127143
{
128144
// We're aborting the SaveChanges call, delist the trigger session now
129-
DelistTriggerSession(eventData);
145+
DelistTriggerSession(eventData.Context);
130146
throw;
131147
}
132148
finally
@@ -145,7 +161,7 @@ public int SavedChanges(SaveChangesCompletedEventData eventData, int result)
145161
_triggerSession.RaiseAfterSaveTriggers();
146162
_triggerSession.RaiseAfterSaveCompletedTriggers();
147163

148-
DelistTriggerSession(eventData);
164+
DelistTriggerSession(eventData.Context);
149165

150166
return result;
151167
}
@@ -163,7 +179,7 @@ public async ValueTask<int> SavedChangesAsync(SaveChangesCompletedEventData even
163179
_triggerSession.RaiseAfterSaveCompletedTriggers();
164180
await _triggerSession.RaiseAfterSaveCompletedAsyncTriggers(cancellationToken).ConfigureAwait(false);
165181

166-
DelistTriggerSession(eventData);
182+
DelistTriggerSession(eventData.Context);
167183

168184
return result;
169185
}
@@ -172,52 +188,94 @@ public void SaveChangesFailed(DbContextErrorEventData eventData)
172188
{
173189
try
174190
{
175-
RaiseAfterSaveFailedTriggers(eventData.Exception);
191+
if (!AfterSaveFailedTriggersRaised)
192+
{
193+
RaiseAfterSaveFailedTriggers(eventData.Exception);
194+
}
176195
}
177196
finally
178197
{
179-
DelistTriggerSession(eventData);
198+
DelistTriggerSession(eventData.Context);
180199
}
181200
}
182201

183202
public async Task SaveChangesFailedAsync(DbContextErrorEventData eventData, CancellationToken cancellationToken = default)
184203
{
185204
try
186205
{
187-
await RaiseAfterSaveFailedAsyncTriggers(eventData.Exception, cancellationToken).ConfigureAwait(false);
206+
if (!AfterSaveFailedTriggersRaised)
207+
{
208+
await RaiseAfterSaveFailedAsyncTriggers(eventData.Exception, cancellationToken).ConfigureAwait(false);
209+
}
188210
}
189211
finally
190212
{
191-
DelistTriggerSession(eventData);
213+
DelistTriggerSession(eventData.Context);
192214
}
193215
}
194216

217+
// An interceptor registered after us can still suppress the exception, so the session is only released once SaveChanges
218+
// reports its final outcome: SavedChanges, SaveChangesFailed, SaveChangesCanceled or the DbContext.SaveChangesFailed event.
195219
public InterceptionResult ThrowingConcurrencyException(ConcurrencyExceptionEventData eventData, InterceptionResult result)
196220
{
197-
if (!result.IsSuppressed)
221+
if (!result.IsSuppressed && !AfterSaveFailedTriggersRaised)
198222
{
199-
SaveChangesFailed(eventData);
223+
try
224+
{
225+
RaiseAfterSaveFailedTriggers(eventData.Exception);
226+
}
227+
finally
228+
{
229+
_afterSaveFailedTriggersRaisedDepth = _parallelSaveChangesCount;
230+
}
200231
}
201232

202233
return result;
203234
}
204235

205236
public async ValueTask<InterceptionResult> ThrowingConcurrencyExceptionAsync(ConcurrencyExceptionEventData eventData, InterceptionResult result, CancellationToken cancellationToken = default)
206237
{
207-
if (!result.IsSuppressed)
238+
if (!result.IsSuppressed && !AfterSaveFailedTriggersRaised)
208239
{
209-
await SaveChangesFailedAsync(eventData, cancellationToken).ConfigureAwait(false);
240+
try
241+
{
242+
await RaiseAfterSaveFailedAsyncTriggers(eventData.Exception, cancellationToken).ConfigureAwait(false);
243+
}
244+
finally
245+
{
246+
_afterSaveFailedTriggersRaisedDepth = _parallelSaveChangesCount;
247+
}
210248
}
211249

212250
return result;
213251
}
214252

253+
private void OnSaveChangesFailed(object? sender, SaveChangesFailedEventArgs eventArgs)
254+
{
255+
if (eventArgs.Exception is not DbUpdateConcurrencyException || _triggerSession is null)
256+
{
257+
return;
258+
}
259+
260+
try
261+
{
262+
if (!AfterSaveFailedTriggersRaised)
263+
{
264+
RaiseAfterSaveFailedTriggers(eventArgs.Exception);
265+
}
266+
}
267+
finally
268+
{
269+
DelistTriggerSession(sender as DbContext);
270+
}
271+
}
272+
215273
public void SaveChangesCanceled(DbContextEventData eventData)
216-
=> DelistTriggerSession(eventData);
274+
=> DelistTriggerSession(eventData.Context);
217275

218276
public Task SaveChangesCanceledAsync(DbContextEventData eventData, CancellationToken cancellationToken = default)
219277
{
220-
DelistTriggerSession(eventData);
278+
DelistTriggerSession(eventData.Context);
221279
return Task.CompletedTask;
222280
}
223281

@@ -226,6 +284,7 @@ public void ResetState()
226284
_triggerSession?.Dispose();
227285
_triggerSession = null;
228286
_parallelSaveChangesCount = 0;
287+
_afterSaveFailedTriggersRaisedDepth = 0;
229288
#if DEBUG
230289
_capturedDbContext = null;
231290
#endif

‎test/EntityFrameworkCore.Triggered.Tests/Internal/TriggerSessionSaveChangesInterceptorReleaseTests.cs‎

Lines changed: 111 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,15 @@ public override ValueTask<InterceptionResult<DbDataReader>> ReaderExecutingAsync
4848
=> Enabled ? throw new OperationCanceledException() : new ValueTask<InterceptionResult<DbDataReader>>(result);
4949
}
5050

51+
class SuppressingConcurrencyInterceptor : ISaveChangesInterceptor
52+
{
53+
public InterceptionResult ThrowingConcurrencyException(ConcurrencyExceptionEventData eventData, InterceptionResult result)
54+
=> InterceptionResult.Suppress();
55+
56+
public ValueTask<InterceptionResult> ThrowingConcurrencyExceptionAsync(ConcurrencyExceptionEventData eventData, InterceptionResult result, CancellationToken cancellationToken = default)
57+
=> new(InterceptionResult.Suppress());
58+
}
59+
5160
readonly SqliteConnection _connection = new("DataSource=:memory:");
5261
readonly CancelingCommandInterceptor _cancelingInterceptor = new();
5362
readonly TriggerStub<Item> _trigger = new();
@@ -59,10 +68,14 @@ public TriggerSessionSaveChangesInterceptorReleaseTests()
5968

6069
public void Dispose() => _connection.Dispose();
6170

71+
bool _suppressConcurrencyExceptions;
72+
6273
void ConfigureOptions(DbContextOptionsBuilder optionsBuilder)
6374
=> optionsBuilder
6475
.UseSqlite(_connection)
65-
.AddInterceptors(_cancelingInterceptor)
76+
.AddInterceptors(_suppressConcurrencyExceptions
77+
? new IInterceptor[] { _cancelingInterceptor, new SuppressingConcurrencyInterceptor() }
78+
: new IInterceptor[] { _cancelingInterceptor })
6679
.ConfigureWarnings(warnings => warnings.Ignore(CoreEventId.ManyServiceProvidersCreatedWarning))
6780
.UseTriggers(triggerOptions => triggerOptions.AddTrigger(_trigger));
6881

@@ -187,6 +200,103 @@ public async Task ThrowingAfterSaveFailedTrigger_ReleasesTriggerSession(bool asy
187200
await AssertNextSaveRaisesBeforeSaveTriggers(context, async);
188201
}
189202

203+
[Theory]
204+
[InlineData(false)]
205+
[InlineData(true)]
206+
public async Task ConcurrencyException_WithThrowingAfterSaveFailedTrigger_ReleasesTriggerSession(bool async)
207+
{
208+
using var context = CreateContext();
209+
CauseConcurrencyConflict(context, SeedItem(context));
210+
_trigger.AfterSaveFailedHandler = (_, _) => throw new InvalidOperationException("trigger failed");
211+
212+
var exception = await Assert.ThrowsAsync<DbUpdateException>(() => Save(context, async));
213+
Assert.Equal("trigger failed", Assert.IsType<InvalidOperationException>(exception.InnerException).Message);
214+
Assert.Single(_trigger.AfterSaveFailedInvocations);
215+
216+
_trigger.AfterSaveFailedHandler = null;
217+
await AssertNextSaveRaisesBeforeSaveTriggers(context, async);
218+
}
219+
220+
[Theory]
221+
[InlineData(false)]
222+
[InlineData(true)]
223+
public async Task ConcurrencyException_SuppressedByLaterInterceptor_ReleasesTriggerSession(bool async)
224+
{
225+
_suppressConcurrencyExceptions = true;
226+
using var context = CreateContext();
227+
CauseConcurrencyConflict(context, SeedItem(context));
228+
229+
await Save(context, async);
230+
231+
await AssertNextSaveRaisesBeforeSaveTriggers(context, async);
232+
}
233+
234+
[Theory]
235+
[InlineData(false)]
236+
[InlineData(true)]
237+
public async Task ConcurrencyException_SuppressedForMultipleCommands_RaisesAfterSaveFailedTriggersOnce(bool async)
238+
{
239+
_suppressConcurrencyExceptions = true;
240+
using var context = CreateContext();
241+
var first = SeedItem(context);
242+
var second = SeedItem(context);
243+
CauseConcurrencyConflict(context, first);
244+
second.Name = "conflicting";
245+
246+
await Save(context, async);
247+
248+
Assert.Equal(2, _trigger.AfterSaveFailedInvocations.Count);
249+
Assert.Contains(_trigger.AfterSaveFailedInvocations, invocation => invocation.context.Entity == first);
250+
Assert.Contains(_trigger.AfterSaveFailedInvocations, invocation => invocation.context.Entity == second);
251+
}
252+
253+
[Theory]
254+
[InlineData(false)]
255+
[InlineData(true)]
256+
public async Task ConcurrencyException_Suppressed_NestedSaveFailureRaisesItsOwnAfterSaveFailedTriggers(bool async)
257+
{
258+
_suppressConcurrencyExceptions = true;
259+
using var context = CreateContext();
260+
var seeded = SeedItem(context);
261+
CauseConcurrencyConflict(context, seeded);
262+
263+
var duplicate = new Item { Id = seeded.Id, Name = "duplicate" };
264+
var nestedSaveAttempted = false;
265+
_trigger.AfterSaveHandler = _ => {
266+
if (nestedSaveAttempted)
267+
{
268+
return;
269+
}
270+
271+
nestedSaveAttempted = true;
272+
context.Entry(seeded).State = EntityState.Detached;
273+
context.Items.Add(duplicate);
274+
Assert.Throws<DbUpdateException>(() => context.SaveChanges());
275+
};
276+
277+
await Save(context, async);
278+
279+
Assert.True(nestedSaveAttempted);
280+
Assert.Contains(_trigger.AfterSaveFailedInvocations, invocation => invocation.context.Entity == duplicate);
281+
}
282+
283+
[Theory]
284+
[InlineData(false)]
285+
[InlineData(true)]
286+
public async Task ConcurrencyExceptionThrownByAfterSaveTrigger_ReleasesTriggerSession(bool async)
287+
{
288+
using var context = CreateContext();
289+
var item = SeedItem(context);
290+
item.Name = "changed";
291+
_trigger.AfterSaveHandler = _ => throw new DbUpdateConcurrencyException("from trigger");
292+
293+
await Assert.ThrowsAsync<DbUpdateConcurrencyException>(() => Save(context, async));
294+
Assert.Contains(_trigger.AfterSaveFailedInvocations, invocation => invocation.context.Entity == item);
295+
296+
_trigger.AfterSaveHandler = null;
297+
await AssertNextSaveRaisesBeforeSaveTriggers(context, async);
298+
}
299+
190300
[Fact]
191301
public async Task PooledContext_AfterConcurrencyException_NextLeaseRaisesAsyncBeforeSaveTriggers()
192302
{

0 commit comments

Comments
 (0)