From e44bf9b243a68eb5f114a09e482efb708c7de3c2 Mon Sep 17 00:00:00 2001 From: eanzhao Date: Mon, 27 Apr 2026 14:27:12 +0800 Subject: [PATCH 1/4] feat(studio): optimistic concurrency for role/connector catalog drafts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #432. Catalog state and draft persistence is now version-aware. Proto: - RoleCatalogState / ConnectorCatalogState: add `last_applied_event_version` - *CatalogSavedEvent / *DraftSavedEvent / *DraftDeletedEvent: add `optional int64 expected_version` (proto3 optional; unset = skip check) Actor (RoleCatalogGAgent, ConnectorCatalogGAgent): - Each Apply step bumps `LastAppliedEventVersion`. - Each handler validates `expected_version` if set; mismatch raises EventStoreOptimisticConcurrencyException (foundation-defined). Store / contracts: - IRoleCatalogStore / IConnectorCatalogStore: write methods now accept optional `expectedVersion` (default null = backward compatible). - Stored DTOs (StoredRole/ConnectorCatalog/Draft) carry `Version`. - Service requests (Save*Request) carry `ExpectedVersion?`; responses carry `Version`. HTTP (RolesController, ConnectorsController): - GET writes `ETag: ""` response header. - PUT/DELETE read `If-Match` header (also accept `expectedVersion` in request body); 409 Conflict on EventStoreOptimisticConcurrencyException with `code: VERSION_CONFLICT`. - New ETagSupport helper for header parsing/emission. Backward compatibility: - Callers omitting `expected_version` / `If-Match` continue to work as last-writer-wins (existing CLI tests / fixtures unchanged). Tests: - Apply increment: RoleCatalog_Apply_IncrementsLastAppliedEventVersion + ConnectorCatalog equivalent. - Store-level event plumbing: SaveDraft/DeleteDraft with/without expectedVersion → event ExpectedVersion field correctly set/unset. - Store-level read: GetDraft surfaces Version from projection state. Out of scope (deferred): - End-to-end actor harness for the conflict-throw path. Foundation's EventStoreOptimisticConcurrencyException semantics are exercised separately; integration coverage to follow. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../ConnectorCatalogGAgent.cs | 23 ++- .../connector_catalog_messages.proto | 4 + .../RoleCatalogGAgent.cs | 22 +++ .../role_catalog_messages.proto | 7 +- .../Abstractions/IConnectorCatalogStore.cs | 6 +- .../Studio/Abstractions/IRoleCatalogStore.cs | 6 +- .../Abstractions/IStudioWorkspaceStore.cs | 12 +- .../Studio/Contracts/ConnectorContracts.cs | 12 +- .../Studio/Contracts/RoleContracts.cs | 12 +- .../Studio/Services/ConnectorService.cs | 14 +- .../Studio/Services/RoleCatalogService.cs | 14 +- .../Controllers/ConnectorsController.cs | 50 +++++- .../Controllers/ETagSupport.cs | 32 ++++ .../Controllers/RolesController.cs | 50 +++++- .../ActorBackedConnectorCatalogStore.cs | 38 +++-- .../ActorBackedRoleCatalogStore.cs | 33 +++- .../ActorBackedGAgentStateTransitionTests.cs | 42 ++++++ .../ActorBackedStoreAdapterTests.cs | 142 ++++++++++++++++++ .../ConnectorServiceTests.cs | 6 +- 19 files changed, 470 insertions(+), 55 deletions(-) create mode 100644 src/Aevatar.Studio.Hosting/Controllers/ETagSupport.cs diff --git a/agents/Aevatar.GAgents.ConnectorCatalog/ConnectorCatalogGAgent.cs b/agents/Aevatar.GAgents.ConnectorCatalog/ConnectorCatalogGAgent.cs index 928cae92f..5eb9fd46c 100644 --- a/agents/Aevatar.GAgents.ConnectorCatalog/ConnectorCatalogGAgent.cs +++ b/agents/Aevatar.GAgents.ConnectorCatalog/ConnectorCatalogGAgent.cs @@ -1,5 +1,6 @@ using Aevatar.Foundation.Abstractions; using Aevatar.Foundation.Abstractions.Attributes; +using Aevatar.Foundation.Abstractions.Persistence; using Aevatar.Foundation.Core; using Aevatar.Foundation.Core.EventSourcing; using Google.Protobuf; @@ -21,19 +22,22 @@ public sealed class ConnectorCatalogGAgent : GAgentBase, [EventHandler(EndpointName = "saveCatalog")] public async Task HandleCatalogSaved(ConnectorCatalogSavedEvent evt) { + EnsureExpectedVersionMatches(evt.HasExpectedVersion, evt.ExpectedVersion); await PersistDomainEventAsync(evt); } [EventHandler(EndpointName = "saveDraft")] public async Task HandleDraftSaved(ConnectorDraftSavedEvent evt) { + EnsureExpectedVersionMatches(evt.HasExpectedVersion, evt.ExpectedVersion); await PersistDomainEventAsync(evt); } [EventHandler(EndpointName = "deleteDraft")] public async Task HandleDraftDeleted(ConnectorDraftDeletedEvent evt) { - // Idempotent: skip if no draft exists + EnsureExpectedVersionMatches(evt.HasExpectedVersion, evt.ExpectedVersion); + if (State.Draft is null) return; @@ -56,12 +60,27 @@ protected override ConnectorCatalogState TransitionState( .OrCurrent(); } + private void EnsureExpectedVersionMatches(bool hasExpectedVersion, long expectedVersion) + { + if (!hasExpectedVersion) + return; + + if (expectedVersion != State.LastAppliedEventVersion) + { + throw new EventStoreOptimisticConcurrencyException( + Id, + expectedVersion, + State.LastAppliedEventVersion); + } + } + private static ConnectorCatalogState ApplyCatalogSaved( ConnectorCatalogState state, ConnectorCatalogSavedEvent evt) { var next = state.Clone(); next.Connectors.Clear(); next.Connectors.AddRange(evt.Connectors); + next.LastAppliedEventVersion = state.LastAppliedEventVersion + 1; return next; } @@ -74,6 +93,7 @@ private static ConnectorCatalogState ApplyDraftSaved( Draft = evt.Draft?.Clone(), UpdatedAtUtc = evt.UpdatedAtUtc, }; + next.LastAppliedEventVersion = state.LastAppliedEventVersion + 1; return next; } @@ -82,6 +102,7 @@ private static ConnectorCatalogState ApplyDraftDeleted( { var next = state.Clone(); next.Draft = null; + next.LastAppliedEventVersion = state.LastAppliedEventVersion + 1; return next; } diff --git a/agents/Aevatar.GAgents.ConnectorCatalog/connector_catalog_messages.proto b/agents/Aevatar.GAgents.ConnectorCatalog/connector_catalog_messages.proto index 6fa2e3b3a..ecef29294 100644 --- a/agents/Aevatar.GAgents.ConnectorCatalog/connector_catalog_messages.proto +++ b/agents/Aevatar.GAgents.ConnectorCatalog/connector_catalog_messages.proto @@ -66,19 +66,23 @@ message ConnectorDraftEntry { message ConnectorCatalogState { repeated ConnectorDefinitionEntry connectors = 1; ConnectorDraftEntry draft = 2; // null when no draft exists + int64 last_applied_event_version = 3; } // ─── Events ─── message ConnectorCatalogSavedEvent { repeated ConnectorDefinitionEntry connectors = 1; + optional int64 expected_version = 2; // unset = skip optimistic concurrency check } message ConnectorDraftSavedEvent { ConnectorDefinitionEntry draft = 1; google.protobuf.Timestamp updated_at_utc = 2; + optional int64 expected_version = 3; // unset = skip optimistic concurrency check } message ConnectorDraftDeletedEvent { + optional int64 expected_version = 1; // unset = skip optimistic concurrency check } diff --git a/agents/Aevatar.GAgents.RoleCatalog/RoleCatalogGAgent.cs b/agents/Aevatar.GAgents.RoleCatalog/RoleCatalogGAgent.cs index 58f44b6c0..82a3cdd4a 100644 --- a/agents/Aevatar.GAgents.RoleCatalog/RoleCatalogGAgent.cs +++ b/agents/Aevatar.GAgents.RoleCatalog/RoleCatalogGAgent.cs @@ -1,5 +1,6 @@ using Aevatar.Foundation.Abstractions; using Aevatar.Foundation.Abstractions.Attributes; +using Aevatar.Foundation.Abstractions.Persistence; using Aevatar.Foundation.Core; using Aevatar.Foundation.Core.EventSourcing; using Google.Protobuf; @@ -21,18 +22,22 @@ public sealed class RoleCatalogGAgent : GAgentBase, IProjected [EventHandler(EndpointName = "saveCatalog")] public async Task HandleCatalogSaved(RoleCatalogSavedEvent evt) { + EnsureExpectedVersionMatches(evt.HasExpectedVersion, evt.ExpectedVersion); await PersistDomainEventAsync(evt); } [EventHandler(EndpointName = "saveDraft")] public async Task HandleDraftSaved(RoleDraftSavedEvent evt) { + EnsureExpectedVersionMatches(evt.HasExpectedVersion, evt.ExpectedVersion); await PersistDomainEventAsync(evt); } [EventHandler(EndpointName = "deleteDraft")] public async Task HandleDraftDeleted(RoleDraftDeletedEvent evt) { + EnsureExpectedVersionMatches(evt.HasExpectedVersion, evt.ExpectedVersion); + if (State.Draft is null) return; @@ -55,12 +60,27 @@ protected override RoleCatalogState TransitionState( .OrCurrent(); } + private void EnsureExpectedVersionMatches(bool hasExpectedVersion, long expectedVersion) + { + if (!hasExpectedVersion) + return; + + if (expectedVersion != State.LastAppliedEventVersion) + { + throw new EventStoreOptimisticConcurrencyException( + Id, + expectedVersion, + State.LastAppliedEventVersion); + } + } + private static RoleCatalogState ApplyCatalogSaved( RoleCatalogState state, RoleCatalogSavedEvent evt) { var next = state.Clone(); next.Roles.Clear(); next.Roles.AddRange(evt.Roles); + next.LastAppliedEventVersion = state.LastAppliedEventVersion + 1; return next; } @@ -73,6 +93,7 @@ private static RoleCatalogState ApplyDraftSaved( Draft = evt.Draft?.Clone(), UpdatedAtUtc = evt.UpdatedAtUtc, }; + next.LastAppliedEventVersion = state.LastAppliedEventVersion + 1; return next; } @@ -81,6 +102,7 @@ private static RoleCatalogState ApplyDraftDeleted( { var next = state.Clone(); next.Draft = null; + next.LastAppliedEventVersion = state.LastAppliedEventVersion + 1; return next; } diff --git a/agents/Aevatar.GAgents.RoleCatalog/role_catalog_messages.proto b/agents/Aevatar.GAgents.RoleCatalog/role_catalog_messages.proto index c3b995e1c..57a553641 100644 --- a/agents/Aevatar.GAgents.RoleCatalog/role_catalog_messages.proto +++ b/agents/Aevatar.GAgents.RoleCatalog/role_catalog_messages.proto @@ -23,18 +23,23 @@ message RoleDraftEntry { message RoleCatalogState { repeated RoleDefinitionEntry roles = 1; RoleDraftEntry draft = 2; // null when no draft exists + int64 last_applied_event_version = 3; } // ─── Events ─── message RoleCatalogSavedEvent { repeated RoleDefinitionEntry roles = 1; + optional int64 expected_version = 2; // unset = skip optimistic concurrency check } message RoleDraftSavedEvent { RoleDefinitionEntry draft = 1; google.protobuf.Timestamp updated_at_utc = 2; + optional int64 expected_version = 3; // unset = skip optimistic concurrency check } -message RoleDraftDeletedEvent {} +message RoleDraftDeletedEvent { + optional int64 expected_version = 1; // unset = skip optimistic concurrency check +} diff --git a/src/Aevatar.Studio.Application/Studio/Abstractions/IConnectorCatalogStore.cs b/src/Aevatar.Studio.Application/Studio/Abstractions/IConnectorCatalogStore.cs index e4728ee03..d36617fb2 100644 --- a/src/Aevatar.Studio.Application/Studio/Abstractions/IConnectorCatalogStore.cs +++ b/src/Aevatar.Studio.Application/Studio/Abstractions/IConnectorCatalogStore.cs @@ -6,6 +6,7 @@ public interface IConnectorCatalogStore Task SaveConnectorCatalogAsync( StoredConnectorCatalog catalog, + long? expectedVersion = null, CancellationToken cancellationToken = default); Task ImportLocalCatalogAsync(CancellationToken cancellationToken = default); @@ -14,9 +15,12 @@ Task SaveConnectorCatalogAsync( Task SaveConnectorDraftAsync( StoredConnectorDraft draft, + long? expectedVersion = null, CancellationToken cancellationToken = default); - Task DeleteConnectorDraftAsync(CancellationToken cancellationToken = default); + Task DeleteConnectorDraftAsync( + long? expectedVersion = null, + CancellationToken cancellationToken = default); } public sealed record ImportedConnectorCatalog( diff --git a/src/Aevatar.Studio.Application/Studio/Abstractions/IRoleCatalogStore.cs b/src/Aevatar.Studio.Application/Studio/Abstractions/IRoleCatalogStore.cs index cc9094f1d..8fee88550 100644 --- a/src/Aevatar.Studio.Application/Studio/Abstractions/IRoleCatalogStore.cs +++ b/src/Aevatar.Studio.Application/Studio/Abstractions/IRoleCatalogStore.cs @@ -6,6 +6,7 @@ public interface IRoleCatalogStore Task SaveRoleCatalogAsync( StoredRoleCatalog catalog, + long? expectedVersion = null, CancellationToken cancellationToken = default); Task ImportLocalCatalogAsync(CancellationToken cancellationToken = default); @@ -14,9 +15,12 @@ Task SaveRoleCatalogAsync( Task SaveRoleDraftAsync( StoredRoleDraft draft, + long? expectedVersion = null, CancellationToken cancellationToken = default); - Task DeleteRoleDraftAsync(CancellationToken cancellationToken = default); + Task DeleteRoleDraftAsync( + long? expectedVersion = null, + CancellationToken cancellationToken = default); } public sealed record ImportedRoleCatalog( diff --git a/src/Aevatar.Studio.Application/Studio/Abstractions/IStudioWorkspaceStore.cs b/src/Aevatar.Studio.Application/Studio/Abstractions/IStudioWorkspaceStore.cs index 15319b502..db2fc507a 100644 --- a/src/Aevatar.Studio.Application/Studio/Abstractions/IStudioWorkspaceStore.cs +++ b/src/Aevatar.Studio.Application/Studio/Abstractions/IStudioWorkspaceStore.cs @@ -91,27 +91,31 @@ public sealed record StoredConnectorCatalog( string HomeDirectory, string FilePath, bool FileExists, - IReadOnlyList Connectors); + IReadOnlyList Connectors, + long Version = 0); public sealed record StoredRoleCatalog( string HomeDirectory, string FilePath, bool FileExists, - IReadOnlyList Roles); + IReadOnlyList Roles, + long Version = 0); public sealed record StoredConnectorDraft( string HomeDirectory, string FilePath, bool FileExists, DateTimeOffset? UpdatedAtUtc, - StoredConnectorDefinition? Draft); + StoredConnectorDefinition? Draft, + long Version = 0); public sealed record StoredRoleDraft( string HomeDirectory, string FilePath, bool FileExists, DateTimeOffset? UpdatedAtUtc, - StoredRoleDefinition? Draft); + StoredRoleDefinition? Draft, + long Version = 0); public sealed record StoredConnectorDefinition( string Name, diff --git a/src/Aevatar.Studio.Application/Studio/Contracts/ConnectorContracts.cs b/src/Aevatar.Studio.Application/Studio/Contracts/ConnectorContracts.cs index 3a6ada389..639892d39 100644 --- a/src/Aevatar.Studio.Application/Studio/Contracts/ConnectorContracts.cs +++ b/src/Aevatar.Studio.Application/Studio/Contracts/ConnectorContracts.cs @@ -4,20 +4,24 @@ public sealed record ConnectorCatalogResponse( string HomeDirectory, string FilePath, bool FileExists, - IReadOnlyList Connectors); + IReadOnlyList Connectors, + long Version = 0); public sealed record ConnectorDraftResponse( string HomeDirectory, string FilePath, bool FileExists, DateTimeOffset? UpdatedAtUtc, - ConnectorDefinitionDto? Draft); + ConnectorDefinitionDto? Draft, + long Version = 0); public sealed record SaveConnectorCatalogRequest( - IReadOnlyList Connectors); + IReadOnlyList Connectors, + long? ExpectedVersion = null); public sealed record SaveConnectorDraftRequest( - ConnectorDefinitionDto? Draft); + ConnectorDefinitionDto? Draft, + long? ExpectedVersion = null); public sealed record ImportConnectorCatalogResponse( string SourceFilePath, diff --git a/src/Aevatar.Studio.Application/Studio/Contracts/RoleContracts.cs b/src/Aevatar.Studio.Application/Studio/Contracts/RoleContracts.cs index 46e2b94de..8825c53ed 100644 --- a/src/Aevatar.Studio.Application/Studio/Contracts/RoleContracts.cs +++ b/src/Aevatar.Studio.Application/Studio/Contracts/RoleContracts.cs @@ -4,20 +4,24 @@ public sealed record RoleCatalogResponse( string HomeDirectory, string FilePath, bool FileExists, - IReadOnlyList Roles); + IReadOnlyList Roles, + long Version = 0); public sealed record RoleDraftResponse( string HomeDirectory, string FilePath, bool FileExists, DateTimeOffset? UpdatedAtUtc, - RoleDefinitionDto? Draft); + RoleDefinitionDto? Draft, + long Version = 0); public sealed record SaveRoleCatalogRequest( - IReadOnlyList Roles); + IReadOnlyList Roles, + long? ExpectedVersion = null); public sealed record SaveRoleDraftRequest( - RoleDefinitionDto? Draft); + RoleDefinitionDto? Draft, + long? ExpectedVersion = null); public sealed record ImportRoleCatalogResponse( string SourceFilePath, diff --git a/src/Aevatar.Studio.Application/Studio/Services/ConnectorService.cs b/src/Aevatar.Studio.Application/Studio/Services/ConnectorService.cs index 91672e2de..362e5140d 100644 --- a/src/Aevatar.Studio.Application/Studio/Services/ConnectorService.cs +++ b/src/Aevatar.Studio.Application/Studio/Services/ConnectorService.cs @@ -52,6 +52,7 @@ public async Task SaveCatalogAsync( .Where(connector => !string.IsNullOrWhiteSpace(connector.Name)) .Select(ToStoredConnector) .ToList()), + request.ExpectedVersion, cancellationToken); return ToResponse(saved); @@ -103,7 +104,7 @@ public async Task SaveDraftAsync( { if (request.Draft is null) { - await _store.DeleteConnectorDraftAsync(cancellationToken); + await _store.DeleteConnectorDraftAsync(request.ExpectedVersion, cancellationToken); return await GetDraftAsync(cancellationToken); } @@ -114,13 +115,14 @@ public async Task SaveDraftAsync( FileExists: false, UpdatedAtUtc: DateTimeOffset.UtcNow, Draft: ToStoredConnectorDraft(request.Draft)), + request.ExpectedVersion, cancellationToken); return ToDraftResponse(saved); } - public Task DeleteDraftAsync(CancellationToken cancellationToken = default) => - _store.DeleteConnectorDraftAsync(cancellationToken); + public Task DeleteDraftAsync(long? expectedVersion = null, CancellationToken cancellationToken = default) => + _store.DeleteConnectorDraftAsync(expectedVersion, cancellationToken); private static void EnsureUniqueNames(IEnumerable connectors) { @@ -279,7 +281,8 @@ private static ConnectorCatalogResponse ToResponse(StoredConnectorCatalog catalo catalog.HomeDirectory, catalog.FilePath, catalog.FileExists, - catalog.Connectors.Select(ToDto).ToList()); + catalog.Connectors.Select(ToDto).ToList(), + catalog.Version); private static ImportConnectorCatalogResponse ToImportResponse(ImportedConnectorCatalog imported) => new( @@ -297,7 +300,8 @@ private static ConnectorDraftResponse ToDraftResponse(StoredConnectorDraft draft draft.FilePath, draft.FileExists, draft.UpdatedAtUtc, - draft.Draft is null ? null : ToDto(draft.Draft)); + draft.Draft is null ? null : ToDto(draft.Draft), + draft.Version); private static ConnectorDefinitionDto ToDto(StoredConnectorDefinition connector) => new( diff --git a/src/Aevatar.Studio.Application/Studio/Services/RoleCatalogService.cs b/src/Aevatar.Studio.Application/Studio/Services/RoleCatalogService.cs index 8e692c278..9f5e61340 100644 --- a/src/Aevatar.Studio.Application/Studio/Services/RoleCatalogService.cs +++ b/src/Aevatar.Studio.Application/Studio/Services/RoleCatalogService.cs @@ -45,6 +45,7 @@ public async Task SaveCatalogAsync( .Where(role => !string.IsNullOrWhiteSpace(role.Id)) .Select(ToStoredRole) .ToList()), + request.ExpectedVersion, cancellationToken); return ToResponse(saved); @@ -96,7 +97,7 @@ public async Task SaveDraftAsync( { if (request.Draft is null) { - await _store.DeleteRoleDraftAsync(cancellationToken); + await _store.DeleteRoleDraftAsync(request.ExpectedVersion, cancellationToken); return await GetDraftAsync(cancellationToken); } @@ -107,13 +108,14 @@ public async Task SaveDraftAsync( FileExists: false, UpdatedAtUtc: DateTimeOffset.UtcNow, Draft: ToStoredRoleDraft(request.Draft)), + request.ExpectedVersion, cancellationToken); return ToDraftResponse(saved); } - public Task DeleteDraftAsync(CancellationToken cancellationToken = default) => - _store.DeleteRoleDraftAsync(cancellationToken); + public Task DeleteDraftAsync(long? expectedVersion = null, CancellationToken cancellationToken = default) => + _store.DeleteRoleDraftAsync(expectedVersion, cancellationToken); private static void EnsureUniqueIds(IEnumerable roles) { @@ -164,7 +166,8 @@ private static RoleCatalogResponse ToResponse(StoredRoleCatalog catalog) => catalog.HomeDirectory, catalog.FilePath, catalog.FileExists, - catalog.Roles.Select(ToDto).ToList()); + catalog.Roles.Select(ToDto).ToList(), + catalog.Version); private static RoleDraftResponse ToDraftResponse(StoredRoleDraft draft) => new( @@ -172,7 +175,8 @@ private static RoleDraftResponse ToDraftResponse(StoredRoleDraft draft) => draft.FilePath, draft.FileExists, draft.UpdatedAtUtc, - draft.Draft is null ? null : ToDto(draft.Draft)); + draft.Draft is null ? null : ToDto(draft.Draft), + draft.Version); private static RoleDefinitionDto ToDto(StoredRoleDefinition role) => new( diff --git a/src/Aevatar.Studio.Hosting/Controllers/ConnectorsController.cs b/src/Aevatar.Studio.Hosting/Controllers/ConnectorsController.cs index 63c4d6033..a2e97e6c9 100644 --- a/src/Aevatar.Studio.Hosting/Controllers/ConnectorsController.cs +++ b/src/Aevatar.Studio.Hosting/Controllers/ConnectorsController.cs @@ -1,3 +1,4 @@ +using Aevatar.Foundation.Abstractions.Persistence; using Aevatar.Studio.Application.Studio.Contracts; using Aevatar.Studio.Application.Studio.Services; using Aevatar.Studio.Infrastructure.Storage; @@ -22,7 +23,9 @@ public async Task> Get(CancellationToken { try { - return Ok(await _connectorService.GetCatalogAsync(cancellationToken)); + var response = await _connectorService.GetCatalogAsync(cancellationToken); + ETagSupport.WriteETag(Response, response.Version); + return Ok(response); } catch (ChronoStorageServiceException exception) { @@ -43,7 +46,9 @@ public async Task> GetDraft(CancellationTok { try { - return Ok(await _connectorService.GetDraftAsync(cancellationToken)); + var response = await _connectorService.GetDraftAsync(cancellationToken); + ETagSupport.WriteETag(Response, response.Version); + return Ok(response); } catch (ChronoStorageServiceException exception) { @@ -64,9 +69,16 @@ public async Task> Save( [FromBody] SaveConnectorCatalogRequest request, CancellationToken cancellationToken) { + var effectiveRequest = ApplyIfMatch(request); try { - return Ok(await _connectorService.SaveCatalogAsync(request, cancellationToken)); + var response = await _connectorService.SaveCatalogAsync(effectiveRequest, cancellationToken); + ETagSupport.WriteETag(Response, response.Version); + return Ok(response); + } + catch (EventStoreOptimisticConcurrencyException exception) + { + return Conflict(new { code = "VERSION_CONFLICT", message = exception.Message }); } catch (ChronoStorageServiceException exception) { @@ -112,9 +124,16 @@ public async Task> SaveDraft( [FromBody] SaveConnectorDraftRequest request, CancellationToken cancellationToken) { + var effectiveRequest = ApplyIfMatch(request); try { - return Ok(await _connectorService.SaveDraftAsync(request, cancellationToken)); + var response = await _connectorService.SaveDraftAsync(effectiveRequest, cancellationToken); + ETagSupport.WriteETag(Response, response.Version); + return Ok(response); + } + catch (EventStoreOptimisticConcurrencyException exception) + { + return Conflict(new { code = "VERSION_CONFLICT", message = exception.Message }); } catch (ChronoStorageServiceException exception) { @@ -133,11 +152,16 @@ public async Task> SaveDraft( [HttpDelete("draft")] public async Task DeleteDraft(CancellationToken cancellationToken) { + var expectedVersion = ETagSupport.TryParseIfMatch(Request); try { - await _connectorService.DeleteDraftAsync(cancellationToken); + await _connectorService.DeleteDraftAsync(expectedVersion, cancellationToken); return NoContent(); } + catch (EventStoreOptimisticConcurrencyException exception) + { + return Conflict(new { code = "VERSION_CONFLICT", message = exception.Message }); + } catch (ChronoStorageServiceException exception) { return ChronoStorageErrorResponses.ToActionResult(exception); @@ -151,4 +175,20 @@ public async Task DeleteDraft(CancellationToken cancellationToken return StatusCode(StatusCodes.Status502BadGateway, new { message = exception.Message }); } } + + private SaveConnectorCatalogRequest ApplyIfMatch(SaveConnectorCatalogRequest request) + { + if (request.ExpectedVersion is not null) + return request; + var expected = ETagSupport.TryParseIfMatch(Request); + return expected is null ? request : request with { ExpectedVersion = expected }; + } + + private SaveConnectorDraftRequest ApplyIfMatch(SaveConnectorDraftRequest request) + { + if (request.ExpectedVersion is not null) + return request; + var expected = ETagSupport.TryParseIfMatch(Request); + return expected is null ? request : request with { ExpectedVersion = expected }; + } } diff --git a/src/Aevatar.Studio.Hosting/Controllers/ETagSupport.cs b/src/Aevatar.Studio.Hosting/Controllers/ETagSupport.cs new file mode 100644 index 000000000..e4aa20954 --- /dev/null +++ b/src/Aevatar.Studio.Hosting/Controllers/ETagSupport.cs @@ -0,0 +1,32 @@ +using Microsoft.AspNetCore.Http; +using Microsoft.Net.Http.Headers; + +namespace Aevatar.Studio.Hosting.Controllers; + +internal static class ETagSupport +{ + private const string IfMatchHeader = "If-Match"; + private const string ETagHeader = "ETag"; + + /// + /// Parse an If-Match header into a long version. Accepts both quoted ETag form + /// ("5") and bare integer (5). Returns null when missing or unparseable. + /// + public static long? TryParseIfMatch(HttpRequest request) + { + if (!request.Headers.TryGetValue(IfMatchHeader, out var values) || values.Count == 0) + return null; + + var raw = values.ToString().Trim(); + if (string.IsNullOrEmpty(raw)) + return null; + + var unquoted = raw.Trim('"'); + return long.TryParse(unquoted, out var version) ? version : null; + } + + public static void WriteETag(HttpResponse response, long version) + { + response.Headers[HeaderNames.ETag] = $"\"{version}\""; + } +} diff --git a/src/Aevatar.Studio.Hosting/Controllers/RolesController.cs b/src/Aevatar.Studio.Hosting/Controllers/RolesController.cs index a02085382..b1bbf2955 100644 --- a/src/Aevatar.Studio.Hosting/Controllers/RolesController.cs +++ b/src/Aevatar.Studio.Hosting/Controllers/RolesController.cs @@ -1,3 +1,4 @@ +using Aevatar.Foundation.Abstractions.Persistence; using Aevatar.Studio.Application.Studio.Contracts; using Aevatar.Studio.Application.Studio.Services; using Aevatar.Studio.Infrastructure.Storage; @@ -22,7 +23,9 @@ public async Task> Get(CancellationToken cance { try { - return Ok(await _roleCatalogService.GetCatalogAsync(cancellationToken)); + var response = await _roleCatalogService.GetCatalogAsync(cancellationToken); + ETagSupport.WriteETag(Response, response.Version); + return Ok(response); } catch (ChronoStorageServiceException exception) { @@ -43,7 +46,9 @@ public async Task> GetDraft(CancellationToken ca { try { - return Ok(await _roleCatalogService.GetDraftAsync(cancellationToken)); + var response = await _roleCatalogService.GetDraftAsync(cancellationToken); + ETagSupport.WriteETag(Response, response.Version); + return Ok(response); } catch (ChronoStorageServiceException exception) { @@ -64,9 +69,16 @@ public async Task> Save( [FromBody] SaveRoleCatalogRequest request, CancellationToken cancellationToken) { + var effectiveRequest = ApplyIfMatch(request); try { - return Ok(await _roleCatalogService.SaveCatalogAsync(request, cancellationToken)); + var response = await _roleCatalogService.SaveCatalogAsync(effectiveRequest, cancellationToken); + ETagSupport.WriteETag(Response, response.Version); + return Ok(response); + } + catch (EventStoreOptimisticConcurrencyException exception) + { + return Conflict(new { code = "VERSION_CONFLICT", message = exception.Message }); } catch (ChronoStorageServiceException exception) { @@ -112,9 +124,16 @@ public async Task> SaveDraft( [FromBody] SaveRoleDraftRequest request, CancellationToken cancellationToken) { + var effectiveRequest = ApplyIfMatch(request); try { - return Ok(await _roleCatalogService.SaveDraftAsync(request, cancellationToken)); + var response = await _roleCatalogService.SaveDraftAsync(effectiveRequest, cancellationToken); + ETagSupport.WriteETag(Response, response.Version); + return Ok(response); + } + catch (EventStoreOptimisticConcurrencyException exception) + { + return Conflict(new { code = "VERSION_CONFLICT", message = exception.Message }); } catch (ChronoStorageServiceException exception) { @@ -133,11 +152,16 @@ public async Task> SaveDraft( [HttpDelete("draft")] public async Task DeleteDraft(CancellationToken cancellationToken) { + var expectedVersion = ETagSupport.TryParseIfMatch(Request); try { - await _roleCatalogService.DeleteDraftAsync(cancellationToken); + await _roleCatalogService.DeleteDraftAsync(expectedVersion, cancellationToken); return NoContent(); } + catch (EventStoreOptimisticConcurrencyException exception) + { + return Conflict(new { code = "VERSION_CONFLICT", message = exception.Message }); + } catch (ChronoStorageServiceException exception) { return ChronoStorageErrorResponses.ToActionResult(exception); @@ -151,4 +175,20 @@ public async Task DeleteDraft(CancellationToken cancellationToken return StatusCode(StatusCodes.Status502BadGateway, new { message = exception.Message }); } } + + private SaveRoleCatalogRequest ApplyIfMatch(SaveRoleCatalogRequest request) + { + if (request.ExpectedVersion is not null) + return request; + var expected = ETagSupport.TryParseIfMatch(Request); + return expected is null ? request : request with { ExpectedVersion = expected }; + } + + private SaveRoleDraftRequest ApplyIfMatch(SaveRoleDraftRequest request) + { + if (request.ExpectedVersion is not null) + return request; + var expected = ETagSupport.TryParseIfMatch(Request); + return expected is null ? request : request with { ExpectedVersion = expected }; + } } diff --git a/src/Aevatar.Studio.Infrastructure/ActorBacked/ActorBackedConnectorCatalogStore.cs b/src/Aevatar.Studio.Infrastructure/ActorBacked/ActorBackedConnectorCatalogStore.cs index edd18b0a3..e33b65bd4 100644 --- a/src/Aevatar.Studio.Infrastructure/ActorBacked/ActorBackedConnectorCatalogStore.cs +++ b/src/Aevatar.Studio.Infrastructure/ActorBacked/ActorBackedConnectorCatalogStore.cs @@ -48,13 +48,15 @@ public async Task GetConnectorCatalogAsync( CancellationToken cancellationToken = default) { var state = await ReadProjectedStateAsync(cancellationToken); + var version = state?.LastAppliedEventVersion ?? 0; if (state is null) { return new StoredConnectorCatalog( HomeDirectory: ActorHomeDirectory, FilePath: ActorFilePath, FileExists: false, - Connectors: []); + Connectors: [], + Version: version); } var connectors = state.Connectors @@ -66,23 +68,29 @@ public async Task GetConnectorCatalogAsync( HomeDirectory: ActorHomeDirectory, FilePath: ActorFilePath, FileExists: connectors.Count > 0, - Connectors: connectors); + Connectors: connectors, + Version: version); } public async Task SaveConnectorCatalogAsync( StoredConnectorCatalog catalog, + long? expectedVersion = null, CancellationToken cancellationToken = default) { var actor = await EnsureWriteActorAsync(cancellationToken); var evt = new ConnectorCatalogSavedEvent(); evt.Connectors.AddRange(catalog.Connectors.Select(ToProtoConnectorDefinition)); + if (expectedVersion is not null) + evt.ExpectedVersion = expectedVersion.Value; await ActorCommandDispatcher.SendAsync(_dispatchPort, actor, evt, cancellationToken); + var refreshed = await ReadProjectedStateAsync(cancellationToken); return new StoredConnectorCatalog( HomeDirectory: ActorHomeDirectory, FilePath: ActorFilePath, FileExists: true, - Connectors: catalog.Connectors); + Connectors: catalog.Connectors, + Version: refreshed?.LastAppliedEventVersion ?? ((expectedVersion ?? catalog.Version) + 1)); } public async Task ImportLocalCatalogAsync( @@ -114,6 +122,7 @@ public async Task GetConnectorDraftAsync( { var state = await ReadProjectedStateAsync(cancellationToken); var draftEntry = state?.Draft; + var version = state?.LastAppliedEventVersion ?? 0; if (draftEntry is null) { return new StoredConnectorDraft( @@ -121,7 +130,8 @@ public async Task GetConnectorDraftAsync( FilePath: ActorFilePath + "/draft", FileExists: false, UpdatedAtUtc: null, - Draft: null); + Draft: null, + Version: version); } return new StoredConnectorDraft( @@ -129,11 +139,13 @@ public async Task GetConnectorDraftAsync( FilePath: ActorFilePath + "/draft", FileExists: true, UpdatedAtUtc: draftEntry.UpdatedAtUtc?.ToDateTimeOffset(), - Draft: draftEntry.Draft is not null ? ToStoredConnectorDefinition(draftEntry.Draft) : null); + Draft: draftEntry.Draft is not null ? ToStoredConnectorDefinition(draftEntry.Draft) : null, + Version: version); } public async Task SaveConnectorDraftAsync( StoredConnectorDraft draft, + long? expectedVersion = null, CancellationToken cancellationToken = default) { var actor = await EnsureWriteActorAsync(cancellationToken); @@ -143,23 +155,31 @@ public async Task SaveConnectorDraftAsync( Draft = draft.Draft is not null ? ToProtoConnectorDefinition(draft.Draft) : null, UpdatedAtUtc = Timestamp.FromDateTimeOffset(updatedAtUtc), }; + if (expectedVersion is not null) + evt.ExpectedVersion = expectedVersion.Value; await ActorCommandDispatcher.SendAsync(_dispatchPort, actor, evt, cancellationToken); - // Also persist to local workspace for offline access await _workspaceStore.SaveConnectorDraftAsync(draft, cancellationToken); + var refreshed = await ReadProjectedStateAsync(cancellationToken); return new StoredConnectorDraft( HomeDirectory: ActorHomeDirectory, FilePath: ActorFilePath + "/draft", FileExists: true, UpdatedAtUtc: updatedAtUtc, - Draft: draft.Draft); + Draft: draft.Draft, + Version: refreshed?.LastAppliedEventVersion ?? ((expectedVersion ?? draft.Version) + 1)); } - public async Task DeleteConnectorDraftAsync(CancellationToken cancellationToken = default) + public async Task DeleteConnectorDraftAsync( + long? expectedVersion = null, + CancellationToken cancellationToken = default) { var actor = await EnsureWriteActorAsync(cancellationToken); - await ActorCommandDispatcher.SendAsync(_dispatchPort, actor, new ConnectorDraftDeletedEvent(), cancellationToken); + var evt = new ConnectorDraftDeletedEvent(); + if (expectedVersion is not null) + evt.ExpectedVersion = expectedVersion.Value; + await ActorCommandDispatcher.SendAsync(_dispatchPort, actor, evt, cancellationToken); await _workspaceStore.DeleteConnectorDraftAsync(cancellationToken); } diff --git a/src/Aevatar.Studio.Infrastructure/ActorBacked/ActorBackedRoleCatalogStore.cs b/src/Aevatar.Studio.Infrastructure/ActorBacked/ActorBackedRoleCatalogStore.cs index f1ca60e25..1c303df75 100644 --- a/src/Aevatar.Studio.Infrastructure/ActorBacked/ActorBackedRoleCatalogStore.cs +++ b/src/Aevatar.Studio.Infrastructure/ActorBacked/ActorBackedRoleCatalogStore.cs @@ -58,23 +58,29 @@ public async Task GetRoleCatalogAsync(CancellationToken cance HomeDirectory: ActorHomeDirectory, FilePath: ActorFilePath, FileExists: roles.Count > 0, - Roles: roles); + Roles: roles, + Version: state?.LastAppliedEventVersion ?? 0); } public async Task SaveRoleCatalogAsync( StoredRoleCatalog catalog, + long? expectedVersion = null, CancellationToken cancellationToken = default) { var actor = await EnsureWriteActorAsync(cancellationToken); var evt = new RoleCatalogSavedEvent(); evt.Roles.AddRange(catalog.Roles.Select(ToProtoRoleDefinition)); + if (expectedVersion is not null) + evt.ExpectedVersion = expectedVersion.Value; await ActorCommandDispatcher.SendAsync(_dispatchPort, actor, evt, cancellationToken); + var refreshed = await ReadProjectedStateAsync(cancellationToken); return new StoredRoleCatalog( HomeDirectory: ActorHomeDirectory, FilePath: ActorFilePath, FileExists: true, - Roles: catalog.Roles); + Roles: catalog.Roles, + Version: refreshed?.LastAppliedEventVersion ?? ((expectedVersion ?? catalog.Version) + 1)); } public async Task ImportLocalCatalogAsync(CancellationToken cancellationToken = default) @@ -103,6 +109,7 @@ public async Task GetRoleDraftAsync(CancellationToken cancellat { var state = await ReadProjectedStateAsync(cancellationToken); var draftEntry = state?.Draft; + var version = state?.LastAppliedEventVersion ?? 0; if (draftEntry is null) { return new StoredRoleDraft( @@ -110,7 +117,8 @@ public async Task GetRoleDraftAsync(CancellationToken cancellat FilePath: ActorFilePath + "/draft", FileExists: false, UpdatedAtUtc: null, - Draft: null); + Draft: null, + Version: version); } return new StoredRoleDraft( @@ -118,11 +126,13 @@ public async Task GetRoleDraftAsync(CancellationToken cancellat FilePath: ActorFilePath + "/draft", FileExists: true, UpdatedAtUtc: draftEntry.UpdatedAtUtc?.ToDateTimeOffset(), - Draft: draftEntry.Draft is not null ? ToStoredRoleDefinition(draftEntry.Draft) : null); + Draft: draftEntry.Draft is not null ? ToStoredRoleDefinition(draftEntry.Draft) : null, + Version: version); } public async Task SaveRoleDraftAsync( StoredRoleDraft draft, + long? expectedVersion = null, CancellationToken cancellationToken = default) { var actor = await EnsureWriteActorAsync(cancellationToken); @@ -132,22 +142,31 @@ public async Task SaveRoleDraftAsync( Draft = draft.Draft is not null ? ToProtoRoleDefinition(draft.Draft) : null, UpdatedAtUtc = Timestamp.FromDateTimeOffset(updatedAtUtc), }; + if (expectedVersion is not null) + evt.ExpectedVersion = expectedVersion.Value; await ActorCommandDispatcher.SendAsync(_dispatchPort, actor, evt, cancellationToken); await _localWorkspaceStore.SaveRoleDraftAsync(draft, cancellationToken); + var refreshed = await ReadProjectedStateAsync(cancellationToken); return new StoredRoleDraft( HomeDirectory: ActorHomeDirectory, FilePath: ActorFilePath + "/draft", FileExists: true, UpdatedAtUtc: updatedAtUtc, - Draft: draft.Draft); + Draft: draft.Draft, + Version: refreshed?.LastAppliedEventVersion ?? ((expectedVersion ?? draft.Version) + 1)); } - public async Task DeleteRoleDraftAsync(CancellationToken cancellationToken = default) + public async Task DeleteRoleDraftAsync( + long? expectedVersion = null, + CancellationToken cancellationToken = default) { var actor = await EnsureWriteActorAsync(cancellationToken); - await ActorCommandDispatcher.SendAsync(_dispatchPort, actor, new RoleDraftDeletedEvent(), cancellationToken); + var evt = new RoleDraftDeletedEvent(); + if (expectedVersion is not null) + evt.ExpectedVersion = expectedVersion.Value; + await ActorCommandDispatcher.SendAsync(_dispatchPort, actor, evt, cancellationToken); await _localWorkspaceStore.DeleteRoleDraftAsync(cancellationToken); } diff --git a/test/Aevatar.Tools.Cli.Tests/ActorBackedGAgentStateTransitionTests.cs b/test/Aevatar.Tools.Cli.Tests/ActorBackedGAgentStateTransitionTests.cs index 77ff8c997..36e620cea 100644 --- a/test/Aevatar.Tools.Cli.Tests/ActorBackedGAgentStateTransitionTests.cs +++ b/test/Aevatar.Tools.Cli.Tests/ActorBackedGAgentStateTransitionTests.cs @@ -1176,6 +1176,7 @@ private static ConnectorCatalogState ApplyConnectorCatalogSaved( var next = state.Clone(); next.Connectors.Clear(); next.Connectors.AddRange(evt.Connectors); + next.LastAppliedEventVersion = state.LastAppliedEventVersion + 1; return next; } @@ -1188,6 +1189,7 @@ private static ConnectorCatalogState ApplyConnectorDraftSaved( Draft = evt.Draft?.Clone(), UpdatedAtUtc = evt.UpdatedAtUtc, }; + next.LastAppliedEventVersion = state.LastAppliedEventVersion + 1; return next; } @@ -1196,6 +1198,7 @@ private static ConnectorCatalogState ApplyConnectorDraftDeleted( { var next = state.Clone(); next.Draft = null; + next.LastAppliedEventVersion = state.LastAppliedEventVersion + 1; return next; } @@ -1396,6 +1399,7 @@ private static RoleCatalogState ApplyRoleCatalogSaved( var next = state.Clone(); next.Roles.Clear(); next.Roles.AddRange(evt.Roles); + next.LastAppliedEventVersion = state.LastAppliedEventVersion + 1; return next; } @@ -1408,6 +1412,7 @@ private static RoleCatalogState ApplyRoleDraftSaved( Draft = evt.Draft?.Clone(), UpdatedAtUtc = evt.UpdatedAtUtc, }; + next.LastAppliedEventVersion = state.LastAppliedEventVersion + 1; return next; } @@ -1416,6 +1421,7 @@ private static RoleCatalogState ApplyRoleDraftDeleted( { var next = state.Clone(); next.Draft = null; + next.LastAppliedEventVersion = state.LastAppliedEventVersion + 1; return next; } @@ -1553,6 +1559,42 @@ public void RoleCatalog_DeleteDraft_NoDraft_ReturnsNullDraft() next.Draft.Should().BeNull(); } + [Fact] + public void RoleCatalog_Apply_IncrementsLastAppliedEventVersion() + { + var state = new RoleCatalogState(); + + var afterFirst = ApplyRoleCatalog(state, new RoleCatalogSavedEvent()); + afterFirst.LastAppliedEventVersion.Should().Be(1); + + var afterSecond = ApplyRoleCatalog(afterFirst, new RoleDraftSavedEvent + { + UpdatedAtUtc = Timestamp.FromDateTimeOffset(DateTimeOffset.UtcNow), + }); + afterSecond.LastAppliedEventVersion.Should().Be(2); + + var afterThird = ApplyRoleCatalog(afterSecond, new RoleDraftDeletedEvent()); + afterThird.LastAppliedEventVersion.Should().Be(3); + } + + [Fact] + public void ConnectorCatalog_Apply_IncrementsLastAppliedEventVersion() + { + var state = new ConnectorCatalogState(); + + var afterFirst = ApplyConnectorCatalog(state, new ConnectorCatalogSavedEvent()); + afterFirst.LastAppliedEventVersion.Should().Be(1); + + var afterSecond = ApplyConnectorCatalog(afterFirst, new ConnectorDraftSavedEvent + { + UpdatedAtUtc = Timestamp.FromDateTimeOffset(DateTimeOffset.UtcNow), + }); + afterSecond.LastAppliedEventVersion.Should().Be(2); + + var afterThird = ApplyConnectorCatalog(afterSecond, new ConnectorDraftDeletedEvent()); + afterThird.LastAppliedEventVersion.Should().Be(3); + } + [Fact] public void RoleCatalog_SaveCatalog_DoesNotAffectDraft() { diff --git a/test/Aevatar.Tools.Cli.Tests/ActorBackedStoreAdapterTests.cs b/test/Aevatar.Tools.Cli.Tests/ActorBackedStoreAdapterTests.cs index c72ce83ee..0e944693d 100644 --- a/test/Aevatar.Tools.Cli.Tests/ActorBackedStoreAdapterTests.cs +++ b/test/Aevatar.Tools.Cli.Tests/ActorBackedStoreAdapterTests.cs @@ -1483,6 +1483,148 @@ public async Task RoleCatalogStore_DeleteDraft_SendsEventAndSyncsWorkspace() workspaceStore.RoleDraftDeleted.Should().BeTrue(); } + [Fact] + public async Task RoleCatalogStore_SaveDraft_PlumbsExpectedVersionToEvent() + { + var runtime = new FakeActorRuntime(); + var scopeResolver = new FakeScopeResolver { ScopeIdToReturn = "scope-1" }; + var workspaceStore = new StubWorkspaceStore(); + var logger = NullLogger.Instance; + var store = new ActorBackedRoleCatalogStore( + new FakeStudioActorBootstrap(runtime), new FakeActorDispatchPort(runtime), scopeResolver, workspaceStore, EmptyReader(), logger); + + var draft = new StoredRoleDraft( + HomeDirectory: "test", + FilePath: "test/draft", + FileExists: true, + UpdatedAtUtc: DateTimeOffset.UtcNow, + Draft: new StoredRoleDefinition("r1", "My Role", "prompt", "anthropic", "claude-opus", [])); + + await store.SaveRoleDraftAsync(draft, expectedVersion: 5); + + var evt = runtime.Actors["role-catalog-scope-1"].ReceivedEnvelopes[0].Payload.Unpack(); + evt.HasExpectedVersion.Should().BeTrue(); + evt.ExpectedVersion.Should().Be(5); + } + + [Fact] + public async Task RoleCatalogStore_SaveDraft_WithoutExpectedVersion_LeavesEventUnsetForOptimisticBypass() + { + var runtime = new FakeActorRuntime(); + var scopeResolver = new FakeScopeResolver { ScopeIdToReturn = "scope-1" }; + var workspaceStore = new StubWorkspaceStore(); + var logger = NullLogger.Instance; + var store = new ActorBackedRoleCatalogStore( + new FakeStudioActorBootstrap(runtime), new FakeActorDispatchPort(runtime), scopeResolver, workspaceStore, EmptyReader(), logger); + + var draft = new StoredRoleDraft( + HomeDirectory: "test", + FilePath: "test/draft", + FileExists: true, + UpdatedAtUtc: DateTimeOffset.UtcNow, + Draft: new StoredRoleDefinition("r1", "My Role", "prompt", "anthropic", "claude-opus", [])); + + await store.SaveRoleDraftAsync(draft); + + var evt = runtime.Actors["role-catalog-scope-1"].ReceivedEnvelopes[0].Payload.Unpack(); + evt.HasExpectedVersion.Should().BeFalse(); + } + + [Fact] + public async Task RoleCatalogStore_DeleteDraft_PlumbsExpectedVersionToEvent() + { + var runtime = new FakeActorRuntime(); + var scopeResolver = new FakeScopeResolver { ScopeIdToReturn = "scope-1" }; + var workspaceStore = new StubWorkspaceStore(); + var logger = NullLogger.Instance; + var store = new ActorBackedRoleCatalogStore( + new FakeStudioActorBootstrap(runtime), new FakeActorDispatchPort(runtime), scopeResolver, workspaceStore, EmptyReader(), logger); + + await store.DeleteRoleDraftAsync(expectedVersion: 7); + + var evt = runtime.Actors["role-catalog-scope-1"].ReceivedEnvelopes[0].Payload.Unpack(); + evt.HasExpectedVersion.Should().BeTrue(); + evt.ExpectedVersion.Should().Be(7); + } + + [Fact] + public async Task RoleCatalogStore_GetDraft_ReturnsVersionFromProjectionState() + { + var runtime = new FakeActorRuntime(); + var state = new RoleCatalogState + { + LastAppliedEventVersion = 42, + Draft = new RoleDraftEntry + { + UpdatedAtUtc = Timestamp.FromDateTimeOffset(DateTimeOffset.UtcNow), + Draft = new RoleDefinitionEntry { Id = "d", Name = "D" }, + }, + }; + var reader = PackedReader("role-catalog-scope-1", state); + var scopeResolver = new FakeScopeResolver { ScopeIdToReturn = "scope-1" }; + var workspaceStore = new StubWorkspaceStore(); + var logger = NullLogger.Instance; + var store = new ActorBackedRoleCatalogStore( + new FakeStudioActorBootstrap(runtime), new FakeActorDispatchPort(runtime), scopeResolver, workspaceStore, reader, logger); + + var draft = await store.GetRoleDraftAsync(); + + draft.Version.Should().Be(42); + } + + [Fact] + public async Task ConnectorCatalogStore_SaveDraft_PlumbsExpectedVersionToEvent() + { + var runtime = new FakeActorRuntime(); + var scopeResolver = new FakeScopeResolver { ScopeIdToReturn = "scope-1" }; + var workspaceStore = new StubWorkspaceStore(); + var logger = NullLogger.Instance; + var store = new ActorBackedConnectorCatalogStore( + new FakeStudioActorBootstrap(runtime), new FakeActorDispatchPort(runtime), scopeResolver, workspaceStore, EmptyReader(), logger); + + var draft = new StoredConnectorDraft( + HomeDirectory: "test", + FilePath: "test/draft", + FileExists: true, + UpdatedAtUtc: DateTimeOffset.UtcNow, + Draft: new StoredConnectorDefinition( + "conn-1", "http", true, 30000, 3, + new StoredHttpConnectorConfig("http://x", [], [], [], new Dictionary(), new StoredConnectorAuthConfig("", "", "", "", "")), + new StoredCliConnectorConfig("", [], [], [], "", new Dictionary()), + new StoredMcpConnectorConfig("", "", "", [], new Dictionary(), new Dictionary(), new StoredConnectorAuthConfig("", "", "", "", ""), "", [], []))); + + await store.SaveConnectorDraftAsync(draft, expectedVersion: 9); + + var evt = runtime.Actors["connector-catalog-scope-1"].ReceivedEnvelopes[0].Payload.Unpack(); + evt.HasExpectedVersion.Should().BeTrue(); + evt.ExpectedVersion.Should().Be(9); + } + + [Fact] + public async Task ConnectorCatalogStore_GetDraft_ReturnsVersionFromProjectionState() + { + var runtime = new FakeActorRuntime(); + var state = new ConnectorCatalogState + { + LastAppliedEventVersion = 13, + Draft = new ConnectorDraftEntry + { + UpdatedAtUtc = Timestamp.FromDateTimeOffset(DateTimeOffset.UtcNow), + Draft = new ConnectorDefinitionEntry { Name = "d", Type = "http" }, + }, + }; + var reader = PackedReader("connector-catalog-scope-1", state); + var scopeResolver = new FakeScopeResolver { ScopeIdToReturn = "scope-1" }; + var workspaceStore = new StubWorkspaceStore(); + var logger = NullLogger.Instance; + var store = new ActorBackedConnectorCatalogStore( + new FakeStudioActorBootstrap(runtime), new FakeActorDispatchPort(runtime), scopeResolver, workspaceStore, reader, logger); + + var draft = await store.GetConnectorDraftAsync(); + + draft.Version.Should().Be(13); + } + [Fact] public async Task RoleCatalogStore_GetCatalog_MapsStateCorrectly() { diff --git a/test/Aevatar.Tools.Cli.Tests/ConnectorServiceTests.cs b/test/Aevatar.Tools.Cli.Tests/ConnectorServiceTests.cs index 5f41cff47..d9ed30b60 100644 --- a/test/Aevatar.Tools.Cli.Tests/ConnectorServiceTests.cs +++ b/test/Aevatar.Tools.Cli.Tests/ConnectorServiceTests.cs @@ -114,7 +114,7 @@ private sealed class RecordingConnectorCatalogStore : IConnectorCatalogStore public Task GetConnectorCatalogAsync(CancellationToken cancellationToken = default) => Task.FromResult(LastSavedCatalog ?? new StoredConnectorCatalog(string.Empty, string.Empty, false, [])); - public Task SaveConnectorCatalogAsync(StoredConnectorCatalog catalog, CancellationToken cancellationToken = default) + public Task SaveConnectorCatalogAsync(StoredConnectorCatalog catalog, long? expectedVersion = null, CancellationToken cancellationToken = default) { LastSavedCatalog = catalog with { FileExists = true }; return Task.FromResult(LastSavedCatalog); @@ -126,10 +126,10 @@ public Task ImportLocalCatalogAsync(CancellationToken public Task GetConnectorDraftAsync(CancellationToken cancellationToken = default) => Task.FromResult(new StoredConnectorDraft(string.Empty, string.Empty, false, null, null)); - public Task SaveConnectorDraftAsync(StoredConnectorDraft draft, CancellationToken cancellationToken = default) => + public Task SaveConnectorDraftAsync(StoredConnectorDraft draft, long? expectedVersion = null, CancellationToken cancellationToken = default) => Task.FromResult(draft); - public Task DeleteConnectorDraftAsync(CancellationToken cancellationToken = default) => + public Task DeleteConnectorDraftAsync(long? expectedVersion = null, CancellationToken cancellationToken = default) => Task.CompletedTask; } From 8b1d922e40b7e7d8b76030b9bb5ece032fa4a050 Mon Sep 17 00:00:00 2001 From: eanzhao Date: Mon, 27 Apr 2026 14:56:42 +0800 Subject: [PATCH 2/4] fix(studio): make ETag flow correct under projection lag and malformed preconditions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the two correctness issues raised in #434 review: 1) Stale post-write ETag (ActorBackedRoleCatalogStore.cs / ConnectorCatalogStore.cs): The previous code re-read the projection immediately after dispatch to compute the response Version. The projection is eventually consistent and may still contain the pre-write document, so the response ETag could be the same one the caller just used as If-Match — guaranteeing a false VERSION_CONFLICT on the next guarded write. Fix: derive the post-write Version deterministically from `expected_version + 1` (the actor enforces the match and increments by exactly one in Apply). When the caller did not supply expected_version, return Version = 0 to signal "unknown — re-GET for authoritative version", and the controller no longer emits an ETag in that case. 2) Malformed If-Match silently bypassing concurrency control (ETagSupport.cs): The previous TryParseIfMatch returned null for both "absent" and "invalid", letting `If-Match: W/"5"`, `If-Match: "5","6"`, `If-Match: *`, typos, etc. silently fall back to last-writer-wins. Fix: ParseIfMatch is now tristate (Absent / Valid / Invalid). Controllers reject Invalid with 400 MALFORMED_IF_MATCH; only Valid values bind to ExpectedVersion; Absent passes through unchanged. Tests: - New ETagSupportTests: theory matrix covering valid forms (bare / quoted), absent/empty header, weak validators, multi-value, wildcard, non-numeric, negative. - New RolesControllerETagTests: malformed If-Match → 400 on PUT/DELETE with no store invocation; valid If-Match plumbs ExpectedVersion and emits deterministic ETag; absent If-Match emits no ETag; store-thrown EventStoreOptimisticConcurrencyException → 409. - Updated existing store tests to assert deterministic Version semantics (expected_version=5 → response Version=6; no expected_version → Version=0). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Controllers/ConnectorsController.cs | 67 ++++++-- .../Controllers/ETagSupport.cs | 59 ++++++- .../Controllers/RolesController.cs | 74 ++++++-- .../ActorBackedConnectorCatalogStore.cs | 13 +- .../ActorBackedRoleCatalogStore.cs | 13 +- .../ActorBackedStoreAdapterTests.cs | 17 +- .../ETagSupportTests.cs | 67 ++++++++ .../RolesControllerETagTests.cs | 159 ++++++++++++++++++ 8 files changed, 416 insertions(+), 53 deletions(-) create mode 100644 test/Aevatar.Tools.Cli.Tests/ETagSupportTests.cs create mode 100644 test/Aevatar.Tools.Cli.Tests/RolesControllerETagTests.cs diff --git a/src/Aevatar.Studio.Hosting/Controllers/ConnectorsController.cs b/src/Aevatar.Studio.Hosting/Controllers/ConnectorsController.cs index a2e97e6c9..4e5f0e38c 100644 --- a/src/Aevatar.Studio.Hosting/Controllers/ConnectorsController.cs +++ b/src/Aevatar.Studio.Hosting/Controllers/ConnectorsController.cs @@ -69,11 +69,12 @@ public async Task> Save( [FromBody] SaveConnectorCatalogRequest request, CancellationToken cancellationToken) { - var effectiveRequest = ApplyIfMatch(request); + if (TryApplyIfMatch(request, out var effectiveRequest, out var malformed)) + return malformed!; try { var response = await _connectorService.SaveCatalogAsync(effectiveRequest, cancellationToken); - ETagSupport.WriteETag(Response, response.Version); + EmitETagIfDeterministic(response.Version); return Ok(response); } catch (EventStoreOptimisticConcurrencyException exception) @@ -124,11 +125,12 @@ public async Task> SaveDraft( [FromBody] SaveConnectorDraftRequest request, CancellationToken cancellationToken) { - var effectiveRequest = ApplyIfMatch(request); + if (TryApplyIfMatch(request, out var effectiveRequest, out var malformed)) + return malformed!; try { var response = await _connectorService.SaveDraftAsync(effectiveRequest, cancellationToken); - ETagSupport.WriteETag(Response, response.Version); + EmitETagIfDeterministic(response.Version); return Ok(response); } catch (EventStoreOptimisticConcurrencyException exception) @@ -152,7 +154,11 @@ public async Task> SaveDraft( [HttpDelete("draft")] public async Task DeleteDraft(CancellationToken cancellationToken) { - var expectedVersion = ETagSupport.TryParseIfMatch(Request); + var status = ETagSupport.ParseIfMatch(Request, out var ifMatchVersion); + if (status == IfMatchStatus.Invalid) + return MalformedIfMatch(); + + long? expectedVersion = status == IfMatchStatus.Valid ? ifMatchVersion : null; try { await _connectorService.DeleteDraftAsync(expectedVersion, cancellationToken); @@ -176,19 +182,50 @@ public async Task DeleteDraft(CancellationToken cancellationToken } } - private SaveConnectorCatalogRequest ApplyIfMatch(SaveConnectorCatalogRequest request) + private bool TryApplyIfMatch(SaveConnectorCatalogRequest request, out SaveConnectorCatalogRequest effective, out ActionResult? malformed) + { + var status = ETagSupport.ParseIfMatch(Request, out var version); + if (status == IfMatchStatus.Invalid) + { + effective = request; + malformed = MalformedIfMatch(); + return true; + } + + effective = status == IfMatchStatus.Valid && request.ExpectedVersion is null + ? request with { ExpectedVersion = version } + : request; + malformed = null; + return false; + } + + private bool TryApplyIfMatch(SaveConnectorDraftRequest request, out SaveConnectorDraftRequest effective, out ActionResult? malformed) { - if (request.ExpectedVersion is not null) - return request; - var expected = ETagSupport.TryParseIfMatch(Request); - return expected is null ? request : request with { ExpectedVersion = expected }; + var status = ETagSupport.ParseIfMatch(Request, out var version); + if (status == IfMatchStatus.Invalid) + { + effective = request; + malformed = MalformedIfMatch(); + return true; + } + + effective = status == IfMatchStatus.Valid && request.ExpectedVersion is null + ? request with { ExpectedVersion = version } + : request; + malformed = null; + return false; } - private SaveConnectorDraftRequest ApplyIfMatch(SaveConnectorDraftRequest request) + private void EmitETagIfDeterministic(long version) { - if (request.ExpectedVersion is not null) - return request; - var expected = ETagSupport.TryParseIfMatch(Request); - return expected is null ? request : request with { ExpectedVersion = expected }; + if (version > 0) + ETagSupport.WriteETag(Response, version); } + + private BadRequestObjectResult MalformedIfMatch() => + BadRequest(new + { + code = "MALFORMED_IF_MATCH", + message = "If-Match must be a strong validator with a single non-negative integer version (e.g. \"5\").", + }); } diff --git a/src/Aevatar.Studio.Hosting/Controllers/ETagSupport.cs b/src/Aevatar.Studio.Hosting/Controllers/ETagSupport.cs index e4aa20954..3c25501ca 100644 --- a/src/Aevatar.Studio.Hosting/Controllers/ETagSupport.cs +++ b/src/Aevatar.Studio.Hosting/Controllers/ETagSupport.cs @@ -3,26 +3,67 @@ namespace Aevatar.Studio.Hosting.Controllers; +internal enum IfMatchStatus +{ + /// Header is not present or empty — caller did not request a precondition. + Absent, + + /// Header parsed cleanly as a strong, single-version ETag. + Valid, + + /// Header is present but malformed (weak validator, multiple values, wildcard, non-integer). + Invalid, +} + internal static class ETagSupport { private const string IfMatchHeader = "If-Match"; - private const string ETagHeader = "ETag"; /// - /// Parse an If-Match header into a long version. Accepts both quoted ETag form - /// ("5") and bare integer (5). Returns null when missing or unparseable. + /// Parse an If-Match header into a strong, single-value version. /// - public static long? TryParseIfMatch(HttpRequest request) + /// + /// Accepted forms: bare integer (5) or strong-quoted ETag ("5"). + /// Rejected as : weak validators (W/"5"), + /// multi-value lists ("5","6"), wildcard (*), and any value that does + /// not parse to a non-negative . Distinguishing absent from invalid + /// is essential — falling back to last-writer-wins on a malformed precondition would + /// silently bypass the optimistic concurrency guarantee the caller asked for. + /// + public static IfMatchStatus ParseIfMatch(HttpRequest request, out long version) { + version = 0; + if (!request.Headers.TryGetValue(IfMatchHeader, out var values) || values.Count == 0) - return null; + return IfMatchStatus.Absent; var raw = values.ToString().Trim(); - if (string.IsNullOrEmpty(raw)) - return null; + if (raw.Length == 0) + return IfMatchStatus.Absent; + + if (raw == "*") + return IfMatchStatus.Invalid; + + if (raw.StartsWith("W/", StringComparison.Ordinal)) + return IfMatchStatus.Invalid; + + if (raw.Contains(',')) + return IfMatchStatus.Invalid; + + var unquoted = raw.Length >= 2 && raw[0] == '"' && raw[^1] == '"' + ? raw[1..^1] + : raw; + + if (unquoted.Length == 0) + return IfMatchStatus.Invalid; + + if (long.TryParse(unquoted, out var parsed) && parsed >= 0) + { + version = parsed; + return IfMatchStatus.Valid; + } - var unquoted = raw.Trim('"'); - return long.TryParse(unquoted, out var version) ? version : null; + return IfMatchStatus.Invalid; } public static void WriteETag(HttpResponse response, long version) diff --git a/src/Aevatar.Studio.Hosting/Controllers/RolesController.cs b/src/Aevatar.Studio.Hosting/Controllers/RolesController.cs index b1bbf2955..92a090ab2 100644 --- a/src/Aevatar.Studio.Hosting/Controllers/RolesController.cs +++ b/src/Aevatar.Studio.Hosting/Controllers/RolesController.cs @@ -69,11 +69,12 @@ public async Task> Save( [FromBody] SaveRoleCatalogRequest request, CancellationToken cancellationToken) { - var effectiveRequest = ApplyIfMatch(request); + if (TryApplyIfMatch(request, out var effectiveRequest, out var malformed)) + return malformed!; try { var response = await _roleCatalogService.SaveCatalogAsync(effectiveRequest, cancellationToken); - ETagSupport.WriteETag(Response, response.Version); + EmitETagIfDeterministic(response.Version); return Ok(response); } catch (EventStoreOptimisticConcurrencyException exception) @@ -124,11 +125,12 @@ public async Task> SaveDraft( [FromBody] SaveRoleDraftRequest request, CancellationToken cancellationToken) { - var effectiveRequest = ApplyIfMatch(request); + if (TryApplyIfMatch(request, out var effectiveRequest, out var malformed)) + return malformed!; try { var response = await _roleCatalogService.SaveDraftAsync(effectiveRequest, cancellationToken); - ETagSupport.WriteETag(Response, response.Version); + EmitETagIfDeterministic(response.Version); return Ok(response); } catch (EventStoreOptimisticConcurrencyException exception) @@ -152,7 +154,11 @@ public async Task> SaveDraft( [HttpDelete("draft")] public async Task DeleteDraft(CancellationToken cancellationToken) { - var expectedVersion = ETagSupport.TryParseIfMatch(Request); + var status = ETagSupport.ParseIfMatch(Request, out var ifMatchVersion); + if (status == IfMatchStatus.Invalid) + return MalformedIfMatch(); + + long? expectedVersion = status == IfMatchStatus.Valid ? ifMatchVersion : null; try { await _roleCatalogService.DeleteDraftAsync(expectedVersion, cancellationToken); @@ -176,19 +182,57 @@ public async Task DeleteDraft(CancellationToken cancellationToken } } - private SaveRoleCatalogRequest ApplyIfMatch(SaveRoleCatalogRequest request) + /// + /// Returns true and assigns when the request's If-Match header is invalid; + /// otherwise binds the expected version (header takes precedence over body). + /// + private bool TryApplyIfMatch(SaveRoleCatalogRequest request, out SaveRoleCatalogRequest effective, out ActionResult? malformed) + { + var status = ETagSupport.ParseIfMatch(Request, out var version); + if (status == IfMatchStatus.Invalid) + { + effective = request; + malformed = MalformedIfMatch(); + return true; + } + + effective = status == IfMatchStatus.Valid && request.ExpectedVersion is null + ? request with { ExpectedVersion = version } + : request; + malformed = null; + return false; + } + + private bool TryApplyIfMatch(SaveRoleDraftRequest request, out SaveRoleDraftRequest effective, out ActionResult? malformed) { - if (request.ExpectedVersion is not null) - return request; - var expected = ETagSupport.TryParseIfMatch(Request); - return expected is null ? request : request with { ExpectedVersion = expected }; + var status = ETagSupport.ParseIfMatch(Request, out var version); + if (status == IfMatchStatus.Invalid) + { + effective = request; + malformed = MalformedIfMatch(); + return true; + } + + effective = status == IfMatchStatus.Valid && request.ExpectedVersion is null + ? request with { ExpectedVersion = version } + : request; + malformed = null; + return false; } - private SaveRoleDraftRequest ApplyIfMatch(SaveRoleDraftRequest request) + private void EmitETagIfDeterministic(long version) { - if (request.ExpectedVersion is not null) - return request; - var expected = ETagSupport.TryParseIfMatch(Request); - return expected is null ? request : request with { ExpectedVersion = expected }; + // Storage returns 0 when the post-write version is non-deterministic + // (caller did not supply expected_version; projection lag would race a re-read). + // Only emit ETag when we can guarantee the value reflects this write. + if (version > 0) + ETagSupport.WriteETag(Response, version); } + + private BadRequestObjectResult MalformedIfMatch() => + BadRequest(new + { + code = "MALFORMED_IF_MATCH", + message = "If-Match must be a strong validator with a single non-negative integer version (e.g. \"5\").", + }); } diff --git a/src/Aevatar.Studio.Infrastructure/ActorBacked/ActorBackedConnectorCatalogStore.cs b/src/Aevatar.Studio.Infrastructure/ActorBacked/ActorBackedConnectorCatalogStore.cs index e33b65bd4..5cb095253 100644 --- a/src/Aevatar.Studio.Infrastructure/ActorBacked/ActorBackedConnectorCatalogStore.cs +++ b/src/Aevatar.Studio.Infrastructure/ActorBacked/ActorBackedConnectorCatalogStore.cs @@ -84,13 +84,12 @@ public async Task SaveConnectorCatalogAsync( evt.ExpectedVersion = expectedVersion.Value; await ActorCommandDispatcher.SendAsync(_dispatchPort, actor, evt, cancellationToken); - var refreshed = await ReadProjectedStateAsync(cancellationToken); return new StoredConnectorCatalog( HomeDirectory: ActorHomeDirectory, FilePath: ActorFilePath, FileExists: true, Connectors: catalog.Connectors, - Version: refreshed?.LastAppliedEventVersion ?? ((expectedVersion ?? catalog.Version) + 1)); + Version: NextDeterministicVersion(expectedVersion)); } public async Task ImportLocalCatalogAsync( @@ -161,14 +160,13 @@ public async Task SaveConnectorDraftAsync( await _workspaceStore.SaveConnectorDraftAsync(draft, cancellationToken); - var refreshed = await ReadProjectedStateAsync(cancellationToken); return new StoredConnectorDraft( HomeDirectory: ActorHomeDirectory, FilePath: ActorFilePath + "/draft", FileExists: true, UpdatedAtUtc: updatedAtUtc, Draft: draft.Draft, - Version: refreshed?.LastAppliedEventVersion ?? ((expectedVersion ?? draft.Version) + 1)); + Version: NextDeterministicVersion(expectedVersion)); } public async Task DeleteConnectorDraftAsync( @@ -184,6 +182,13 @@ public async Task DeleteConnectorDraftAsync( await _workspaceStore.DeleteConnectorDraftAsync(cancellationToken); } + // Post-write version is deterministic only when caller supplied expected_version + // (actor enforces match → Apply increments by exactly one). Without expected_version + // the projection is eventually consistent and may still report the pre-write value, + // so we return 0 to signal "unknown — re-GET for authoritative version". + private static long NextDeterministicVersion(long? expectedVersion) => + expectedVersion is null ? 0 : expectedVersion.Value + 1; + // ── Read from projection ── private async Task ReadProjectedStateAsync(CancellationToken ct) diff --git a/src/Aevatar.Studio.Infrastructure/ActorBacked/ActorBackedRoleCatalogStore.cs b/src/Aevatar.Studio.Infrastructure/ActorBacked/ActorBackedRoleCatalogStore.cs index 1c303df75..f5b002947 100644 --- a/src/Aevatar.Studio.Infrastructure/ActorBacked/ActorBackedRoleCatalogStore.cs +++ b/src/Aevatar.Studio.Infrastructure/ActorBacked/ActorBackedRoleCatalogStore.cs @@ -74,13 +74,12 @@ public async Task SaveRoleCatalogAsync( evt.ExpectedVersion = expectedVersion.Value; await ActorCommandDispatcher.SendAsync(_dispatchPort, actor, evt, cancellationToken); - var refreshed = await ReadProjectedStateAsync(cancellationToken); return new StoredRoleCatalog( HomeDirectory: ActorHomeDirectory, FilePath: ActorFilePath, FileExists: true, Roles: catalog.Roles, - Version: refreshed?.LastAppliedEventVersion ?? ((expectedVersion ?? catalog.Version) + 1)); + Version: NextDeterministicVersion(expectedVersion)); } public async Task ImportLocalCatalogAsync(CancellationToken cancellationToken = default) @@ -148,14 +147,13 @@ public async Task SaveRoleDraftAsync( await _localWorkspaceStore.SaveRoleDraftAsync(draft, cancellationToken); - var refreshed = await ReadProjectedStateAsync(cancellationToken); return new StoredRoleDraft( HomeDirectory: ActorHomeDirectory, FilePath: ActorFilePath + "/draft", FileExists: true, UpdatedAtUtc: updatedAtUtc, Draft: draft.Draft, - Version: refreshed?.LastAppliedEventVersion ?? ((expectedVersion ?? draft.Version) + 1)); + Version: NextDeterministicVersion(expectedVersion)); } public async Task DeleteRoleDraftAsync( @@ -171,6 +169,13 @@ public async Task DeleteRoleDraftAsync( await _localWorkspaceStore.DeleteRoleDraftAsync(cancellationToken); } + // Post-write version is deterministic only when caller supplied expected_version + // (actor enforces match → Apply increments by exactly one). Without expected_version + // the projection is eventually consistent and may still report the pre-write value, + // so we return 0 to signal "unknown — re-GET for authoritative version". + private static long NextDeterministicVersion(long? expectedVersion) => + expectedVersion is null ? 0 : expectedVersion.Value + 1; + // ── Read from projection ── private async Task ReadProjectedStateAsync(CancellationToken ct) diff --git a/test/Aevatar.Tools.Cli.Tests/ActorBackedStoreAdapterTests.cs b/test/Aevatar.Tools.Cli.Tests/ActorBackedStoreAdapterTests.cs index 0e944693d..60bf1477b 100644 --- a/test/Aevatar.Tools.Cli.Tests/ActorBackedStoreAdapterTests.cs +++ b/test/Aevatar.Tools.Cli.Tests/ActorBackedStoreAdapterTests.cs @@ -1484,7 +1484,7 @@ public async Task RoleCatalogStore_DeleteDraft_SendsEventAndSyncsWorkspace() } [Fact] - public async Task RoleCatalogStore_SaveDraft_PlumbsExpectedVersionToEvent() + public async Task RoleCatalogStore_SaveDraft_PlumbsExpectedVersionToEventAndReturnsDeterministicNextVersion() { var runtime = new FakeActorRuntime(); var scopeResolver = new FakeScopeResolver { ScopeIdToReturn = "scope-1" }; @@ -1500,15 +1500,16 @@ public async Task RoleCatalogStore_SaveDraft_PlumbsExpectedVersionToEvent() UpdatedAtUtc: DateTimeOffset.UtcNow, Draft: new StoredRoleDefinition("r1", "My Role", "prompt", "anthropic", "claude-opus", [])); - await store.SaveRoleDraftAsync(draft, expectedVersion: 5); + var saved = await store.SaveRoleDraftAsync(draft, expectedVersion: 5); var evt = runtime.Actors["role-catalog-scope-1"].ReceivedEnvelopes[0].Payload.Unpack(); evt.HasExpectedVersion.Should().BeTrue(); evt.ExpectedVersion.Should().Be(5); + saved.Version.Should().Be(6); } [Fact] - public async Task RoleCatalogStore_SaveDraft_WithoutExpectedVersion_LeavesEventUnsetForOptimisticBypass() + public async Task RoleCatalogStore_SaveDraft_WithoutExpectedVersion_LeavesEventUnsetAndReturnsZeroVersion() { var runtime = new FakeActorRuntime(); var scopeResolver = new FakeScopeResolver { ScopeIdToReturn = "scope-1" }; @@ -1524,10 +1525,13 @@ public async Task RoleCatalogStore_SaveDraft_WithoutExpectedVersion_LeavesEventU UpdatedAtUtc: DateTimeOffset.UtcNow, Draft: new StoredRoleDefinition("r1", "My Role", "prompt", "anthropic", "claude-opus", [])); - await store.SaveRoleDraftAsync(draft); + var saved = await store.SaveRoleDraftAsync(draft); var evt = runtime.Actors["role-catalog-scope-1"].ReceivedEnvelopes[0].Payload.Unpack(); evt.HasExpectedVersion.Should().BeFalse(); + // Version is 0 (unknown) — signals "re-GET for authoritative version" + // because the projection is eventually consistent and would race a re-read. + saved.Version.Should().Be(0); } [Fact] @@ -1573,7 +1577,7 @@ public async Task RoleCatalogStore_GetDraft_ReturnsVersionFromProjectionState() } [Fact] - public async Task ConnectorCatalogStore_SaveDraft_PlumbsExpectedVersionToEvent() + public async Task ConnectorCatalogStore_SaveDraft_PlumbsExpectedVersionToEventAndReturnsDeterministicNextVersion() { var runtime = new FakeActorRuntime(); var scopeResolver = new FakeScopeResolver { ScopeIdToReturn = "scope-1" }; @@ -1593,11 +1597,12 @@ public async Task ConnectorCatalogStore_SaveDraft_PlumbsExpectedVersionToEvent() new StoredCliConnectorConfig("", [], [], [], "", new Dictionary()), new StoredMcpConnectorConfig("", "", "", [], new Dictionary(), new Dictionary(), new StoredConnectorAuthConfig("", "", "", "", ""), "", [], []))); - await store.SaveConnectorDraftAsync(draft, expectedVersion: 9); + var saved = await store.SaveConnectorDraftAsync(draft, expectedVersion: 9); var evt = runtime.Actors["connector-catalog-scope-1"].ReceivedEnvelopes[0].Payload.Unpack(); evt.HasExpectedVersion.Should().BeTrue(); evt.ExpectedVersion.Should().Be(9); + saved.Version.Should().Be(10); } [Fact] diff --git a/test/Aevatar.Tools.Cli.Tests/ETagSupportTests.cs b/test/Aevatar.Tools.Cli.Tests/ETagSupportTests.cs new file mode 100644 index 000000000..5565f471b --- /dev/null +++ b/test/Aevatar.Tools.Cli.Tests/ETagSupportTests.cs @@ -0,0 +1,67 @@ +using Aevatar.Studio.Hosting.Controllers; +using FluentAssertions; +using Microsoft.AspNetCore.Http; + +namespace Aevatar.Tools.Cli.Tests; + +/// +/// Locks in If-Match parsing semantics. Distinguishing absent from invalid is essential — a +/// malformed precondition silently falling back to last-writer-wins would defeat the +/// optimistic concurrency guarantee that the caller is requesting. +/// +public sealed class ETagSupportTests +{ + [Fact] + public void ParseIfMatch_WhenHeaderMissing_ReturnsAbsent() + { + var status = ETagSupport.ParseIfMatch(MakeRequest(headerValue: null), out var version); + + status.Should().Be(IfMatchStatus.Absent); + version.Should().Be(0); + } + + [Fact] + public void ParseIfMatch_WhenHeaderEmpty_ReturnsAbsent() + { + var status = ETagSupport.ParseIfMatch(MakeRequest(" "), out _); + status.Should().Be(IfMatchStatus.Absent); + } + + [Theory] + [InlineData("5", 5)] + [InlineData("\"5\"", 5)] + [InlineData("0", 0)] + [InlineData("\"42\"", 42)] + public void ParseIfMatch_WhenStrongSingleVersion_ReturnsValid(string header, long expected) + { + var status = ETagSupport.ParseIfMatch(MakeRequest(header), out var version); + + status.Should().Be(IfMatchStatus.Valid); + version.Should().Be(expected); + } + + [Theory] + [InlineData("W/\"5\"")] // weak validator + [InlineData("\"5\", \"6\"")] // multi-value + [InlineData("\"5\",\"6\"")] // multi-value (no space) + [InlineData("*")] // wildcard + [InlineData("not-a-number")] // non-numeric + [InlineData("\"abc\"")] // non-numeric quoted + [InlineData("\"\"")] // empty quoted + [InlineData("-1")] // negative + [InlineData("\"-1\"")] // negative quoted + public void ParseIfMatch_WhenMalformed_ReturnsInvalid(string header) + { + var status = ETagSupport.ParseIfMatch(MakeRequest(header), out _); + + status.Should().Be(IfMatchStatus.Invalid); + } + + private static HttpRequest MakeRequest(string? headerValue) + { + var ctx = new DefaultHttpContext(); + if (headerValue is not null) + ctx.Request.Headers["If-Match"] = headerValue; + return ctx.Request; + } +} diff --git a/test/Aevatar.Tools.Cli.Tests/RolesControllerETagTests.cs b/test/Aevatar.Tools.Cli.Tests/RolesControllerETagTests.cs new file mode 100644 index 000000000..54198cf65 --- /dev/null +++ b/test/Aevatar.Tools.Cli.Tests/RolesControllerETagTests.cs @@ -0,0 +1,159 @@ +using Aevatar.Studio.Application.Studio.Abstractions; +using Aevatar.Studio.Application.Studio.Contracts; +using Aevatar.Studio.Application.Studio.Services; +using Aevatar.Studio.Hosting.Controllers; +using Aevatar.Foundation.Abstractions.Persistence; +using FluentAssertions; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; + +namespace Aevatar.Tools.Cli.Tests; + +/// +/// HTTP-boundary contract for the new ETag / If-Match flow. Covers the two correctness +/// concerns surfaced in PR #434 review: +/// 1) Malformed If-Match must reject (400), not silently fall back to last-writer-wins. +/// 2) Successful guarded write returns a deterministic next ETag (expected_version + 1). +/// +public sealed class RolesControllerETagTests +{ + [Fact] + public async Task SaveDraft_WithMalformedIfMatch_Returns400_AndDoesNotInvokeStore() + { + var store = new RecordingRoleCatalogStore(); + var controller = CreateController(store, ifMatch: "W/\"5\""); + + var result = await controller.SaveDraft( + new SaveRoleDraftRequest(Draft: SampleRole()), + CancellationToken.None); + + var bad = result.Result.Should().BeOfType().Subject; + bad.Value.Should().NotBeNull(); + store.SavedDraft.Should().BeNull(); + store.SavedExpectedVersion.Should().BeNull(); + } + + [Fact] + public async Task DeleteDraft_WithMalformedIfMatch_Returns400_AndDoesNotInvokeStore() + { + var store = new RecordingRoleCatalogStore(); + var controller = CreateController(store, ifMatch: "*"); + + var result = await controller.DeleteDraft(CancellationToken.None); + + result.Should().BeOfType(); + store.DraftDeletes.Should().Be(0); + } + + [Fact] + public async Task SaveDraft_WithValidIfMatch_PassesExpectedVersionToStore_AndEmitsDeterministicETag() + { + var store = new RecordingRoleCatalogStore(); + var controller = CreateController(store, ifMatch: "\"5\""); + + var result = await controller.SaveDraft( + new SaveRoleDraftRequest(Draft: SampleRole()), + CancellationToken.None); + + result.Result.Should().BeOfType(); + store.SavedExpectedVersion.Should().Be(5); + controller.Response.Headers["ETag"].ToString().Should().Be("\"6\""); + } + + [Fact] + public async Task SaveDraft_WithoutIfMatch_DoesNotEmitETag() + { + var store = new RecordingRoleCatalogStore(); + var controller = CreateController(store, ifMatch: null); + + var result = await controller.SaveDraft( + new SaveRoleDraftRequest(Draft: SampleRole()), + CancellationToken.None); + + result.Result.Should().BeOfType(); + store.SavedExpectedVersion.Should().BeNull(); + controller.Response.Headers.ContainsKey("ETag").Should().BeFalse(); + } + + [Fact] + public async Task SaveDraft_WhenStoreThrowsOptimisticConflict_Returns409() + { + var store = new RecordingRoleCatalogStore + { + SaveDraftException = new EventStoreOptimisticConcurrencyException("role-catalog-test", 5, 7), + }; + var controller = CreateController(store, ifMatch: "\"5\""); + + var result = await controller.SaveDraft( + new SaveRoleDraftRequest(Draft: SampleRole()), + CancellationToken.None); + + var conflict = result.Result.Should().BeOfType().Subject; + conflict.Value.Should().NotBeNull(); + } + + private static RolesController CreateController(IRoleCatalogStore store, string? ifMatch) + { + var service = new RoleCatalogService(store, new StubRoleCatalogImportParser()); + var controller = new RolesController(service); + var httpContext = new DefaultHttpContext(); + if (ifMatch is not null) + httpContext.Request.Headers["If-Match"] = ifMatch; + controller.ControllerContext = new ControllerContext { HttpContext = httpContext }; + return controller; + } + + private static RoleDefinitionDto SampleRole() => + new(Id: "r1", Name: "Test", SystemPrompt: "p", Provider: "anthropic", Model: "claude", Connectors: []); + + private sealed class RecordingRoleCatalogStore : IRoleCatalogStore + { + public StoredRoleDraft? SavedDraft { get; private set; } + public long? SavedExpectedVersion { get; private set; } + public int DraftDeletes { get; private set; } + public Exception? SaveDraftException { get; set; } + + public Task GetRoleCatalogAsync(CancellationToken cancellationToken = default) => + Task.FromResult(new StoredRoleCatalog(string.Empty, string.Empty, false, [], 0)); + + public Task SaveRoleCatalogAsync(StoredRoleCatalog catalog, long? expectedVersion = null, CancellationToken cancellationToken = default) + { + if (SaveDraftException is not null) + throw SaveDraftException; + return Task.FromResult(catalog with + { + Version = expectedVersion is null ? 0 : expectedVersion.Value + 1, + }); + } + + public Task ImportLocalCatalogAsync(CancellationToken cancellationToken = default) => + throw new NotSupportedException(); + + public Task GetRoleDraftAsync(CancellationToken cancellationToken = default) => + Task.FromResult(new StoredRoleDraft(string.Empty, string.Empty, false, null, null, 0)); + + public Task SaveRoleDraftAsync(StoredRoleDraft draft, long? expectedVersion = null, CancellationToken cancellationToken = default) + { + if (SaveDraftException is not null) + throw SaveDraftException; + SavedDraft = draft; + SavedExpectedVersion = expectedVersion; + return Task.FromResult(draft with + { + Version = expectedVersion is null ? 0 : expectedVersion.Value + 1, + }); + } + + public Task DeleteRoleDraftAsync(long? expectedVersion = null, CancellationToken cancellationToken = default) + { + DraftDeletes++; + return Task.CompletedTask; + } + } + + private sealed class StubRoleCatalogImportParser : IRoleCatalogImportParser + { + public Task> ParseCatalogAsync(Stream stream, CancellationToken cancellationToken = default) => + Task.FromResult>([]); + } +} From d795eeca3f225c109d00fe61e77971fd2de28ee7 Mon Sep 17 00:00:00 2001 From: eanzhao Date: Mon, 27 Apr 2026 15:31:49 +0800 Subject: [PATCH 3/4] test(studio): expand controller ETag coverage to address codecov/patch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add ConnectorsControllerETagTests mirroring RolesControllerETagTests (ConnectorsController.cs was at 0% — never exercised). Expand the role tests to cover Get / GetDraft (ETag emission), catalog Save with malformed/valid/conflict If-Match, and DeleteDraft happy + conflict paths. Per controller, locked-in HTTP-boundary contract is: - GET / GET draft → ETag header from current state version - PUT / PUT draft - malformed If-Match → 400 MALFORMED_IF_MATCH, store not invoked - valid If-Match → store called with expected_version, ETag header is expected+1 - absent If-Match → no ETag header (Version 0 = unknown) - store throws OCC → 409 VERSION_CONFLICT - DELETE draft - malformed/valid/conflict mirror the PUT semantics Total: 24 new controller-boundary tests across the two files. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../ConnectorsControllerETagTests.cs | 303 ++++++++++++++++++ .../RolesControllerETagTests.cs | 166 ++++++++-- 2 files changed, 439 insertions(+), 30 deletions(-) create mode 100644 test/Aevatar.Tools.Cli.Tests/ConnectorsControllerETagTests.cs diff --git a/test/Aevatar.Tools.Cli.Tests/ConnectorsControllerETagTests.cs b/test/Aevatar.Tools.Cli.Tests/ConnectorsControllerETagTests.cs new file mode 100644 index 000000000..1e201cfb5 --- /dev/null +++ b/test/Aevatar.Tools.Cli.Tests/ConnectorsControllerETagTests.cs @@ -0,0 +1,303 @@ +using Aevatar.Studio.Application.Studio.Abstractions; +using Aevatar.Studio.Application.Studio.Contracts; +using Aevatar.Studio.Application.Studio.Services; +using Aevatar.Studio.Hosting.Controllers; +using Aevatar.Foundation.Abstractions.Persistence; +using FluentAssertions; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; + +namespace Aevatar.Tools.Cli.Tests; + +/// +/// Mirror of for the connector surface. +/// Locks in identical ETag / If-Match semantics: GET emits ETag, PUT/DELETE accept +/// If-Match with strict tristate parse, malformed → 400, valid → deterministic next +/// ETag, optimistic conflict → 409. +/// +public sealed class ConnectorsControllerETagTests +{ + [Fact] + public async Task Get_EmitsETagFromStoreVersion() + { + var store = new RecordingConnectorCatalogStore { CatalogVersion = 12 }; + var controller = CreateController(store, ifMatch: null); + + var result = await controller.Get(CancellationToken.None); + + result.Result.Should().BeOfType(); + controller.Response.Headers["ETag"].ToString().Should().Be("\"12\""); + } + + [Fact] + public async Task GetDraft_EmitsETagFromStoreVersion() + { + var store = new RecordingConnectorCatalogStore { DraftVersion = 7 }; + var controller = CreateController(store, ifMatch: null); + + var result = await controller.GetDraft(CancellationToken.None); + + result.Result.Should().BeOfType(); + controller.Response.Headers["ETag"].ToString().Should().Be("\"7\""); + } + + [Fact] + public async Task Save_WithMalformedIfMatch_Returns400_AndDoesNotInvokeStore() + { + var store = new RecordingConnectorCatalogStore(); + var controller = CreateController(store, ifMatch: "W/\"3\""); + + var result = await controller.Save( + new SaveConnectorCatalogRequest(Connectors: [SampleHttpConnector()]), + CancellationToken.None); + + result.Result.Should().BeOfType(); + store.SavedCatalog.Should().BeNull(); + } + + [Fact] + public async Task Save_WithValidIfMatch_PassesExpectedVersion_AndEmitsDeterministicETag() + { + var store = new RecordingConnectorCatalogStore(); + var controller = CreateController(store, ifMatch: "\"3\""); + + var result = await controller.Save( + new SaveConnectorCatalogRequest(Connectors: [SampleHttpConnector()]), + CancellationToken.None); + + result.Result.Should().BeOfType(); + store.SavedCatalogExpectedVersion.Should().Be(3); + controller.Response.Headers["ETag"].ToString().Should().Be("\"4\""); + } + + [Fact] + public async Task Save_WhenStoreThrowsOptimisticConflict_Returns409() + { + var store = new RecordingConnectorCatalogStore + { + ThrowOnWrite = new EventStoreOptimisticConcurrencyException("connector-catalog-test", 3, 5), + }; + var controller = CreateController(store, ifMatch: "\"3\""); + + var result = await controller.Save( + new SaveConnectorCatalogRequest(Connectors: [SampleHttpConnector()]), + CancellationToken.None); + + result.Result.Should().BeOfType(); + } + + [Fact] + public async Task SaveDraft_WithMalformedIfMatch_Returns400_AndDoesNotInvokeStore() + { + var store = new RecordingConnectorCatalogStore(); + var controller = CreateController(store, ifMatch: "*"); + + var result = await controller.SaveDraft( + new SaveConnectorDraftRequest(Draft: SampleHttpConnector()), + CancellationToken.None); + + result.Result.Should().BeOfType(); + store.SavedDraft.Should().BeNull(); + store.SavedDraftExpectedVersion.Should().BeNull(); + } + + [Fact] + public async Task SaveDraft_WithValidIfMatch_PassesExpectedVersion_AndEmitsDeterministicETag() + { + var store = new RecordingConnectorCatalogStore(); + var controller = CreateController(store, ifMatch: "\"5\""); + + var result = await controller.SaveDraft( + new SaveConnectorDraftRequest(Draft: SampleHttpConnector()), + CancellationToken.None); + + result.Result.Should().BeOfType(); + store.SavedDraftExpectedVersion.Should().Be(5); + controller.Response.Headers["ETag"].ToString().Should().Be("\"6\""); + } + + [Fact] + public async Task SaveDraft_WithoutIfMatch_DoesNotEmitETag() + { + var store = new RecordingConnectorCatalogStore(); + var controller = CreateController(store, ifMatch: null); + + var result = await controller.SaveDraft( + new SaveConnectorDraftRequest(Draft: SampleHttpConnector()), + CancellationToken.None); + + result.Result.Should().BeOfType(); + store.SavedDraftExpectedVersion.Should().BeNull(); + controller.Response.Headers.ContainsKey("ETag").Should().BeFalse(); + } + + [Fact] + public async Task SaveDraft_WhenStoreThrowsOptimisticConflict_Returns409() + { + var store = new RecordingConnectorCatalogStore + { + ThrowOnWrite = new EventStoreOptimisticConcurrencyException("connector-catalog-test", 5, 7), + }; + var controller = CreateController(store, ifMatch: "\"5\""); + + var result = await controller.SaveDraft( + new SaveConnectorDraftRequest(Draft: SampleHttpConnector()), + CancellationToken.None); + + result.Result.Should().BeOfType(); + } + + [Fact] + public async Task DeleteDraft_WithMalformedIfMatch_Returns400_AndDoesNotInvokeStore() + { + var store = new RecordingConnectorCatalogStore(); + var controller = CreateController(store, ifMatch: "not-a-number"); + + var result = await controller.DeleteDraft(CancellationToken.None); + + result.Should().BeOfType(); + store.DraftDeletes.Should().Be(0); + } + + [Fact] + public async Task DeleteDraft_WithValidIfMatch_PassesExpectedVersionToStore() + { + var store = new RecordingConnectorCatalogStore(); + var controller = CreateController(store, ifMatch: "\"9\""); + + var result = await controller.DeleteDraft(CancellationToken.None); + + result.Should().BeOfType(); + store.DraftDeletes.Should().Be(1); + store.DraftDeleteExpectedVersion.Should().Be(9); + } + + [Fact] + public async Task DeleteDraft_WhenStoreThrowsOptimisticConflict_Returns409() + { + var store = new RecordingConnectorCatalogStore + { + ThrowOnDelete = new EventStoreOptimisticConcurrencyException("connector-catalog-test", 9, 11), + }; + var controller = CreateController(store, ifMatch: "\"9\""); + + var result = await controller.DeleteDraft(CancellationToken.None); + + result.Should().BeOfType(); + } + + private static ConnectorsController CreateController(IConnectorCatalogStore store, string? ifMatch) + { + var service = new ConnectorService(store, new StubConnectorCatalogImportParser()); + var controller = new ConnectorsController(service); + var httpContext = new DefaultHttpContext(); + if (ifMatch is not null) + httpContext.Request.Headers["If-Match"] = ifMatch; + controller.ControllerContext = new ControllerContext { HttpContext = httpContext }; + return controller; + } + + private static ConnectorDefinitionDto SampleHttpConnector() => + new( + Name: "conn-1", + Type: "http", + Enabled: true, + TimeoutMs: 30_000, + Retry: 1, + Http: new HttpConnectorDefinitionDto( + BaseUrl: "https://example.com/api", + AllowedMethods: ["GET"], + AllowedPaths: [], + AllowedInputKeys: [], + DefaultHeaders: new Dictionary(StringComparer.OrdinalIgnoreCase), + Auth: EmptyAuth()), + Cli: EmptyCli(), + Mcp: EmptyMcp()); + + private static ConnectorAuthDefinitionDto EmptyAuth() => + new(string.Empty, string.Empty, string.Empty, string.Empty, string.Empty); + + private static CliConnectorDefinitionDto EmptyCli() => + new( + Command: string.Empty, + FixedArguments: [], + AllowedOperations: [], + AllowedInputKeys: [], + WorkingDirectory: string.Empty, + Environment: new Dictionary(StringComparer.OrdinalIgnoreCase)); + + private static McpConnectorDefinitionDto EmptyMcp() => + new( + ServerName: string.Empty, + Command: string.Empty, + Url: string.Empty, + Arguments: [], + Environment: new Dictionary(StringComparer.OrdinalIgnoreCase), + AdditionalHeaders: new Dictionary(StringComparer.OrdinalIgnoreCase), + Auth: EmptyAuth(), + DefaultTool: string.Empty, + AllowedTools: [], + AllowedInputKeys: []); + + private sealed class RecordingConnectorCatalogStore : IConnectorCatalogStore + { + public StoredConnectorCatalog? SavedCatalog { get; private set; } + public long? SavedCatalogExpectedVersion { get; private set; } + public StoredConnectorDraft? SavedDraft { get; private set; } + public long? SavedDraftExpectedVersion { get; private set; } + public int DraftDeletes { get; private set; } + public long? DraftDeleteExpectedVersion { get; private set; } + public long CatalogVersion { get; set; } + public long DraftVersion { get; set; } + public Exception? ThrowOnWrite { get; set; } + public Exception? ThrowOnDelete { get; set; } + + public Task GetConnectorCatalogAsync(CancellationToken cancellationToken = default) => + Task.FromResult(new StoredConnectorCatalog(string.Empty, string.Empty, false, [], CatalogVersion)); + + public Task SaveConnectorCatalogAsync(StoredConnectorCatalog catalog, long? expectedVersion = null, CancellationToken cancellationToken = default) + { + if (ThrowOnWrite is not null) + throw ThrowOnWrite; + SavedCatalog = catalog; + SavedCatalogExpectedVersion = expectedVersion; + return Task.FromResult(catalog with + { + Version = expectedVersion is null ? 0 : expectedVersion.Value + 1, + }); + } + + public Task ImportLocalCatalogAsync(CancellationToken cancellationToken = default) => + throw new NotSupportedException(); + + public Task GetConnectorDraftAsync(CancellationToken cancellationToken = default) => + Task.FromResult(new StoredConnectorDraft(string.Empty, string.Empty, false, null, null, DraftVersion)); + + public Task SaveConnectorDraftAsync(StoredConnectorDraft draft, long? expectedVersion = null, CancellationToken cancellationToken = default) + { + if (ThrowOnWrite is not null) + throw ThrowOnWrite; + SavedDraft = draft; + SavedDraftExpectedVersion = expectedVersion; + return Task.FromResult(draft with + { + Version = expectedVersion is null ? 0 : expectedVersion.Value + 1, + }); + } + + public Task DeleteConnectorDraftAsync(long? expectedVersion = null, CancellationToken cancellationToken = default) + { + if (ThrowOnDelete is not null) + throw ThrowOnDelete; + DraftDeletes++; + DraftDeleteExpectedVersion = expectedVersion; + return Task.CompletedTask; + } + } + + private sealed class StubConnectorCatalogImportParser : IConnectorCatalogImportParser + { + public Task> ParseCatalogAsync(Stream stream, CancellationToken cancellationToken = default) => + Task.FromResult>([]); + } +} diff --git a/test/Aevatar.Tools.Cli.Tests/RolesControllerETagTests.cs b/test/Aevatar.Tools.Cli.Tests/RolesControllerETagTests.cs index 54198cf65..f42b4a09f 100644 --- a/test/Aevatar.Tools.Cli.Tests/RolesControllerETagTests.cs +++ b/test/Aevatar.Tools.Cli.Tests/RolesControllerETagTests.cs @@ -10,43 +10,100 @@ namespace Aevatar.Tools.Cli.Tests; /// -/// HTTP-boundary contract for the new ETag / If-Match flow. Covers the two correctness -/// concerns surfaced in PR #434 review: +/// HTTP-boundary contract for the new ETag / If-Match flow. +/// Covers PR #434 review concerns: /// 1) Malformed If-Match must reject (400), not silently fall back to last-writer-wins. /// 2) Successful guarded write returns a deterministic next ETag (expected_version + 1). +/// And the GET surface that emits the ETag clients use as If-Match. /// public sealed class RolesControllerETagTests { [Fact] - public async Task SaveDraft_WithMalformedIfMatch_Returns400_AndDoesNotInvokeStore() + public async Task Get_EmitsETagFromStoreVersion() + { + var store = new RecordingRoleCatalogStore { CatalogVersion = 12 }; + var controller = CreateController(store, ifMatch: null); + + var result = await controller.Get(CancellationToken.None); + + result.Result.Should().BeOfType(); + controller.Response.Headers["ETag"].ToString().Should().Be("\"12\""); + } + + [Fact] + public async Task GetDraft_EmitsETagFromStoreVersion() + { + var store = new RecordingRoleCatalogStore { DraftVersion = 7 }; + var controller = CreateController(store, ifMatch: null); + + var result = await controller.GetDraft(CancellationToken.None); + + result.Result.Should().BeOfType(); + controller.Response.Headers["ETag"].ToString().Should().Be("\"7\""); + } + + [Fact] + public async Task Save_WithMalformedIfMatch_Returns400_AndDoesNotInvokeStore() { var store = new RecordingRoleCatalogStore(); - var controller = CreateController(store, ifMatch: "W/\"5\""); + var controller = CreateController(store, ifMatch: "W/\"3\""); - var result = await controller.SaveDraft( - new SaveRoleDraftRequest(Draft: SampleRole()), + var result = await controller.Save( + new SaveRoleCatalogRequest(Roles: [SampleRole()]), CancellationToken.None); - var bad = result.Result.Should().BeOfType().Subject; - bad.Value.Should().NotBeNull(); - store.SavedDraft.Should().BeNull(); - store.SavedExpectedVersion.Should().BeNull(); + result.Result.Should().BeOfType(); + store.SavedCatalog.Should().BeNull(); } [Fact] - public async Task DeleteDraft_WithMalformedIfMatch_Returns400_AndDoesNotInvokeStore() + public async Task Save_WithValidIfMatch_PassesExpectedVersion_AndEmitsDeterministicETag() { var store = new RecordingRoleCatalogStore(); - var controller = CreateController(store, ifMatch: "*"); + var controller = CreateController(store, ifMatch: "\"3\""); - var result = await controller.DeleteDraft(CancellationToken.None); + var result = await controller.Save( + new SaveRoleCatalogRequest(Roles: [SampleRole()]), + CancellationToken.None); - result.Should().BeOfType(); - store.DraftDeletes.Should().Be(0); + result.Result.Should().BeOfType(); + store.SavedCatalogExpectedVersion.Should().Be(3); + controller.Response.Headers["ETag"].ToString().Should().Be("\"4\""); + } + + [Fact] + public async Task Save_WhenStoreThrowsOptimisticConflict_Returns409() + { + var store = new RecordingRoleCatalogStore + { + ThrowOnWrite = new EventStoreOptimisticConcurrencyException("role-catalog-test", 3, 5), + }; + var controller = CreateController(store, ifMatch: "\"3\""); + + var result = await controller.Save( + new SaveRoleCatalogRequest(Roles: [SampleRole()]), + CancellationToken.None); + + result.Result.Should().BeOfType(); } [Fact] - public async Task SaveDraft_WithValidIfMatch_PassesExpectedVersionToStore_AndEmitsDeterministicETag() + public async Task SaveDraft_WithMalformedIfMatch_Returns400_AndDoesNotInvokeStore() + { + var store = new RecordingRoleCatalogStore(); + var controller = CreateController(store, ifMatch: "W/\"5\""); + + var result = await controller.SaveDraft( + new SaveRoleDraftRequest(Draft: SampleRole()), + CancellationToken.None); + + result.Result.Should().BeOfType(); + store.SavedDraft.Should().BeNull(); + store.SavedDraftExpectedVersion.Should().BeNull(); + } + + [Fact] + public async Task SaveDraft_WithValidIfMatch_PassesExpectedVersion_AndEmitsDeterministicETag() { var store = new RecordingRoleCatalogStore(); var controller = CreateController(store, ifMatch: "\"5\""); @@ -56,7 +113,7 @@ public async Task SaveDraft_WithValidIfMatch_PassesExpectedVersionToStore_AndEmi CancellationToken.None); result.Result.Should().BeOfType(); - store.SavedExpectedVersion.Should().Be(5); + store.SavedDraftExpectedVersion.Should().Be(5); controller.Response.Headers["ETag"].ToString().Should().Be("\"6\""); } @@ -71,7 +128,7 @@ public async Task SaveDraft_WithoutIfMatch_DoesNotEmitETag() CancellationToken.None); result.Result.Should().BeOfType(); - store.SavedExpectedVersion.Should().BeNull(); + store.SavedDraftExpectedVersion.Should().BeNull(); controller.Response.Headers.ContainsKey("ETag").Should().BeFalse(); } @@ -80,7 +137,7 @@ public async Task SaveDraft_WhenStoreThrowsOptimisticConflict_Returns409() { var store = new RecordingRoleCatalogStore { - SaveDraftException = new EventStoreOptimisticConcurrencyException("role-catalog-test", 5, 7), + ThrowOnWrite = new EventStoreOptimisticConcurrencyException("role-catalog-test", 5, 7), }; var controller = CreateController(store, ifMatch: "\"5\""); @@ -88,8 +145,46 @@ public async Task SaveDraft_WhenStoreThrowsOptimisticConflict_Returns409() new SaveRoleDraftRequest(Draft: SampleRole()), CancellationToken.None); - var conflict = result.Result.Should().BeOfType().Subject; - conflict.Value.Should().NotBeNull(); + result.Result.Should().BeOfType(); + } + + [Fact] + public async Task DeleteDraft_WithMalformedIfMatch_Returns400_AndDoesNotInvokeStore() + { + var store = new RecordingRoleCatalogStore(); + var controller = CreateController(store, ifMatch: "*"); + + var result = await controller.DeleteDraft(CancellationToken.None); + + result.Should().BeOfType(); + store.DraftDeletes.Should().Be(0); + } + + [Fact] + public async Task DeleteDraft_WithValidIfMatch_PassesExpectedVersionToStore() + { + var store = new RecordingRoleCatalogStore(); + var controller = CreateController(store, ifMatch: "\"9\""); + + var result = await controller.DeleteDraft(CancellationToken.None); + + result.Should().BeOfType(); + store.DraftDeletes.Should().Be(1); + store.DraftDeleteExpectedVersion.Should().Be(9); + } + + [Fact] + public async Task DeleteDraft_WhenStoreThrowsOptimisticConflict_Returns409() + { + var store = new RecordingRoleCatalogStore + { + ThrowOnDelete = new EventStoreOptimisticConcurrencyException("role-catalog-test", 9, 11), + }; + var controller = CreateController(store, ifMatch: "\"9\""); + + var result = await controller.DeleteDraft(CancellationToken.None); + + result.Should().BeOfType(); } private static RolesController CreateController(IRoleCatalogStore store, string? ifMatch) @@ -108,18 +203,26 @@ private static RoleDefinitionDto SampleRole() => private sealed class RecordingRoleCatalogStore : IRoleCatalogStore { + public StoredRoleCatalog? SavedCatalog { get; private set; } + public long? SavedCatalogExpectedVersion { get; private set; } public StoredRoleDraft? SavedDraft { get; private set; } - public long? SavedExpectedVersion { get; private set; } + public long? SavedDraftExpectedVersion { get; private set; } public int DraftDeletes { get; private set; } - public Exception? SaveDraftException { get; set; } + public long? DraftDeleteExpectedVersion { get; private set; } + public long CatalogVersion { get; set; } + public long DraftVersion { get; set; } + public Exception? ThrowOnWrite { get; set; } + public Exception? ThrowOnDelete { get; set; } public Task GetRoleCatalogAsync(CancellationToken cancellationToken = default) => - Task.FromResult(new StoredRoleCatalog(string.Empty, string.Empty, false, [], 0)); + Task.FromResult(new StoredRoleCatalog(string.Empty, string.Empty, false, [], CatalogVersion)); public Task SaveRoleCatalogAsync(StoredRoleCatalog catalog, long? expectedVersion = null, CancellationToken cancellationToken = default) { - if (SaveDraftException is not null) - throw SaveDraftException; + if (ThrowOnWrite is not null) + throw ThrowOnWrite; + SavedCatalog = catalog; + SavedCatalogExpectedVersion = expectedVersion; return Task.FromResult(catalog with { Version = expectedVersion is null ? 0 : expectedVersion.Value + 1, @@ -130,14 +233,14 @@ public Task ImportLocalCatalogAsync(CancellationToken cance throw new NotSupportedException(); public Task GetRoleDraftAsync(CancellationToken cancellationToken = default) => - Task.FromResult(new StoredRoleDraft(string.Empty, string.Empty, false, null, null, 0)); + Task.FromResult(new StoredRoleDraft(string.Empty, string.Empty, false, null, null, DraftVersion)); public Task SaveRoleDraftAsync(StoredRoleDraft draft, long? expectedVersion = null, CancellationToken cancellationToken = default) { - if (SaveDraftException is not null) - throw SaveDraftException; + if (ThrowOnWrite is not null) + throw ThrowOnWrite; SavedDraft = draft; - SavedExpectedVersion = expectedVersion; + SavedDraftExpectedVersion = expectedVersion; return Task.FromResult(draft with { Version = expectedVersion is null ? 0 : expectedVersion.Value + 1, @@ -146,7 +249,10 @@ public Task SaveRoleDraftAsync(StoredRoleDraft draft, long? exp public Task DeleteRoleDraftAsync(long? expectedVersion = null, CancellationToken cancellationToken = default) { + if (ThrowOnDelete is not null) + throw ThrowOnDelete; DraftDeletes++; + DraftDeleteExpectedVersion = expectedVersion; return Task.CompletedTask; } } From 819b6abeb04d22e4c72fef43bf0b629d92dde4a4 Mon Sep 17 00:00:00 2001 From: eanzhao Date: Mon, 27 Apr 2026 15:45:14 +0800 Subject: [PATCH 4/4] fix(studio): reject when If-Match header and body expectedVersion disagree Addresses PR #434 review: previously when both an HTTP If-Match header and a body `expectedVersion` were present, the controller silently kept the body value. That meant `If-Match: "5"` paired with body `expectedVersion: 6` would let the write succeed against actor v=6 even though the HTTP precondition would have failed at v=5. New rule: when If-Match parses cleanly, the header is authoritative. If the body explicitly disagrees, the request is internally inconsistent and is rejected with 400 IF_MATCH_BODY_MISMATCH. Body-only (header absent) and header-only (body null) paths are unchanged. Tests: - {Save, SaveDraft}_WhenIfMatchHeaderDisagreesWithBodyExpectedVersion_Returns400 - SaveDraft_WhenIfMatchHeaderAgreesWithBodyExpectedVersion_HeaderWinsAndStoreReceivesIt Same fix in both RolesController and ConnectorsController. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Controllers/ConnectorsController.cs | 47 +++++++++++++--- .../Controllers/RolesController.cs | 54 +++++++++++++++---- .../ConnectorsControllerETagTests.cs | 43 +++++++++++++++ .../RolesControllerETagTests.cs | 43 +++++++++++++++ 4 files changed, 169 insertions(+), 18 deletions(-) diff --git a/src/Aevatar.Studio.Hosting/Controllers/ConnectorsController.cs b/src/Aevatar.Studio.Hosting/Controllers/ConnectorsController.cs index 4e5f0e38c..ce3270dc9 100644 --- a/src/Aevatar.Studio.Hosting/Controllers/ConnectorsController.cs +++ b/src/Aevatar.Studio.Hosting/Controllers/ConnectorsController.cs @@ -184,7 +184,7 @@ public async Task DeleteDraft(CancellationToken cancellationToken private bool TryApplyIfMatch(SaveConnectorCatalogRequest request, out SaveConnectorCatalogRequest effective, out ActionResult? malformed) { - var status = ETagSupport.ParseIfMatch(Request, out var version); + var status = ETagSupport.ParseIfMatch(Request, out var headerVersion); if (status == IfMatchStatus.Invalid) { effective = request; @@ -192,16 +192,28 @@ private bool TryApplyIfMatch(SaveConnectorCatalogRequest request, out SaveConnec return true; } - effective = status == IfMatchStatus.Valid && request.ExpectedVersion is null - ? request with { ExpectedVersion = version } - : request; + if (status == IfMatchStatus.Valid) + { + if (request.ExpectedVersion is { } bodyVersion && bodyVersion != headerVersion) + { + effective = request; + malformed = IfMatchBodyMismatch(headerVersion, bodyVersion); + return true; + } + + effective = request with { ExpectedVersion = headerVersion }; + malformed = null; + return false; + } + + effective = request; malformed = null; return false; } private bool TryApplyIfMatch(SaveConnectorDraftRequest request, out SaveConnectorDraftRequest effective, out ActionResult? malformed) { - var status = ETagSupport.ParseIfMatch(Request, out var version); + var status = ETagSupport.ParseIfMatch(Request, out var headerVersion); if (status == IfMatchStatus.Invalid) { effective = request; @@ -209,9 +221,21 @@ private bool TryApplyIfMatch(SaveConnectorDraftRequest request, out SaveConnecto return true; } - effective = status == IfMatchStatus.Valid && request.ExpectedVersion is null - ? request with { ExpectedVersion = version } - : request; + if (status == IfMatchStatus.Valid) + { + if (request.ExpectedVersion is { } bodyVersion && bodyVersion != headerVersion) + { + effective = request; + malformed = IfMatchBodyMismatch(headerVersion, bodyVersion); + return true; + } + + effective = request with { ExpectedVersion = headerVersion }; + malformed = null; + return false; + } + + effective = request; malformed = null; return false; } @@ -228,4 +252,11 @@ private BadRequestObjectResult MalformedIfMatch() => code = "MALFORMED_IF_MATCH", message = "If-Match must be a strong validator with a single non-negative integer version (e.g. \"5\").", }); + + private BadRequestObjectResult IfMatchBodyMismatch(long headerVersion, long bodyVersion) => + BadRequest(new + { + code = "IF_MATCH_BODY_MISMATCH", + message = $"If-Match header (\"{headerVersion}\") disagrees with body expectedVersion ({bodyVersion}). Send only one, or set them to the same value.", + }); } diff --git a/src/Aevatar.Studio.Hosting/Controllers/RolesController.cs b/src/Aevatar.Studio.Hosting/Controllers/RolesController.cs index 92a090ab2..e3a4e479a 100644 --- a/src/Aevatar.Studio.Hosting/Controllers/RolesController.cs +++ b/src/Aevatar.Studio.Hosting/Controllers/RolesController.cs @@ -183,12 +183,15 @@ public async Task DeleteDraft(CancellationToken cancellationToken } /// - /// Returns true and assigns when the request's If-Match header is invalid; - /// otherwise binds the expected version (header takes precedence over body). + /// When the request's If-Match header is malformed, returns true and yields a 400. + /// When the header is valid AND the body specifies a different expectedVersion, returns + /// true and yields a 400 — the request is internally inconsistent and would otherwise let + /// the body silently bypass an explicit HTTP precondition. Otherwise binds the header's + /// version (header is authoritative when present). /// private bool TryApplyIfMatch(SaveRoleCatalogRequest request, out SaveRoleCatalogRequest effective, out ActionResult? malformed) { - var status = ETagSupport.ParseIfMatch(Request, out var version); + var status = ETagSupport.ParseIfMatch(Request, out var headerVersion); if (status == IfMatchStatus.Invalid) { effective = request; @@ -196,16 +199,28 @@ private bool TryApplyIfMatch(SaveRoleCatalogRequest request, out SaveRoleCatalog return true; } - effective = status == IfMatchStatus.Valid && request.ExpectedVersion is null - ? request with { ExpectedVersion = version } - : request; + if (status == IfMatchStatus.Valid) + { + if (request.ExpectedVersion is { } bodyVersion && bodyVersion != headerVersion) + { + effective = request; + malformed = IfMatchBodyMismatch(headerVersion, bodyVersion); + return true; + } + + effective = request with { ExpectedVersion = headerVersion }; + malformed = null; + return false; + } + + effective = request; malformed = null; return false; } private bool TryApplyIfMatch(SaveRoleDraftRequest request, out SaveRoleDraftRequest effective, out ActionResult? malformed) { - var status = ETagSupport.ParseIfMatch(Request, out var version); + var status = ETagSupport.ParseIfMatch(Request, out var headerVersion); if (status == IfMatchStatus.Invalid) { effective = request; @@ -213,9 +228,21 @@ private bool TryApplyIfMatch(SaveRoleDraftRequest request, out SaveRoleDraftRequ return true; } - effective = status == IfMatchStatus.Valid && request.ExpectedVersion is null - ? request with { ExpectedVersion = version } - : request; + if (status == IfMatchStatus.Valid) + { + if (request.ExpectedVersion is { } bodyVersion && bodyVersion != headerVersion) + { + effective = request; + malformed = IfMatchBodyMismatch(headerVersion, bodyVersion); + return true; + } + + effective = request with { ExpectedVersion = headerVersion }; + malformed = null; + return false; + } + + effective = request; malformed = null; return false; } @@ -235,4 +262,11 @@ private BadRequestObjectResult MalformedIfMatch() => code = "MALFORMED_IF_MATCH", message = "If-Match must be a strong validator with a single non-negative integer version (e.g. \"5\").", }); + + private BadRequestObjectResult IfMatchBodyMismatch(long headerVersion, long bodyVersion) => + BadRequest(new + { + code = "IF_MATCH_BODY_MISMATCH", + message = $"If-Match header (\"{headerVersion}\") disagrees with body expectedVersion ({bodyVersion}). Send only one, or set them to the same value.", + }); } diff --git a/test/Aevatar.Tools.Cli.Tests/ConnectorsControllerETagTests.cs b/test/Aevatar.Tools.Cli.Tests/ConnectorsControllerETagTests.cs index 1e201cfb5..433e8bcc3 100644 --- a/test/Aevatar.Tools.Cli.Tests/ConnectorsControllerETagTests.cs +++ b/test/Aevatar.Tools.Cli.Tests/ConnectorsControllerETagTests.cs @@ -70,6 +70,20 @@ public async Task Save_WithValidIfMatch_PassesExpectedVersion_AndEmitsDeterminis controller.Response.Headers["ETag"].ToString().Should().Be("\"4\""); } + [Fact] + public async Task Save_WhenIfMatchHeaderDisagreesWithBodyExpectedVersion_Returns400() + { + var store = new RecordingConnectorCatalogStore(); + var controller = CreateController(store, ifMatch: "\"3\""); + + var result = await controller.Save( + new SaveConnectorCatalogRequest(Connectors: [SampleHttpConnector()], ExpectedVersion: 4), + CancellationToken.None); + + result.Result.Should().BeOfType(); + store.SavedCatalog.Should().BeNull(); + } + [Fact] public async Task Save_WhenStoreThrowsOptimisticConflict_Returns409() { @@ -131,6 +145,35 @@ public async Task SaveDraft_WithoutIfMatch_DoesNotEmitETag() controller.Response.Headers.ContainsKey("ETag").Should().BeFalse(); } + [Fact] + public async Task SaveDraft_WhenIfMatchHeaderDisagreesWithBodyExpectedVersion_Returns400() + { + var store = new RecordingConnectorCatalogStore(); + var controller = CreateController(store, ifMatch: "\"5\""); + + var result = await controller.SaveDraft( + new SaveConnectorDraftRequest(Draft: SampleHttpConnector(), ExpectedVersion: 6), + CancellationToken.None); + + result.Result.Should().BeOfType(); + store.SavedDraft.Should().BeNull(); + store.SavedDraftExpectedVersion.Should().BeNull(); + } + + [Fact] + public async Task SaveDraft_WhenIfMatchHeaderAgreesWithBodyExpectedVersion_HeaderWinsAndStoreReceivesIt() + { + var store = new RecordingConnectorCatalogStore(); + var controller = CreateController(store, ifMatch: "\"5\""); + + var result = await controller.SaveDraft( + new SaveConnectorDraftRequest(Draft: SampleHttpConnector(), ExpectedVersion: 5), + CancellationToken.None); + + result.Result.Should().BeOfType(); + store.SavedDraftExpectedVersion.Should().Be(5); + } + [Fact] public async Task SaveDraft_WhenStoreThrowsOptimisticConflict_Returns409() { diff --git a/test/Aevatar.Tools.Cli.Tests/RolesControllerETagTests.cs b/test/Aevatar.Tools.Cli.Tests/RolesControllerETagTests.cs index f42b4a09f..30d4ec6f1 100644 --- a/test/Aevatar.Tools.Cli.Tests/RolesControllerETagTests.cs +++ b/test/Aevatar.Tools.Cli.Tests/RolesControllerETagTests.cs @@ -71,6 +71,20 @@ public async Task Save_WithValidIfMatch_PassesExpectedVersion_AndEmitsDeterminis controller.Response.Headers["ETag"].ToString().Should().Be("\"4\""); } + [Fact] + public async Task Save_WhenIfMatchHeaderDisagreesWithBodyExpectedVersion_Returns400() + { + var store = new RecordingRoleCatalogStore(); + var controller = CreateController(store, ifMatch: "\"3\""); + + var result = await controller.Save( + new SaveRoleCatalogRequest(Roles: [SampleRole()], ExpectedVersion: 4), + CancellationToken.None); + + result.Result.Should().BeOfType(); + store.SavedCatalog.Should().BeNull(); + } + [Fact] public async Task Save_WhenStoreThrowsOptimisticConflict_Returns409() { @@ -132,6 +146,35 @@ public async Task SaveDraft_WithoutIfMatch_DoesNotEmitETag() controller.Response.Headers.ContainsKey("ETag").Should().BeFalse(); } + [Fact] + public async Task SaveDraft_WhenIfMatchHeaderDisagreesWithBodyExpectedVersion_Returns400() + { + var store = new RecordingRoleCatalogStore(); + var controller = CreateController(store, ifMatch: "\"5\""); + + var result = await controller.SaveDraft( + new SaveRoleDraftRequest(Draft: SampleRole(), ExpectedVersion: 6), + CancellationToken.None); + + result.Result.Should().BeOfType(); + store.SavedDraft.Should().BeNull(); + store.SavedDraftExpectedVersion.Should().BeNull(); + } + + [Fact] + public async Task SaveDraft_WhenIfMatchHeaderAgreesWithBodyExpectedVersion_HeaderWinsAndStoreReceivesIt() + { + var store = new RecordingRoleCatalogStore(); + var controller = CreateController(store, ifMatch: "\"5\""); + + var result = await controller.SaveDraft( + new SaveRoleDraftRequest(Draft: SampleRole(), ExpectedVersion: 5), + CancellationToken.None); + + result.Result.Should().BeOfType(); + store.SavedDraftExpectedVersion.Should().Be(5); + } + [Fact] public async Task SaveDraft_WhenStoreThrowsOptimisticConflict_Returns409() {