From 06401bbe1535145c5b9b943e0cdbdc11482d3121 Mon Sep 17 00:00:00 2001 From: "jack.lewis" Date: Fri, 20 Sep 2024 14:28:24 +0100 Subject: [PATCH 1/7] Adding initial ETag caching changes --- .../API/Infrastructure/Helpers/ETagManager.cs | 6 + .../API/Infrastructure/ServiceCollectionX.cs | 12 ++ src/IIIFPresentation/API/Program.cs | 5 + .../API/Settings/CacheSettings.cs | 105 ++++++++++++++++++ 4 files changed, 128 insertions(+) create mode 100644 src/IIIFPresentation/API/Infrastructure/Helpers/ETagManager.cs create mode 100644 src/IIIFPresentation/API/Settings/CacheSettings.cs diff --git a/src/IIIFPresentation/API/Infrastructure/Helpers/ETagManager.cs b/src/IIIFPresentation/API/Infrastructure/Helpers/ETagManager.cs new file mode 100644 index 00000000..63334c62 --- /dev/null +++ b/src/IIIFPresentation/API/Infrastructure/Helpers/ETagManager.cs @@ -0,0 +1,6 @@ +namespace API.Infrastructure.Helpers; + +public class ETagManager +{ + +} \ No newline at end of file diff --git a/src/IIIFPresentation/API/Infrastructure/ServiceCollectionX.cs b/src/IIIFPresentation/API/Infrastructure/ServiceCollectionX.cs index 7caf5516..3ac79f71 100644 --- a/src/IIIFPresentation/API/Infrastructure/ServiceCollectionX.cs +++ b/src/IIIFPresentation/API/Infrastructure/ServiceCollectionX.cs @@ -1,6 +1,7 @@ using System.Reflection; using API.Infrastructure.Mediatr.Behaviours; using API.Infrastructure.Requests.Pipelines; +using API.Settings; using MediatR; using Repository; @@ -16,6 +17,17 @@ public static IServiceCollection AddDataAccess(this IServiceCollection services, return services .AddPresentationContext(configuration); } + + /// + /// Configure caching + /// + public static IServiceCollection AddCaching(this IServiceCollection services, CacheSettings cacheSettings) + => services.AddMemoryCache(memoryCacheOptions => + { + memoryCacheOptions.SizeLimit = cacheSettings.MemoryCacheSizeLimit; + memoryCacheOptions.CompactionPercentage = cacheSettings.MemoryCacheCompactionPercentage; + }) + .AddLazyCache(); /// /// Add MediatR services and pipeline behaviours to service collection. diff --git a/src/IIIFPresentation/API/Program.cs b/src/IIIFPresentation/API/Program.cs index 243b3a1e..b563fb6b 100644 --- a/src/IIIFPresentation/API/Program.cs +++ b/src/IIIFPresentation/API/Program.cs @@ -29,8 +29,13 @@ builder.Services.AddOptions() .BindConfiguration(nameof(ApiSettings)); +builder.Services.AddOptions() + .BindConfiguration(nameof(CacheSettings)); + +var cacheSettings = builder.Configuration.Get() ?? new CacheSettings(); builder.Services.AddDataAccess(builder.Configuration); +builder.Services.AddCaching(cacheSettings); builder.Services.ConfigureMediatR(); builder.Services.AddHealthChecks(); builder.Services.Configure(opts => diff --git a/src/IIIFPresentation/API/Settings/CacheSettings.cs b/src/IIIFPresentation/API/Settings/CacheSettings.cs new file mode 100644 index 00000000..26a1de51 --- /dev/null +++ b/src/IIIFPresentation/API/Settings/CacheSettings.cs @@ -0,0 +1,105 @@ +namespace API.Settings; + +/// +/// Settings related to caching +/// +public class CacheSettings +{ + /// + /// A collection CacheTtls per source + /// + public Dictionary TimeToLive { get; set; } = new(); + + /// + /// The size limit for MemoryCache. Maps to MemoryCacheOptions.SizeLimit property + /// + public long MemoryCacheSizeLimit { get; set; } = 10000; + + /// + /// The amount to compact the cache by when the maximum size is exceeded, value be between 0 and 1. + /// Maps to MemoryCacheOptions.CompactionPercentage property. + /// + public double MemoryCacheCompactionPercentage { get; set; } = 0.05; + + /// + /// Get pre configured Ttl for a source. + /// Falls back to Memory cache duration if not found. + /// + /// Pre configured ttl to fetch + /// Cache source to get ttl for + /// Ttl, in secs + public int GetTtl(CacheDuration duration = CacheDuration.Default, CacheSource source = CacheSource.Memory) + => TimeToLive.TryGetValue(source, out var settings) + ? settings.GetTtl(duration) + : GetFallback(duration); + + /// + /// Get specific named cache override for source. + /// Falls back to default memory duration if not found. + /// + /// Name of cache override to fetch + /// Cache source to get ttl for + /// Ttl, in secs + public int GetTtl(string named, CacheSource source = CacheSource.Memory) + => TimeToLive.TryGetValue(source, out var settings) + ? settings.GetTtl(named) + : GetFallback(); + + private readonly CacheGroupSettings fallback = new(); + + private int GetFallback(CacheDuration duration = CacheDuration.Default) => + TimeToLive.TryGetValue(CacheSource.Memory, out var settings) + ? settings.GetTtl(duration) + : fallback.GetTtl(duration); +} + +public class CacheGroupSettings +{ + public int ShortTtlSecs { get; set; } = 60; + public int DefaultTtlSecs { get; set; } = 600; + public int LongTtlSecs { get; set; } = 1800; + public Dictionary Overrides { get; set; } + + public int GetTtl(CacheDuration duration) + => duration switch + { + CacheDuration.Short => ShortTtlSecs, + CacheDuration.Default => DefaultTtlSecs, + CacheDuration.Long => LongTtlSecs, + _ => DefaultTtlSecs + }; + + public int GetTtl(string named) + => Overrides.TryGetValue(named, out var ttl) ? ttl : DefaultTtlSecs; +} + +/// +/// Available caching sources +/// +public enum CacheSource +{ + /// + /// Local in-memory cache + /// + Memory, + + /// + /// External distributed cache + /// + Distributed, + + /// + /// Http caching (via headers) + /// + Http +} + +/// +/// Default preconfigured cache durations +/// +public enum CacheDuration +{ + Short, + Default, + Long +} \ No newline at end of file From cd9db66d48caee7e372f94bc01a052015fe6bef1 Mon Sep 17 00:00:00 2001 From: "jack.lewis" Date: Mon, 23 Sep 2024 16:19:53 +0100 Subject: [PATCH 2/7] Adding ability to set the ID of a collection on PUT --- .../API/Attributes/EtagCachingAttribute.cs | 14 +- .../Storage/Requests/UpdateCollection.cs | 105 ------------- .../Storage/Requests/UpsertCollection.cs | 146 ++++++++++++++++++ .../API/Features/Storage/StorageController.cs | 10 +- .../API/Infrastructure/ControllerBaseX.cs | 2 + .../API/Infrastructure/Helpers/ETagManager.cs | 32 +++- src/IIIFPresentation/API/Program.cs | 3 + src/IIIFPresentation/Core/WriteResult.cs | 7 +- 8 files changed, 201 insertions(+), 118 deletions(-) delete mode 100644 src/IIIFPresentation/API/Features/Storage/Requests/UpdateCollection.cs create mode 100644 src/IIIFPresentation/API/Features/Storage/Requests/UpsertCollection.cs diff --git a/src/IIIFPresentation/API/Attributes/EtagCachingAttribute.cs b/src/IIIFPresentation/API/Attributes/EtagCachingAttribute.cs index 466396e3..75c5a07b 100644 --- a/src/IIIFPresentation/API/Attributes/EtagCachingAttribute.cs +++ b/src/IIIFPresentation/API/Attributes/EtagCachingAttribute.cs @@ -10,7 +10,7 @@ namespace API.Attributes; -public class EtagCachingAttribute : ActionFilterAttribute +public class EtagCachingAttribute() : ActionFilterAttribute { // When a "304 Not Modified" response is to be sent back to the client, all headers apart from the following list should be stripped from the response to keep the response size minimal. See https://datatracker.ietf.org/doc/html/rfc7232#section-4.1:~:text=200%20(OK)%20response.-,The%20server%20generating%20a%20304,-response%20MUST%20generate private static readonly string[] headersToKeepFor304 = @@ -22,7 +22,7 @@ public class EtagCachingAttribute : ActionFilterAttribute HeaderNames.Vary }; - private static readonly Dictionary etagHashes = new(); + // private static readonly Dictionary etagHashes = new(); // Adds cache headers to response public override async Task OnResultExecutionAsync( @@ -32,6 +32,7 @@ ResultExecutionDelegate next { var request = context.HttpContext.Request; var response = context.HttpContext.Response; + var eTagManager = context.HttpContext.RequestServices.GetService(); // For more info on this technique, see https://stackoverflow.com/a/65901913 and https://www.madskristensen.net/blog/send-etag-headers-in-aspnet-core/ and https://gist.github.com/madskristensen/36357b1df9ddbfd123162cd4201124c4 var originalStream = response.Body; @@ -55,7 +56,7 @@ ResultExecutionDelegate next { responseHeaders.ETag ??= GenerateETag(memoryStream, - request.Path); // This request generates a hash from the response - this would come from S3 in live + request.Path, eTagManager!); // This request generates a hash from the response - this would come from S3 in live } var requestHeaders = request.GetTypedHeaders(); @@ -90,7 +91,7 @@ private static bool IsEtagSupported(HttpResponse response) return true; } - private static EntityTagHeaderValue GenerateETag(Stream stream, string path) + private static EntityTagHeaderValue GenerateETag(Stream stream, string path, IETagManager eTagManager) { var hashBytes = MD5.HashData(stream); stream.Position = 0; @@ -100,7 +101,7 @@ private static EntityTagHeaderValue GenerateETag(Stream stream, string path) new EntityTagHeaderValue('"' + hashString + '"'); - etagHashes[path] = enityTagHeader.Tag.ToString(); + eTagManager.UpsertETag(path, enityTagHeader.Tag.ToString()); return enityTagHeader; } @@ -126,6 +127,7 @@ public override async Task OnActionExecutionAsync(ActionExecutingContext context ArgumentNullException.ThrowIfNull(next); var request = context.HttpContext.Request; + var eTagManager = context.HttpContext.RequestServices.GetService(); if (request.Method == HttpMethod.Put.ToString()) { @@ -135,7 +137,7 @@ public override async Task OnActionExecutionAsync(ActionExecutingContext context StatusCode = StatusCodes.Status400BadRequest }; - etagHashes.TryGetValue(request.Path, out var etag); + eTagManager!.TryGetETag(request.Path, out var etag); if (!request.Headers.IfMatch.Equals(etag)) { diff --git a/src/IIIFPresentation/API/Features/Storage/Requests/UpdateCollection.cs b/src/IIIFPresentation/API/Features/Storage/Requests/UpdateCollection.cs deleted file mode 100644 index 1495927e..00000000 --- a/src/IIIFPresentation/API/Features/Storage/Requests/UpdateCollection.cs +++ /dev/null @@ -1,105 +0,0 @@ -using API.Auth; -using API.Converters; -using API.Features.Storage.Helpers; -using API.Helpers; -using API.Infrastructure.Requests; -using API.Settings; -using Core; -using Core.Helpers; -using MediatR; -using Microsoft.EntityFrameworkCore; -using Microsoft.Extensions.Options; -using Models.API.Collection; -using Models.API.Collection.Upsert; -using Repository; -using Repository.Helpers; - -namespace API.Features.Storage.Requests; - -public class UpdateCollection(int customerId, string collectionId, UpsertFlatCollection collection, UrlRoots urlRoots) - : IRequest> -{ - public int CustomerId { get; } = customerId; - - public string CollectionId { get; set; } = collectionId; - - public UpsertFlatCollection Collection { get; } = collection; - - public UrlRoots UrlRoots { get; } = urlRoots; -} - -public class UpdateCollectionHandler( - PresentationContext dbContext, - ILogger logger, - IOptions options) - : IRequestHandler> -{ - private readonly ApiSettings settings = options.Value; - - private const int DefaultCurrentPage = 1; - - public async Task> Handle(UpdateCollection request, CancellationToken cancellationToken) - { - var collectionFromDatabase = - await dbContext.Collections.FirstOrDefaultAsync(c => c.Id == request.CollectionId, cancellationToken); - - if (collectionFromDatabase == null) - { - return ModifyEntityResult.Failure( - "Could not find a matching record for the provided collection id", WriteResult.NotFound); - } - - if (collectionFromDatabase.Parent != request.Collection.Parent) - { - var parentCollection = await dbContext.RetrieveCollection(request.CustomerId, - request.Collection.Parent.GetLastPathElement(), cancellationToken); - - if (parentCollection == null) - { - return ModifyEntityResult.Failure( - $"The parent collection could not be found", WriteResult.BadRequest); - } - } - - collectionFromDatabase.Modified = DateTime.UtcNow; - collectionFromDatabase.ModifiedBy = Authorizer.GetUser(); - collectionFromDatabase.IsPublic = request.Collection.Behavior.IsPublic(); - collectionFromDatabase.IsStorageCollection = request.Collection.Behavior.IsStorageCollection(); - collectionFromDatabase.Label = request.Collection.Label; - collectionFromDatabase.Parent = request.Collection.Parent; - collectionFromDatabase.Slug = request.Collection.Slug; - collectionFromDatabase.Thumbnail = request.Collection.Thumbnail; - collectionFromDatabase.Tags = request.Collection.Tags; - collectionFromDatabase.ItemsOrder = request.Collection.ItemsOrder; - - var saveErrors = await dbContext.TrySaveCollection(request.CustomerId, logger, cancellationToken); - - if (saveErrors != null) - { - return saveErrors; - } - - var total = await dbContext.Collections.CountAsync( - c => c.CustomerId == request.CustomerId && c.Parent == collectionFromDatabase.Id, - cancellationToken: cancellationToken); - - var items = dbContext.Collections - .Where(s => s.CustomerId == request.CustomerId && s.Parent == collectionFromDatabase.Id) - .Take(settings.PageSize); - - foreach (var item in items) - { - item.FullPath = $"{(collectionFromDatabase.Parent != null ? $"{collectionFromDatabase.Slug}/" : string.Empty)}{item.Slug}"; - } - - if (collectionFromDatabase.Parent != null) - { - collectionFromDatabase.FullPath = - CollectionRetrieval.RetrieveFullPathForCollection(collectionFromDatabase, dbContext); - } - - return ModifyEntityResult.Success( - collectionFromDatabase.ToFlatCollection(request.UrlRoots, settings.PageSize, DefaultCurrentPage, total, - await items.ToListAsync(cancellationToken: cancellationToken))); - } -} \ No newline at end of file diff --git a/src/IIIFPresentation/API/Features/Storage/Requests/UpsertCollection.cs b/src/IIIFPresentation/API/Features/Storage/Requests/UpsertCollection.cs new file mode 100644 index 00000000..a8db4a57 --- /dev/null +++ b/src/IIIFPresentation/API/Features/Storage/Requests/UpsertCollection.cs @@ -0,0 +1,146 @@ +using API.Auth; +using API.Converters; +using API.Features.Storage.Helpers; +using API.Infrastructure.Helpers; +using API.Infrastructure.Requests; +using API.Settings; +using Core; +using Core.Helpers; +using MediatR; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Options; +using Models.API.Collection; +using Models.API.Collection.Upsert; +using Models.Database.Collections; +using Repository; +using Repository.Helpers; + +namespace API.Features.Storage.Requests; + +public class UpsertCollection(int customerId, string collectionId, UpsertFlatCollection collection, UrlRoots urlRoots, string? eTag) + : IRequest> +{ + public int CustomerId { get; } = customerId; + + public string CollectionId { get; set; } = collectionId; + + public UpsertFlatCollection Collection { get; } = collection; + + public UrlRoots UrlRoots { get; } = urlRoots; + + public string? ETag { get; set; } = eTag; +} + +public class UpsertCollectionHandler( + PresentationContext dbContext, + IETagManager eTagManager, + ILogger logger, + IOptions options) + : IRequestHandler> +{ + private readonly ApiSettings settings = options.Value; + + private const int DefaultCurrentPage = 1; + + public async Task> Handle(UpsertCollection request, CancellationToken cancellationToken) + { + var databaseCollection = + await dbContext.Collections.FirstOrDefaultAsync(c => c.Id == request.CollectionId, cancellationToken); + + if (databaseCollection == null) + { + var createdDate = DateTime.UtcNow; + + var parentCollection = await dbContext.RetrieveCollection(request.CustomerId, + request.Collection.Parent.GetLastPathElement(), cancellationToken); + if (parentCollection == null) + { + return ModifyEntityResult.Failure( + "The parent collection could not be found", WriteResult.BadRequest); + } + + databaseCollection = new Collection + { + Id = request.CollectionId, + Created = createdDate, + Modified = createdDate, + CreatedBy = Authorizer.GetUser(), + CustomerId = request.CustomerId, + IsPublic = request.Collection.Behavior.IsPublic(), + IsStorageCollection = request.Collection.Behavior.IsStorageCollection(), + Label = request.Collection.Label, + Parent = parentCollection.Id, + Slug = request.Collection.Slug, + Thumbnail = request.Collection.Thumbnail, + Tags = request.Collection.Tags, + ItemsOrder = request.Collection.ItemsOrder + }; + + await dbContext.AddAsync(databaseCollection, cancellationToken); + } + else + { + eTagManager.TryGetETag($"/{request.CustomerId}/collections/{request.CollectionId}", out var eTag); + + if (request.ETag != eTag) + { + return ModifyEntityResult.Failure( + "ETag does not match", WriteResult.PreConditionFailed); + } + + if (databaseCollection.Parent != request.Collection.Parent) + { + var parentCollection = await dbContext.RetrieveCollection(request.CustomerId, + request.Collection.Parent.GetLastPathElement(), cancellationToken); + + if (parentCollection == null) + { + return ModifyEntityResult.Failure( + $"The parent collection could not be found", WriteResult.BadRequest); + } + } + + databaseCollection.Modified = DateTime.UtcNow; + databaseCollection.ModifiedBy = Authorizer.GetUser(); + databaseCollection.IsPublic = request.Collection.Behavior.IsPublic(); + databaseCollection.IsStorageCollection = request.Collection.Behavior.IsStorageCollection(); + databaseCollection.Label = request.Collection.Label; + databaseCollection.Parent = request.Collection.Parent; + databaseCollection.Slug = request.Collection.Slug; + databaseCollection.Thumbnail = request.Collection.Thumbnail; + databaseCollection.Tags = request.Collection.Tags; + databaseCollection.ItemsOrder = request.Collection.ItemsOrder; + } + + + var saveErrors = await dbContext.TrySaveCollection(request.CustomerId, logger, cancellationToken); + + if (saveErrors != null) + { + return saveErrors; + } + + var total = await dbContext.Collections.CountAsync( + c => c.CustomerId == request.CustomerId && c.Parent == databaseCollection.Id, + cancellationToken: cancellationToken); + + var items = dbContext.Collections + .Where(s => s.CustomerId == request.CustomerId && s.Parent == databaseCollection.Id) + .Take(settings.PageSize); + + foreach (var item in items) + { + item.FullPath = $"{(databaseCollection.Parent != null ? $"{databaseCollection.Slug}/" : string.Empty)}{item.Slug}"; + } + + if (databaseCollection.Parent != null) + { + databaseCollection.FullPath = + CollectionRetrieval.RetrieveFullPathForCollection(databaseCollection, dbContext); + } + + return ModifyEntityResult.Success( + databaseCollection.ToFlatCollection(request.UrlRoots, settings.PageSize, DefaultCurrentPage, total, + await items.ToListAsync(cancellationToken: cancellationToken))); + } +} \ No newline at end of file diff --git a/src/IIIFPresentation/API/Features/Storage/StorageController.cs b/src/IIIFPresentation/API/Features/Storage/StorageController.cs index 1424cc85..f687a7ce 100644 --- a/src/IIIFPresentation/API/Features/Storage/StorageController.cs +++ b/src/IIIFPresentation/API/Features/Storage/StorageController.cs @@ -18,11 +18,13 @@ namespace API.Features.Storage; [Route("/{customerId}")] [ApiController] -public class StorageController(IOptions options, IMediator mediator) +public class StorageController(IOptions options, IMediator mediator, IETagManager etagManager) : PresentationController(options.Value, mediator) { + private readonly IETagManager etagManager = etagManager; + [HttpGet("{*slug}")] - [EtagCaching] + [EtagCaching()] public async Task GetHierarchicalCollection(int customerId, string slug = "") { var storageRoot = await Mediator.Send(new GetHierarchicalCollection(customerId, slug)); @@ -83,7 +85,6 @@ public async Task Post(int customerId, [FromBody] UpsertFlatColle } [HttpPut("collections/{id}")] - [EtagCaching] public async Task Put(int customerId, string id, [FromBody] UpsertFlatCollection collection, [FromServices] UpsertFlatCollectionValidator validator) { @@ -99,7 +100,8 @@ public async Task Put(int customerId, string id, [FromBody] Upser return this.ValidationFailed(validation); } - return await HandleUpsert(new UpdateCollection(customerId, id, collection, GetUrlRoots())); + return await HandleUpsert(new UpsertCollection(customerId, id, collection, GetUrlRoots(), + Request.Headers.IfMatch)); } [HttpDelete("collections/{id}")] diff --git a/src/IIIFPresentation/API/Infrastructure/ControllerBaseX.cs b/src/IIIFPresentation/API/Infrastructure/ControllerBaseX.cs index 21fe85b1..720a681d 100644 --- a/src/IIIFPresentation/API/Infrastructure/ControllerBaseX.cs +++ b/src/IIIFPresentation/API/Infrastructure/ControllerBaseX.cs @@ -72,6 +72,8 @@ public static IActionResult ModifyResultToHttpResult(this ControllerBase cont $"{errorTitle}: Validation failed"), WriteResult.StorageLimitExceeded => controller.PresentationProblem(entityResult.Error, instance, (int)HttpStatusCode.InsufficientStorage, $"{errorTitle}: Storage limit exceeded"), + WriteResult.PreConditionFailed => controller.PresentationProblem(entityResult.Error, instance, (int)HttpStatusCode.PreconditionFailed, + $"{errorTitle}: Pre-condition failed"), _ => controller.PresentationProblem(entityResult.Error, instance, (int)HttpStatusCode.InternalServerError, errorTitle), }; diff --git a/src/IIIFPresentation/API/Infrastructure/Helpers/ETagManager.cs b/src/IIIFPresentation/API/Infrastructure/Helpers/ETagManager.cs index 63334c62..df14918c 100644 --- a/src/IIIFPresentation/API/Infrastructure/Helpers/ETagManager.cs +++ b/src/IIIFPresentation/API/Infrastructure/Helpers/ETagManager.cs @@ -1,6 +1,34 @@ -namespace API.Infrastructure.Helpers; +using LazyCache; +using Microsoft.Extensions.Caching.Memory; -public class ETagManager +namespace API.Infrastructure.Helpers; + +public class ETagManager(IAppCache appCache, ILogger logger) : IETagManager { + MemoryCacheEntryOptions options = new MemoryCacheEntryOptions().SetSize(1); + + public bool TryGetETag(string id, out string? eTag) + { + try + { + return appCache.TryGetValue(id, out eTag); + } + catch (Exception ex) + { + logger.LogWarning(ex, "Error retrieving ETag"); + eTag = null; + return false; + } + } + public void UpsertETag(string id, string etag) + { + appCache.Add(id, etag, options); + } +} + +public interface IETagManager +{ + bool TryGetETag(string id, out string? eTag); + void UpsertETag(string id, string eTag); } \ No newline at end of file diff --git a/src/IIIFPresentation/API/Program.cs b/src/IIIFPresentation/API/Program.cs index b563fb6b..6e530c9b 100644 --- a/src/IIIFPresentation/API/Program.cs +++ b/src/IIIFPresentation/API/Program.cs @@ -1,6 +1,7 @@ using System.Text.Json.Serialization; using API.Features.Storage.Validators; using API.Infrastructure; +using API.Infrastructure.Helpers; using API.Settings; using Microsoft.AspNetCore.HttpOverrides; using Newtonsoft.Json; @@ -36,7 +37,9 @@ builder.Services.AddDataAccess(builder.Configuration); builder.Services.AddCaching(cacheSettings); +builder.Services.AddSingleton(); builder.Services.ConfigureMediatR(); +builder.Services.AddSingleton(); builder.Services.AddHealthChecks(); builder.Services.Configure(opts => { diff --git a/src/IIIFPresentation/Core/WriteResult.cs b/src/IIIFPresentation/Core/WriteResult.cs index 934699d1..f67e131c 100644 --- a/src/IIIFPresentation/Core/WriteResult.cs +++ b/src/IIIFPresentation/Core/WriteResult.cs @@ -48,5 +48,10 @@ public enum WriteResult /// /// Predefined storage limits exceeded /// - StorageLimitExceeded + StorageLimitExceeded, + + /// + /// Request failed a precondition + /// + PreConditionFailed } \ No newline at end of file From 573cba093726a1e1016b08d0a8b43f504ce7cf7d Mon Sep 17 00:00:00 2001 From: "jack.lewis" Date: Mon, 23 Sep 2024 17:02:43 +0100 Subject: [PATCH 3/7] adding test for allowing creation --- .../Integration/ModifyCollectionTests.cs | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/src/IIIFPresentation/API.Tests/Integration/ModifyCollectionTests.cs b/src/IIIFPresentation/API.Tests/Integration/ModifyCollectionTests.cs index 59315d32..e98c3447 100644 --- a/src/IIIFPresentation/API.Tests/Integration/ModifyCollectionTests.cs +++ b/src/IIIFPresentation/API.Tests/Integration/ModifyCollectionTests.cs @@ -292,6 +292,51 @@ public async Task UpdateCollection_UpdatesCollection_WhenAllValuesProvided() responseCollection.View.Id.Should().Contain("?page=1&pageSize=20"); } + [Fact] + public async Task UpdateCollection_CreatesCollection_WhenUnknownCollectionIdProvided() + { + // Arrange + var updatedCollection = new UpsertFlatCollection() + { + Behavior = new List() + { + Behavior.IsPublic, + Behavior.IsStorageCollection + }, + Label = new LanguageMap("en", ["test collection - create from update"]), + Slug = "create-from-update", + Parent = parent, + ItemsOrder = 1, + Thumbnail = "some/location/2", + Tags = "some, tags, 2", + }; + + var updateRequestMessage = HttpRequestMessageBuilder.GetPrivateRequest(HttpMethod.Put, + $"{Customer}/collections/createFromUpdate", JsonSerializer.Serialize(updatedCollection)); + + // Act + var response = await httpClient.AsCustomer(1).SendAsync(updateRequestMessage); + + var responseCollection = await response.ReadAsPresentationResponseAsync(); + + var fromDatabase = dbContext.Collections.First(c => c.Id == responseCollection!.Id!.Split('/', StringSplitOptions.TrimEntries).Last()); + + // Assert + response.StatusCode.Should().Be(HttpStatusCode.OK); + fromDatabase.Id.Should().Be("createFromUpdate"); + fromDatabase.Parent.Should().Be(parent); + fromDatabase.Label!.Values.First()[0].Should().Be("test collection - create from update"); + fromDatabase.Slug.Should().Be("create-from-update"); + fromDatabase.ItemsOrder.Should().Be(1); + fromDatabase.Thumbnail.Should().Be("some/location/2"); + fromDatabase.Tags.Should().Be("some, tags, 2"); + fromDatabase.IsPublic.Should().BeTrue(); + fromDatabase.IsStorageCollection.Should().BeTrue(); + responseCollection!.View!.PageSize.Should().Be(20); + responseCollection.View.Page.Should().Be(1); + responseCollection.View.Id.Should().Contain("?page=1&pageSize=20"); + } + [Fact] public async Task UpdateCollection_UpdatesCollection_WhenAllValuesProvidedWithoutLabel() { From aae4c82c686ed614be4f567553b879a5f56ef688 Mon Sep 17 00:00:00 2001 From: "jack.lewis" Date: Tue, 24 Sep 2024 11:40:30 +0100 Subject: [PATCH 4/7] Updates to support ETags being created and stored properly --- .../Integration/ModifyCollectionTests.cs | 30 ++++++++++++ .../API/Attributes/EtagCachingAttribute.cs | 46 ++----------------- .../Storage/Requests/UpsertCollection.cs | 12 ++++- .../API/Features/Storage/StorageController.cs | 11 ++--- .../API/Infrastructure/Helpers/ETagManager.cs | 8 +--- .../Infrastructure/Helpers/IETagManager.cs | 7 +++ 6 files changed, 57 insertions(+), 57 deletions(-) create mode 100644 src/IIIFPresentation/API/Infrastructure/Helpers/IETagManager.cs diff --git a/src/IIIFPresentation/API.Tests/Integration/ModifyCollectionTests.cs b/src/IIIFPresentation/API.Tests/Integration/ModifyCollectionTests.cs index e98c3447..8905557a 100644 --- a/src/IIIFPresentation/API.Tests/Integration/ModifyCollectionTests.cs +++ b/src/IIIFPresentation/API.Tests/Integration/ModifyCollectionTests.cs @@ -337,6 +337,36 @@ public async Task UpdateCollection_CreatesCollection_WhenUnknownCollectionIdProv responseCollection.View.Id.Should().Contain("?page=1&pageSize=20"); } + [Fact] + public async Task UpdateCollection_FailsToCreateCollection_WhenUnknownCollectionWithETag() + { + // Arrange + var updatedCollection = new UpsertFlatCollection() + { + Behavior = new List() + { + Behavior.IsPublic, + Behavior.IsStorageCollection + }, + Label = new LanguageMap("en", ["test collection - create from update"]), + Slug = "create-from-update-2", + Parent = parent, + ItemsOrder = 1, + Thumbnail = "some/location/2", + Tags = "some, tags, 2", + }; + + var updateRequestMessage = HttpRequestMessageBuilder.GetPrivateRequest(HttpMethod.Put, + $"{Customer}/collections/createFromUpdate2", JsonSerializer.Serialize(updatedCollection)); + updateRequestMessage.Headers.IfMatch.Add(new EntityTagHeaderValue("\"someTag\"")); + + // Act + var response = await httpClient.AsCustomer(1).SendAsync(updateRequestMessage); + + // Assert + response.StatusCode.Should().Be(HttpStatusCode.PreconditionFailed); + } + [Fact] public async Task UpdateCollection_UpdatesCollection_WhenAllValuesProvidedWithoutLabel() { diff --git a/src/IIIFPresentation/API/Attributes/EtagCachingAttribute.cs b/src/IIIFPresentation/API/Attributes/EtagCachingAttribute.cs index 75c5a07b..da43c781 100644 --- a/src/IIIFPresentation/API/Attributes/EtagCachingAttribute.cs +++ b/src/IIIFPresentation/API/Attributes/EtagCachingAttribute.cs @@ -10,10 +10,10 @@ namespace API.Attributes; -public class EtagCachingAttribute() : ActionFilterAttribute +public class ETagCachingAttribute : ActionFilterAttribute { // When a "304 Not Modified" response is to be sent back to the client, all headers apart from the following list should be stripped from the response to keep the response size minimal. See https://datatracker.ietf.org/doc/html/rfc7232#section-4.1:~:text=200%20(OK)%20response.-,The%20server%20generating%20a%20304,-response%20MUST%20generate - private static readonly string[] headersToKeepFor304 = + private static readonly string[] HeadersToKeepFor304 = { HeaderNames.CacheControl, HeaderNames.ContentLocation, @@ -22,8 +22,6 @@ public class EtagCachingAttribute() : ActionFilterAttribute HeaderNames.Vary }; - // private static readonly Dictionary etagHashes = new(); - // Adds cache headers to response public override async Task OnResultExecutionAsync( ResultExecutingContext context, @@ -42,7 +40,7 @@ ResultExecutionDelegate next await next(); memoryStream.Position = 0; - if (response.StatusCode == StatusCodes.Status200OK) + if (response.StatusCode is StatusCodes.Status200OK or StatusCodes.Status201Created) { var responseHeaders = response.GetTypedHeaders(); @@ -67,7 +65,7 @@ ResultExecutionDelegate next // Remove all unnecessary headers while only keeping the ones that should be included in a `304` response. foreach (var header in response.Headers) - if (!headersToKeepFor304.Contains(header.Key)) + if (!HeadersToKeepFor304.Contains(header.Key)) response.Headers.Remove(header.Key); return; @@ -119,40 +117,4 @@ private static bool IsClientCacheValid(RequestHeaders reqHeaders, ResponseHeader return false; } - - // checks request for valid cache headers - public override async Task OnActionExecutionAsync(ActionExecutingContext context, ActionExecutionDelegate next) - { - ArgumentNullException.ThrowIfNull(context); - ArgumentNullException.ThrowIfNull(next); - - var request = context.HttpContext.Request; - var eTagManager = context.HttpContext.RequestServices.GetService(); - - if (request.Method == HttpMethod.Put.ToString()) - { - if (request.Headers.IfMatch.Count == 0) - context.Result = new ObjectResult("This method requires a valid ETag to be present") - { - StatusCode = StatusCodes.Status400BadRequest - }; - - eTagManager!.TryGetETag(request.Path, out var etag); - - if (!request.Headers.IfMatch.Equals(etag)) - { - context.Result = new ObjectResult(new Error() - { - Detail = "Cannot match ETag", - Status = 412 - }) - { - StatusCode = StatusCodes.Status412PreconditionFailed - }; - } - } - - OnActionExecuting(context); - if (context.Result == null) OnActionExecuted(await next()); - } } \ No newline at end of file diff --git a/src/IIIFPresentation/API/Features/Storage/Requests/UpsertCollection.cs b/src/IIIFPresentation/API/Features/Storage/Requests/UpsertCollection.cs index a8db4a57..c1d3392a 100644 --- a/src/IIIFPresentation/API/Features/Storage/Requests/UpsertCollection.cs +++ b/src/IIIFPresentation/API/Features/Storage/Requests/UpsertCollection.cs @@ -17,7 +17,8 @@ namespace API.Features.Storage.Requests; -public class UpsertCollection(int customerId, string collectionId, UpsertFlatCollection collection, UrlRoots urlRoots, string? eTag) +public class UpsertCollection(int customerId, string collectionId, UpsertFlatCollection collection, UrlRoots urlRoots, + string? eTag) : IRequest> { public int CustomerId { get; } = customerId; @@ -42,13 +43,20 @@ public class UpsertCollectionHandler( private const int DefaultCurrentPage = 1; - public async Task> Handle(UpsertCollection request, CancellationToken cancellationToken) + public async Task> Handle(UpsertCollection request, + CancellationToken cancellationToken) { var databaseCollection = await dbContext.Collections.FirstOrDefaultAsync(c => c.Id == request.CollectionId, cancellationToken); if (databaseCollection == null) { + if (request.ETag is not null) + { + return ModifyEntityResult.Failure( + "ETag should not be added when inserting collection via PUT", WriteResult.PreConditionFailed); + } + var createdDate = DateTime.UtcNow; var parentCollection = await dbContext.RetrieveCollection(request.CustomerId, diff --git a/src/IIIFPresentation/API/Features/Storage/StorageController.cs b/src/IIIFPresentation/API/Features/Storage/StorageController.cs index f687a7ce..e40530c7 100644 --- a/src/IIIFPresentation/API/Features/Storage/StorageController.cs +++ b/src/IIIFPresentation/API/Features/Storage/StorageController.cs @@ -18,13 +18,11 @@ namespace API.Features.Storage; [Route("/{customerId}")] [ApiController] -public class StorageController(IOptions options, IMediator mediator, IETagManager etagManager) +public class StorageController(IOptions options, IMediator mediator) : PresentationController(options.Value, mediator) { - private readonly IETagManager etagManager = etagManager; - [HttpGet("{*slug}")] - [EtagCaching()] + [ETagCaching()] public async Task GetHierarchicalCollection(int customerId, string slug = "") { var storageRoot = await Mediator.Send(new GetHierarchicalCollection(customerId, slug)); @@ -41,7 +39,7 @@ public async Task GetHierarchicalCollection(int customerId, strin } [HttpGet("collections/{id}")] - [EtagCaching] + [ETagCaching] public async Task Get(int customerId, string id, int? page = 1, int? pageSize = -1, string? orderBy = null, string? orderByDescending = null) { @@ -65,7 +63,7 @@ public async Task Get(int customerId, string id, int? page = 1, i } [HttpPost("collections")] - [EtagCaching] + [ETagCaching] public async Task Post(int customerId, [FromBody] UpsertFlatCollection collection, [FromServices] UpsertFlatCollectionValidator validator) { @@ -85,6 +83,7 @@ public async Task Post(int customerId, [FromBody] UpsertFlatColle } [HttpPut("collections/{id}")] + [ETagCaching] public async Task Put(int customerId, string id, [FromBody] UpsertFlatCollection collection, [FromServices] UpsertFlatCollectionValidator validator) { diff --git a/src/IIIFPresentation/API/Infrastructure/Helpers/ETagManager.cs b/src/IIIFPresentation/API/Infrastructure/Helpers/ETagManager.cs index df14918c..f83205e2 100644 --- a/src/IIIFPresentation/API/Infrastructure/Helpers/ETagManager.cs +++ b/src/IIIFPresentation/API/Infrastructure/Helpers/ETagManager.cs @@ -5,7 +5,7 @@ namespace API.Infrastructure.Helpers; public class ETagManager(IAppCache appCache, ILogger logger) : IETagManager { - MemoryCacheEntryOptions options = new MemoryCacheEntryOptions().SetSize(1); + private readonly MemoryCacheEntryOptions options = new MemoryCacheEntryOptions().SetSize(1); public bool TryGetETag(string id, out string? eTag) { @@ -25,10 +25,4 @@ public void UpsertETag(string id, string etag) { appCache.Add(id, etag, options); } -} - -public interface IETagManager -{ - bool TryGetETag(string id, out string? eTag); - void UpsertETag(string id, string eTag); } \ No newline at end of file diff --git a/src/IIIFPresentation/API/Infrastructure/Helpers/IETagManager.cs b/src/IIIFPresentation/API/Infrastructure/Helpers/IETagManager.cs new file mode 100644 index 00000000..3dc45b77 --- /dev/null +++ b/src/IIIFPresentation/API/Infrastructure/Helpers/IETagManager.cs @@ -0,0 +1,7 @@ +namespace API.Infrastructure.Helpers; + +public interface IETagManager +{ + bool TryGetETag(string id, out string? eTag); + void UpsertETag(string id, string eTag); +} \ No newline at end of file From c0c08e1b48ec26185fa3bbacced96da2431f1d9e Mon Sep 17 00:00:00 2001 From: "jack.lewis" Date: Tue, 24 Sep 2024 11:55:30 +0100 Subject: [PATCH 5/7] change wording slightly --- .../API/Features/Storage/Requests/UpsertCollection.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/IIIFPresentation/API/Features/Storage/Requests/UpsertCollection.cs b/src/IIIFPresentation/API/Features/Storage/Requests/UpsertCollection.cs index c1d3392a..9122af1c 100644 --- a/src/IIIFPresentation/API/Features/Storage/Requests/UpsertCollection.cs +++ b/src/IIIFPresentation/API/Features/Storage/Requests/UpsertCollection.cs @@ -54,7 +54,7 @@ public async Task> Handle(UpsertCollection re if (request.ETag is not null) { return ModifyEntityResult.Failure( - "ETag should not be added when inserting collection via PUT", WriteResult.PreConditionFailed); + "ETag should not be added when inserting a collection via PUT", WriteResult.PreConditionFailed); } var createdDate = DateTime.UtcNow; From 2f199360711cd8e5a5ddffc4071fd2c54b39f1e1 Mon Sep 17 00:00:00 2001 From: "jack.lewis" Date: Tue, 24 Sep 2024 15:43:11 +0100 Subject: [PATCH 6/7] Setting correct cache timeout --- src/IIIFPresentation/API/Attributes/EtagCachingAttribute.cs | 4 ++-- .../API/Infrastructure/Helpers/ETagManager.cs | 2 ++ .../API/Infrastructure/Helpers/IETagManager.cs | 2 ++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/IIIFPresentation/API/Attributes/EtagCachingAttribute.cs b/src/IIIFPresentation/API/Attributes/EtagCachingAttribute.cs index da43c781..ec319665 100644 --- a/src/IIIFPresentation/API/Attributes/EtagCachingAttribute.cs +++ b/src/IIIFPresentation/API/Attributes/EtagCachingAttribute.cs @@ -30,7 +30,7 @@ ResultExecutionDelegate next { var request = context.HttpContext.Request; var response = context.HttpContext.Response; - var eTagManager = context.HttpContext.RequestServices.GetService(); + var eTagManager = context.HttpContext.RequestServices.GetService()!; // For more info on this technique, see https://stackoverflow.com/a/65901913 and https://www.madskristensen.net/blog/send-etag-headers-in-aspnet-core/ and https://gist.github.com/madskristensen/36357b1df9ddbfd123162cd4201124c4 var originalStream = response.Body; @@ -47,7 +47,7 @@ ResultExecutionDelegate next responseHeaders.CacheControl = new CacheControlHeaderValue() // how long clients should cache the response { Public = request.HasShowExtraHeader(), - MaxAge = TimeSpan.FromDays(365) + MaxAge = TimeSpan.FromSeconds(eTagManager.CacheTimeoutSeconds) }; if (IsEtagSupported(response)) diff --git a/src/IIIFPresentation/API/Infrastructure/Helpers/ETagManager.cs b/src/IIIFPresentation/API/Infrastructure/Helpers/ETagManager.cs index f83205e2..d9cd8a63 100644 --- a/src/IIIFPresentation/API/Infrastructure/Helpers/ETagManager.cs +++ b/src/IIIFPresentation/API/Infrastructure/Helpers/ETagManager.cs @@ -6,6 +6,8 @@ namespace API.Infrastructure.Helpers; public class ETagManager(IAppCache appCache, ILogger logger) : IETagManager { private readonly MemoryCacheEntryOptions options = new MemoryCacheEntryOptions().SetSize(1); + + public int CacheTimeoutSeconds { get; } = appCache.DefaultCachePolicy.DefaultCacheDurationSeconds; public bool TryGetETag(string id, out string? eTag) { diff --git a/src/IIIFPresentation/API/Infrastructure/Helpers/IETagManager.cs b/src/IIIFPresentation/API/Infrastructure/Helpers/IETagManager.cs index 3dc45b77..952adb1b 100644 --- a/src/IIIFPresentation/API/Infrastructure/Helpers/IETagManager.cs +++ b/src/IIIFPresentation/API/Infrastructure/Helpers/IETagManager.cs @@ -2,6 +2,8 @@ namespace API.Infrastructure.Helpers; public interface IETagManager { + public int CacheTimeoutSeconds { get; } + bool TryGetETag(string id, out string? eTag); void UpsertETag(string id, string eTag); } \ No newline at end of file From b44be1fa3c48cc633d853fd4fcaeca0a200b8a0b Mon Sep 17 00:00:00 2001 From: "jack.lewis" Date: Wed, 25 Sep 2024 11:18:48 +0100 Subject: [PATCH 7/7] code review comments --- .../Integration/ModifyCollectionTests.cs | 2 +- .../API/Attributes/EtagCachingAttribute.cs | 20 ++++++++--------- src/IIIFPresentation/Core/WriteResult.cs | 22 +++++++++---------- 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/IIIFPresentation/API.Tests/Integration/ModifyCollectionTests.cs b/src/IIIFPresentation/API.Tests/Integration/ModifyCollectionTests.cs index 8905557a..9ca69064 100644 --- a/src/IIIFPresentation/API.Tests/Integration/ModifyCollectionTests.cs +++ b/src/IIIFPresentation/API.Tests/Integration/ModifyCollectionTests.cs @@ -292,7 +292,7 @@ public async Task UpdateCollection_UpdatesCollection_WhenAllValuesProvided() responseCollection.View.Id.Should().Contain("?page=1&pageSize=20"); } - [Fact] + [Fact] public async Task UpdateCollection_CreatesCollection_WhenUnknownCollectionIdProvided() { // Arrange diff --git a/src/IIIFPresentation/API/Attributes/EtagCachingAttribute.cs b/src/IIIFPresentation/API/Attributes/EtagCachingAttribute.cs index ec319665..87d2b806 100644 --- a/src/IIIFPresentation/API/Attributes/EtagCachingAttribute.cs +++ b/src/IIIFPresentation/API/Attributes/EtagCachingAttribute.cs @@ -66,7 +66,9 @@ ResultExecutionDelegate next // Remove all unnecessary headers while only keeping the ones that should be included in a `304` response. foreach (var header in response.Headers) if (!HeadersToKeepFor304.Contains(header.Key)) - response.Headers.Remove(header.Key); + { + response.Headers.Remove(header.Key); + } return; } @@ -80,11 +82,9 @@ await memoryStream private static bool IsEtagSupported(HttpResponse response) { // 20kb length limit - can be changed - if (response.Body.Length > 20 * 1024) - return false; + if (response.Body.Length > 20 * 1024) return false; - if (response.Headers.ContainsKey(HeaderNames.ETag)) - return false; + if (response.Headers.ContainsKey(HeaderNames.ETag)) return false; return true; } @@ -95,12 +95,10 @@ private static EntityTagHeaderValue GenerateETag(Stream stream, string path, IET stream.Position = 0; var hashString = Convert.ToBase64String(hashBytes); - var enityTagHeader = - new EntityTagHeaderValue('"' + hashString + - '"'); + var entityTagHeader = new EntityTagHeaderValue($"\"{hashString}\""); - eTagManager.UpsertETag(path, enityTagHeader.Tag.ToString()); - return enityTagHeader; + eTagManager.UpsertETag(path, entityTagHeader.Tag.ToString()); + return entityTagHeader; } private static bool IsClientCacheValid(RequestHeaders reqHeaders, ResponseHeaders resHeaders) @@ -113,7 +111,9 @@ private static bool IsClientCacheValid(RequestHeaders reqHeaders, ResponseHeader ); if (reqHeaders.IfModifiedSince is not null && resHeaders.LastModified is not null) + { return reqHeaders.IfModifiedSince >= resHeaders.LastModified; + } return false; } diff --git a/src/IIIFPresentation/Core/WriteResult.cs b/src/IIIFPresentation/Core/WriteResult.cs index f67e131c..6b877d6e 100644 --- a/src/IIIFPresentation/Core/WriteResult.cs +++ b/src/IIIFPresentation/Core/WriteResult.cs @@ -1,57 +1,57 @@ namespace Core; /// -/// Represents the result of a Create or Update operation +/// Represents the result of a Create or Update operation /// public enum WriteResult { /// - /// Default state - likely operation has yet to be run. + /// Default state - likely operation has yet to be run. /// Unknown, /// - /// Source item not found + /// Source item not found /// NotFound, /// - /// An error occurred handling update + /// An error occurred handling update /// Error, /// - /// The update values would have resulted in a conflict with an existing resource + /// The update values would have resulted in a conflict with an existing resource /// Conflict, /// - /// Request failed validation + /// Request failed validation /// FailedValidation, /// - /// Entity was successfully updated + /// Entity was successfully updated /// Updated, /// - /// Entity was successfully created + /// Entity was successfully created /// Created, /// - /// Entity had an invalid request + /// Entity had an invalid request /// BadRequest, /// - /// Predefined storage limits exceeded + /// Predefined storage limits exceeded /// StorageLimitExceeded, /// - /// Request failed a precondition + /// Request failed a precondition /// PreConditionFailed } \ No newline at end of file