diff --git a/src/Squidex.Domain.Apps.Entities/Contents/Queries/ContentLoader.cs b/src/Squidex.Domain.Apps.Entities/Contents/Queries/ContentLoader.cs index 572e4d9fc..5abb7f687 100644 --- a/src/Squidex.Domain.Apps.Entities/Contents/Queries/ContentLoader.cs +++ b/src/Squidex.Domain.Apps.Entities/Contents/Queries/ContentLoader.cs @@ -32,7 +32,7 @@ namespace Squidex.Domain.Apps.Entities.Contents.Queries var content = await grain.GetStateAsync(version); - if (content.Value == null || content.Value.Version != version) + if (content.Value == null || (version > EtagVersion.Any && content.Value.Version != version)) { throw new DomainObjectNotFoundException(id.ToString(), typeof(IContentEntity)); } diff --git a/src/Squidex.Domain.Apps.Entities/SquidexCommand.cs b/src/Squidex.Domain.Apps.Entities/SquidexCommand.cs index b9e15f41e..e5dbccb20 100644 --- a/src/Squidex.Domain.Apps.Entities/SquidexCommand.cs +++ b/src/Squidex.Domain.Apps.Entities/SquidexCommand.cs @@ -17,6 +17,6 @@ namespace Squidex.Domain.Apps.Entities public ClaimsPrincipal User { get; set; } - public long ExpectedVersion { get; set; } = EtagVersion.Any; + public long ExpectedVersion { get; set; } = EtagVersion.Auto; } } diff --git a/src/Squidex.Infrastructure.Azure/EventSourcing/CosmosDbEventStore_Writer.cs b/src/Squidex.Infrastructure.Azure/EventSourcing/CosmosDbEventStore_Writer.cs index 45144e56e..0c9186e15 100644 --- a/src/Squidex.Infrastructure.Azure/EventSourcing/CosmosDbEventStore_Writer.cs +++ b/src/Squidex.Infrastructure.Azure/EventSourcing/CosmosDbEventStore_Writer.cs @@ -60,7 +60,7 @@ namespace Squidex.Infrastructure.EventSourcing var currentVersion = await GetEventStreamOffsetAsync(streamName); - if (expectedVersion != EtagVersion.Any && expectedVersion != currentVersion) + if (expectedVersion > EtagVersion.Any && expectedVersion != currentVersion) { throw new WrongEventVersionException(currentVersion, expectedVersion); } @@ -81,7 +81,7 @@ namespace Squidex.Infrastructure.EventSourcing { currentVersion = await GetEventStreamOffsetAsync(streamName); - if (expectedVersion != EtagVersion.Any) + if (expectedVersion > EtagVersion.Any) { throw new WrongEventVersionException(currentVersion, expectedVersion); } diff --git a/src/Squidex.Infrastructure.MongoDb/EventSourcing/MongoEventStore_Writer.cs b/src/Squidex.Infrastructure.MongoDb/EventSourcing/MongoEventStore_Writer.cs index 29a9de831..a18a836d2 100644 --- a/src/Squidex.Infrastructure.MongoDb/EventSourcing/MongoEventStore_Writer.cs +++ b/src/Squidex.Infrastructure.MongoDb/EventSourcing/MongoEventStore_Writer.cs @@ -51,7 +51,7 @@ namespace Squidex.Infrastructure.EventSourcing var currentVersion = await GetEventStreamOffsetAsync(streamName); - if (expectedVersion != EtagVersion.Any && expectedVersion != currentVersion) + if (expectedVersion > EtagVersion.Any && expectedVersion != currentVersion) { throw new WrongEventVersionException(currentVersion, expectedVersion); } @@ -74,7 +74,7 @@ namespace Squidex.Infrastructure.EventSourcing { currentVersion = await GetEventStreamOffsetAsync(streamName); - if (expectedVersion != EtagVersion.Any) + if (expectedVersion > EtagVersion.Any) { throw new WrongEventVersionException(currentVersion, expectedVersion); } diff --git a/src/Squidex.Infrastructure/Commands/DomainObjectGrainBase.cs b/src/Squidex.Infrastructure/Commands/DomainObjectGrainBase.cs index 8d69c27c6..daf4c21c5 100644 --- a/src/Squidex.Infrastructure/Commands/DomainObjectGrainBase.cs +++ b/src/Squidex.Infrastructure/Commands/DomainObjectGrainBase.cs @@ -152,7 +152,7 @@ namespace Squidex.Infrastructure.Commands { Guard.NotNull(command, nameof(command)); - if (command.ExpectedVersion != EtagVersion.Any && command.ExpectedVersion != Version) + if (command.ExpectedVersion > EtagVersion.Any && command.ExpectedVersion != Version) { throw new DomainObjectVersionException(id.ToString(), GetType(), Version, command.ExpectedVersion); } diff --git a/src/Squidex.Infrastructure/Commands/LogSnapshotDomainObjectGrain.cs b/src/Squidex.Infrastructure/Commands/LogSnapshotDomainObjectGrain.cs index 8e820282f..d2054d18c 100644 --- a/src/Squidex.Infrastructure/Commands/LogSnapshotDomainObjectGrain.cs +++ b/src/Squidex.Infrastructure/Commands/LogSnapshotDomainObjectGrain.cs @@ -36,7 +36,7 @@ namespace Squidex.Infrastructure.Commands public T GetSnapshot(long version) { - if (version == EtagVersion.Any) + if (version <= EtagVersion.Any) { return Snapshot; } diff --git a/src/Squidex.Infrastructure/EtagVersion.cs b/src/Squidex.Infrastructure/EtagVersion.cs index 85d1175b4..21c305132 100644 --- a/src/Squidex.Infrastructure/EtagVersion.cs +++ b/src/Squidex.Infrastructure/EtagVersion.cs @@ -9,10 +9,12 @@ namespace Squidex.Infrastructure { public static class EtagVersion { + public const long NotFound = long.MinValue; + + public const long Auto = -3; + public const long Any = -2; public const long Empty = -1; - - public const long NotFound = long.MinValue; } } diff --git a/src/Squidex.Infrastructure/States/Persistence{TSnapshot,TKey}.cs b/src/Squidex.Infrastructure/States/Persistence{TSnapshot,TKey}.cs index 17608e24c..c59303c17 100644 --- a/src/Squidex.Infrastructure/States/Persistence{TSnapshot,TKey}.cs +++ b/src/Squidex.Infrastructure/States/Persistence{TSnapshot,TKey}.cs @@ -69,7 +69,7 @@ namespace Squidex.Infrastructure.States UpdateVersion(); - if (expectedVersion != EtagVersion.Any && expectedVersion != version) + if (expectedVersion > EtagVersion.Any && expectedVersion != version) { if (version == EtagVersion.Empty) { diff --git a/src/Squidex.Web/CommandMiddlewares/ETagCommandMiddleware.cs b/src/Squidex.Web/CommandMiddlewares/ETagCommandMiddleware.cs index 06cf91311..84ecd94ee 100644 --- a/src/Squidex.Web/CommandMiddlewares/ETagCommandMiddleware.cs +++ b/src/Squidex.Web/CommandMiddlewares/ETagCommandMiddleware.cs @@ -10,6 +10,7 @@ using System.Globalization; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.Net.Http.Headers; +using Squidex.Domain.Apps.Entities; using Squidex.Infrastructure; using Squidex.Infrastructure.Commands; @@ -35,22 +36,17 @@ namespace Squidex.Web.CommandMiddlewares return; } - context.Command.ExpectedVersion = EtagVersion.Any; + var command = context.Command; - var headers = httpContext.Request.Headers; - - if (headers.TryGetValue(HeaderNames.IfMatch, out var etag) && !string.IsNullOrWhiteSpace(etag)) + if (command.ExpectedVersion == EtagVersion.Auto) { - var etagValue = etag.ToString(); - - if (etagValue.StartsWith("W/", StringComparison.OrdinalIgnoreCase)) + if (TryParseEtag(httpContext, out var expectedVersion)) { - etagValue = etagValue.Substring(2); + command.ExpectedVersion = expectedVersion; } - - if (long.TryParse(etagValue, NumberStyles.Any, CultureInfo.InvariantCulture, out var expectedVersion)) + else { - context.Command.ExpectedVersion = expectedVersion; + command.ExpectedVersion = EtagVersion.Any; } } @@ -58,8 +54,37 @@ namespace Squidex.Web.CommandMiddlewares if (context.PlainResult is EntitySavedResult result) { - httpContext.Response.Headers[HeaderNames.ETag] = result.Version.ToString(); + SetResponsEtag(httpContext, result.Version); + } + else if (context.PlainResult is IEntityWithVersion entity) + { + SetResponsEtag(httpContext, entity.Version); + } + } + + private static void SetResponsEtag(HttpContext httpContext, long version) + { + httpContext.Response.Headers[HeaderNames.ETag] = version.ToString(); + } + + private static bool TryParseEtag(HttpContext httpContext, out long version) + { + version = default; + + if (httpContext.Request.Headers.TryGetHeaderString(HeaderNames.IfMatch, out var etag)) + { + if (etag.StartsWith("W/", StringComparison.OrdinalIgnoreCase)) + { + etag = etag.Substring(2); + } + + if (long.TryParse(etag, NumberStyles.Any, CultureInfo.InvariantCulture, out version)) + { + return true; + } } + + return false; } } } diff --git a/src/Squidex.Web/Extensions.cs b/src/Squidex.Web/Extensions.cs index 4349a864f..d2c2fec97 100644 --- a/src/Squidex.Web/Extensions.cs +++ b/src/Squidex.Web/Extensions.cs @@ -7,6 +7,7 @@ using System; using System.Security.Claims; +using Microsoft.AspNetCore.Http; using Squidex.Infrastructure.Security; namespace Squidex.Web @@ -43,5 +44,23 @@ namespace Squidex.Web return string.Equals(subject, userId, StringComparison.OrdinalIgnoreCase); } + + public static bool TryGetHeaderString(this IHeaderDictionary headers, string header, out string result) + { + result = null; + + if (headers.TryGetValue(header, out var value)) + { + string valueString = value; + + if (!string.IsNullOrWhiteSpace(valueString)) + { + result = valueString; + return true; + } + } + + return false; + } } } diff --git a/src/Squidex.Web/Pipeline/ETagFilter.cs b/src/Squidex.Web/Pipeline/ETagFilter.cs index 4dd680374..224fbe4f1 100644 --- a/src/Squidex.Web/Pipeline/ETagFilter.cs +++ b/src/Squidex.Web/Pipeline/ETagFilter.cs @@ -5,6 +5,7 @@ // All rights reserved. Licensed under the MIT license. // ========================================================================== +using System; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; @@ -29,21 +30,19 @@ namespace Squidex.Web.Pipeline var httpContext = context.HttpContext; - if (httpContext.Response.Headers.TryGetValue(HeaderNames.ETag, out var etag) && !string.IsNullOrWhiteSpace(etag)) + if (httpContext.Response.Headers.TryGetHeaderString(HeaderNames.ETag, out var etag)) { - string etagValue = etag; - - if (!options.Strong) + if (!options.Strong && !etag.StartsWith("W/", StringComparison.OrdinalIgnoreCase)) { - etagValue = "W/" + etag; + etag = $"W/{etag}"; - httpContext.Response.Headers[HeaderNames.ETag] = etagValue; + httpContext.Response.Headers[HeaderNames.ETag] = etag; } if (HttpMethods.IsGet(httpContext.Request.Method) && httpContext.Response.StatusCode == 200 && - httpContext.Request.Headers.TryGetValue(HeaderNames.IfNoneMatch, out var noneMatch) && !string.IsNullOrWhiteSpace(noneMatch) && - string.Equals(etagValue, noneMatch, System.StringComparison.Ordinal)) + httpContext.Request.Headers.TryGetHeaderString(HeaderNames.IfNoneMatch, out var noneMatch) && + string.Equals(etag, noneMatch, StringComparison.Ordinal)) { resultContext.Result = new StatusCodeResult(304); } diff --git a/src/Squidex/app/shared/state/contents.state.ts b/src/Squidex/app/shared/state/contents.state.ts index 38248e97e..958c62cb9 100644 --- a/src/Squidex/app/shared/state/contents.state.ts +++ b/src/Squidex/app/shared/state/contents.state.ts @@ -56,7 +56,7 @@ interface Snapshot { } function sameContent(lhs: ContentDto, rhs?: ContentDto): boolean { - return lhs === rhs || (!!lhs && !!rhs && lhs.id === rhs.id && lhs.version.eq(lhs.version)); + return lhs === rhs || (!!lhs && !!rhs && lhs.id === rhs.id && lhs.version.eq(rhs.version)); } export abstract class ContentsStateBase extends State { diff --git a/tests/Squidex.Domain.Apps.Entities.Tests/Contents/Queries/ContentLoaderTests.cs b/tests/Squidex.Domain.Apps.Entities.Tests/Contents/Queries/ContentLoaderTests.cs index 4047f916d..bbf6e5eed 100644 --- a/tests/Squidex.Domain.Apps.Entities.Tests/Contents/Queries/ContentLoaderTests.cs +++ b/tests/Squidex.Domain.Apps.Entities.Tests/Contents/Queries/ContentLoaderTests.cs @@ -50,6 +50,17 @@ namespace Squidex.Domain.Apps.Entities.Contents.Queries await Assert.ThrowsAsync(() => sut.GetAsync(id, 10)); } + [Fact] + public async Task Should_not_throw_exception_if_state_has_other_version_than_any() + { + var content = new ContentEntity { Version = 5 }; + + A.CallTo(() => grain.GetStateAsync(10)) + .Returns(J.Of(content)); + + await sut.GetAsync(id, EtagVersion.Any); + } + [Fact] public async Task Should_return_content_from_state() { diff --git a/tests/Squidex.Web.Tests/CommandMiddlewares/ETagCommandMiddlewareTests.cs b/tests/Squidex.Web.Tests/CommandMiddlewares/ETagCommandMiddlewareTests.cs index 05b58b31b..094055cc6 100644 --- a/tests/Squidex.Web.Tests/CommandMiddlewares/ETagCommandMiddlewareTests.cs +++ b/tests/Squidex.Web.Tests/CommandMiddlewares/ETagCommandMiddlewareTests.cs @@ -10,6 +10,7 @@ using FakeItEasy; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Primitives; using Microsoft.Net.Http.Headers; +using Squidex.Domain.Apps.Entities.Contents; using Squidex.Domain.Apps.Entities.Contents.Commands; using Squidex.Infrastructure.Commands; using Xunit; @@ -45,6 +46,19 @@ namespace Squidex.Web.CommandMiddlewares Assert.Null(command.Actor); } + [Fact] + public async Task Should_do_nothing_if_command_has_etag_defined() + { + httpContext.Request.Headers[HeaderNames.IfMatch] = "13"; + + var command = new CreateContent { ExpectedVersion = 1 }; + var context = Ctx(command); + + await sut.HandleAsync(context); + + Assert.Equal(1, context.Command.ExpectedVersion); + } + [Fact] public async Task Should_add_expected_version_to_command() { @@ -72,7 +86,7 @@ namespace Squidex.Web.CommandMiddlewares } [Fact] - public async Task Should_add_etag_header_to_response() + public async Task Should_add_version_from_result_as_etag_to_response() { var command = new CreateContent(); var context = Ctx(command); @@ -84,6 +98,19 @@ namespace Squidex.Web.CommandMiddlewares Assert.Equal(new StringValues("17"), httpContextAccessor.HttpContext.Response.Headers[HeaderNames.ETag]); } + [Fact] + public async Task Should_add_version_from_entity_as_etag_to_response() + { + var command = new CreateContent(); + var context = Ctx(command); + + context.Complete(new ContentEntity { Version = 17 }); + + await sut.HandleAsync(context); + + Assert.Equal(new StringValues("17"), httpContextAccessor.HttpContext.Response.Headers[HeaderNames.ETag]); + } + private CommandContext Ctx(ICommand command) { return new CommandContext(command, commandBus); diff --git a/tests/Squidex.Web.Tests/Pipeline/ETagFilterTests.cs b/tests/Squidex.Web.Tests/Pipeline/ETagFilterTests.cs index 9a1e30abb..db46eba5c 100644 --- a/tests/Squidex.Web.Tests/Pipeline/ETagFilterTests.cs +++ b/tests/Squidex.Web.Tests/Pipeline/ETagFilterTests.cs @@ -38,12 +38,22 @@ namespace Squidex.Web.Pipeline }; } + [Fact] + public async Task Should_not_convert_already_weak_tag() + { + httpContext.Response.Headers[HeaderNames.ETag] = "W/13"; + + await sut.OnActionExecutionAsync(executingContext, Next()); + + Assert.Equal("W/13", httpContext.Response.Headers[HeaderNames.ETag]); + } + [Fact] public async Task Should_convert_strong_to_weak_tag() { httpContext.Response.Headers[HeaderNames.ETag] = "13"; - await sut.OnActionExecutionAsync(executingContext, () => Task.FromResult(executedContext)); + await sut.OnActionExecutionAsync(executingContext, Next()); Assert.Equal("W/13", httpContext.Response.Headers[HeaderNames.ETag]); } @@ -53,7 +63,7 @@ namespace Squidex.Web.Pipeline { httpContext.Response.Headers[HeaderNames.ETag] = string.Empty; - await sut.OnActionExecutionAsync(executingContext, () => Task.FromResult(executedContext)); + await sut.OnActionExecutionAsync(executingContext, Next()); Assert.Null((string)httpContext.Response.Headers[HeaderNames.ETag]); } @@ -66,7 +76,7 @@ namespace Squidex.Web.Pipeline httpContext.Response.Headers[HeaderNames.ETag] = "13"; - await sut.OnActionExecutionAsync(executingContext, () => Task.FromResult(executedContext)); + await sut.OnActionExecutionAsync(executingContext, Next()); Assert.Equal(304, (executedContext.Result as StatusCodeResult).StatusCode); } @@ -79,9 +89,14 @@ namespace Squidex.Web.Pipeline httpContext.Response.Headers[HeaderNames.ETag] = "13"; - await sut.OnActionExecutionAsync(executingContext, () => Task.FromResult(executedContext)); + await sut.OnActionExecutionAsync(executingContext, Next()); Assert.Equal(200, (executedContext.Result as StatusCodeResult).StatusCode); } + + private ActionExecutionDelegate Next() + { + return () => Task.FromResult(executedContext); + } } }