Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: 412 status on multiple requests without revision header #427

Merged
merged 18 commits into from
Feb 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -48,5 +48,4 @@ Task<List<Guid>> GetExistingIds<TEntity>(
IEnumerable<TEntity> entities,
CancellationToken cancellationToken)
where TEntity : class, IIdentifiableEntity;
bool TrySetOriginalRevision<TEntity>(TEntity entity, Guid? revision) where TEntity : class, IVersionableEntity;
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Digdir.Domain.Dialogporten.Application.Common.ReturnTypes;
using Digdir.Library.Entity.Abstractions.Features.Versionable;
using OneOf.Types;
using OneOf;

Expand All @@ -8,6 +9,11 @@ public interface IUnitOfWork
{
IUnitOfWork WithoutAuditableSideEffects();
Task<SaveChangesResult> SaveChangesAsync(CancellationToken cancellationToken = default);

IUnitOfWork EnableConcurrencyCheck<TEntity>(
TEntity? entity,
Guid? revision)
where TEntity : class, IVersionableEntity;
}

[GenerateOneOf]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ namespace Digdir.Domain.Dialogporten.Application.Features.V1.EndUser.Dialogs.Que
public sealed class GetDialogDto
{
public Guid Id { get; set; }
public Guid Revision { get; set; }
public Guid IfMatchDialogRevision { get; set; }
oskogstad marked this conversation as resolved.
Show resolved Hide resolved
public string Org { get; set; } = null!;
public string ServiceResource { get; set; } = null!;
public string Party { get; set; } = null!;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ internal sealed class MappingProfile : Profile
public MappingProfile()
{
CreateMap<DialogEntity, GetDialogDto>()
.ForMember(dest => dest.IfMatchDialogRevision, opt => opt.MapFrom(src => src.Revision))
.ForMember(dest => dest.Status, opt => opt.MapFrom(src => src.StatusId));

CreateMap<DialogActivity, GetDialogDialogActivityDto>()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ namespace Digdir.Domain.Dialogporten.Application.Features.V1.ServiceOwner.Dialog
public sealed class DeleteDialogCommand : IRequest<DeleteDialogResult>
{
public Guid Id { get; set; }
public Guid? Revision { get; set; }
public Guid? IfMatchDialogRevision { get; set; }
}

[GenerateOneOf]
Expand Down Expand Up @@ -51,9 +51,11 @@ public async Task<DeleteDialogResult> Handle(DeleteDialogCommand request, Cancel
return new EntityNotFound<DialogEntity>(request.Id);
}

_db.TrySetOriginalRevision(dialog, request.Revision);
_db.Dialogs.SoftRemove(dialog);
var saveResult = await _unitOfWork.SaveChangesAsync(cancellationToken);
var saveResult = await _unitOfWork
.EnableConcurrencyCheck(dialog, request.IfMatchDialogRevision)
.SaveChangesAsync(cancellationToken);

return saveResult.Match<DeleteDialogResult>(
success => success,
domainError => throw new UnreachableException("Should never get a domain error when creating a new dialog"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ namespace Digdir.Domain.Dialogporten.Application.Features.V1.ServiceOwner.Dialog
public sealed class UpdateDialogCommand : IRequest<UpdateDialogResult>
{
public Guid Id { get; set; }
public Guid? Revision { get; set; }
public Guid? IfMatchDialogRevision { get; set; }
public UpdateDialogDto Dto { get; set; } = null!;
}

Expand Down Expand Up @@ -75,8 +75,6 @@ public async Task<UpdateDialogResult> Handle(UpdateDialogCommand request, Cancel
return new EntityNotFound<DialogEntity>(request.Id);
}

_db.TrySetOriginalRevision(dialog, request.Revision);

// Update primitive properties
_mapper.Map(request.Dto, dialog);
ValidateTimeFields(dialog);
Expand Down Expand Up @@ -123,8 +121,10 @@ await dialog.Elements
update: UpdateApiActions,
delete: DeleteDelegate.NoOp);

var saveResult = await _unitOfWork
.EnableConcurrencyCheck(dialog, request.IfMatchDialogRevision)
.SaveChangesAsync(cancellationToken);

var saveResult = await _unitOfWork.SaveChangesAsync(cancellationToken);
return saveResult.Match<UpdateDialogResult>(
success => success,
domainError => domainError,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ namespace Digdir.Domain.Dialogporten.Application.Features.V1.ServiceOwner.Dialog
public sealed class GetDialogDto
{
public Guid Id { get; set; }
public Guid Revision { get; set; }
public Guid IfMatchDialogRevision { get; set; }
public string Org { get; set; } = null!;
public string ServiceResource { get; set; } = null!;
public string Party { get; set; } = null!;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ internal sealed class MappingProfile : Profile
public MappingProfile()
{
CreateMap<DialogEntity, GetDialogDto>()
.ForMember(dest => dest.IfMatchDialogRevision, opt => opt.MapFrom(src => src.Revision))
.ForMember(dest => dest.Status, opt => opt.MapFrom(src => src.StatusId));

CreateMap<DialogActivity, GetDialogDialogActivityDto>()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,21 @@ public class DialogActivity : IImmutableEntity, IAggregateCreatedHandler, IEvent
[AggregateChild]
public DialogActivityPerformedBy? PerformedBy { get; set; }

public List<DialogActivity> RelatedActivities { get; set; } = new();
public List<DialogActivity> RelatedActivities { get; set; } = [];

public void OnCreate(AggregateNode self, DateTimeOffset utcNow)
{
_domainEvents.Add(new DialogActivityCreatedDomainEvent(DialogId, Id));
}

private readonly List<IDomainEvent> _domainEvents = new();
public IReadOnlyCollection<IDomainEvent> DomainEvents => _domainEvents;
private readonly List<IDomainEvent> _domainEvents = [];

public IEnumerable<IDomainEvent> PopDomainEvents()
{
var events = _domainEvents.ToList();
_domainEvents.Clear();
return events;
}
}

public class DialogActivityDescription : LocalizationSet
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,5 +128,10 @@ public void UpdateSeenAt(string seenByEndUserId, string? seenByEndUserName)
}

private readonly List<IDomainEvent> _domainEvents = [];
public IReadOnlyCollection<IDomainEvent> DomainEvents => _domainEvents;
public IEnumerable<IDomainEvent> PopDomainEvents()
{
var events = _domainEvents.ToList();
_domainEvents.Clear();
return events;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,11 @@ public class DialogElement : IEntity, IAggregateChangedHandler, IEventPublisher
public DialogElement? RelatedDialogElement { get; set; }

// === Principal relationships ===
[AggregateChild]
public DialogElementDisplayName? DisplayName { get; set; }
[AggregateChild]
public List<DialogElementUrl> Urls { get; set; } = new();
public List<DialogApiAction> ApiActions { get; set; } = new();
public List<DialogActivity> Activities { get; set; } = new();
public List<DialogElement> RelatedDialogElements { get; set; } = new();
[AggregateChild] public DialogElementDisplayName? DisplayName { get; set; }
[AggregateChild] public List<DialogElementUrl> Urls { get; set; } = [];
public List<DialogApiAction> ApiActions { get; set; } = [];
public List<DialogActivity> Activities { get; set; } = [];
public List<DialogElement> RelatedDialogElements { get; set; } = [];

public void OnCreate(AggregateNode self, DateTimeOffset utcNow)
{
Expand All @@ -50,13 +48,19 @@ public void OnDelete(AggregateNode self, DateTimeOffset utcNow)
_domainEvents.Add(new DialogElementDeletedDomainEvent(DialogId, Id, RelatedDialogElementId, Type));
}

public IReadOnlyCollection<IDomainEvent> DomainEvents => _domainEvents;
private readonly List<IDomainEvent> _domainEvents = new();

public void SoftDelete()
{
_domainEvents.Add(new DialogElementDeletedDomainEvent(DialogId, Id, RelatedDialogElementId, Type));
}

private readonly List<IDomainEvent> _domainEvents = [];

public IEnumerable<IDomainEvent> PopDomainEvents()
{
var events = _domainEvents.ToList();
_domainEvents.Clear();
return events;
}
}

public class DialogElementDisplayName : LocalizationSet
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
namespace Digdir.Domain.Dialogporten.Infrastructure.Common.Exceptions;

internal class OptimisticConcurrencyTimeoutException : Exception;
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public override ValueTask<InterceptionResult<int>> SavingChangesAsync(
var domainEvents = dbContext.ChangeTracker.Entries()
.SelectMany(x =>
x.Entity is IEventPublisher publisher
? publisher.DomainEvents
? publisher.PopDomainEvents()
: Enumerable.Empty<IDomainEvent>())
.ToList();

Expand All @@ -43,6 +43,7 @@ x.Entity is IEventPublisher publisher
.ToList();

dbContext.Set<OutboxMessage>().AddRange(outboxMessages);

return base.SavingChangesAsync(eventData, result, cancellationToken);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public DialogDbContext(DbContextOptions<DialogDbContext> options) : base(options
public DbSet<OutboxMessage> OutboxMessages => Set<OutboxMessage>();
public DbSet<OutboxMessageConsumer> OutboxMessageConsumers => Set<OutboxMessageConsumer>();

public bool TrySetOriginalRevision<TEntity>(
internal bool TrySetOriginalRevision<TEntity>(
TEntity? entity,
Guid? revision)
where TEntity : class, IVersionableEntity
Expand Down
80 changes: 80 additions & 0 deletions src/Digdir.Domain.Dialogporten.Infrastructure/UnitOfWork.cs
Original file line number Diff line number Diff line change
@@ -1,20 +1,29 @@
using Digdir.Domain.Dialogporten.Application.Common;
using Digdir.Domain.Dialogporten.Application.Common.ReturnTypes;
using Digdir.Domain.Dialogporten.Application.Externals;
using Digdir.Domain.Dialogporten.Infrastructure.Common.Exceptions;
using Digdir.Domain.Dialogporten.Infrastructure.Persistence;
using Digdir.Library.Entity.Abstractions.Features.Versionable;
using Digdir.Library.Entity.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore;
using OneOf.Types;
using Polly;
using Polly.Contrib.WaitAndRetry;
using Polly.Timeout;
using Polly.Wrap;

namespace Digdir.Domain.Dialogporten.Infrastructure;

internal sealed class UnitOfWork : IUnitOfWork
{
private static readonly AsyncPolicyWrap ConcurrencyRetryPolicy;

private readonly DialogDbContext _dialogDbContext;
private readonly ITransactionTime _transactionTime;
private readonly IDomainContext _domainContext;

private bool _auditableSideEffects = true;
private bool _enableConcurrencyCheck;

public UnitOfWork(DialogDbContext dialogDbContext, ITransactionTime transactionTime, IDomainContext domainContext)
{
Expand All @@ -23,6 +32,44 @@ public UnitOfWork(DialogDbContext dialogDbContext, ITransactionTime transactionT
_domainContext = domainContext ?? throw new ArgumentNullException(nameof(domainContext));
}

static UnitOfWork()
{
// Backoff strategy with jitter for retry policy, starting at ~5ms
const int medianFirstDelayInMs = 5;
// Total timeout for optimistic concurrency handling
const int timeoutInSeconds = 10;

var timeoutPolicy =
Policy.TimeoutAsync(timeoutInSeconds,
TimeoutStrategy.Pessimistic,
(_, _, _) => throw new OptimisticConcurrencyTimeoutException());

// Fetch the db revision and retry
// https://learn.microsoft.com/en-us/ef/core/saving/concurrency?tabs=data-annotations#resolving-concurrency-conflicts
var retryPolicy = Policy
.Handle<DbUpdateConcurrencyException>()
.WaitAndRetryAsync(
sleepDurations: Backoff.DecorrelatedJitterBackoffV2(
medianFirstRetryDelay: TimeSpan.FromMilliseconds(medianFirstDelayInMs),
retryCount: int.MaxValue),
onRetryAsync: FetchCurrentRevision);

ConcurrencyRetryPolicy = timeoutPolicy.WrapAsync(retryPolicy);
}

public IUnitOfWork EnableConcurrencyCheck<TEntity>(
TEntity? entity,
Guid? revision)
where TEntity : class, IVersionableEntity
{
if (_dialogDbContext.TrySetOriginalRevision(entity, revision))
{
_enableConcurrencyCheck = true;
}

return this;
}

public IUnitOfWork WithoutAuditableSideEffects()
{
_auditableSideEffects = false;
Expand All @@ -46,6 +93,14 @@ public async Task<SaveChangesResult> SaveChangesAsync(CancellationToken cancella
await _dialogDbContext.ChangeTracker.HandleAuditableEntities(_transactionTime.Value, cancellationToken);
}

if (!_enableConcurrencyCheck)
{
// Attempt to save changes without concurrency check
await ConcurrencyRetryPolicy.ExecuteAsync(ct => _dialogDbContext.SaveChangesAsync(ct), cancellationToken);

return new Success();
}

try
{
await _dialogDbContext.SaveChangesAsync(cancellationToken);
Expand All @@ -57,4 +112,29 @@ public async Task<SaveChangesResult> SaveChangesAsync(CancellationToken cancella

return new Success();
}

private static async Task FetchCurrentRevision(Exception exception, TimeSpan _)
{
if (exception is not DbUpdateConcurrencyException concurrencyException)
{
return;
}

foreach (var entry in concurrencyException.Entries)
{
if (entry.Entity is not IVersionableEntity)
{
continue;
}

var dbValues = await entry.GetDatabaseValuesAsync();
if (dbValues == null)
{
continue;
}

var currentRevision = dbValues[nameof(IVersionableEntity.Revision)]!;
entry.Property(nameof(IVersionableEntity.Revision)).OriginalValue = currentRevision;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public override async Task HandleAsync(GetDialogQuery req, CancellationToken ct)
await result.Match(
dto =>
{
HttpContext.Response.Headers.ETag = dto.Revision.ToString();
HttpContext.Response.Headers.ETag = dto.IfMatchDialogRevision.ToString();
return SendOkAsync(dto, ct);
},
notFound => this.NotFoundAsync(notFound, ct),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public override async Task HandleAsync(CreateDialogActivityRequest req, Cancella

updateDialogDto.Activities.Add(req);

var updateDialogCommand = new UpdateDialogCommand { Id = req.DialogId, Revision = req.Revision, Dto = updateDialogDto };
var updateDialogCommand = new UpdateDialogCommand { Id = req.DialogId, IfMatchDialogRevision = req.IfMatchDialogRevision, Dto = updateDialogDto };

var result = await _sender.Send(updateDialogCommand, ct);
await result.Match(
Expand All @@ -76,8 +76,8 @@ public sealed class CreateDialogActivityRequest : UpdateDialogDialogActivityDto
{
public Guid DialogId { get; set; }

[FromHeader(headerName: Constants.IfMatch, isRequired: false)]
public Guid? Revision { get; set; }
[FromHeader(headerName: Constants.IfMatch, isRequired: false, removeFromSchema: true)]
public Guid? IfMatchDialogRevision { get; set; }
}

public sealed class CreateDialogActivityEndpointSummary : Summary<CreateDialogActivityEndpoint>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public override async Task HandleAsync(CreateDialogElementRequest req, Cancellat

updateDialogDto.Elements.Add(req);

var updateDialogCommand = new UpdateDialogCommand { Id = req.DialogId, Revision = req.Revision, Dto = updateDialogDto };
var updateDialogCommand = new UpdateDialogCommand { Id = req.DialogId, IfMatchDialogRevision = req.IfMatchDialogRevision, Dto = updateDialogDto };

var result = await _sender.Send(updateDialogCommand, ct);
await result.Match(
Expand All @@ -72,8 +72,8 @@ public sealed class CreateDialogElementRequest : UpdateDialogDialogElementDto
{
public Guid DialogId { get; set; }

[FromHeader(headerName: Constants.IfMatch, isRequired: false)]
public Guid? Revision { get; set; }
[FromHeader(headerName: Constants.IfMatch, isRequired: false, removeFromSchema: true)]
public Guid? IfMatchDialogRevision { get; set; }
}

public sealed class CreateDialogElementEndpointSummary : Summary<CreateDialogElementEndpoint>
Expand Down
Loading
Loading