Skip to content

Commit 669c3c9

Browse files
heuveaCopilot
heuvea
and
Copilot
authored
FUND-2421 DRC Conflict 409 http status code (#157)
* fixed 409 and locking issues * co-pilot fixes * Update src/OneGround.ZGW.Documenten.Web/Concurrency/ResilienceConcurrencyRetryPipeline.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * co-pilot fixes ii * polly retry on delete eoi added * code review point fixed * Update src/OneGround.ZGW.Documenten.Web/Concurrency/ResilienceConcurrencyRetryPipeline.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update src/OneGround.ZGW.Documenten.Web/Concurrency/ResilienceConcurrencyRetryPipeline.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * dynamically load changed options * fixed UnitTest --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 6f25c64 commit 669c3c9

14 files changed

+478
-217
lines changed

‎src/OneGround.ZGW.Documenten.Web/Concurrency/ConcurrencyConflictException.cs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
using System;
2+
3+
namespace OneGround.ZGW.Documenten.Web.Concurrency;
4+
5+
public class ConcurrencyConflictException : Exception
6+
{
7+
public ConcurrencyConflictException(string message, Guid id)
8+
: base(message)
9+
{
10+
Id = id;
11+
}
12+
13+
public ConcurrencyConflictException(string message, Exception innerException, Guid id)
14+
: base(message, innerException)
15+
{
16+
Id = id;
17+
}
18+
19+
public Guid Id { get; private set; }
20+
}

‎src/OneGround.ZGW.Documenten.Web/Concurrency/ResilienceConcurrencyRetryPipeline.cs‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
using System;
2+
using System.Threading;
3+
using System.Threading.Tasks;
4+
using Microsoft.Extensions.Http.Resilience;
5+
using Microsoft.Extensions.Logging;
6+
using Microsoft.Extensions.Options;
7+
using OneGround.ZGW.Common.Handlers;
8+
using Polly;
9+
using Polly.Retry;
10+
11+
namespace OneGround.ZGW.Documenten.Web.Concurrency;
12+
13+
public class ResilienceConcurrencyRetryPipeline<TObjectType>
14+
where TObjectType : class
15+
{
16+
private readonly ILogger _logger;
17+
private readonly IOptionsMonitor<HttpRetryStrategyOptions> _retryOptionsMonitor;
18+
19+
public ResilienceConcurrencyRetryPipeline(
20+
ILogger<ResilienceConcurrencyRetryPipeline<TObjectType>> logger,
21+
IOptionsMonitor<HttpRetryStrategyOptions> retryOptionsMonitor
22+
)
23+
{
24+
_logger = logger;
25+
_retryOptionsMonitor = retryOptionsMonitor;
26+
}
27+
28+
// Method which executes pipeline and return (null, CommandStatus.Conflict) if all retries fail due to ConcurrencyConflictException
29+
public async Task<(TObjectType enkelvoudiginformatieobject, CommandStatus status)> ExecuteWithResultAsync(
30+
Func<CancellationToken, Task<(TObjectType enkelvoudiginformatieobject, CommandStatus status)>> action,
31+
CancellationToken cancellationToken
32+
)
33+
{
34+
// Build pipeline dynamically with current configuration values
35+
var options = _retryOptionsMonitor.CurrentValue;
36+
37+
var concurrencyRetryPipeline = new ResiliencePipelineBuilder<(TObjectType, CommandStatus)>()
38+
.AddRetry(
39+
new RetryStrategyOptions<(TObjectType, CommandStatus)>
40+
{
41+
ShouldHandle = new PredicateBuilder<(TObjectType, CommandStatus)>().Handle<ConcurrencyConflictException>(),
42+
MaxRetryAttempts = options.MaxRetryAttempts,
43+
BackoffType = options.BackoffType,
44+
Delay = options.Delay,
45+
UseJitter = options.UseJitter,
46+
OnRetry = args =>
47+
{
48+
_logger.LogWarning(
49+
"Retry {AttemptNumber} after concurrency conflict for {ObjectType}...",
50+
args.AttemptNumber + 1,
51+
typeof(TObjectType).Name
52+
);
53+
return default;
54+
},
55+
}
56+
)
57+
.Build();
58+
59+
try
60+
{
61+
return await concurrencyRetryPipeline.ExecuteAsync(async ct => await action(ct), cancellationToken);
62+
}
63+
catch (ConcurrencyConflictException ex)
64+
{
65+
_logger.LogWarning("All retries for concurrency conflict exhausted for {ObjectType} with ID {Id}", typeof(TObjectType).Name, ex.Id);
66+
return (null, CommandStatus.Conflict);
67+
}
68+
}
69+
}

‎src/OneGround.ZGW.Documenten.Web/Controllers/v1/ObjectInformatieObjectenController.cs‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -136,13 +136,11 @@ public async Task<IActionResult> GetAsync(Guid id, CancellationToken cancellatio
136136
/// </remarks>
137137
/// <response code="401">Unauthorized</response>
138138
/// <response code="403">Forbidden</response>
139-
/// <response code="409">EnkelvoudigInformatieObject was modified by another user</response>
140139
/// <response code="429">Too Many Requests</response>
141140
/// <response code="500">Internal Server Error</response>
142141
[HttpPost(ApiRoutes.ObjectInformatieObjecten.Create, Name = Operations.ObjectInformatieObjecten.Create)]
143142
[Scope(AuthorizationScopes.Documenten.Create)]
144143
[SwaggerResponse(StatusCodes.Status400BadRequest, Type = typeof(ErrorResponse))]
145-
[SwaggerResponse(StatusCodes.Status409Conflict, Type = typeof(ErrorResponse))]
146144
[SwaggerResponse(StatusCodes.Status201Created, Type = typeof(ObjectInformatieObjectResponseDto))]
147145
[ZgwApiVersion(Api.LatestVersion_1_0)]
148146
[ZgwApiVersion(Api.LatestVersion_1_1)]
@@ -175,11 +173,6 @@ CancellationToken cancellationToken
175173
return _errorResponseBuilder.Forbidden();
176174
}
177175

178-
if (result.Status == CommandStatus.Conflict)
179-
{
180-
return _errorResponseBuilder.Conflict(result.Errors);
181-
}
182-
183176
var objectInformatieObjectResponse = _mapper.Map<ObjectInformatieObjectResponseDto>(result.Result);
184177

185178
return Created(objectInformatieObjectResponse.Url, objectInformatieObjectResponse);

‎src/OneGround.ZGW.Documenten.Web/Handlers/v1/1/LockEnkelvoudigInformatieObjectCommandHandler.cs‎

Lines changed: 45 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
using OneGround.ZGW.Documenten.DataModel;
2020
using OneGround.ZGW.Documenten.Services;
2121
using OneGround.ZGW.Documenten.Web.Authorization;
22+
using OneGround.ZGW.Documenten.Web.Concurrency;
2223

2324
namespace OneGround.ZGW.Documenten.Web.Handlers.v1._1;
2425

@@ -29,6 +30,7 @@ public class LockEnkelvoudigInformatieObjectCommandHandler
2930
private readonly DrcDbContext _context;
3031
private readonly IAuditTrailFactory _auditTrailFactory;
3132
private readonly IDocumentService _documentService;
33+
private readonly ResilienceConcurrencyRetryPipeline<EnkelvoudigInformatieObject> _concurrencyRetryPipeline;
3234

3335
public LockEnkelvoudigInformatieObjectCommandHandler(
3436
ILogger<LockEnkelvoudigInformatieObjectCommandHandler> logger,
@@ -39,12 +41,14 @@ public LockEnkelvoudigInformatieObjectCommandHandler(
3941
IAuthorizationContextAccessor authorizationContextAccessor,
4042
INotificatieService notificatieService,
4143
IDocumentServicesResolver documentServicesResolver,
42-
IDocumentKenmerkenResolver documentKenmerkenResolver
44+
IDocumentKenmerkenResolver documentKenmerkenResolver,
45+
ResilienceConcurrencyRetryPipeline<EnkelvoudigInformatieObject> concurrencyRetryPipeline
4346
)
4447
: base(logger, configuration, uriService, authorizationContextAccessor, notificatieService, documentKenmerkenResolver)
4548
{
4649
_context = context;
4750
_auditTrailFactory = auditTrailFactory;
51+
_concurrencyRetryPipeline = concurrencyRetryPipeline;
4852

4953
_documentService = documentServicesResolver.GetDefault();
5054
}
@@ -65,35 +69,52 @@ public async Task<CommandResult<string>> Handle(LockEnkelvoudigInformatieObjectC
6569

6670
var rsinFilter = GetRsinFilterPredicate<EnkelvoudigInformatieObject>();
6771

68-
var enkelvoudigInformatieObject = await _context
69-
.EnkelvoudigInformatieObjecten.LockForUpdate(_context, c => c.Id, [request.Id])
70-
.Where(rsinFilter)
71-
.Include(e => e.LatestEnkelvoudigInformatieObjectVersie)
72-
.SingleOrDefaultAsync(e => e.Id == request.Id, cancellationToken);
73-
74-
ValidationError error;
72+
var (enkelvoudigInformatieObject, status) = await _concurrencyRetryPipeline.ExecuteWithResultAsync(
73+
async (token) =>
74+
{
75+
// First, try to acquire lock on the EnkelvoudigInformatieObject
76+
var _enkelvoudigInformatieObject = await _context
77+
.EnkelvoudigInformatieObjecten.LockForUpdate(_context, c => c.Id, [request.Id])
78+
.Where(rsinFilter)
79+
.Include(e => e.LatestEnkelvoudigInformatieObjectVersie)
80+
.SingleOrDefaultAsync(e => e.Id == request.Id, token);
81+
82+
// The object might be locked OR not exist - check if it exists without lock
83+
if (_enkelvoudigInformatieObject == null)
84+
{
85+
// The object might be locked OR not exist - check if it exists without lock
86+
var exists = await _context.EnkelvoudigInformatieObjecten.Where(rsinFilter).AnyAsync(e => e.Id == request.Id, token);
7587

76-
// The object might be locked OR not exist - check if it exists without lock
77-
if (enkelvoudigInformatieObject == null)
78-
{
79-
// The object might be locked OR not exist - check if it exists without lock
80-
var exists = await _context.EnkelvoudigInformatieObjecten.Where(rsinFilter).AnyAsync(e => e.Id == request.Id, cancellationToken);
88+
if (!exists)
89+
{
90+
// Object truly doesn't exist
91+
return (enkelvoudiginformatieobject: null, status: CommandStatus.NotFound);
92+
}
8193

82-
if (!exists)
83-
{
84-
// Object truly doesn't exist
85-
error = new ValidationError("id", ErrorCode.NotFound, $"EnkelvoudigInformatieObject {request.Id} is onbekend.");
94+
// Throw the exception again so Polly knows a retry is needed (giving up after maximum reached retries)
95+
throw new ConcurrencyConflictException("Concurrency conflict detected.", request.Id);
96+
}
97+
else
98+
{
99+
return (enkelvoudiginformatieobject: _enkelvoudigInformatieObject, status: CommandStatus.OK);
100+
}
101+
},
102+
cancellationToken
103+
);
86104

87-
return new CommandResult<string>(null, CommandStatus.NotFound, error);
88-
}
105+
if (status == CommandStatus.NotFound)
106+
{
107+
return new CommandResult<string>(null, CommandStatus.NotFound);
108+
}
89109

110+
if (status == CommandStatus.Conflict)
111+
{
90112
// Object exists but is locked by another process
91-
error = new ValidationError(
113+
var error = new ValidationError(
92114
"nonFieldErrors",
93115
ErrorCode.Conflict,
94116
$"Het enkelvoudiginformatieobject {request.Id} is vergrendeld door een andere bewerking."
95117
);
96-
97118
return new CommandResult<string>(null, CommandStatus.Conflict, error);
98119
}
99120

@@ -105,7 +126,7 @@ public async Task<CommandResult<string>> Handle(LockEnkelvoudigInformatieObjectC
105126
{
106127
if (enkelvoudigInformatieObject.Locked)
107128
{
108-
error = new ValidationError("nonFieldErrors", ErrorCode.ExistingLock, "Het document is al gelockt.");
129+
var error = new ValidationError("nonFieldErrors", ErrorCode.ExistingLock, "Het document is al gelockt.");
109130
return new CommandResult<string>(null, CommandStatus.ValidationError, error);
110131
}
111132

@@ -116,15 +137,15 @@ public async Task<CommandResult<string>> Handle(LockEnkelvoudigInformatieObjectC
116137
{
117138
if (request.Lock != null && enkelvoudigInformatieObject.Lock != request.Lock)
118139
{
119-
error = new ValidationError("nonFieldErrors", ErrorCode.IncorrectLockId, "Incorrect lock ID.");
140+
var error = new ValidationError("nonFieldErrors", ErrorCode.IncorrectLockId, "Incorrect lock ID.");
120141
return new CommandResult<string>(null, CommandStatus.ValidationError, error);
121142
}
122143

123144
if (request.Lock == null)
124145
{
125146
if (!AuthorizationContextAccessor.AuthorizationContext.IsForcedUnlockAuthorized())
126147
{
127-
error = new ValidationError("nonFieldErrors", ErrorCode.MissingLockId, "Dit is een verplicht veld.");
148+
var error = new ValidationError("nonFieldErrors", ErrorCode.MissingLockId, "Dit is een verplicht veld.");
128149
return new CommandResult<string>(null, CommandStatus.ValidationError, error);
129150
}
130151
}

‎src/OneGround.ZGW.Documenten.Web/Handlers/v1/1/UpdateEnkelvoudigInformatieObjectCommandHandler.cs‎

Lines changed: 46 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
using OneGround.ZGW.Documenten.Services;
2626
using OneGround.ZGW.Documenten.Web.Authorization;
2727
using OneGround.ZGW.Documenten.Web.BusinessRules.v1;
28+
using OneGround.ZGW.Documenten.Web.Concurrency;
2829
using OneGround.ZGW.Documenten.Web.Extensions;
2930
using OneGround.ZGW.Documenten.Web.Notificaties;
3031
using OneGround.ZGW.Documenten.Web.Services;
@@ -36,6 +37,7 @@ public class UpdateEnkelvoudigInformatieObjectCommandHandler
3637
IRequestHandler<UpdateEnkelvoudigInformatieObjectCommand, CommandResult<EnkelvoudigInformatieObjectVersie>>
3738
{
3839
private readonly IEnkelvoudigInformatieObjectMerger _entityMerger;
40+
private readonly ResilienceConcurrencyRetryPipeline<EnkelvoudigInformatieObject> _concurrencyRetryPipeline;
3941

4042
public UpdateEnkelvoudigInformatieObjectCommandHandler(
4143
ILogger<UpdateEnkelvoudigInformatieObjectCommandHandler> logger,
@@ -52,7 +54,8 @@ public UpdateEnkelvoudigInformatieObjectCommandHandler(
5254
IOptions<FormOptions> formOptions,
5355
INotificatieService notificatieService,
5456
IDocumentKenmerkenResolver documentKenmerkenResolver,
55-
IEnkelvoudigInformatieObjectMergerFactory entityMergerFactory
57+
IEnkelvoudigInformatieObjectMergerFactory entityMergerFactory,
58+
ResilienceConcurrencyRetryPipeline<EnkelvoudigInformatieObject> concurrencyRetryPipeline
5659
)
5760
: base(
5861
logger,
@@ -72,6 +75,7 @@ IEnkelvoudigInformatieObjectMergerFactory entityMergerFactory
7275
)
7376
{
7477
_entityMerger = entityMergerFactory.Create<EnkelvoudigInformatieObjectUpdateRequestDto>();
78+
_concurrencyRetryPipeline = concurrencyRetryPipeline;
7579
}
7680

7781
public async Task<CommandResult<EnkelvoudigInformatieObjectVersie>> Handle(
@@ -92,34 +96,54 @@ CancellationToken cancellationToken
9296

9397
var rsinFilter = GetRsinFilterPredicate<EnkelvoudigInformatieObject>();
9498

95-
// First, try to acquire lock on the EnkelvoudigInformatieObject
96-
var existingEnkelvoudigInformatieObject = await _context
97-
.EnkelvoudigInformatieObjecten.LockForUpdate(_context, c => c.Id, [request.ExistingEnkelvoudigInformatieObjectId])
98-
.Where(rsinFilter)
99-
.AsSplitQuery()
100-
.Include(e => e.LatestEnkelvoudigInformatieObjectVersie)
101-
.SingleOrDefaultAsync(e => e.Id == request.ExistingEnkelvoudigInformatieObjectId, cancellationToken);
99+
var (existingEnkelvoudigInformatieObject, status) = await _concurrencyRetryPipeline.ExecuteWithResultAsync(
100+
async (token) =>
101+
{
102+
// First, try to acquire lock on the EnkelvoudigInformatieObject
103+
var _existingEnkelvoudigInformatieObject = await _context
104+
.EnkelvoudigInformatieObjecten.LockForUpdate(_context, c => c.Id, [request.ExistingEnkelvoudigInformatieObjectId])
105+
.Where(rsinFilter)
106+
.Include(e => e.LatestEnkelvoudigInformatieObjectVersie)
107+
.SingleOrDefaultAsync(e => e.Id == request.ExistingEnkelvoudigInformatieObjectId, token);
108+
109+
// The object might be locked OR not exist - check if it exists without lock
110+
if (_existingEnkelvoudigInformatieObject == null)
111+
{
112+
// The object might be locked OR not exist - check if it exists without lock
113+
var exists = await _context
114+
.EnkelvoudigInformatieObjecten.Where(rsinFilter)
115+
.AnyAsync(e => e.Id == request.ExistingEnkelvoudigInformatieObjectId, token);
116+
117+
if (!exists)
118+
{
119+
// Object truly doesn't exist
120+
return (enkelvoudiginformatieobject: null, status: CommandStatus.NotFound);
121+
}
122+
123+
// Throw the exception again so Polly knows a retry is needed (giving up after maximum reached retries)
124+
throw new ConcurrencyConflictException("Concurrency conflict detected.", request.ExistingEnkelvoudigInformatieObjectId);
125+
}
126+
else
127+
{
128+
return (enkelvoudiginformatieobject: _existingEnkelvoudigInformatieObject, status: CommandStatus.OK);
129+
}
130+
},
131+
cancellationToken
132+
);
102133

103-
if (existingEnkelvoudigInformatieObject == null)
134+
if (status == CommandStatus.NotFound)
104135
{
105-
// The object might be locked OR not exist - check if it exists without lock
106-
var exists = await _context
107-
.EnkelvoudigInformatieObjecten.Where(rsinFilter)
108-
.AnyAsync(e => e.Id == request.ExistingEnkelvoudigInformatieObjectId, cancellationToken);
109-
110-
if (!exists)
111-
{
112-
// Object truly doesn't exist
113-
return new CommandResult<EnkelvoudigInformatieObjectVersie>(null, CommandStatus.NotFound);
114-
}
136+
return new CommandResult<EnkelvoudigInformatieObjectVersie>(null, CommandStatus.NotFound);
137+
}
115138

139+
if (status == CommandStatus.Conflict)
140+
{
116141
// Object exists but is locked by another process
117142
var error = new ValidationError(
118143
"nonFieldErrors",
119144
ErrorCode.Conflict,
120145
$"Het enkelvoudiginformatieobject {request.ExistingEnkelvoudigInformatieObjectId} is vergrendeld door een andere bewerking."
121146
);
122-
123147
return new CommandResult<EnkelvoudigInformatieObjectVersie>(null, CommandStatus.Conflict, error);
124148
}
125149

@@ -194,9 +218,9 @@ await _enkelvoudigInformatieObjectBusinessRuleService.ValidateAsync(
194218
// We have enabled (some) metadata fields for the underlying document provider
195219
var metadata = new DocumentMeta { Rsin = versie.InformatieObject.Owner, Version = versie.Versie };
196220

197-
var result = await DocumentService.InitiateMultipartUploadAsync(versie.Bestandsnaam, metadata, cancellationToken);
221+
var multipartUploadResult = await DocumentService.InitiateMultipartUploadAsync(versie.Bestandsnaam, metadata, cancellationToken);
198222

199-
versie.MultiPartDocumentId = result.Context;
223+
versie.MultiPartDocumentId = multipartUploadResult.Context;
200224

201225
AddBestandsDelenToEnkelvoudigeInformatieObjectVersie(versie);
202226

0 commit comments

Comments
 (0)