From e6e59c1e72dd927bbb31fe0caae36a600738a34d Mon Sep 17 00:00:00 2001 From: Donald Gray Date: Wed, 5 Jun 2024 14:22:42 +0100 Subject: [PATCH 01/60] Ensure ImageDeliveryChannels are ordered in API results --- .../Assets/AssetQueryXTests.cs | 56 +++++++++++++++++++ .../DLCS.Repository/Assets/AssetQueryX.cs | 5 +- 2 files changed, 59 insertions(+), 2 deletions(-) create mode 100644 src/protagonist/DLCS.Repository.Tests/Assets/AssetQueryXTests.cs diff --git a/src/protagonist/DLCS.Repository.Tests/Assets/AssetQueryXTests.cs b/src/protagonist/DLCS.Repository.Tests/Assets/AssetQueryXTests.cs new file mode 100644 index 000000000..e216fa693 --- /dev/null +++ b/src/protagonist/DLCS.Repository.Tests/Assets/AssetQueryXTests.cs @@ -0,0 +1,56 @@ +using System.Collections.Generic; +using System.Linq; +using DLCS.Model.Assets; +using DLCS.Model.Policies; +using DLCS.Repository.Assets; +using Microsoft.EntityFrameworkCore; +using Test.Helpers.Data; +using Test.Helpers.Integration; + +namespace DLCS.Repository.Tests.Assets; + +[Trait("Category", "Database")] +[Collection(DatabaseCollection.CollectionName)] +public class AssetQueryXTests +{ + private readonly DlcsContext dbContext; + + public AssetQueryXTests(DlcsDatabaseFixture dbFixture) + { + dbContext = dbFixture.DbContext; + dbFixture.CleanUp(); + } + + [Fact] + public async Task IncludeDeliveryChannelsWithPolicy_ReturnsDeliveryChannels_ByOrderOfChannel() + { + var assetId = AssetIdGenerator.GetAssetId(); + await dbContext.ImageDeliveryChannels.AddRangeAsync( + new() + { + ImageId = assetId, Channel = "gamma", + DeliveryChannelPolicyId = KnownDeliveryChannelPolicies.ImageDefault + }, + new() + { + ImageId = assetId, Channel = "alpha", + DeliveryChannelPolicyId = KnownDeliveryChannelPolicies.ImageDefault + }, + new() + { + ImageId = assetId, Channel = "beta", + DeliveryChannelPolicyId = KnownDeliveryChannelPolicies.ImageDefault + }); + await dbContext.Images.AddTestAsset(assetId); + await dbContext.SaveChangesAsync(); + + // Act + var result = await dbContext.Images + .Where(i => i.Id == assetId) + .IncludeDeliveryChannelsWithPolicy() + .ToListAsync(); + + // Assert + result.Single().ImageDeliveryChannels.Should().BeInAscendingOrder(idc => idc.Channel); + } +} \ No newline at end of file diff --git a/src/protagonist/DLCS.Repository/Assets/AssetQueryX.cs b/src/protagonist/DLCS.Repository/Assets/AssetQueryX.cs index 7bf97f454..64cd06037 100644 --- a/src/protagonist/DLCS.Repository/Assets/AssetQueryX.cs +++ b/src/protagonist/DLCS.Repository/Assets/AssetQueryX.cs @@ -115,11 +115,12 @@ public static IQueryable ApplyAssetFilter(this IQueryable queryabl return filtered; } - + /// /// Include asset delivery channels and their associated policies. /// public static IQueryable IncludeDeliveryChannelsWithPolicy(this IQueryable assetQuery) - => assetQuery.Include(a => a.ImageDeliveryChannels) + => assetQuery + .Include(a => a.ImageDeliveryChannels.OrderBy(idc => idc.Channel)) .ThenInclude(dc => dc.DeliveryChannelPolicy); } \ No newline at end of file From 751a7dd8b79a71c216a698dbb1d768a0e237d751 Mon Sep 17 00:00:00 2001 From: Donald Gray Date: Wed, 5 Jun 2024 14:28:52 +0100 Subject: [PATCH 02/60] Small tweaks in API asset processing classes --- .../API/Features/Image/Ingest/AssetProcessor.cs | 5 ++--- .../Image/Ingest/DeliveryChannelProcessor.cs | 17 ++++++----------- .../DLCS.Repository/Assets/AssetQueryX.cs | 3 +-- 3 files changed, 9 insertions(+), 16 deletions(-) diff --git a/src/protagonist/API/Features/Image/Ingest/AssetProcessor.cs b/src/protagonist/API/Features/Image/Ingest/AssetProcessor.cs index e832730e1..1d7ca6d98 100644 --- a/src/protagonist/API/Features/Image/Ingest/AssetProcessor.cs +++ b/src/protagonist/API/Features/Image/Ingest/AssetProcessor.cs @@ -49,10 +49,9 @@ public async Task Process(AssetBeforeProcessing assetBeforeP Func? requiresReingestPreSave = null, CancellationToken cancellationToken = default) { - Asset? existingAsset; try { - existingAsset = await assetRepository.GetAsset(assetBeforeProcessing.Asset.Id, true); + var existingAsset = await assetRepository.GetAsset(assetBeforeProcessing.Asset.Id, true); if (existingAsset == null) { @@ -98,7 +97,7 @@ public async Task Process(AssetBeforeProcessing assetBeforeP return new ProcessAssetResult { Result = ModifyEntityResult.Failure( - "Delivery channels are required when updating an existing Asset via PUT", + "Delivery channels are required when updating an existing Asset", WriteResult.BadRequest ) }; diff --git a/src/protagonist/API/Features/Image/Ingest/DeliveryChannelProcessor.cs b/src/protagonist/API/Features/Image/Ingest/DeliveryChannelProcessor.cs index da8198c90..eb32c4648 100644 --- a/src/protagonist/API/Features/Image/Ingest/DeliveryChannelProcessor.cs +++ b/src/protagonist/API/Features/Image/Ingest/DeliveryChannelProcessor.cs @@ -45,12 +45,9 @@ public async Task ProcessImageDeliveryChannels(Asset? existingAsset, Asset deliveryChannelsBeforeProcessing, existingAsset != null); return deliveryChannelChanged; } - catch (InvalidOperationException) + catch (InvalidOperationException ioEx) { - throw new APIException("Failed to match delivery channel policy") - { - StatusCode = 400 - }; + throw new BadRequestException("Failed to match delivery channel policy", ioEx); } } @@ -160,7 +157,7 @@ private async Task SetImageDeliveryChannels(Asset asset, DeliveryChannelsB private async Task GetDeliveryChannelPolicy(Asset asset, DeliveryChannelsBeforeProcessing deliveryChannel) { DeliveryChannelPolicy deliveryChannelPolicy; - if (deliveryChannel.Policy.IsNullOrEmpty()) + if (string.IsNullOrEmpty(deliveryChannel.Policy)) { deliveryChannelPolicy = await defaultDeliveryChannelRepository.MatchDeliveryChannelPolicyForChannel( asset.MediaType!, asset.Space, asset.Customer, deliveryChannel.Channel); @@ -201,11 +198,9 @@ await defaultDeliveryChannelRepository.MatchedDeliveryChannels(asset.MediaType!, if (matchedDeliveryChannels.Any(x => x.Channel == AssetDeliveryChannels.None) && matchedDeliveryChannels.Count != 1) { - throw new APIException("An asset can only be automatically assigned a delivery channel of type 'None' when it is the only one available. " + - "Please check your default delivery channel configuration.") - { - StatusCode = 400 - }; + throw new BadRequestException( + "An asset can only be automatically assigned a delivery channel of type 'None' when it is the only one available. " + + "Please check your default delivery channel configuration."); } foreach (var deliveryChannel in matchedDeliveryChannels) diff --git a/src/protagonist/DLCS.Repository/Assets/AssetQueryX.cs b/src/protagonist/DLCS.Repository/Assets/AssetQueryX.cs index 64cd06037..f2d5986b8 100644 --- a/src/protagonist/DLCS.Repository/Assets/AssetQueryX.cs +++ b/src/protagonist/DLCS.Repository/Assets/AssetQueryX.cs @@ -26,7 +26,7 @@ public static IQueryable AsOrderedAssetQuery(this IQueryable asset /// The orderBy field can be the API version of property or the full property version. /// Defaults to "Created" field ordering if no field specified. /// - public static IQueryable AsOrderedAssetQuery(this IQueryable assetQuery, string? orderBy, + private static IQueryable AsOrderedAssetQuery(this IQueryable assetQuery, string? orderBy, bool descending = false) { var field = GetPropertyName(orderBy); @@ -57,7 +57,6 @@ private static string GetPropertyName(string? orderBy) }; } - // Create an Expression from the PropertyName. // I think Split(".") handles nested properties maybe - seems unnecessary but from an SO post // "x" means nothing when creating the Parameter, it's just used for debug messages From 76ac631b6d2b4722c3da03cf2d9c8caae1c45740 Mon Sep 17 00:00:00 2001 From: "jack.lewis" Date: Tue, 18 Jun 2024 17:18:40 +0100 Subject: [PATCH 03/60] Initial commit adding ailty to use a stickiness cookie when generating thumbs through cantaloupe --- .../Clients/CantaloupeThumbsClientTests.cs | 39 ++++++++++++++++++- .../Infrastructure/ServiceCollectionX.cs | 5 ++- .../Clients/CantaloupeThumbsClient.cs | 16 +++++++- 3 files changed, 56 insertions(+), 4 deletions(-) diff --git a/src/protagonist/Engine.Tests/Ingest/Image/ImageServer/Clients/CantaloupeThumbsClientTests.cs b/src/protagonist/Engine.Tests/Ingest/Image/ImageServer/Clients/CantaloupeThumbsClientTests.cs index 27fc69718..a18153acd 100644 --- a/src/protagonist/Engine.Tests/Ingest/Image/ImageServer/Clients/CantaloupeThumbsClientTests.cs +++ b/src/protagonist/Engine.Tests/Ingest/Image/ImageServer/Clients/CantaloupeThumbsClientTests.cs @@ -17,6 +17,7 @@ public class CantaloupeThumbsClientTests private readonly ControllableHttpMessageHandler httpHandler; private readonly CantaloupeThumbsClient sut; private readonly IImageMeasurer imageMeasurer; + private readonly HttpClient httpClient; private readonly List defaultThumbs = new() { @@ -33,7 +34,7 @@ public CantaloupeThumbsClientTests() A.CallTo(() => imageMeasurer.MeasureImage(A._, A._)).Returns(new ImageOnDisk()); - var httpClient = new HttpClient(httpHandler); + httpClient = new HttpClient(httpHandler); httpClient.BaseAddress = new Uri("http://image-processor/"); sut = new CantaloupeThumbsClient(httpClient, fileSystem, imageMeasurer, new NullLogger()); } @@ -299,6 +300,42 @@ public async Task GenerateThumbnails_ReturnsThumbForSuccessfulResponse_AfterFirs "Landscape images - invalid sizes altered", }, }; + + [Fact] + public async Task GenerateThumbnails_UpdatesHandlerWithCookies() + { + // Arrange + var assetId = new AssetId(2, 1, nameof(GenerateThumbnails_ReturnsThumbForSuccessfulResponse)); + var context = IngestionContextFactory.GetIngestionContext(assetId: assetId.ToString()); + + var response = new HttpResponseMessage(HttpStatusCode.OK); + response.Headers.Add("Set-Cookie", new List() + { + "AWSALBAPP-0=_remove_; Expires=Tue, 25 Jun 2024 10:56:45 GMT; Path=/", + "AWSALBAPP-1=_remove_; Expires=Tue, 25 Jun 2024 10:56:45 GMT; Path=/", + "AWSALBAPP-2=_remove_; Expires=Tue, 25 Jun 2024 10:56:45 GMT; Path=/" + }); + httpHandler.SetResponse(response); + context.Asset.Width = 2000; + context.Asset.Height = 2000; + + context.WithLocation(new ImageLocation + { + S3 = "//some/location/with/s3" + }); + + // Act + await sut.GenerateThumbnails(context, defaultThumbs, ThumbsRoot); + + var cookies = httpClient.DefaultRequestHeaders.GetCookies(); + + // Assert + cookies.Count.Should().Be(3); + cookies[0].Cookies[0].Name.Should().Be("AWSALBAPP-0"); + cookies[0].Cookies[0].Value.Should().Be("_remove_"); + cookies[1].Cookies[0].Name.Should().Be("AWSALBAPP-1"); + cookies[2].Cookies[0].Name.Should().Be("AWSALBAPP-2"); + } public class ImageOnDiskResults { diff --git a/src/protagonist/Engine/Infrastructure/ServiceCollectionX.cs b/src/protagonist/Engine/Infrastructure/ServiceCollectionX.cs index c8edfe3e8..647b3889c 100644 --- a/src/protagonist/Engine/Infrastructure/ServiceCollectionX.cs +++ b/src/protagonist/Engine/Infrastructure/ServiceCollectionX.cs @@ -1,3 +1,4 @@ +using System.Net; using DLCS.AWS.Configuration; using DLCS.AWS.ElasticTranscoder; using DLCS.AWS.S3; @@ -102,13 +103,13 @@ public static IServiceCollection AddAssetIngestion(this IServiceCollection servi services.AddTransient(); services.AddScoped() .AddScoped(); - + services.AddHttpClient(client => { client.BaseAddress = engineSettings.ImageIngest.ImageProcessorUrl; client.Timeout = TimeSpan.FromMilliseconds(engineSettings.ImageIngest.ImageProcessorTimeoutMs); }).AddHttpMessageHandler(); - + services.AddHttpClient(client => { client.BaseAddress = engineSettings.ImageIngest.ThumbsProcessorUrl; diff --git a/src/protagonist/Engine/Ingest/Image/ImageServer/Clients/CantaloupeThumbsClient.cs b/src/protagonist/Engine/Ingest/Image/ImageServer/Clients/CantaloupeThumbsClient.cs index fb85599ef..6042358c7 100644 --- a/src/protagonist/Engine/Ingest/Image/ImageServer/Clients/CantaloupeThumbsClient.cs +++ b/src/protagonist/Engine/Ingest/Image/ImageServer/Clients/CantaloupeThumbsClient.cs @@ -17,7 +17,7 @@ public class CantaloupeThumbsClient : IThumbsClient private readonly IFileSystem fileSystem; private readonly IImageMeasurer imageMeasurer; private readonly ILogger logger; - + public CantaloupeThumbsClient( HttpClient cantaloupeClient, IFileSystem fileSystem, @@ -66,6 +66,8 @@ public async Task> GenerateThumbnails(IngestionContext context using var response = await cantaloupeClient.GetAsync( $"iiif/3/{convertedS3Location}/full/{size}/0/default.jpg", cancellationToken); + + AttemptToAddStickinessCookie(response); if (response.StatusCode == HttpStatusCode.BadRequest) { @@ -84,6 +86,18 @@ await cantaloupeClient.GetAsync( throw new HttpException(response.StatusCode, "failed to retrieve data from the thumbs processor"); } + private void AttemptToAddStickinessCookie(HttpResponseMessage response) + { + if (!cantaloupeClient.DefaultRequestHeaders.Contains("Cookie")) + { + var hasCookie = response.Headers.TryGetValues("Set-Cookie", out var cookies); + if (hasCookie) + { + cantaloupeClient.DefaultRequestHeaders.Add("Cookie", cookies!); + } + } + } + private async Task SaveImageToDisk(HttpResponseMessage response, string size, string thumbFolder, int count, bool shouldRetry, string convertedS3Location, AssetId assetId, List thumbsResponse, Size imageSize, CancellationToken cancellationToken) From e8c9ebfdaf690aebc27ebd800058c586e38cc55c Mon Sep 17 00:00:00 2001 From: "jack.lewis" Date: Tue, 18 Jun 2024 17:28:57 +0100 Subject: [PATCH 04/60] remove unnecessary using --- src/protagonist/Engine/Infrastructure/ServiceCollectionX.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/protagonist/Engine/Infrastructure/ServiceCollectionX.cs b/src/protagonist/Engine/Infrastructure/ServiceCollectionX.cs index 647b3889c..01c7db149 100644 --- a/src/protagonist/Engine/Infrastructure/ServiceCollectionX.cs +++ b/src/protagonist/Engine/Infrastructure/ServiceCollectionX.cs @@ -1,4 +1,3 @@ -using System.Net; using DLCS.AWS.Configuration; using DLCS.AWS.ElasticTranscoder; using DLCS.AWS.S3; From 9323a1020077280f94068ac22665fd0904874431 Mon Sep 17 00:00:00 2001 From: "jack.lewis" Date: Wed, 19 Jun 2024 10:44:16 +0100 Subject: [PATCH 05/60] adding additional test --- .../Clients/CantaloupeThumbsClientTests.cs | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/protagonist/Engine.Tests/Ingest/Image/ImageServer/Clients/CantaloupeThumbsClientTests.cs b/src/protagonist/Engine.Tests/Ingest/Image/ImageServer/Clients/CantaloupeThumbsClientTests.cs index a18153acd..7225efe0a 100644 --- a/src/protagonist/Engine.Tests/Ingest/Image/ImageServer/Clients/CantaloupeThumbsClientTests.cs +++ b/src/protagonist/Engine.Tests/Ingest/Image/ImageServer/Clients/CantaloupeThumbsClientTests.cs @@ -337,6 +337,31 @@ public async Task GenerateThumbnails_UpdatesHandlerWithCookies() cookies[2].Cookies[0].Name.Should().Be("AWSALBAPP-2"); } + [Fact] + public async Task GenerateThumbnails_UpdatesHandlerWithNoCookiesSet() + { + // Arrange + var assetId = new AssetId(2, 1, nameof(GenerateThumbnails_ReturnsThumbForSuccessfulResponse)); + var context = IngestionContextFactory.GetIngestionContext(assetId: assetId.ToString()); + + httpHandler.SetResponse(new HttpResponseMessage(HttpStatusCode.OK)); + context.Asset.Width = 2000; + context.Asset.Height = 2000; + + context.WithLocation(new ImageLocation + { + S3 = "//some/location/with/s3" + }); + + // Act + await sut.GenerateThumbnails(context, defaultThumbs, ThumbsRoot); + + var cookies = httpClient.DefaultRequestHeaders.GetCookies(); + + // Assert + cookies.Count.Should().Be(0); + } + public class ImageOnDiskResults { public ImageOnDisk ReturnedFromImageServer { get; } From 51d646576362ae0f529488acf75557d0cfbd1c1c Mon Sep 17 00:00:00 2001 From: "jack.lewis" Date: Thu, 20 Jun 2024 10:40:07 +0100 Subject: [PATCH 06/60] removing expired from test to avoid issues after cookie expires --- .../ImageServer/Clients/CantaloupeThumbsClientTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/protagonist/Engine.Tests/Ingest/Image/ImageServer/Clients/CantaloupeThumbsClientTests.cs b/src/protagonist/Engine.Tests/Ingest/Image/ImageServer/Clients/CantaloupeThumbsClientTests.cs index 7225efe0a..3cab0416b 100644 --- a/src/protagonist/Engine.Tests/Ingest/Image/ImageServer/Clients/CantaloupeThumbsClientTests.cs +++ b/src/protagonist/Engine.Tests/Ingest/Image/ImageServer/Clients/CantaloupeThumbsClientTests.cs @@ -311,9 +311,9 @@ public async Task GenerateThumbnails_UpdatesHandlerWithCookies() var response = new HttpResponseMessage(HttpStatusCode.OK); response.Headers.Add("Set-Cookie", new List() { - "AWSALBAPP-0=_remove_; Expires=Tue, 25 Jun 2024 10:56:45 GMT; Path=/", - "AWSALBAPP-1=_remove_; Expires=Tue, 25 Jun 2024 10:56:45 GMT; Path=/", - "AWSALBAPP-2=_remove_; Expires=Tue, 25 Jun 2024 10:56:45 GMT; Path=/" + "AWSALBAPP-0=_remove_; Path=/", + "AWSALBAPP-1=_remove_; Path=/", + "AWSALBAPP-2=_remove_; Path=/" }); httpHandler.SetResponse(response); context.Asset.Width = 2000; From 5223e920d9b16761360b78fe45333e7710eef59f Mon Sep 17 00:00:00 2001 From: Donald Gray Date: Mon, 24 Jun 2024 15:57:50 +0100 Subject: [PATCH 07/60] Do not include ImageService3 on v2 manifests --- .../Integration/ManifestHandlingTests.cs | 23 ++++++++++++++ .../Integration/NamedQueryTests.cs | 20 ++++++++++++- .../Infrastructure/IIIF/IIIFCanvasFactory.cs | 30 ++++++++++++------- 3 files changed, 61 insertions(+), 12 deletions(-) diff --git a/src/protagonist/Orchestrator.Tests/Integration/ManifestHandlingTests.cs b/src/protagonist/Orchestrator.Tests/Integration/ManifestHandlingTests.cs index a68ff7546..729e02479 100644 --- a/src/protagonist/Orchestrator.Tests/Integration/ManifestHandlingTests.cs +++ b/src/protagonist/Orchestrator.Tests/Integration/ManifestHandlingTests.cs @@ -237,6 +237,29 @@ public async Task Get_ManifestForImage_ReturnsManifest() response.Headers.CacheControl.MaxAge.Should().BeGreaterThan(TimeSpan.FromSeconds(2)); } + [Fact] + public async Task Get_ManifestForImage_V2_ReturnsManifest_WithoutIIIFImage3Services() + { + // Arrange + var id = AssetIdGenerator.GetAssetId(); + await dbFixture.DbContext.Images.AddTestAsset(id, origin: "testorigin", imageDeliveryChannels: imageDeliveryChannels); + await dbFixture.DbContext.SaveChangesAsync(); + + var path = $"iiif-manifest/v2/{id}"; + + // Act + var response = await httpClient.GetAsync(path); + + // Assert + var json = await response.Content.ReadAsStringAsync(); + json.Should().NotContain("ImageService3"); + + response.StatusCode.Should().Be(HttpStatusCode.OK); + response.Headers.Should().ContainKey("x-asset-id").WhoseValue.Should().ContainSingle(id.ToString()); + response.Headers.CacheControl.Public.Should().BeTrue(); + response.Headers.CacheControl.MaxAge.Should().BeGreaterThan(TimeSpan.FromSeconds(2)); + } + [Fact] public async Task Get_V2ManifestForImage_ReturnsManifest_FromMetadata() { diff --git a/src/protagonist/Orchestrator.Tests/Integration/NamedQueryTests.cs b/src/protagonist/Orchestrator.Tests/Integration/NamedQueryTests.cs index 083ae50cb..f2b67b68e 100644 --- a/src/protagonist/Orchestrator.Tests/Integration/NamedQueryTests.cs +++ b/src/protagonist/Orchestrator.Tests/Integration/NamedQueryTests.cs @@ -1,5 +1,4 @@ using System; -using System.Collections.Generic; using System.Linq; using System.Net; using System.Net.Http; @@ -195,6 +194,25 @@ public async Task Get_ReturnsV2Manifest_WithCorrectId_IgnoringQueryParam() jsonResponse["@id"].ToString().Should().Be($"http://localhost/{path}"); } + [Fact] + public async Task Get_ReturnsV2Manifest_WithoutImageService3Services() + { + // Arrange + const string path = "iiif-resource/v2/99/test-named-query/my-ref/1"; + const string iiif2 = "application/ld+json; profile=\"http://iiif.io/api/presentation/2/context.json\""; + + // Act + var response = await httpClient.GetAsync($"{path}?foo=bar"); + + // Assert + response.StatusCode.Should().Be(HttpStatusCode.OK); + response.Headers.Vary.Should().Contain("Accept"); + response.Content.Headers.ContentType.ToString().Should().Be(iiif2); + + var json = await response.Content.ReadAsStringAsync(); + json.Should().NotContain("ImageService3"); + } + [Fact] public async Task Get_ReturnsV3ManifestWithCorrectCount_ViaConneg() { diff --git a/src/protagonist/Orchestrator/Infrastructure/IIIF/IIIFCanvasFactory.cs b/src/protagonist/Orchestrator/Infrastructure/IIIF/IIIFCanvasFactory.cs index c1eed0191..f06c46c73 100644 --- a/src/protagonist/Orchestrator/Infrastructure/IIIF/IIIFCanvasFactory.cs +++ b/src/protagonist/Orchestrator/Infrastructure/IIIF/IIIFCanvasFactory.cs @@ -85,7 +85,7 @@ public IIIFCanvasFactory( Format = "image/jpeg", Width = thumbnailSizes.MaxDerivativeSize.Width, Height = thumbnailSizes.MaxDerivativeSize.Height, - Service = GetImageServices(asset, customerPathElement, authProbeServices) + Service = GetImageServices(asset, customerPathElement, false, authProbeServices) } : null, }.AsListOf() @@ -98,7 +98,7 @@ public IIIFCanvasFactory( { Id = GetFullQualifiedThumbPath(asset, customerPathElement, thumbnailSizes.OpenThumbnails), Format = "image/jpeg", - Service = GetImageServiceForThumbnail(asset, customerPathElement, + Service = GetImageServiceForThumbnail(asset, customerPathElement, false, thumbnailSizes.OpenThumbnails) }.AsListOf(); } @@ -147,7 +147,7 @@ public IIIFCanvasFactory( thumbnailSizes.MaxDerivativeSize, false), Width = thumbnailSizes.MaxDerivativeSize.Width, Height = thumbnailSizes.MaxDerivativeSize.Height, - Service = GetImageServices(asset, customerPathElement, null) + Service = GetImageServices(asset, customerPathElement, true, null) } : null, }.AsList() @@ -158,7 +158,8 @@ public IIIFCanvasFactory( canvas.Thumbnail = new IIIF2.Thumbnail { Id = GetFullQualifiedThumbPath(asset, customerPathElement, thumbnailSizes.OpenThumbnails), - Service = GetImageServiceForThumbnail(asset, customerPathElement, thumbnailSizes.OpenThumbnails) + Service = GetImageServiceForThumbnail(asset, customerPathElement, true, + thumbnailSizes.OpenThumbnails) }.AsList(); } @@ -183,10 +184,10 @@ public IIIFCanvasFactory( }; } - private List GetImageServiceForThumbnail(Asset asset, CustomerPathElement customerPathElement, - List thumbnailSizes) + private List GetImageServiceForThumbnail(Asset asset, CustomerPathElement customerPathElement, + bool forPresentation2, List thumbnailSizes) { - var services = new List(2); + var services = new List(); if (orchestratorSettings.ImageServerConfig.VersionPathTemplates.ContainsKey(ImageApi.Version.V2)) { services.Add(new ImageService2 @@ -198,6 +199,9 @@ private List GetImageServiceForThumbnail(Asset asset, CustomerPathElem }); } + // NOTE - we never include ImageService3 on Presentation2 manifests + if (forPresentation2) return services; + if (orchestratorSettings.ImageServerConfig.VersionPathTemplates.ContainsKey(ImageApi.Version.V3)) { services.Add(new ImageService3 @@ -259,13 +263,14 @@ private string GetFullyQualifiedId(Asset asset, CustomerPathElement customerPath return assetPathGenerator.GetFullPathForRequest(imageRequest, true, false); } - private List GetImageServices(Asset asset, CustomerPathElement customerPathElement, + private List GetImageServices(Asset asset, CustomerPathElement customerPathElement, bool forPresentation2, Dictionary? authProbeServices) { var noAuthServices = authProbeServices.IsNullOrEmpty(); + var versionPathTemplates = orchestratorSettings.ImageServerConfig.VersionPathTemplates; - var services = new List(2); - if (orchestratorSettings.ImageServerConfig.VersionPathTemplates.ContainsKey(ImageApi.Version.V2)) + var services = new List(); + if (versionPathTemplates.ContainsKey(ImageApi.Version.V2)) { services.Add(new ImageService2 { @@ -278,7 +283,10 @@ private List GetImageServices(Asset asset, CustomerPathElement custome }); } - if (orchestratorSettings.ImageServerConfig.VersionPathTemplates.ContainsKey(ImageApi.Version.V3)) + // NOTE - we never include ImageService3 on Presentation2 manifests + if (forPresentation2) return services; + + if (versionPathTemplates.ContainsKey(ImageApi.Version.V3)) { services.Add(new ImageService3 { From ee3f39d09ddb2fc33684550c7a788495fbc4298b Mon Sep 17 00:00:00 2001 From: Donald Gray Date: Wed, 26 Jun 2024 14:56:14 +0100 Subject: [PATCH 08/60] Allow batch with empty 'members' in legacy mode --- .../Integration/CustomerQueueTests.cs | 44 +++++++++++++++++++ .../API/Converters/LegacyModeConverter.cs | 8 +++- .../Queues/CustomerQueueController.cs | 18 +++++++- .../Queues/Requests/CreateEmptyBatch.cs | 35 +++++++++++++++ .../Collections/CollectionXTests.cs | 24 ++++++++++ .../DLCS.Core/Collections/CollectionX.cs | 7 +++ 6 files changed, 133 insertions(+), 3 deletions(-) create mode 100644 src/protagonist/API/Features/Queues/Requests/CreateEmptyBatch.cs diff --git a/src/protagonist/API.Tests/Integration/CustomerQueueTests.cs b/src/protagonist/API.Tests/Integration/CustomerQueueTests.cs index a0e50f472..c4d7577cf 100644 --- a/src/protagonist/API.Tests/Integration/CustomerQueueTests.cs +++ b/src/protagonist/API.Tests/Integration/CustomerQueueTests.cs @@ -558,6 +558,30 @@ public async Task Post_CreateBatch_201_IfLegacyModeEnabled() response.StatusCode.Should().Be(HttpStatusCode.Created); } + [Fact] + public async Task Post_CreateBatch_201_IfLegacyModeEnabled_MembersEmpty() + { + // Arrange + var hydraImageBody = @"{ + ""@context"": ""http://www.w3.org/ns/hydra/context.jsonld"", + ""@type"": ""Collection"", + ""member"": [] +}"; + + var content = new StringContent(hydraImageBody, Encoding.UTF8, "application/json"); + var path = $"/customers/{LegacyModeHelpers.LegacyCustomer}/queue"; + + // Act + var response = await httpClient.AsCustomer(LegacyModeHelpers.LegacyCustomer).PostAsync(path, content); + + // Assert + response.StatusCode.Should().Be(HttpStatusCode.Created); + + var model = await response.ReadAsHydraResponseAsync(); + var dbBatch = dbContext.Batches.Single(a => a.Id == model.Id.GetLastPathElementAsInt()); + dbBatch.Count.Should().Be(0); + } + [Fact] public async Task Post_CreateBatch_201_IfLegacyModeEnabledWithAtIdFieldSet() { @@ -698,6 +722,26 @@ public async Task Post_CreateBatch_400_WithInvalidIdsSet() response.StatusCode.Should().Be(HttpStatusCode.BadRequest); } + [Fact] + public async Task Post_CreateBatch_400_MembersEmpty() + { + // Arrange + var hydraImageBody = @"{ + ""@context"": ""http://www.w3.org/ns/hydra/context.jsonld"", + ""@type"": ""Collection"", + ""member"": [] +}"; + + var content = new StringContent(hydraImageBody, Encoding.UTF8, "application/json"); + const string path = "/customers/99/queue"; + + // Act + var response = await httpClient.AsCustomer(99).PostAsync(path, content); + + // Assert + response.StatusCode.Should().Be(HttpStatusCode.BadRequest); + } + [Fact] public async Task Post_CreateBatch_400_IfSpaceNotFound() { diff --git a/src/protagonist/API/Converters/LegacyModeConverter.cs b/src/protagonist/API/Converters/LegacyModeConverter.cs index 2542d0047..4c71e7a74 100644 --- a/src/protagonist/API/Converters/LegacyModeConverter.cs +++ b/src/protagonist/API/Converters/LegacyModeConverter.cs @@ -4,6 +4,7 @@ using DLCS.HydraModel; using DLCS.Model.Assets; using Hydra; +using Microsoft.Extensions.Logging; using AssetFamily = DLCS.HydraModel.AssetFamily; namespace API.Converters; @@ -14,13 +15,16 @@ namespace API.Converters; public static class LegacyModeConverter { private const string DefaultMediaType = "image/unknown"; + + internal static void LogLegacyUsage(this ILogger logger, string message, params object?[] args) + => logger.LogWarning("LEGACY USE:" + message, args); /// /// Converts from legacy format to new format /// /// The image to convert should be emulated and translated into delivery channels /// A converted image - public static T VerifyAndConvertToModernFormat(T image) + public static T VerifyAndConvertToModernFormat(T image, ILogger? logger = null) where T : Image { if (image.Origin.IsNullOrEmpty()) @@ -30,6 +34,7 @@ public static T VerifyAndConvertToModernFormat(T image) if (image.MediaType.IsNullOrEmpty()) { + logger?.LogLegacyUsage("Null or empty media type"); var contentType = image.Origin?.Split('.').Last() ?? string.Empty; image.MediaType = MIMEHelper.GetContentTypeForExtension(contentType) ?? DefaultMediaType; @@ -43,6 +48,7 @@ public static T VerifyAndConvertToModernFormat(T image) if (image.MaxUnauthorised is null or 0 && image.Roles.IsNullOrEmpty()) { + logger?.LogLegacyUsage("MaxUnauthorised"); image.MaxUnauthorised = -1; } diff --git a/src/protagonist/API/Features/Queues/CustomerQueueController.cs b/src/protagonist/API/Features/Queues/CustomerQueueController.cs index dda627229..89e588165 100644 --- a/src/protagonist/API/Features/Queues/CustomerQueueController.cs +++ b/src/protagonist/API/Features/Queues/CustomerQueueController.cs @@ -1,5 +1,4 @@ using System.Collections.Generic; -using System.Net; using API.Converters; using API.Exceptions; using API.Features.Image; @@ -8,6 +7,7 @@ using API.Features.Queues.Validation; using API.Infrastructure; using API.Settings; +using DLCS.Core.Collections; using DLCS.Core.Strings; using DLCS.HydraModel; using DLCS.Model.Assets; @@ -16,6 +16,7 @@ using MediatR; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Batch = DLCS.HydraModel.Batch; @@ -28,8 +29,12 @@ namespace API.Features.Queues; [ApiController] public class CustomerQueueController : HydraController { - public CustomerQueueController(IOptions settings, IMediator mediator) : base(settings.Value, mediator) + private readonly ILogger logger; + + public CustomerQueueController(IOptions settings, IMediator mediator, + ILogger logger) : base(settings.Value, mediator) { + this.logger = logger; } /// @@ -331,6 +336,15 @@ private async Task CreateBatchInternal(int customerId, HydraColle try { UpdateMembers(customerId, images.Members); + + if (images.Members.IsEmpty() && Settings.LegacyModeEnabledForCustomer(customerId)) + { + logger.LogLegacyUsage("Empty batch received for customer {CustomerId}", customerId); + return await HandleUpsert(new CreateEmptyBatch(customerId), + batch => batch.ToHydra(GetUrlRoots().BaseUrl), + errorTitle: "Create batch failed", + cancellationToken: cancellationToken); + } } catch (APIException apiEx) { diff --git a/src/protagonist/API/Features/Queues/Requests/CreateEmptyBatch.cs b/src/protagonist/API/Features/Queues/Requests/CreateEmptyBatch.cs new file mode 100644 index 000000000..bf66ff479 --- /dev/null +++ b/src/protagonist/API/Features/Queues/Requests/CreateEmptyBatch.cs @@ -0,0 +1,35 @@ +using API.Infrastructure.Requests; +using DLCS.Core; +using DLCS.Model.Assets; +using MediatR; + +namespace API.Features.Queues.Requests; + +/// +/// Handler that creates an empty batch of 0 images +/// +public class CreateEmptyBatch : IRequest> +{ + public int CustomerId { get; } + + public CreateEmptyBatch(int customerId) + { + CustomerId = customerId; + } +} + +public class CreateEmptyBatchHandler : IRequestHandler> +{ + private readonly IBatchRepository batchRepository; + + public CreateEmptyBatchHandler(IBatchRepository batchRepository) + { + this.batchRepository = batchRepository; + } + + public async Task> Handle(CreateEmptyBatch request, CancellationToken cancellationToken) + { + var batch = await batchRepository.CreateBatch(request.CustomerId, Array.Empty(), cancellationToken); + return ModifyEntityResult.Success(batch, WriteResult.Created); + } +} \ No newline at end of file diff --git a/src/protagonist/DLCS.Core.Tests/Collections/CollectionXTests.cs b/src/protagonist/DLCS.Core.Tests/Collections/CollectionXTests.cs index 0319e6e7a..7979325d6 100644 --- a/src/protagonist/DLCS.Core.Tests/Collections/CollectionXTests.cs +++ b/src/protagonist/DLCS.Core.Tests/Collections/CollectionXTests.cs @@ -54,6 +54,30 @@ public void IsNullOrEmpty_List_False_IfHasValues() coll.IsNullOrEmpty().Should().BeFalse(); } + + [Fact] + public void IsEmpty_List_False_IfNull() + { + List coll = null; + + coll.IsEmpty().Should().BeFalse(); + } + + [Fact] + public void IsEmpty_List_True_IfEmpty() + { + var coll = new List(); + + coll.IsEmpty().Should().BeTrue(); + } + + [Fact] + public void IsEmpty_List_False_IfHasValues() + { + var coll = new List {2}; + + coll.IsEmpty().Should().BeFalse(); + } [Fact] public void AsList_ReturnsExpected() diff --git a/src/protagonist/DLCS.Core/Collections/CollectionX.cs b/src/protagonist/DLCS.Core/Collections/CollectionX.cs index 74c897f3e..c64126e3b 100644 --- a/src/protagonist/DLCS.Core/Collections/CollectionX.cs +++ b/src/protagonist/DLCS.Core/Collections/CollectionX.cs @@ -20,6 +20,13 @@ public static bool IsNullOrEmpty([NotNullWhen(false)] this IEnumerable? co /// true if null or empty, else false public static bool IsNullOrEmpty([NotNullWhen(false)] this IList? collection) => collection == null || collection.Count == 0; + + /// + /// Check if IList is empty - this explicitly checks for Empty only, list could still be null + /// + /// true if empty, else false + public static bool IsEmpty(this IList? collection) + => collection?.Count == 0; /// /// Check if list contains single specified item From 5748fa1fe9f331ad109733b14d0dcb553a7413b0 Mon Sep 17 00:00:00 2001 From: "jack.lewis" Date: Wed, 26 Jun 2024 17:37:11 +0100 Subject: [PATCH 09/60] iniitial commit adding support for optional named queries + tightening some values --- .../Validation/HydraNamedQueryValidator.cs | 3 ++ .../Parsing/IIIFNamedQueryParserTests.cs | 20 ++++++++-- .../Parsing/PdfNamedQueryParserTests.cs | 20 ++++++++-- .../Parsing/BaseNamedQueryParser.cs | 39 ++++++++++++------- .../Parsing/PdfNamedQueryParser.cs | 4 +- .../Parsing/StoredNamedQueryParser.cs | 2 +- 6 files changed, 64 insertions(+), 24 deletions(-) diff --git a/src/protagonist/API/Features/NamedQueries/Validation/HydraNamedQueryValidator.cs b/src/protagonist/API/Features/NamedQueries/Validation/HydraNamedQueryValidator.cs index 370fb5b47..a4aa1d22f 100644 --- a/src/protagonist/API/Features/NamedQueries/Validation/HydraNamedQueryValidator.cs +++ b/src/protagonist/API/Features/NamedQueries/Validation/HydraNamedQueryValidator.cs @@ -15,6 +15,9 @@ public HydraNamedQueryValidator() RuleFor(nq => nq.Template) .NotEmpty() .WithMessage("A template is required"); + RuleFor(nq => nq.Template) + .Must(n => n.Contains('=')) + .WithMessage("named query requires at least 1 parameter"); RuleSet("create", () => { RuleFor(nq => nq.Name) diff --git a/src/protagonist/DLCS.Repository.Tests/NamedQueries/Parsing/IIIFNamedQueryParserTests.cs b/src/protagonist/DLCS.Repository.Tests/NamedQueries/Parsing/IIIFNamedQueryParserTests.cs index 6f85b33b6..1706e7b41 100644 --- a/src/protagonist/DLCS.Repository.Tests/NamedQueries/Parsing/IIIFNamedQueryParserTests.cs +++ b/src/protagonist/DLCS.Repository.Tests/NamedQueries/Parsing/IIIFNamedQueryParserTests.cs @@ -33,10 +33,24 @@ public void GenerateParsedNamedQueryFromRequest_Throws_IfTemplateEmptyOrWhiteSpa } [Theory] - [InlineData("space=p1", "")] [InlineData("space=p1&s1=p2", "1")] [InlineData("space=p1&s1=p2&#=1", "")] - public void GenerateParsedNamedQueryFromRequest_ReturnsFaultParsedNQ_IfTooFewParamsPassed(string template, + public void GenerateParsedNamedQueryFromRequest_ReturnTrueNQ_IfTooFewParamsPassed(string template, + string args) + { + // Act + var result = + sut.GenerateParsedNamedQueryFromRequest(Customer, args, template, "my-query"); + + // Assert + result.IsFaulty.Should().BeFalse(); + } + + [Theory] + [InlineData("space=p1", "")] + [InlineData("s1=p1&space=p2", "")] + [InlineData("s1=p1&space=p2", "1")] + public void GenerateParsedNamedQueryFromRequest_ReturnFaultyNQ_IfSpaceNotPassedCorrectlyPassed(string template, string args) { // Act @@ -45,7 +59,7 @@ public void GenerateParsedNamedQueryFromRequest_ReturnsFaultParsedNQ_IfTooFewPar // Assert result.IsFaulty.Should().BeTrue(); - result.ErrorMessage.Should().StartWith("Not enough query arguments to satisfy template element parameter"); + result.ErrorMessage.Should().StartWith("The key \"space\" cannot be a null element"); } [Theory] diff --git a/src/protagonist/DLCS.Repository.Tests/NamedQueries/Parsing/PdfNamedQueryParserTests.cs b/src/protagonist/DLCS.Repository.Tests/NamedQueries/Parsing/PdfNamedQueryParserTests.cs index fbb5251b7..0c38681a0 100644 --- a/src/protagonist/DLCS.Repository.Tests/NamedQueries/Parsing/PdfNamedQueryParserTests.cs +++ b/src/protagonist/DLCS.Repository.Tests/NamedQueries/Parsing/PdfNamedQueryParserTests.cs @@ -35,10 +35,24 @@ public void GenerateParsedNamedQueryFromRequest_Throws_IfTemplateEmptyOrWhiteSpa } [Theory] - [InlineData("space=p1", "")] [InlineData("space=p1&s1=p2", "1")] [InlineData("space=p1&s1=p2&#=1", "")] - public void GenerateParsedNamedQueryFromRequest_ReturnsFaultParsedNQ_IfTooFewParamsPassed(string template, + public void GenerateParsedNamedQueryFromRequest_ReturnsParsedNQ_IfTooFewParamsPassed(string template, + string args) + { + // Act + var result = + sut.GenerateParsedNamedQueryFromRequest(99, args, template, "my-query"); + + // Assert + result.IsFaulty.Should().BeFalse(); + } + + [Theory] + [InlineData("space=p1", "")] + [InlineData("space=p1&s1=p2", "")] + [InlineData("s1=p1&space=p2", "1")] + public void GenerateParsedNamedQueryFromRequest_ReturnFaultyNQ_IfSpaceNotPassedCorrectlyPassed(string template, string args) { // Act @@ -47,7 +61,7 @@ public void GenerateParsedNamedQueryFromRequest_ReturnsFaultParsedNQ_IfTooFewPar // Assert result.IsFaulty.Should().BeTrue(); - result.ErrorMessage.Should().StartWith("Not enough query arguments to satisfy template element parameter"); + result.ErrorMessage.Should().StartWith("The key \"space\" cannot be a null element"); } [Theory] diff --git a/src/protagonist/DLCS.Repository/NamedQueries/Parsing/BaseNamedQueryParser.cs b/src/protagonist/DLCS.Repository/NamedQueries/Parsing/BaseNamedQueryParser.cs index 3f507844f..7dbf74807 100644 --- a/src/protagonist/DLCS.Repository/NamedQueries/Parsing/BaseNamedQueryParser.cs +++ b/src/protagonist/DLCS.Repository/NamedQueries/Parsing/BaseNamedQueryParser.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; +using Amazon.CloudFront.Model; using DLCS.Core.Collections; using DLCS.Core.Guard; using DLCS.Model.Assets.NamedQueries; @@ -36,6 +37,11 @@ public abstract class BaseNamedQueryParser : INamedQueryParser protected const string AssetOrdering = "assetOrder"; protected const string PathReplacement = "%2F"; + private readonly List cannotBeNullOptions = new List() + { + Space + }; + public BaseNamedQueryParser(ILogger logger) { Logger = logger; @@ -91,7 +97,7 @@ private static List GetQueryArgsList(string? namedQueryArgs, string[] te private T GenerateParsedNamedQuery(int customerId, string[] templatePairing, List queryArgs) { var assetQuery = GenerateParsedQueryObject(customerId); - + // Iterate through all of the pairs and generate the NQ model try { @@ -107,31 +113,28 @@ private T GenerateParsedNamedQuery(int customerId, string[] templatePairing, Lis assetQuery.AssetOrdering = GetAssetOrderingFromTemplateElement(elements[1]); break; case Space: - assetQuery.Space = int.Parse(GetQueryArgumentFromTemplateElement(queryArgs, elements[1])); + assetQuery.Space = int.Parse(GetQueryArgumentFromTemplateElement(queryArgs, elements[0], elements[1])!); break; case SpaceName: - assetQuery.SpaceName = GetQueryArgumentFromTemplateElement(queryArgs, elements[1]); + assetQuery.SpaceName = GetQueryArgumentFromTemplateElement(queryArgs, elements[0], elements[1]); break; case String1: - assetQuery.String1 = GetQueryArgumentFromTemplateElement(queryArgs, elements[1]); + assetQuery.String1 = GetQueryArgumentFromTemplateElement(queryArgs, elements[0], elements[1]); break; case String2: - assetQuery.String2 = GetQueryArgumentFromTemplateElement(queryArgs, elements[1]); + assetQuery.String2 = GetQueryArgumentFromTemplateElement(queryArgs, elements[0], elements[1]); break; case String3: - assetQuery.String3 = GetQueryArgumentFromTemplateElement(queryArgs, elements[1]); + assetQuery.String3 = GetQueryArgumentFromTemplateElement(queryArgs, elements[0], elements[1]); break; case Number1: - assetQuery.Number1 = - long.Parse(GetQueryArgumentFromTemplateElement(queryArgs, elements[1])); + assetQuery.Number1 = long.Parse(GetQueryArgumentFromTemplateElement(queryArgs, elements[0], elements[1])); break; case Number2: - assetQuery.Number2 = - long.Parse(GetQueryArgumentFromTemplateElement(queryArgs, elements[1])); + assetQuery.Number2 = long.Parse(GetQueryArgumentFromTemplateElement(queryArgs, elements[0], elements[1])); break; case Number3: - assetQuery.Number3 = - long.Parse(GetQueryArgumentFromTemplateElement(queryArgs, elements[1])); + assetQuery.Number3 = long.Parse(GetQueryArgumentFromTemplateElement(queryArgs, elements[0], elements[1])); break; } @@ -162,7 +165,7 @@ private T GenerateParsedNamedQuery(int customerId, string[] templatePairing, Lis /// Could use Activator.CreateInstance this avoids using reflection protected abstract T GenerateParsedQueryObject(int customerId); - protected string GetQueryArgumentFromTemplateElement(List args, string element) + protected string? GetQueryArgumentFromTemplateElement(List args, string key, string element) { // Arg will be in format p1, p2, p3 etc. Get the index, then extract that element from args list if (!element.StartsWith(ParameterPrefix) || element.Length <= 1) @@ -178,8 +181,14 @@ protected string GetQueryArgumentFromTemplateElement(List args, string e return args[argNumber - 1].Replace(PathReplacement, "/", StringComparison.OrdinalIgnoreCase); } - throw new ArgumentOutOfRangeException(element, - "Not enough query arguments to satisfy template element parameter"); + if (cannotBeNullOptions.Contains(key)) + { + throw new ArgumentException( + $"The key \"{key}\" cannot be a null element"); + } + + // parameter out of range of supplied arguments, assumed to be an optional param to the NQ + return null; } throw new ArgumentException($"Could not parse template element parameter '{element}'", element); diff --git a/src/protagonist/DLCS.Repository/NamedQueries/Parsing/PdfNamedQueryParser.cs b/src/protagonist/DLCS.Repository/NamedQueries/Parsing/PdfNamedQueryParser.cs index ef2b6bd62..63d352f3f 100644 --- a/src/protagonist/DLCS.Repository/NamedQueries/Parsing/PdfNamedQueryParser.cs +++ b/src/protagonist/DLCS.Repository/NamedQueries/Parsing/PdfNamedQueryParser.cs @@ -28,10 +28,10 @@ protected override void CustomHandling(List queryArgs, string key, strin switch (key) { case CoverPage: - assetQuery.CoverPageFormat = GetQueryArgumentFromTemplateElement(queryArgs, value); + assetQuery.CoverPageFormat = GetQueryArgumentFromTemplateElement(queryArgs, key, value); break; case RedactedMessage: - assetQuery.RedactedMessage = GetQueryArgumentFromTemplateElement(queryArgs, value); + assetQuery.RedactedMessage = GetQueryArgumentFromTemplateElement(queryArgs, key, value); break; } } diff --git a/src/protagonist/DLCS.Repository/NamedQueries/Parsing/StoredNamedQueryParser.cs b/src/protagonist/DLCS.Repository/NamedQueries/Parsing/StoredNamedQueryParser.cs index 8f83abc88..7fc2c01b2 100644 --- a/src/protagonist/DLCS.Repository/NamedQueries/Parsing/StoredNamedQueryParser.cs +++ b/src/protagonist/DLCS.Repository/NamedQueries/Parsing/StoredNamedQueryParser.cs @@ -30,7 +30,7 @@ protected override void CustomHandling(List queryArgs, string key, strin switch (key) { case ObjectName: - assetQuery.ObjectNameFormat = GetQueryArgumentFromTemplateElement(queryArgs, value); + assetQuery.ObjectNameFormat = GetQueryArgumentFromTemplateElement(queryArgs, key, value); break; } } From 612ae9c13acae6e31ef45f5369e78ebccbfa0a0e Mon Sep 17 00:00:00 2001 From: "jack.lewis" Date: Thu, 27 Jun 2024 11:19:00 +0100 Subject: [PATCH 10/60] getting named queries to work core4ctly with empty values and removing limits on space --- .../Validation/HydraNamedQueryValidator.cs | 3 -- .../Parsing/IIIFNamedQueryParserTests.cs | 23 +++++++--- .../Parsing/PdfNamedQueryParserTests.cs | 23 +++++----- .../Parsing/BaseNamedQueryParser.cs | 44 ++++++++++++------- 4 files changed, 57 insertions(+), 36 deletions(-) diff --git a/src/protagonist/API/Features/NamedQueries/Validation/HydraNamedQueryValidator.cs b/src/protagonist/API/Features/NamedQueries/Validation/HydraNamedQueryValidator.cs index a4aa1d22f..370fb5b47 100644 --- a/src/protagonist/API/Features/NamedQueries/Validation/HydraNamedQueryValidator.cs +++ b/src/protagonist/API/Features/NamedQueries/Validation/HydraNamedQueryValidator.cs @@ -15,9 +15,6 @@ public HydraNamedQueryValidator() RuleFor(nq => nq.Template) .NotEmpty() .WithMessage("A template is required"); - RuleFor(nq => nq.Template) - .Must(n => n.Contains('=')) - .WithMessage("named query requires at least 1 parameter"); RuleSet("create", () => { RuleFor(nq => nq.Name) diff --git a/src/protagonist/DLCS.Repository.Tests/NamedQueries/Parsing/IIIFNamedQueryParserTests.cs b/src/protagonist/DLCS.Repository.Tests/NamedQueries/Parsing/IIIFNamedQueryParserTests.cs index 1706e7b41..b440af26f 100644 --- a/src/protagonist/DLCS.Repository.Tests/NamedQueries/Parsing/IIIFNamedQueryParserTests.cs +++ b/src/protagonist/DLCS.Repository.Tests/NamedQueries/Parsing/IIIFNamedQueryParserTests.cs @@ -46,11 +46,25 @@ public void GenerateParsedNamedQueryFromRequest_ReturnTrueNQ_IfTooFewParamsPasse result.IsFaulty.Should().BeFalse(); } + [Fact] + public void GenerateParsedNamedQueryFromRequest_ReturnFaultyNQ_IfNoParametersPassed() + { + // Act + var result = + sut.GenerateParsedNamedQueryFromRequest(Customer, "", "s1=p1&space=p2", "my-query"); + + // Assert + result.IsFaulty.Should().BeTrue(); + result.ErrorMessage.Should().StartWith("Named query must have at least 1 argument"); + } + [Theory] - [InlineData("space=p1", "")] - [InlineData("s1=p1&space=p2", "")] [InlineData("s1=p1&space=p2", "1")] - public void GenerateParsedNamedQueryFromRequest_ReturnFaultyNQ_IfSpaceNotPassedCorrectlyPassed(string template, + [InlineData("s1=p1&n1=p2", "1")] + [InlineData("s1=p1&n1=&n2=p2", "1/2")] + [InlineData("space=p1&s1=p2&#=1", "")] + [InlineData("space=p1&s1=p2&#=1", "10")] + public void GenerateParsedNamedQueryFromRequest_ReturnsNQ_IfLessQueriesPassedThanParameters(string template, string args) { // Act @@ -58,8 +72,7 @@ public void GenerateParsedNamedQueryFromRequest_ReturnFaultyNQ_IfSpaceNotPassedC sut.GenerateParsedNamedQueryFromRequest(Customer, args, template, "my-query"); // Assert - result.IsFaulty.Should().BeTrue(); - result.ErrorMessage.Should().StartWith("The key \"space\" cannot be a null element"); + result.IsFaulty.Should().BeFalse(); } [Theory] diff --git a/src/protagonist/DLCS.Repository.Tests/NamedQueries/Parsing/PdfNamedQueryParserTests.cs b/src/protagonist/DLCS.Repository.Tests/NamedQueries/Parsing/PdfNamedQueryParserTests.cs index 0c38681a0..e3488efc9 100644 --- a/src/protagonist/DLCS.Repository.Tests/NamedQueries/Parsing/PdfNamedQueryParserTests.cs +++ b/src/protagonist/DLCS.Repository.Tests/NamedQueries/Parsing/PdfNamedQueryParserTests.cs @@ -34,25 +34,25 @@ public void GenerateParsedNamedQueryFromRequest_Throws_IfTemplateEmptyOrWhiteSpa .WithMessage("Value cannot be null. (Parameter 'namedQueryTemplate')"); } - [Theory] - [InlineData("space=p1&s1=p2", "1")] - [InlineData("space=p1&s1=p2&#=1", "")] - public void GenerateParsedNamedQueryFromRequest_ReturnsParsedNQ_IfTooFewParamsPassed(string template, - string args) + [Fact] + public void GenerateParsedNamedQueryFromRequest_ReturnFaultyNQ_IfNoParametersPassed() { // Act var result = - sut.GenerateParsedNamedQueryFromRequest(99, args, template, "my-query"); + sut.GenerateParsedNamedQueryFromRequest(99, "", "s1=p1&space=p2", "my-query"); // Assert - result.IsFaulty.Should().BeFalse(); + result.IsFaulty.Should().BeTrue(); + result.ErrorMessage.Should().StartWith("Named query must have at least 1 argument"); } [Theory] - [InlineData("space=p1", "")] - [InlineData("space=p1&s1=p2", "")] [InlineData("s1=p1&space=p2", "1")] - public void GenerateParsedNamedQueryFromRequest_ReturnFaultyNQ_IfSpaceNotPassedCorrectlyPassed(string template, + [InlineData("s1=p1&n1=p2", "1")] + [InlineData("s1=p1&n1=&n2=p2", "1/2")] + [InlineData("space=p1&s1=p2&#=1", "")] + [InlineData("space=p1&s1=p2&#=1", "10")] + public void GenerateParsedNamedQueryFromRequest_ReturnsNQ_IfLessQueriesPassedThanParameters(string template, string args) { // Act @@ -60,8 +60,7 @@ public void GenerateParsedNamedQueryFromRequest_ReturnFaultyNQ_IfSpaceNotPassedC sut.GenerateParsedNamedQueryFromRequest(99, args, template, "my-query"); // Assert - result.IsFaulty.Should().BeTrue(); - result.ErrorMessage.Should().StartWith("The key \"space\" cannot be a null element"); + result.IsFaulty.Should().BeFalse(); } [Theory] diff --git a/src/protagonist/DLCS.Repository/NamedQueries/Parsing/BaseNamedQueryParser.cs b/src/protagonist/DLCS.Repository/NamedQueries/Parsing/BaseNamedQueryParser.cs index 7dbf74807..9bf879fad 100644 --- a/src/protagonist/DLCS.Repository/NamedQueries/Parsing/BaseNamedQueryParser.cs +++ b/src/protagonist/DLCS.Repository/NamedQueries/Parsing/BaseNamedQueryParser.cs @@ -36,12 +36,7 @@ public abstract class BaseNamedQueryParser : INamedQueryParser protected const string String3 = "s3"; protected const string AssetOrdering = "assetOrder"; protected const string PathReplacement = "%2F"; - - private readonly List cannotBeNullOptions = new List() - { - Space - }; - + public BaseNamedQueryParser(ILogger logger) { Logger = logger; @@ -113,7 +108,9 @@ private T GenerateParsedNamedQuery(int customerId, string[] templatePairing, Lis assetQuery.AssetOrdering = GetAssetOrderingFromTemplateElement(elements[1]); break; case Space: - assetQuery.Space = int.Parse(GetQueryArgumentFromTemplateElement(queryArgs, elements[0], elements[1])!); + assetQuery.Space = + ConvertIntegerQueryArg( + GetQueryArgumentFromTemplateElement(queryArgs, elements[0], elements[1])); break; case SpaceName: assetQuery.SpaceName = GetQueryArgumentFromTemplateElement(queryArgs, elements[0], elements[1]); @@ -128,13 +125,19 @@ private T GenerateParsedNamedQuery(int customerId, string[] templatePairing, Lis assetQuery.String3 = GetQueryArgumentFromTemplateElement(queryArgs, elements[0], elements[1]); break; case Number1: - assetQuery.Number1 = long.Parse(GetQueryArgumentFromTemplateElement(queryArgs, elements[0], elements[1])); + assetQuery.Number1 = + ConvertIntegerQueryArg( + GetQueryArgumentFromTemplateElement(queryArgs, elements[0], elements[1])); break; case Number2: - assetQuery.Number2 = long.Parse(GetQueryArgumentFromTemplateElement(queryArgs, elements[0], elements[1])); + assetQuery.Number2 = + ConvertIntegerQueryArg( + GetQueryArgumentFromTemplateElement(queryArgs, elements[0], elements[1])); break; case Number3: - assetQuery.Number3 = long.Parse(GetQueryArgumentFromTemplateElement(queryArgs, elements[0], elements[1])); + assetQuery.Number3 = + ConvertIntegerQueryArg( + GetQueryArgumentFromTemplateElement(queryArgs, elements[0], elements[1])); break; } @@ -150,6 +153,16 @@ private T GenerateParsedNamedQuery(int customerId, string[] templatePairing, Lis return assetQuery; } + private int? ConvertIntegerQueryArg(string? argToConvert) + { + if (argToConvert.IsNullOrEmpty()) + { + return null; + } + + return int.Parse(argToConvert); + } + /// /// Adds handling for any custom key/value pairs, in addition to the core s1, s2, p1 etc /// @@ -173,6 +186,11 @@ private T GenerateParsedNamedQuery(int customerId, string[] templatePairing, Lis // default to just return the element as a literal return element; } + + if (args.Count == 0) + { + throw new ArgumentException("Named query must have at least 1 argument"); + } if (int.TryParse(element[1..], out int argNumber)) { @@ -181,12 +199,6 @@ private T GenerateParsedNamedQuery(int customerId, string[] templatePairing, Lis return args[argNumber - 1].Replace(PathReplacement, "/", StringComparison.OrdinalIgnoreCase); } - if (cannotBeNullOptions.Contains(key)) - { - throw new ArgumentException( - $"The key \"{key}\" cannot be a null element"); - } - // parameter out of range of supplied arguments, assumed to be an optional param to the NQ return null; } From 5ec5593545ec832e4fa1ce3a728fff440b186f98 Mon Sep 17 00:00:00 2001 From: "jack.lewis" Date: Thu, 27 Jun 2024 12:10:32 +0100 Subject: [PATCH 11/60] update to remove unneeded test + method parameter + fixing orchestrator tests --- .../Integration/CustomerResourcesTests.cs | 15 +++++- .../Parsing/IIIFNamedQueryParserTests.cs | 14 ----- .../Parsing/BaseNamedQueryParser.cs | 24 ++++----- .../Parsing/PdfNamedQueryParser.cs | 4 +- .../Parsing/StoredNamedQueryParser.cs | 2 +- .../Integration/NamedQueryTests.cs | 21 ++++++-- .../Integration/PdfTests.cs | 39 +++++++++++--- .../Integration/ZipTests.cs | 52 ++++++++----------- 8 files changed, 99 insertions(+), 72 deletions(-) diff --git a/src/protagonist/API.Tests/Integration/CustomerResourcesTests.cs b/src/protagonist/API.Tests/Integration/CustomerResourcesTests.cs index 4ad297929..ac48b8570 100644 --- a/src/protagonist/API.Tests/Integration/CustomerResourcesTests.cs +++ b/src/protagonist/API.Tests/Integration/CustomerResourcesTests.cs @@ -54,7 +54,7 @@ public async Task Delete_PDF_Returns400_IfUnableToFind() } [Fact] - public async Task Delete_PDF_Returns400_IfArgsIncorrect() + public async Task Delete_PDF_Returns200_IfLessArgsThanQuery() { // Arrange var path = "/customers/99/resources/pdf/cust-resource?args=too-little"; @@ -62,6 +62,19 @@ public async Task Delete_PDF_Returns400_IfArgsIncorrect() // Act var response = await httpClient.AsCustomer().DeleteAsync(path); + // Assert + response.StatusCode.Should().Be(HttpStatusCode.OK); + } + + [Fact] + public async Task Delete_PDF_Returns400_IfNoArgs() + { + // Arrange + var path = "/customers/99/resources/pdf/cust-resource"; + + // Act + var response = await httpClient.AsCustomer().DeleteAsync(path); + // Assert response.StatusCode.Should().Be(HttpStatusCode.BadRequest); } diff --git a/src/protagonist/DLCS.Repository.Tests/NamedQueries/Parsing/IIIFNamedQueryParserTests.cs b/src/protagonist/DLCS.Repository.Tests/NamedQueries/Parsing/IIIFNamedQueryParserTests.cs index b440af26f..c5b750ad8 100644 --- a/src/protagonist/DLCS.Repository.Tests/NamedQueries/Parsing/IIIFNamedQueryParserTests.cs +++ b/src/protagonist/DLCS.Repository.Tests/NamedQueries/Parsing/IIIFNamedQueryParserTests.cs @@ -32,20 +32,6 @@ public void GenerateParsedNamedQueryFromRequest_Throws_IfTemplateEmptyOrWhiteSpa .WithMessage("Value cannot be null. (Parameter 'namedQueryTemplate')"); } - [Theory] - [InlineData("space=p1&s1=p2", "1")] - [InlineData("space=p1&s1=p2&#=1", "")] - public void GenerateParsedNamedQueryFromRequest_ReturnTrueNQ_IfTooFewParamsPassed(string template, - string args) - { - // Act - var result = - sut.GenerateParsedNamedQueryFromRequest(Customer, args, template, "my-query"); - - // Assert - result.IsFaulty.Should().BeFalse(); - } - [Fact] public void GenerateParsedNamedQueryFromRequest_ReturnFaultyNQ_IfNoParametersPassed() { diff --git a/src/protagonist/DLCS.Repository/NamedQueries/Parsing/BaseNamedQueryParser.cs b/src/protagonist/DLCS.Repository/NamedQueries/Parsing/BaseNamedQueryParser.cs index 9bf879fad..8866ac0fb 100644 --- a/src/protagonist/DLCS.Repository/NamedQueries/Parsing/BaseNamedQueryParser.cs +++ b/src/protagonist/DLCS.Repository/NamedQueries/Parsing/BaseNamedQueryParser.cs @@ -109,35 +109,31 @@ private T GenerateParsedNamedQuery(int customerId, string[] templatePairing, Lis break; case Space: assetQuery.Space = - ConvertIntegerQueryArg( - GetQueryArgumentFromTemplateElement(queryArgs, elements[0], elements[1])); + ConvertIntegerQueryArg(GetQueryArgumentFromTemplateElement(queryArgs, elements[1])); break; case SpaceName: - assetQuery.SpaceName = GetQueryArgumentFromTemplateElement(queryArgs, elements[0], elements[1]); + assetQuery.SpaceName = GetQueryArgumentFromTemplateElement(queryArgs, elements[1]); break; case String1: - assetQuery.String1 = GetQueryArgumentFromTemplateElement(queryArgs, elements[0], elements[1]); + assetQuery.String1 = GetQueryArgumentFromTemplateElement(queryArgs, elements[1]); break; case String2: - assetQuery.String2 = GetQueryArgumentFromTemplateElement(queryArgs, elements[0], elements[1]); + assetQuery.String2 = GetQueryArgumentFromTemplateElement(queryArgs, elements[1]); break; case String3: - assetQuery.String3 = GetQueryArgumentFromTemplateElement(queryArgs, elements[0], elements[1]); + assetQuery.String3 = GetQueryArgumentFromTemplateElement(queryArgs, elements[1]); break; case Number1: - assetQuery.Number1 = - ConvertIntegerQueryArg( - GetQueryArgumentFromTemplateElement(queryArgs, elements[0], elements[1])); + assetQuery.Number1 = + ConvertIntegerQueryArg(GetQueryArgumentFromTemplateElement(queryArgs, elements[1])); break; case Number2: assetQuery.Number2 = - ConvertIntegerQueryArg( - GetQueryArgumentFromTemplateElement(queryArgs, elements[0], elements[1])); + ConvertIntegerQueryArg(GetQueryArgumentFromTemplateElement(queryArgs, elements[1])); break; case Number3: assetQuery.Number3 = - ConvertIntegerQueryArg( - GetQueryArgumentFromTemplateElement(queryArgs, elements[0], elements[1])); + ConvertIntegerQueryArg(GetQueryArgumentFromTemplateElement(queryArgs, elements[1])); break; } @@ -178,7 +174,7 @@ private T GenerateParsedNamedQuery(int customerId, string[] templatePairing, Lis /// Could use Activator.CreateInstance this avoids using reflection protected abstract T GenerateParsedQueryObject(int customerId); - protected string? GetQueryArgumentFromTemplateElement(List args, string key, string element) + protected string? GetQueryArgumentFromTemplateElement(List args, string element) { // Arg will be in format p1, p2, p3 etc. Get the index, then extract that element from args list if (!element.StartsWith(ParameterPrefix) || element.Length <= 1) diff --git a/src/protagonist/DLCS.Repository/NamedQueries/Parsing/PdfNamedQueryParser.cs b/src/protagonist/DLCS.Repository/NamedQueries/Parsing/PdfNamedQueryParser.cs index 63d352f3f..ef2b6bd62 100644 --- a/src/protagonist/DLCS.Repository/NamedQueries/Parsing/PdfNamedQueryParser.cs +++ b/src/protagonist/DLCS.Repository/NamedQueries/Parsing/PdfNamedQueryParser.cs @@ -28,10 +28,10 @@ protected override void CustomHandling(List queryArgs, string key, strin switch (key) { case CoverPage: - assetQuery.CoverPageFormat = GetQueryArgumentFromTemplateElement(queryArgs, key, value); + assetQuery.CoverPageFormat = GetQueryArgumentFromTemplateElement(queryArgs, value); break; case RedactedMessage: - assetQuery.RedactedMessage = GetQueryArgumentFromTemplateElement(queryArgs, key, value); + assetQuery.RedactedMessage = GetQueryArgumentFromTemplateElement(queryArgs, value); break; } } diff --git a/src/protagonist/DLCS.Repository/NamedQueries/Parsing/StoredNamedQueryParser.cs b/src/protagonist/DLCS.Repository/NamedQueries/Parsing/StoredNamedQueryParser.cs index 7fc2c01b2..8f83abc88 100644 --- a/src/protagonist/DLCS.Repository/NamedQueries/Parsing/StoredNamedQueryParser.cs +++ b/src/protagonist/DLCS.Repository/NamedQueries/Parsing/StoredNamedQueryParser.cs @@ -30,7 +30,7 @@ protected override void CustomHandling(List queryArgs, string key, strin switch (key) { case ObjectName: - assetQuery.ObjectNameFormat = GetQueryArgumentFromTemplateElement(queryArgs, key, value); + assetQuery.ObjectNameFormat = GetQueryArgumentFromTemplateElement(queryArgs, value); break; } } diff --git a/src/protagonist/Orchestrator.Tests/Integration/NamedQueryTests.cs b/src/protagonist/Orchestrator.Tests/Integration/NamedQueryTests.cs index f2b67b68e..196d8d5af 100644 --- a/src/protagonist/Orchestrator.Tests/Integration/NamedQueryTests.cs +++ b/src/protagonist/Orchestrator.Tests/Integration/NamedQueryTests.cs @@ -116,10 +116,23 @@ public async Task Get_Returns404_IfCustomerNotFound(string path) } [Theory] - [InlineData("iiif-resource/99/test-named-query/too-little-params")] - [InlineData("iiif-resource/v2/99/test-named-query/too-little-params")] - [InlineData("iiif-resource/v3/99/test-named-query/too-little-params")] - public async Task Get_Returns400_IfNamedQueryParametersIncorrect(string path) + [InlineData("iiif-resource/99/test-named-query/my-ref")] + [InlineData("iiif-resource/v2/99/test-named-query/my-ref")] + [InlineData("iiif-resource/v3/99/test-named-query/my-ref")] + public async Task Get_Returns200_IfNamedQueryParametersLessThanMax(string path) + { + // Act + var response = await httpClient.GetAsync(path); + + // Assert + response.StatusCode.Should().Be(HttpStatusCode.OK); + } + + [Theory] + [InlineData("iiif-resource/99/test-named-query")] + [InlineData("iiif-resource/v2/99/test-named-query")] + [InlineData("iiif-resource/v3/99/test-named-query")] + public async Task Get_Returns400_IfNoNamedQueryParameters(string path) { // Act var response = await httpClient.GetAsync(path); diff --git a/src/protagonist/Orchestrator.Tests/Integration/PdfTests.cs b/src/protagonist/Orchestrator.Tests/Integration/PdfTests.cs index 727fc5ffd..e960f801c 100644 --- a/src/protagonist/Orchestrator.Tests/Integration/PdfTests.cs +++ b/src/protagonist/Orchestrator.Tests/Integration/PdfTests.cs @@ -63,6 +63,8 @@ public PdfTests(ProtagonistAppFactory factory, StorageFixture orchestra maxUnauthorised: 10, roles: "clickthrough"); dbFixture.DbContext.Images.AddTestAsset(AssetId.FromString("99/1/not-for-delivery"), num1: 6, ref1: "my-ref", notForDelivery: true); + dbFixture.DbContext.Images.AddTestAsset(AssetId.FromString("99/1/limited-projection"), num1: 2, + ref1: "limited-ref"); dbFixture.DbContext.SaveChanges(); } @@ -110,10 +112,10 @@ public async Task GetPdf_Returns404_IfNQNotFound() } [Fact] - public async Task GetPdf_Returns400_IfParametersIncorrect() + public async Task GetPdf_Returns400_IfNoParameters() { // Arrange - const string path = "pdf/99/test-pdf/too-little-params"; + const string path = "pdf/99/test-pdf"; // Act var response = await httpClient.GetAsync(path); @@ -150,6 +152,28 @@ await AddPdfControlFile("99/pdf/test-pdf/my-ref/1/1/tester.json", response.StatusCode.Should().Be(HttpStatusCode.Accepted); response.Headers.Should().ContainKey("Retry-After"); } + + [Fact] + public async Task GetPdf_Returns200_IfParametersLessThanMax() + { + // Arrange + var fakePdfContent = nameof(GetPdf_Returns200_IfParametersLessThanMax); + const string path = "pdf/99/test-pdf/limited-ref"; + const string pdfStorageKey = "99/pdf/test-pdf/limited-ref/tester"; + await AddPdfControlFile("99/pdf/test-pdf/limited-ref/tester", + new ControlFile { Created = DateTime.UtcNow, InProcess = false }); + pdfCreator.AddCallbackFor(pdfStorageKey, (query, assets) => + { + AddPdf(pdfStorageKey, fakePdfContent).Wait(); + return true; + }); + + // Act + var response = await httpClient.GetAsync(path); + + // Assert + response.StatusCode.Should().Be(HttpStatusCode.OK); + } [Fact] public async Task GetPdf_Returns200_WithExistingPdf_IfPdfControlFileAndPdfExist() @@ -398,10 +422,10 @@ public async Task GetPdfControlFile_Returns404_IfNQNotFound() } [Fact] - public async Task GetPdfControlFile_Returns404_IfParametersIncorrect() + public async Task GetPdfControlFile_Returns404_IfNoParameters() { // Arrange - const string path = "pdf-control/99/test-pdf/too-little-params"; + const string path = "pdf-control/99/test-pdf"; // Act var response = await httpClient.GetAsync(path); @@ -410,11 +434,12 @@ public async Task GetPdfControlFile_Returns404_IfParametersIncorrect() response.StatusCode.Should().Be(HttpStatusCode.NotFound); } - [Fact] - public async Task GetPdfControlFile_Returns200_WithEmptyControlFile_IfNQValidButNoControlFile() + [Theory] + [InlineData("pdf-control/99/test-pdf/any-ref/1/2")] + [InlineData("pdf-control/99/test-pdf/any-ref")] + public async Task GetPdfControlFile_Returns200_WithEmptyControlFile_IfNQValidButNoControlFile(string path) { // Arrange - const string path = "pdf-control/99/test-pdf/any-ref/1/2"; var pdfControlFile = new PdfControlFile { Created = DateTime.MinValue, InProcess = false, Exists = false, Key = string.Empty, ItemCount = 0, diff --git a/src/protagonist/Orchestrator.Tests/Integration/ZipTests.cs b/src/protagonist/Orchestrator.Tests/Integration/ZipTests.cs index 468db6ef1..bceba31da 100644 --- a/src/protagonist/Orchestrator.Tests/Integration/ZipTests.cs +++ b/src/protagonist/Orchestrator.Tests/Integration/ZipTests.cs @@ -57,6 +57,8 @@ public ZipTests(ProtagonistAppFactory factory, StorageFixture orchestra dbFixture.DbContext.Images.AddTestAsset(AssetId.FromString("99/1/matching-zip-5"), num1: 5, ref1: "my-ref"); dbFixture.DbContext.Images.AddTestAsset(AssetId.FromString("99/1/not-for-delivery"), num1: 6, ref1: "my-ref", notForDelivery: true); + dbFixture.DbContext.Images.AddTestAsset(AssetId.FromString("99/1/limited-parameter-zip-1"), num1: 2, + ref1: "limited-ref"); dbFixture.DbContext.SaveChanges(); } @@ -103,19 +105,6 @@ public async Task GetZip_Returns404_IfNQNotFound() response.StatusCode.Should().Be(HttpStatusCode.NotFound); } - [Fact] - public async Task GetZip_Returns400_IfParametersIncorrect() - { - // Arrange - const string path = "zip/99/test-zip/too-little-params"; - - // Act - var response = await httpClient.GetAsync(path); - - // Assert - response.StatusCode.Should().Be(HttpStatusCode.BadRequest); - } - [Fact] public async Task GetZip_Returns404_IfNoMatchingRecordsFound() { @@ -163,6 +152,23 @@ await AddControlFile("99/zip/test-zip/my-ref/1/1/tester.zip.json", (await response.Content.ReadAsStringAsync()).Should().Be(fakeContent); response.Content.Headers.ContentType.Should().Be(new MediaTypeHeaderValue("application/zip")); } + + [Fact] + public async Task GetZip_Returns404_IfLessParametersThanTotal() + { + // Arrange + var fakeContent = nameof(GetZip_Returns404_IfLessParametersThanTotal); + const string path = "zip/99/test-zip/limited-ref"; + await AddControlFile("99/zip/test-zip/limited-ref/tester.zip.json", + new ControlFile { Created = DateTime.UtcNow, InProcess = false }); + await AddZipArchive("99/zip/test-zip/limited-ref/tester.zip", fakeContent); + + // Act + var response = await httpClient.GetAsync(path); + + // Assert + response.StatusCode.Should().Be(HttpStatusCode.OK); + } [Fact] public async Task GetZip_Returns200_WithNewlyCreatedZip_IfControlFileExistsButZipDoesnt() @@ -327,24 +333,12 @@ public async Task GetZipControlFile_Returns404_IfNQNotFound() response.StatusCode.Should().Be(HttpStatusCode.NotFound); } - [Fact] - public async Task GetZipControlFile_Returns404_IfParametersIncorrect() - { - // Arrange - const string path = "zip-control/99/test-zip/too-little-params"; - - // Act - var response = await httpClient.GetAsync(path); - - // Assert - response.StatusCode.Should().Be(HttpStatusCode.NotFound); - } - - [Fact] - public async Task GetZipControlFile_Returns200_WithEmptyControlFile_IfNQValidButNoControlFile() + [Theory] + [InlineData("zip-control/99/test-zip/any-ref/1/2")] + [InlineData("zip-control/99/test-zip/any-ref")] + public async Task GetZipControlFile_Returns200_WithEmptyControlFile_IfNQValidButNoControlFile(string path) { // Arrange - const string path = "zip-control/99/test-zip/any-ref/1/2"; var controlFileJson = JsonConvert.SerializeObject(ControlFile.Empty); // Act From 5d2137d1817bc9dc3d7ef73c425a0fa61bea28c1 Mon Sep 17 00:00:00 2001 From: "jack.lewis" Date: Thu, 27 Jun 2024 12:16:28 +0100 Subject: [PATCH 12/60] remove unneeded using + rename test to be more accurate --- .../NamedQueries/Parsing/BaseNamedQueryParser.cs | 1 - src/protagonist/Orchestrator.Tests/Integration/ZipTests.cs | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/protagonist/DLCS.Repository/NamedQueries/Parsing/BaseNamedQueryParser.cs b/src/protagonist/DLCS.Repository/NamedQueries/Parsing/BaseNamedQueryParser.cs index 8866ac0fb..e9ebc21b2 100644 --- a/src/protagonist/DLCS.Repository/NamedQueries/Parsing/BaseNamedQueryParser.cs +++ b/src/protagonist/DLCS.Repository/NamedQueries/Parsing/BaseNamedQueryParser.cs @@ -1,7 +1,6 @@ using System; using System.Collections.Generic; using System.Linq; -using Amazon.CloudFront.Model; using DLCS.Core.Collections; using DLCS.Core.Guard; using DLCS.Model.Assets.NamedQueries; diff --git a/src/protagonist/Orchestrator.Tests/Integration/ZipTests.cs b/src/protagonist/Orchestrator.Tests/Integration/ZipTests.cs index bceba31da..72e6b2c89 100644 --- a/src/protagonist/Orchestrator.Tests/Integration/ZipTests.cs +++ b/src/protagonist/Orchestrator.Tests/Integration/ZipTests.cs @@ -154,10 +154,10 @@ await AddControlFile("99/zip/test-zip/my-ref/1/1/tester.zip.json", } [Fact] - public async Task GetZip_Returns404_IfLessParametersThanTotal() + public async Task GetZip_Returns200_IfLessParametersThanTotal() { // Arrange - var fakeContent = nameof(GetZip_Returns404_IfLessParametersThanTotal); + var fakeContent = nameof(GetZip_Returns200_IfLessParametersThanTotal); const string path = "zip/99/test-zip/limited-ref"; await AddControlFile("99/zip/test-zip/limited-ref/tester.zip.json", new ControlFile { Created = DateTime.UtcNow, InProcess = false }); From a4205dc92da0a805a5fe641b03b2aa2d671d402a Mon Sep 17 00:00:00 2001 From: griffri Date: Thu, 27 Jun 2024 12:34:15 +0100 Subject: [PATCH 13/60] Omit '@Type' for ImageService in Presentation2 manifests --- .../Orchestrator/Infrastructure/IIIF/IIIFCanvasFactory.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/protagonist/Orchestrator/Infrastructure/IIIF/IIIFCanvasFactory.cs b/src/protagonist/Orchestrator/Infrastructure/IIIF/IIIFCanvasFactory.cs index f06c46c73..23cbacf91 100644 --- a/src/protagonist/Orchestrator/Infrastructure/IIIF/IIIFCanvasFactory.cs +++ b/src/protagonist/Orchestrator/Infrastructure/IIIF/IIIFCanvasFactory.cs @@ -192,6 +192,7 @@ private List GetImageServiceForThumbnail(Asset asset, CustomerPathElem { services.Add(new ImageService2 { + Type = forPresentation2 ? null : default, // Omit '@Type' if for Presentation2 Id = GetFullyQualifiedId(asset, customerPathElement, true, ImageApi.Version.V2), Profile = ImageService2.Level0Profile, Sizes = thumbnailSizes, @@ -274,6 +275,7 @@ private List GetImageServices(Asset asset, CustomerPathElement custome { services.Add(new ImageService2 { + Type = forPresentation2 ? null : default, // Omit '@Type' if for Presentation2 Id = GetFullyQualifiedId(asset, customerPathElement, false, ImageApi.Version.V2), Profile = ImageService2.Level2Profile, Context = ImageService2.Image2Context, From c6522c951d62ebec77397cf8a815f2fe9ab80ee4 Mon Sep 17 00:00:00 2001 From: griffri Date: Thu, 27 Jun 2024 12:34:35 +0100 Subject: [PATCH 14/60] Update iiif-net to 0.2.5 --- src/protagonist/DLCS.Model/DLCS.Model.csproj | 2 +- src/protagonist/DLCS.Repository/DLCS.Repository.csproj | 2 +- src/protagonist/DLCS.Web/DLCS.Web.csproj | 2 +- src/protagonist/Engine/Engine.csproj | 2 +- src/protagonist/Portal/Portal.csproj | 2 +- src/protagonist/Thumbs/Thumbs.csproj | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/protagonist/DLCS.Model/DLCS.Model.csproj b/src/protagonist/DLCS.Model/DLCS.Model.csproj index f9dffa14c..8f423dac8 100644 --- a/src/protagonist/DLCS.Model/DLCS.Model.csproj +++ b/src/protagonist/DLCS.Model/DLCS.Model.csproj @@ -11,7 +11,7 @@ - + diff --git a/src/protagonist/DLCS.Repository/DLCS.Repository.csproj b/src/protagonist/DLCS.Repository/DLCS.Repository.csproj index 8f13fc325..b850efeae 100644 --- a/src/protagonist/DLCS.Repository/DLCS.Repository.csproj +++ b/src/protagonist/DLCS.Repository/DLCS.Repository.csproj @@ -8,7 +8,7 @@ - + all diff --git a/src/protagonist/DLCS.Web/DLCS.Web.csproj b/src/protagonist/DLCS.Web/DLCS.Web.csproj index 2f84c51ca..e7205ab18 100644 --- a/src/protagonist/DLCS.Web/DLCS.Web.csproj +++ b/src/protagonist/DLCS.Web/DLCS.Web.csproj @@ -12,7 +12,7 @@ - + diff --git a/src/protagonist/Engine/Engine.csproj b/src/protagonist/Engine/Engine.csproj index b5dd83659..09779ca0d 100644 --- a/src/protagonist/Engine/Engine.csproj +++ b/src/protagonist/Engine/Engine.csproj @@ -7,7 +7,7 @@ - + diff --git a/src/protagonist/Portal/Portal.csproj b/src/protagonist/Portal/Portal.csproj index 238008543..9b53c010a 100644 --- a/src/protagonist/Portal/Portal.csproj +++ b/src/protagonist/Portal/Portal.csproj @@ -10,7 +10,7 @@ - + diff --git a/src/protagonist/Thumbs/Thumbs.csproj b/src/protagonist/Thumbs/Thumbs.csproj index 47a9ec15c..98cab05fb 100644 --- a/src/protagonist/Thumbs/Thumbs.csproj +++ b/src/protagonist/Thumbs/Thumbs.csproj @@ -10,7 +10,7 @@ - + From 8ae55ac84530f548e7f00c8aa9dc199fda99c60f Mon Sep 17 00:00:00 2001 From: griffri Date: Thu, 27 Jun 2024 12:42:22 +0100 Subject: [PATCH 15/60] Update AngleSharp to 0.17.1 for Portal.Tests and Orchestrator.Tests --- src/protagonist/Orchestrator.Tests/Orchestrator.Tests.csproj | 2 +- src/protagonist/Portal.Tests/Portal.Tests.csproj | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/protagonist/Orchestrator.Tests/Orchestrator.Tests.csproj b/src/protagonist/Orchestrator.Tests/Orchestrator.Tests.csproj index 7e3705f9b..1306bb515 100644 --- a/src/protagonist/Orchestrator.Tests/Orchestrator.Tests.csproj +++ b/src/protagonist/Orchestrator.Tests/Orchestrator.Tests.csproj @@ -7,7 +7,7 @@ - + diff --git a/src/protagonist/Portal.Tests/Portal.Tests.csproj b/src/protagonist/Portal.Tests/Portal.Tests.csproj index b5a415982..5e3974e3b 100644 --- a/src/protagonist/Portal.Tests/Portal.Tests.csproj +++ b/src/protagonist/Portal.Tests/Portal.Tests.csproj @@ -8,7 +8,7 @@ - + From e9aacf190c89cd298e5fd29d6447d81fe84dd61e Mon Sep 17 00:00:00 2001 From: griffri Date: Thu, 27 Jun 2024 13:12:59 +0100 Subject: [PATCH 16/60] Use if instead of ternary to set ImageService2 type --- .../Infrastructure/IIIF/IIIFCanvasFactory.cs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/protagonist/Orchestrator/Infrastructure/IIIF/IIIFCanvasFactory.cs b/src/protagonist/Orchestrator/Infrastructure/IIIF/IIIFCanvasFactory.cs index 23cbacf91..393ccc4e0 100644 --- a/src/protagonist/Orchestrator/Infrastructure/IIIF/IIIFCanvasFactory.cs +++ b/src/protagonist/Orchestrator/Infrastructure/IIIF/IIIFCanvasFactory.cs @@ -190,14 +190,17 @@ private List GetImageServiceForThumbnail(Asset asset, CustomerPathElem var services = new List(); if (orchestratorSettings.ImageServerConfig.VersionPathTemplates.ContainsKey(ImageApi.Version.V2)) { - services.Add(new ImageService2 + var imageService = new ImageService2 { - Type = forPresentation2 ? null : default, // Omit '@Type' if for Presentation2 Id = GetFullyQualifiedId(asset, customerPathElement, true, ImageApi.Version.V2), Profile = ImageService2.Level0Profile, Sizes = thumbnailSizes, Context = ImageService2.Image2Context, - }); + }; + + if (forPresentation2) imageService.Type = null; // '@Type' is not used in Presentation2 + + services.Add(imageService); } // NOTE - we never include ImageService3 on Presentation2 manifests @@ -273,16 +276,19 @@ private List GetImageServices(Asset asset, CustomerPathElement custome var services = new List(); if (versionPathTemplates.ContainsKey(ImageApi.Version.V2)) { - services.Add(new ImageService2 + var imageService = new ImageService2 { - Type = forPresentation2 ? null : default, // Omit '@Type' if for Presentation2 Id = GetFullyQualifiedId(asset, customerPathElement, false, ImageApi.Version.V2), Profile = ImageService2.Level2Profile, Context = ImageService2.Image2Context, Width = asset.Width ?? 0, Height = asset.Height ?? 0, Service = TryGetAuthServices(), - }); + }; + + if (forPresentation2) imageService.Type = null; // '@Type' is not used in Presentation2 + + services.Add(imageService); } // NOTE - we never include ImageService3 on Presentation2 manifests From d274977cf9a6d3e343a1bf4dce55f0a0a509796e Mon Sep 17 00:00:00 2001 From: griffri Date: Thu, 27 Jun 2024 13:54:54 +0100 Subject: [PATCH 17/60] Use 'iiif:Image' @Type in ImageService2 for info.json --- src/protagonist/DLCS.Model/Assets/InfoJsonBuilder.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/protagonist/DLCS.Model/Assets/InfoJsonBuilder.cs b/src/protagonist/DLCS.Model/Assets/InfoJsonBuilder.cs index 19d8721ce..b96fa032b 100644 --- a/src/protagonist/DLCS.Model/Assets/InfoJsonBuilder.cs +++ b/src/protagonist/DLCS.Model/Assets/InfoJsonBuilder.cs @@ -13,6 +13,8 @@ namespace DLCS.Model.Assets; /// public static class InfoJsonBuilder { + private const string ImageService2Type = "iiif:Image"; + /// /// Get level 0 info.json object for IIIF Image 2.1 /// @@ -25,7 +27,7 @@ public static ImageService2 GetImageApi2_1Level0(string serviceEndpoint, List Date: Thu, 27 Jun 2024 14:03:54 +0100 Subject: [PATCH 18/60] Update InfoJsonBuilderTests --- .../DLCS.Model.Tests/Assets/InfoJsonBuilderTests.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/protagonist/DLCS.Model.Tests/Assets/InfoJsonBuilderTests.cs b/src/protagonist/DLCS.Model.Tests/Assets/InfoJsonBuilderTests.cs index 662ac0ac1..58bf4c382 100644 --- a/src/protagonist/DLCS.Model.Tests/Assets/InfoJsonBuilderTests.cs +++ b/src/protagonist/DLCS.Model.Tests/Assets/InfoJsonBuilderTests.cs @@ -5,7 +5,7 @@ using Xunit; namespace DLCS.Model.Tests.Assets; - + public class InfoJsonBuilderTests { [Fact] @@ -15,6 +15,7 @@ public void GetImageApi2_1Level0_ReturnsExpected() var expected = @"{ ""@context"": ""http://iiif.io/api/image/2/context.json"", ""@id"": ""https://test.example.com/iiif-img/2/1/jackal"", + ""@type"": ""iiif:Image"", ""profile"": [ ""http://iiif.io/api/image/2/level0.json"", { @@ -48,6 +49,7 @@ public void GetImageApi2_1Level1_ReturnsExpected() var expected = @"{ ""@context"": ""http://iiif.io/api/image/2/context.json"", ""@id"": ""https://test.example.com/iiif-img/2/1/jackal"", + ""@type"": ""iiif:Image"", ""profile"": [ ""http://iiif.io/api/image/2/level1.json"", { From 564ee5304dc36a40512006ad6c7098c98c3eeaab Mon Sep 17 00:00:00 2001 From: "jack.lewis" Date: Thu, 27 Jun 2024 14:35:22 +0100 Subject: [PATCH 19/60] using long instead of int for converting to a number --- .../NamedQueries/Parsing/BaseNamedQueryParser.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/protagonist/DLCS.Repository/NamedQueries/Parsing/BaseNamedQueryParser.cs b/src/protagonist/DLCS.Repository/NamedQueries/Parsing/BaseNamedQueryParser.cs index e9ebc21b2..c15bc56dd 100644 --- a/src/protagonist/DLCS.Repository/NamedQueries/Parsing/BaseNamedQueryParser.cs +++ b/src/protagonist/DLCS.Repository/NamedQueries/Parsing/BaseNamedQueryParser.cs @@ -108,7 +108,7 @@ private T GenerateParsedNamedQuery(int customerId, string[] templatePairing, Lis break; case Space: assetQuery.Space = - ConvertIntegerQueryArg(GetQueryArgumentFromTemplateElement(queryArgs, elements[1])); + (int?)ConvertToLongQueryArg(GetQueryArgumentFromTemplateElement(queryArgs, elements[1])); break; case SpaceName: assetQuery.SpaceName = GetQueryArgumentFromTemplateElement(queryArgs, elements[1]); @@ -124,15 +124,15 @@ private T GenerateParsedNamedQuery(int customerId, string[] templatePairing, Lis break; case Number1: assetQuery.Number1 = - ConvertIntegerQueryArg(GetQueryArgumentFromTemplateElement(queryArgs, elements[1])); + ConvertToLongQueryArg(GetQueryArgumentFromTemplateElement(queryArgs, elements[1])); break; case Number2: assetQuery.Number2 = - ConvertIntegerQueryArg(GetQueryArgumentFromTemplateElement(queryArgs, elements[1])); + ConvertToLongQueryArg(GetQueryArgumentFromTemplateElement(queryArgs, elements[1])); break; case Number3: assetQuery.Number3 = - ConvertIntegerQueryArg(GetQueryArgumentFromTemplateElement(queryArgs, elements[1])); + ConvertToLongQueryArg(GetQueryArgumentFromTemplateElement(queryArgs, elements[1])); break; } @@ -148,14 +148,14 @@ private T GenerateParsedNamedQuery(int customerId, string[] templatePairing, Lis return assetQuery; } - private int? ConvertIntegerQueryArg(string? argToConvert) + private long? ConvertToLongQueryArg(string? argToConvert) { if (argToConvert.IsNullOrEmpty()) { return null; } - return int.Parse(argToConvert); + return long.Parse(argToConvert); } /// From 903026e4bece26a5cb2344a3b4243bfebd2b1b44 Mon Sep 17 00:00:00 2001 From: griffri Date: Thu, 27 Jun 2024 16:32:55 +0100 Subject: [PATCH 20/60] Set type of imageService2 in InfoJsonService Introduced constant for iiif:Image value --- .../DLCS.Model/Assets/InfoJsonBuilder.cs | 6 ++---- src/protagonist/DLCS.Model/IIIF/Constants.cs | 9 +++++++++ .../Integration/ImageHandlingTests.cs | 4 +++- .../Images/ImageServer/InfoJsonService.cs | 15 +++++++++++++-- 4 files changed, 27 insertions(+), 7 deletions(-) create mode 100644 src/protagonist/DLCS.Model/IIIF/Constants.cs diff --git a/src/protagonist/DLCS.Model/Assets/InfoJsonBuilder.cs b/src/protagonist/DLCS.Model/Assets/InfoJsonBuilder.cs index b96fa032b..07bec1f5b 100644 --- a/src/protagonist/DLCS.Model/Assets/InfoJsonBuilder.cs +++ b/src/protagonist/DLCS.Model/Assets/InfoJsonBuilder.cs @@ -13,8 +13,6 @@ namespace DLCS.Model.Assets; /// public static class InfoJsonBuilder { - private const string ImageService2Type = "iiif:Image"; - /// /// Get level 0 info.json object for IIIF Image 2.1 /// @@ -27,7 +25,7 @@ public static ImageService2 GetImageApi2_1Level0(string serviceEndpoint, List + /// @type value for ImageService2 for rendering as IIIF Image or Presentation v2 + /// + public const string ImageService2Type = "iiif:Image"; +} \ No newline at end of file diff --git a/src/protagonist/Orchestrator.Tests/Integration/ImageHandlingTests.cs b/src/protagonist/Orchestrator.Tests/Integration/ImageHandlingTests.cs index ffd8565b3..81e8f5148 100644 --- a/src/protagonist/Orchestrator.Tests/Integration/ImageHandlingTests.cs +++ b/src/protagonist/Orchestrator.Tests/Integration/ImageHandlingTests.cs @@ -26,6 +26,7 @@ using Orchestrator.Infrastructure.IIIF; using Orchestrator.Tests.Integration.Infrastructure; using Test.Helpers; +using Test.Helpers.Data; using Test.Helpers.Integration; using Yarp.ReverseProxy.Forwarder; using Version = IIIF.ImageApi.Version; @@ -261,7 +262,7 @@ await amazonS3.PutObjectAsync(new PutObjectRequest public async Task GetInfoJsonV2_Correct_ViaDirectPath_NotInS3() { // Arrange - var id = AssetId.FromString($"99/1/{nameof(GetInfoJsonV2_Correct_ViaDirectPath_NotInS3)}"); + var id = AssetIdGenerator.GetAssetId(); await dbFixture.DbContext.Images.AddTestAsset(id, imageDeliveryChannels: deliveryChannelsForImage); await amazonS3.PutObjectAsync(new PutObjectRequest @@ -282,6 +283,7 @@ await amazonS3.PutObjectAsync(new PutObjectRequest jsonResponse.Id.Should().Be($"http://localhost/iiif-img/v2/{id}"); jsonResponse.Context.ToString().Should().Be("http://iiif.io/api/image/2/context.json"); jsonResponse.Sizes.Should().BeEquivalentTo(expectedSizes); + jsonResponse.Type.Should().Be("iiif:Image", "ImageService2 is default on class, overridden when read"); // With correct headers/status response.StatusCode.Should().Be(HttpStatusCode.OK); diff --git a/src/protagonist/Orchestrator/Features/Images/ImageServer/InfoJsonService.cs b/src/protagonist/Orchestrator/Features/Images/ImageServer/InfoJsonService.cs index ba68d1ea7..941a0ff95 100644 --- a/src/protagonist/Orchestrator/Features/Images/ImageServer/InfoJsonService.cs +++ b/src/protagonist/Orchestrator/Features/Images/ImageServer/InfoJsonService.cs @@ -57,8 +57,9 @@ public InfoJsonService( JsonLdBase deserialisedInfoJson = version == Version.V2 ? infoJson.FromJsonStream() : infoJson.FromJsonStream(); + logger.LogTrace("Found info.json version {Version} for {AssetId}", version, orchestrationImage.AssetId); - return new InfoJsonResponse(deserialisedInfoJson, false); + return GetInfoJsonResponse(deserialisedInfoJson, false); } // If not found, build new copy @@ -69,7 +70,17 @@ public InfoJsonService( if (infoJsonResponse == null) return null; await StoreInfoJson(infoJsonKey, infoJsonResponse, cancellationToken); - return new InfoJsonResponse(infoJsonResponse, true); + return GetInfoJsonResponse(infoJsonResponse, true); + } + + private InfoJsonResponse GetInfoJsonResponse(JsonLdBase infoJsonResponse, bool wasOrchestrated) + { + if (infoJsonResponse is ImageService2 imageService2) + { + imageService2.Type = DLCS.Model.IIIF.Constants.ImageService2Type; + } + + return new InfoJsonResponse(infoJsonResponse, wasOrchestrated); } private ObjectInBucket GetInfoJsonKey(OrchestrationImage asset, Version version) From 4e6efea5e88f79f2d05d7d4febea79fdf295eb15 Mon Sep 17 00:00:00 2001 From: griffri Date: Fri, 28 Jun 2024 11:09:35 +0100 Subject: [PATCH 21/60] Add basic named query viewer --- src/protagonist/API.Client/DlcsClient.cs | 10 ++++- src/protagonist/API.Client/IDlcsClient.cs | 1 + .../Requests/GetCustomerNamedQueries.cs | 31 +++++++++++++++ src/protagonist/Portal/Pages/Index.cshtml | 9 +++++ .../Portal/Pages/NamedQueries/Index.cshtml | 38 +++++++++++++++++++ .../Portal/Pages/NamedQueries/Index.cshtml.cs | 27 +++++++++++++ .../Pages/Shared/_ControlPanelLayout.cshtml | 3 ++ 7 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 src/protagonist/Portal/Features/NamedQueries/Requests/GetCustomerNamedQueries.cs create mode 100644 src/protagonist/Portal/Pages/NamedQueries/Index.cshtml create mode 100644 src/protagonist/Portal/Pages/NamedQueries/Index.cshtml.cs diff --git a/src/protagonist/API.Client/DlcsClient.cs b/src/protagonist/API.Client/DlcsClient.cs index 1592cf518..13527b184 100644 --- a/src/protagonist/API.Client/DlcsClient.cs +++ b/src/protagonist/API.Client/DlcsClient.cs @@ -317,7 +317,15 @@ public async Task GetQueue() var queue = await response.ReadAsHydraResponseAsync(jsonSerializerSettings); return queue; } - + + public async Task> GetNamedQueries() + { + var url = $"customers/{currentUser.GetCustomerId()}/namedQueries"; + var response = await httpClient.GetAsync(url); + var namedQueries = await response.ReadAsHydraResponseAsync>(jsonSerializerSettings); + return namedQueries; + } + private HttpContent ApiBody(JsonLdBase apiObject) { var jsonString = JsonConvert.SerializeObject(apiObject, jsonSerializerSettings); diff --git a/src/protagonist/API.Client/IDlcsClient.cs b/src/protagonist/API.Client/IDlcsClient.cs index 6e3567d0e..3a79c9709 100644 --- a/src/protagonist/API.Client/IDlcsClient.cs +++ b/src/protagonist/API.Client/IDlcsClient.cs @@ -34,4 +34,5 @@ Task> GetSpaceImages(int page, int pageSize, int spaceId, Task TestBatch(int batchId); Task> GetBatchImages(int batchId, int page, int pageSize); Task GetQueue(); + Task> GetNamedQueries(); } \ No newline at end of file diff --git a/src/protagonist/Portal/Features/NamedQueries/Requests/GetCustomerNamedQueries.cs b/src/protagonist/Portal/Features/NamedQueries/Requests/GetCustomerNamedQueries.cs new file mode 100644 index 000000000..19360be19 --- /dev/null +++ b/src/protagonist/Portal/Features/NamedQueries/Requests/GetCustomerNamedQueries.cs @@ -0,0 +1,31 @@ +using System.Threading; +using System.Threading.Tasks; +using API.Client; +using DLCS.HydraModel; +using Hydra.Collections; +using MediatR; + +namespace Portal.Features.NamedQueries.Requests; + +/// +/// Get all named queries belonging to the current customer +/// +public class GetCustomerNamedQueries: IRequest> +{ +} + +public class GetCustomerNamedQueriesHandler : IRequestHandler> +{ + private readonly IDlcsClient dlcsClient; + + public GetCustomerNamedQueriesHandler(IDlcsClient dlcsClient) + { + this.dlcsClient = dlcsClient; + } + + public async Task> Handle(GetCustomerNamedQueries request, CancellationToken cancellationToken) + { + var namedQueries = await dlcsClient.GetNamedQueries(); + return namedQueries; + } +} \ No newline at end of file diff --git a/src/protagonist/Portal/Pages/Index.cshtml b/src/protagonist/Portal/Pages/Index.cshtml index e6f0b1ad1..faeac3d7e 100644 --- a/src/protagonist/Portal/Pages/Index.cshtml +++ b/src/protagonist/Portal/Pages/Index.cshtml @@ -57,6 +57,15 @@

View your queue

+
+ +
+

+ Named Queries +

+

View and manage named queries

+
+
diff --git a/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml b/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml new file mode 100644 index 000000000..191e11785 --- /dev/null +++ b/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml @@ -0,0 +1,38 @@ +@page +@using DLCS.Core.Collections +@model Portal.Pages.NamedQueries.IndexModel + +@{ + ViewData["Title"] = "Named Queries"; +} + +
+
+ @if (Model.NamedQueries.Members.IsEmpty()) + { + + } + else + { + + + + + + + + + @foreach (var namedQuery in Model.NamedQueries.Members) + { + + + + + } + +
NameTemplate
@namedQuery.Name@namedQuery.Template
+ } +
+
\ No newline at end of file diff --git a/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml.cs b/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml.cs new file mode 100644 index 000000000..4d68367b7 --- /dev/null +++ b/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml.cs @@ -0,0 +1,27 @@ +using System.Threading.Tasks; +using DLCS.HydraModel; +using Hydra.Collections; +using MediatR; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.RazorPages; +using Portal.Features.NamedQueries.Requests; + +namespace Portal.Pages.NamedQueries; + +public class IndexModel : PageModel +{ + private readonly IMediator mediator; + + [BindProperty] + public HydraCollection NamedQueries { get; set; } + + public IndexModel(IMediator mediator) + { + this.mediator = mediator; + } + + public async Task OnGetAsync() + { + NamedQueries = await mediator.Send(new GetCustomerNamedQueries()); + } +} \ No newline at end of file diff --git a/src/protagonist/Portal/Pages/Shared/_ControlPanelLayout.cshtml b/src/protagonist/Portal/Pages/Shared/_ControlPanelLayout.cshtml index a57938f94..08e7cad90 100644 --- a/src/protagonist/Portal/Pages/Shared/_ControlPanelLayout.cshtml +++ b/src/protagonist/Portal/Pages/Shared/_ControlPanelLayout.cshtml @@ -66,6 +66,9 @@ Upload } +
  • + Named Queries +
  • Users
    • From bfead48363b6c8fa17f2724a6e89976b117b8963 Mon Sep 17 00:00:00 2001 From: griffri Date: Fri, 28 Jun 2024 15:22:50 +0100 Subject: [PATCH 22/60] Implement ability to create, update and delete named queries --- src/protagonist/API.Client/DlcsClient.cs | 32 ++++- src/protagonist/API.Client/IDlcsClient.cs | 5 +- .../NamedQueries/NamedQueryController.cs | 38 ++++++ .../NamedQueries/Requests/CreateNamedQuery.cs | 32 +++++ .../NamedQueries/Requests/DeleteNamedQuery.cs | 30 +++++ .../Requests/GetCustomerNamedQueries.cs | 11 +- .../NamedQueries/Requests/UpdateNamedQuery.cs | 32 +++++ .../Portal/Pages/NamedQueries/Index.cshtml | 127 ++++++++++++++++-- .../Portal/Pages/NamedQueries/Index.cshtml.cs | 4 +- 9 files changed, 289 insertions(+), 22 deletions(-) create mode 100644 src/protagonist/Portal/Features/NamedQueries/NamedQueryController.cs create mode 100644 src/protagonist/Portal/Features/NamedQueries/Requests/CreateNamedQuery.cs create mode 100644 src/protagonist/Portal/Features/NamedQueries/Requests/DeleteNamedQuery.cs create mode 100644 src/protagonist/Portal/Features/NamedQueries/Requests/UpdateNamedQuery.cs diff --git a/src/protagonist/API.Client/DlcsClient.cs b/src/protagonist/API.Client/DlcsClient.cs index 13527b184..bffa1473b 100644 --- a/src/protagonist/API.Client/DlcsClient.cs +++ b/src/protagonist/API.Client/DlcsClient.cs @@ -226,7 +226,7 @@ public async Task DeleteImage(int spaceId, string imageId) var uri = $"customers/{currentUser.GetCustomerId()}/spaces/{spaceId}/images/{imageId}"; try { - var response = await httpClient.DeleteAsync(uri); + await httpClient.DeleteAsync(uri); return true; } catch (Exception ex) @@ -318,12 +318,38 @@ public async Task GetQueue() return queue; } - public async Task> GetNamedQueries() + public async Task> GetNamedQueries(bool includeGlobal) { var url = $"customers/{currentUser.GetCustomerId()}/namedQueries"; var response = await httpClient.GetAsync(url); var namedQueries = await response.ReadAsHydraResponseAsync>(jsonSerializerSettings); - return namedQueries; + + if (!includeGlobal) + { + return namedQueries.Members.Where(nq => nq.Global == false); + } + + return namedQueries.Members; + } + + public async Task DeleteNamedQuery(string namedQueryId) + { + var url = $"customers/{currentUser.GetCustomerId()}/namedQueries/{namedQueryId}"; + await httpClient.DeleteAsync(url); + } + + public async Task UpdateNamedQuery(string namedQueryId, string template) + { + var url = $"customers/{currentUser.GetCustomerId()}/namedQueries/{namedQueryId}"; + await httpClient.PutAsync(url, ApiBody(new NamedQuery(){ Template = template })); + } + + public async Task CreateNamedQuery(NamedQuery newNamedQuery) + { + var url = $"customers/{currentUser.GetCustomerId()}/namedQueries"; + var response = await httpClient.PostAsync(url, ApiBody(newNamedQuery)); + var namedQuery = await response.ReadAsHydraResponseAsync(jsonSerializerSettings); + return namedQuery; } private HttpContent ApiBody(JsonLdBase apiObject) diff --git a/src/protagonist/API.Client/IDlcsClient.cs b/src/protagonist/API.Client/IDlcsClient.cs index 3a79c9709..fe1674341 100644 --- a/src/protagonist/API.Client/IDlcsClient.cs +++ b/src/protagonist/API.Client/IDlcsClient.cs @@ -34,5 +34,8 @@ Task> GetSpaceImages(int page, int pageSize, int spaceId, Task TestBatch(int batchId); Task> GetBatchImages(int batchId, int page, int pageSize); Task GetQueue(); - Task> GetNamedQueries(); + Task> GetNamedQueries(bool includeGlobal); + Task DeleteNamedQuery(string namedQueryId); + Task UpdateNamedQuery(string namedQueryId, string template); + Task CreateNamedQuery(NamedQuery newNamedQuery); } \ No newline at end of file diff --git a/src/protagonist/Portal/Features/NamedQueries/NamedQueryController.cs b/src/protagonist/Portal/Features/NamedQueries/NamedQueryController.cs new file mode 100644 index 000000000..0d42087bb --- /dev/null +++ b/src/protagonist/Portal/Features/NamedQueries/NamedQueryController.cs @@ -0,0 +1,38 @@ +using System.Threading.Tasks; +using MediatR; +using Microsoft.AspNetCore.Mvc; +using Portal.Features.NamedQueries.Requests; + +namespace Portal.Features.NamedQueries; + +[Route("[controller]/[action]")] +public class NamedQueryController : Controller +{ + private readonly IMediator mediator; + + public NamedQueryController(IMediator mediator) + { + this.mediator = mediator; + } + + [HttpPost] + public async Task Delete([FromForm] string namedQueryId) + { + await mediator.Send(new DeleteNamedQuery(){ NamedQueryId = namedQueryId }); + return RedirectToPage("/NamedQueries/Index"); + } + + [HttpPost] + public async Task Update([FromForm] string namedQueryId, [FromForm] string template) + { + await mediator.Send(new UpdateNamedQuery(){ NamedQueryId = namedQueryId, Template = template }); + return RedirectToPage("/NamedQueries/Index"); + } + + [HttpPost] + public async Task Create([FromForm] string queryName, [FromForm] string template) + { + await mediator.Send(new CreateNamedQuery(){ Name = queryName, Template = template }); + return RedirectToPage("/NamedQueries/Index"); + } +} \ No newline at end of file diff --git a/src/protagonist/Portal/Features/NamedQueries/Requests/CreateNamedQuery.cs b/src/protagonist/Portal/Features/NamedQueries/Requests/CreateNamedQuery.cs new file mode 100644 index 000000000..0c51d74ee --- /dev/null +++ b/src/protagonist/Portal/Features/NamedQueries/Requests/CreateNamedQuery.cs @@ -0,0 +1,32 @@ +using System.Threading; +using System.Threading.Tasks; +using API.Client; +using DLCS.HydraModel; +using MediatR; + +namespace Portal.Features.NamedQueries.Requests; + +/// +/// Create a new named query belonging to the customer +/// +public class CreateNamedQuery: IRequest +{ + public string Name { get; set; } + + public string Template { get; set; } +} + +public class CreateNamedQueryHandler : IRequestHandler +{ + private readonly IDlcsClient dlcsClient; + + public CreateNamedQueryHandler(IDlcsClient dlcsClient) + { + this.dlcsClient = dlcsClient; + } + + public async Task Handle(CreateNamedQuery request, CancellationToken cancellationToken) + { + return await dlcsClient.CreateNamedQuery(new NamedQuery() { Name = request.Name, Template = request.Template }); + } +} \ No newline at end of file diff --git a/src/protagonist/Portal/Features/NamedQueries/Requests/DeleteNamedQuery.cs b/src/protagonist/Portal/Features/NamedQueries/Requests/DeleteNamedQuery.cs new file mode 100644 index 000000000..2eed45fd0 --- /dev/null +++ b/src/protagonist/Portal/Features/NamedQueries/Requests/DeleteNamedQuery.cs @@ -0,0 +1,30 @@ +using System.Threading; +using System.Threading.Tasks; +using API.Client; +using MediatR; + +namespace Portal.Features.NamedQueries.Requests; + +/// +/// Delete a specified named query belonging to the current customer +/// +public class DeleteNamedQuery: IRequest +{ + public string NamedQueryId { get; set; } +} + +public class DeleteNamedQueryHandler : IRequestHandler +{ + private readonly IDlcsClient dlcsClient; + + public DeleteNamedQueryHandler(IDlcsClient dlcsClient) + { + this.dlcsClient = dlcsClient; + } + + public async Task Handle(DeleteNamedQuery request, CancellationToken cancellationToken) + { + await dlcsClient.DeleteNamedQuery(request.NamedQueryId); + return Unit.Value; + } +} \ No newline at end of file diff --git a/src/protagonist/Portal/Features/NamedQueries/Requests/GetCustomerNamedQueries.cs b/src/protagonist/Portal/Features/NamedQueries/Requests/GetCustomerNamedQueries.cs index 19360be19..0f8a126c5 100644 --- a/src/protagonist/Portal/Features/NamedQueries/Requests/GetCustomerNamedQueries.cs +++ b/src/protagonist/Portal/Features/NamedQueries/Requests/GetCustomerNamedQueries.cs @@ -1,3 +1,4 @@ +using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; using API.Client; @@ -8,13 +9,13 @@ namespace Portal.Features.NamedQueries.Requests; /// -/// Get all named queries belonging to the current customer +/// Deletes a specified name query belonging to the current customer /// -public class GetCustomerNamedQueries: IRequest> +public class GetCustomerNamedQueries: IRequest> { } -public class GetCustomerNamedQueriesHandler : IRequestHandler> +public class GetCustomerNamedQueriesHandler : IRequestHandler> { private readonly IDlcsClient dlcsClient; @@ -23,9 +24,9 @@ public GetCustomerNamedQueriesHandler(IDlcsClient dlcsClient) this.dlcsClient = dlcsClient; } - public async Task> Handle(GetCustomerNamedQueries request, CancellationToken cancellationToken) + public async Task> Handle(GetCustomerNamedQueries request, CancellationToken cancellationToken) { - var namedQueries = await dlcsClient.GetNamedQueries(); + var namedQueries = await dlcsClient.GetNamedQueries(false); return namedQueries; } } \ No newline at end of file diff --git a/src/protagonist/Portal/Features/NamedQueries/Requests/UpdateNamedQuery.cs b/src/protagonist/Portal/Features/NamedQueries/Requests/UpdateNamedQuery.cs new file mode 100644 index 000000000..6bfdc4635 --- /dev/null +++ b/src/protagonist/Portal/Features/NamedQueries/Requests/UpdateNamedQuery.cs @@ -0,0 +1,32 @@ +using System.Threading; +using System.Threading.Tasks; +using API.Client; +using MediatR; + +namespace Portal.Features.NamedQueries.Requests; + +/// +/// Update a specified named query belonging to the current customer +/// +public class UpdateNamedQuery: IRequest +{ + public string NamedQueryId { get; set; } + + public string Template { get; set; } +} + +public class UpdateNamedQueryHandler : IRequestHandler +{ + private readonly IDlcsClient dlcsClient; + + public UpdateNamedQueryHandler(IDlcsClient dlcsClient) + { + this.dlcsClient = dlcsClient; + } + + public async Task Handle(UpdateNamedQuery request, CancellationToken cancellationToken) + { + await dlcsClient.UpdateNamedQuery(request.NamedQueryId, request.Template); + return Unit.Value; + } +} \ No newline at end of file diff --git a/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml b/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml index 191e11785..97b297e62 100644 --- a/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml +++ b/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml @@ -8,7 +8,7 @@
    - @if (Model.NamedQueries.Members.IsEmpty()) + @if (!Model.NamedQueries.Any()) { -
    \ No newline at end of file +
    + + + +
    + +
    + +
    + +
    + +
    + +
    + +@section Scripts +{ + +} diff --git a/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml.cs b/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml.cs index 4d68367b7..0aef2bb1a 100644 --- a/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml.cs +++ b/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml.cs @@ -1,6 +1,6 @@ +using System.Collections.Generic; using System.Threading.Tasks; using DLCS.HydraModel; -using Hydra.Collections; using MediatR; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.RazorPages; @@ -13,7 +13,7 @@ public class IndexModel : PageModel private readonly IMediator mediator; [BindProperty] - public HydraCollection NamedQueries { get; set; } + public IEnumerable NamedQueries { get; set; } public IndexModel(IMediator mediator) { From a5fd3fa27c6b40d8e14db0eefc11bd52d2f82211 Mon Sep 17 00:00:00 2001 From: griffri Date: Fri, 28 Jun 2024 15:54:52 +0100 Subject: [PATCH 23/60] Add "Create new named query" placeholder, reposition "add" button --- .../NamedQueries/NamedQueryController.cs | 11 ++++- .../Portal/Pages/NamedQueries/Index.cshtml | 44 +++++++++---------- 2 files changed, 32 insertions(+), 23 deletions(-) diff --git a/src/protagonist/Portal/Features/NamedQueries/NamedQueryController.cs b/src/protagonist/Portal/Features/NamedQueries/NamedQueryController.cs index 0d42087bb..830085b8a 100644 --- a/src/protagonist/Portal/Features/NamedQueries/NamedQueryController.cs +++ b/src/protagonist/Portal/Features/NamedQueries/NamedQueryController.cs @@ -1,4 +1,5 @@ using System.Threading.Tasks; +using API.Client; using MediatR; using Microsoft.AspNetCore.Mvc; using Portal.Features.NamedQueries.Requests; @@ -32,7 +33,15 @@ public async Task Update([FromForm] string namedQueryId, [FromFor [HttpPost] public async Task Create([FromForm] string queryName, [FromForm] string template) { - await mediator.Send(new CreateNamedQuery(){ Name = queryName, Template = template }); + try + { + await mediator.Send(new CreateNamedQuery() { Name = queryName, Template = template }); + } + catch (DlcsException dlcsEx) + { + return RedirectToPage("/NamedQueries/Index"); + } + return RedirectToPage("/NamedQueries/Index"); } } \ No newline at end of file diff --git a/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml b/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml index 97b297e62..7f9cb45c3 100644 --- a/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml +++ b/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml @@ -10,9 +10,7 @@
    @if (!Model.NamedQueries.Any()) { - + } else { @@ -25,33 +23,35 @@ - @if (Model.NamedQueries != null) + @foreach (var namedQuery in Model.NamedQueries) { - foreach (var namedQuery in Model.NamedQueries) - { - var namedQueryId = namedQuery.Id.Split('/').Last(); - - @namedQuery.Name - @namedQuery.Template - - - - - - } + var namedQueryId = namedQuery.Id.Split('/').Last(); + + @namedQuery.Name + @namedQuery.Template + + + + + } + + + + + + + }
    - -
    From 930593774dd0b9f9abcbba3e2e0f8cd0e98b4290 Mon Sep 17 00:00:00 2001 From: griffri Date: Fri, 28 Jun 2024 16:38:29 +0100 Subject: [PATCH 26/60] Add success/failure messages on creating NQ, refactor --- src/protagonist/API.Client/DlcsClient.cs | 9 +++------ .../NamedQueries/NamedQueryController.cs | 7 +++---- .../NamedQueries/Requests/CreateNamedQuery.cs | 4 ++-- .../Requests/GetCustomerNamedQueries.cs | 3 +-- .../Portal/Pages/NamedQueries/Index.cshtml | 16 ++++++++++++++++ .../Portal/Pages/NamedQueries/Index.cshtml.cs | 10 +++++++--- 6 files changed, 32 insertions(+), 17 deletions(-) diff --git a/src/protagonist/API.Client/DlcsClient.cs b/src/protagonist/API.Client/DlcsClient.cs index bffa1473b..2c39bdbf4 100644 --- a/src/protagonist/API.Client/DlcsClient.cs +++ b/src/protagonist/API.Client/DlcsClient.cs @@ -324,12 +324,9 @@ public async Task> GetNamedQueries(bool includeGlobal) var response = await httpClient.GetAsync(url); var namedQueries = await response.ReadAsHydraResponseAsync>(jsonSerializerSettings); - if (!includeGlobal) - { - return namedQueries.Members.Where(nq => nq.Global == false); - } - - return namedQueries.Members; + return includeGlobal + ? namedQueries.Members + : namedQueries.Members.Where(nq => nq.Global == false); } public async Task DeleteNamedQuery(string namedQueryId) diff --git a/src/protagonist/Portal/Features/NamedQueries/NamedQueryController.cs b/src/protagonist/Portal/Features/NamedQueries/NamedQueryController.cs index 830085b8a..b24f9551b 100644 --- a/src/protagonist/Portal/Features/NamedQueries/NamedQueryController.cs +++ b/src/protagonist/Portal/Features/NamedQueries/NamedQueryController.cs @@ -36,12 +36,11 @@ public async Task Create([FromForm] string queryName, [FromForm] try { await mediator.Send(new CreateNamedQuery() { Name = queryName, Template = template }); + return RedirectToPage("/NamedQueries/Index", new { success = true }); } - catch (DlcsException dlcsEx) + catch (DlcsException) { - return RedirectToPage("/NamedQueries/Index"); + return RedirectToPage("/NamedQueries/Index",new { success = false }); } - - return RedirectToPage("/NamedQueries/Index"); } } \ No newline at end of file diff --git a/src/protagonist/Portal/Features/NamedQueries/Requests/CreateNamedQuery.cs b/src/protagonist/Portal/Features/NamedQueries/Requests/CreateNamedQuery.cs index 0c51d74ee..12c7b049b 100644 --- a/src/protagonist/Portal/Features/NamedQueries/Requests/CreateNamedQuery.cs +++ b/src/protagonist/Portal/Features/NamedQueries/Requests/CreateNamedQuery.cs @@ -7,7 +7,7 @@ namespace Portal.Features.NamedQueries.Requests; /// -/// Create a new named query belonging to the customer +/// Create a new named query belonging to the current customer /// public class CreateNamedQuery: IRequest { @@ -27,6 +27,6 @@ public CreateNamedQueryHandler(IDlcsClient dlcsClient) public async Task Handle(CreateNamedQuery request, CancellationToken cancellationToken) { - return await dlcsClient.CreateNamedQuery(new NamedQuery() { Name = request.Name, Template = request.Template }); + return await dlcsClient.CreateNamedQuery(new NamedQuery(){ Name = request.Name, Template = request.Template }); } } \ No newline at end of file diff --git a/src/protagonist/Portal/Features/NamedQueries/Requests/GetCustomerNamedQueries.cs b/src/protagonist/Portal/Features/NamedQueries/Requests/GetCustomerNamedQueries.cs index 0f8a126c5..b5ea5f306 100644 --- a/src/protagonist/Portal/Features/NamedQueries/Requests/GetCustomerNamedQueries.cs +++ b/src/protagonist/Portal/Features/NamedQueries/Requests/GetCustomerNamedQueries.cs @@ -3,13 +3,12 @@ using System.Threading.Tasks; using API.Client; using DLCS.HydraModel; -using Hydra.Collections; using MediatR; namespace Portal.Features.NamedQueries.Requests; /// -/// Deletes a specified name query belonging to the current customer +/// Retrieves all named queries belonging to the current customer /// public class GetCustomerNamedQueries: IRequest> { diff --git a/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml b/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml index b304b8cc2..8e387cd73 100644 --- a/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml +++ b/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml @@ -5,6 +5,22 @@ ViewData["Title"] = "Named Queries"; } +@if (Model.Success.HasValue) +{ + if (Model.Success.Value) + { + + } + else + { + + } +} +
    @if (!Model.NamedQueries.Any()) diff --git a/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml.cs b/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml.cs index 0aef2bb1a..e07f7677f 100644 --- a/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml.cs +++ b/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using System.Linq; using System.Threading.Tasks; using DLCS.HydraModel; using MediatR; @@ -13,15 +14,18 @@ public class IndexModel : PageModel private readonly IMediator mediator; [BindProperty] - public IEnumerable NamedQueries { get; set; } - + public IEnumerable NamedQueries { get; set; } = Enumerable.Empty(); + + public bool? Success; + public IndexModel(IMediator mediator) { this.mediator = mediator; } - public async Task OnGetAsync() + public async Task OnGetAsync([FromRoute] bool? success = null) { + Success = success; NamedQueries = await mediator.Send(new GetCustomerNamedQueries()); } } \ No newline at end of file From 5b87589a1275e0a2d5b6fbc9d483c8b3eaca3276 Mon Sep 17 00:00:00 2001 From: griffri Date: Fri, 28 Jun 2024 16:42:01 +0100 Subject: [PATCH 27/60] Use FromQuery instead of FromRoute --- src/protagonist/Portal/Pages/NamedQueries/Index.cshtml.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml.cs b/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml.cs index e07f7677f..dc974a38d 100644 --- a/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml.cs +++ b/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml.cs @@ -23,7 +23,7 @@ public IndexModel(IMediator mediator) this.mediator = mediator; } - public async Task OnGetAsync([FromRoute] bool? success = null) + public async Task OnGetAsync([FromQuery] bool? success = null) { Success = success; NamedQueries = await mediator.Send(new GetCustomerNamedQueries()); From 145104b727a7465ad607676e8b681a28e397081a Mon Sep 17 00:00:00 2001 From: griffri Date: Mon, 1 Jul 2024 09:09:08 +0100 Subject: [PATCH 28/60] Update DeleteNamedQuery and UpdateNamedQuery to log errors --- src/protagonist/API.Client/DlcsClient.cs | 31 +++++++++++++++---- src/protagonist/API.Client/IDlcsClient.cs | 4 +-- .../NamedQueries/Requests/DeleteNamedQuery.cs | 9 +++--- .../NamedQueries/Requests/UpdateNamedQuery.cs | 10 +++--- .../Portal/Pages/NamedQueries/Index.cshtml | 2 +- 5 files changed, 37 insertions(+), 19 deletions(-) diff --git a/src/protagonist/API.Client/DlcsClient.cs b/src/protagonist/API.Client/DlcsClient.cs index 2c39bdbf4..3eee4b636 100644 --- a/src/protagonist/API.Client/DlcsClient.cs +++ b/src/protagonist/API.Client/DlcsClient.cs @@ -118,7 +118,7 @@ public async Task> GetSpaceImages(int page, int pageSize, var response = await httpClient.GetAsync(url); return await response.ReadAsHydraResponseAsync(jsonSerializerSettings); } - catch (Exception ex) + catch (Exception) { logger.LogError("Failed to deserialize storage for space {SpaceStorage}", spaceId); return null; @@ -196,7 +196,7 @@ public async Task DeletePortalUser(string portalUserId) var url = $"customers/{currentUser.GetCustomerId()}/portalUsers/{portalUserId}"; try { - var response = await httpClient.DeleteAsync(url); + await httpClient.DeleteAsync(url); return true; } catch (Exception ex) @@ -329,16 +329,35 @@ public async Task> GetNamedQueries(bool includeGlobal) : namedQueries.Members.Where(nq => nq.Global == false); } - public async Task DeleteNamedQuery(string namedQueryId) + public async Task DeleteNamedQuery(string namedQueryId) { var url = $"customers/{currentUser.GetCustomerId()}/namedQueries/{namedQueryId}"; - await httpClient.DeleteAsync(url); + try + { + await httpClient.DeleteAsync(url); + return true; + } + catch (Exception ex) + { + logger.LogError(ex, "Error deleting named query '{NamedQueryId}'", namedQueryId); + return false; + } } - public async Task UpdateNamedQuery(string namedQueryId, string template) + public async Task UpdateNamedQuery(string namedQueryId, string template) { var url = $"customers/{currentUser.GetCustomerId()}/namedQueries/{namedQueryId}"; - await httpClient.PutAsync(url, ApiBody(new NamedQuery(){ Template = template })); + try + { + var response = await httpClient.PutAsync(url, ApiBody(new NamedQuery(){ Template = template })); + var updatedNamedQuery = await response.ReadAsHydraResponseAsync(jsonSerializerSettings); + return updatedNamedQuery; + } + catch (Exception ex) + { + logger.LogError(ex, "Error updating named query '{NamedQueryId}'", namedQueryId); + return null; + } } public async Task CreateNamedQuery(NamedQuery newNamedQuery) diff --git a/src/protagonist/API.Client/IDlcsClient.cs b/src/protagonist/API.Client/IDlcsClient.cs index fe1674341..edaf4dd97 100644 --- a/src/protagonist/API.Client/IDlcsClient.cs +++ b/src/protagonist/API.Client/IDlcsClient.cs @@ -35,7 +35,7 @@ Task> GetSpaceImages(int page, int pageSize, int spaceId, Task> GetBatchImages(int batchId, int page, int pageSize); Task GetQueue(); Task> GetNamedQueries(bool includeGlobal); - Task DeleteNamedQuery(string namedQueryId); - Task UpdateNamedQuery(string namedQueryId, string template); + Task DeleteNamedQuery(string namedQueryId); + Task UpdateNamedQuery(string namedQueryId, string template); Task CreateNamedQuery(NamedQuery newNamedQuery); } \ No newline at end of file diff --git a/src/protagonist/Portal/Features/NamedQueries/Requests/DeleteNamedQuery.cs b/src/protagonist/Portal/Features/NamedQueries/Requests/DeleteNamedQuery.cs index 2eed45fd0..c17c02937 100644 --- a/src/protagonist/Portal/Features/NamedQueries/Requests/DeleteNamedQuery.cs +++ b/src/protagonist/Portal/Features/NamedQueries/Requests/DeleteNamedQuery.cs @@ -8,12 +8,12 @@ namespace Portal.Features.NamedQueries.Requests; /// /// Delete a specified named query belonging to the current customer /// -public class DeleteNamedQuery: IRequest +public class DeleteNamedQuery: IRequest { public string NamedQueryId { get; set; } } -public class DeleteNamedQueryHandler : IRequestHandler +public class DeleteNamedQueryHandler : IRequestHandler { private readonly IDlcsClient dlcsClient; @@ -22,9 +22,8 @@ public DeleteNamedQueryHandler(IDlcsClient dlcsClient) this.dlcsClient = dlcsClient; } - public async Task Handle(DeleteNamedQuery request, CancellationToken cancellationToken) + public async Task Handle(DeleteNamedQuery request, CancellationToken cancellationToken) { - await dlcsClient.DeleteNamedQuery(request.NamedQueryId); - return Unit.Value; + return await dlcsClient.DeleteNamedQuery(request.NamedQueryId); } } \ No newline at end of file diff --git a/src/protagonist/Portal/Features/NamedQueries/Requests/UpdateNamedQuery.cs b/src/protagonist/Portal/Features/NamedQueries/Requests/UpdateNamedQuery.cs index 6bfdc4635..a24e5508f 100644 --- a/src/protagonist/Portal/Features/NamedQueries/Requests/UpdateNamedQuery.cs +++ b/src/protagonist/Portal/Features/NamedQueries/Requests/UpdateNamedQuery.cs @@ -1,6 +1,7 @@ using System.Threading; using System.Threading.Tasks; using API.Client; +using DLCS.HydraModel; using MediatR; namespace Portal.Features.NamedQueries.Requests; @@ -8,14 +9,14 @@ namespace Portal.Features.NamedQueries.Requests; /// /// Update a specified named query belonging to the current customer /// -public class UpdateNamedQuery: IRequest +public class UpdateNamedQuery: IRequest { public string NamedQueryId { get; set; } public string Template { get; set; } } -public class UpdateNamedQueryHandler : IRequestHandler +public class UpdateNamedQueryHandler : IRequestHandler { private readonly IDlcsClient dlcsClient; @@ -24,9 +25,8 @@ public UpdateNamedQueryHandler(IDlcsClient dlcsClient) this.dlcsClient = dlcsClient; } - public async Task Handle(UpdateNamedQuery request, CancellationToken cancellationToken) + public async Task Handle(UpdateNamedQuery request, CancellationToken cancellationToken) { - await dlcsClient.UpdateNamedQuery(request.NamedQueryId, request.Template); - return Unit.Value; + return await dlcsClient.UpdateNamedQuery(request.NamedQueryId, request.Template); } } \ No newline at end of file diff --git a/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml b/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml index 8e387cd73..eb813941a 100644 --- a/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml +++ b/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml @@ -99,7 +99,7 @@
    - +
    From 6b67b656db52559cdca18bf620abbc0fb4a8e1d3 Mon Sep 17 00:00:00 2001 From: griffri Date: Mon, 1 Jul 2024 11:28:14 +0100 Subject: [PATCH 29/60] Rewrite create/update named query forms to return errors from API --- src/protagonist/API.Client/DlcsClient.cs | 14 +--- .../NamedQueries/NamedQueryController.cs | 22 +++-- .../Portal/Pages/NamedQueries/Index.cshtml | 84 +++++++++++++------ .../Portal/Pages/NamedQueries/Index.cshtml.cs | 3 - 4 files changed, 76 insertions(+), 47 deletions(-) diff --git a/src/protagonist/API.Client/DlcsClient.cs b/src/protagonist/API.Client/DlcsClient.cs index 3eee4b636..1116d96b8 100644 --- a/src/protagonist/API.Client/DlcsClient.cs +++ b/src/protagonist/API.Client/DlcsClient.cs @@ -347,17 +347,9 @@ public async Task DeleteNamedQuery(string namedQueryId) public async Task UpdateNamedQuery(string namedQueryId, string template) { var url = $"customers/{currentUser.GetCustomerId()}/namedQueries/{namedQueryId}"; - try - { - var response = await httpClient.PutAsync(url, ApiBody(new NamedQuery(){ Template = template })); - var updatedNamedQuery = await response.ReadAsHydraResponseAsync(jsonSerializerSettings); - return updatedNamedQuery; - } - catch (Exception ex) - { - logger.LogError(ex, "Error updating named query '{NamedQueryId}'", namedQueryId); - return null; - } + var response = await httpClient.PutAsync(url, ApiBody(new NamedQuery(){ Template = template })); + var updatedNamedQuery = await response.ReadAsHydraResponseAsync(jsonSerializerSettings); + return updatedNamedQuery; } public async Task CreateNamedQuery(NamedQuery newNamedQuery) diff --git a/src/protagonist/Portal/Features/NamedQueries/NamedQueryController.cs b/src/protagonist/Portal/Features/NamedQueries/NamedQueryController.cs index b24f9551b..9c9ebb56e 100644 --- a/src/protagonist/Portal/Features/NamedQueries/NamedQueryController.cs +++ b/src/protagonist/Portal/Features/NamedQueries/NamedQueryController.cs @@ -1,3 +1,4 @@ +using System; using System.Threading.Tasks; using API.Client; using MediatR; @@ -26,21 +27,28 @@ public async Task Delete([FromForm] string namedQueryId) [HttpPost] public async Task Update([FromForm] string namedQueryId, [FromForm] string template) { - await mediator.Send(new UpdateNamedQuery(){ NamedQueryId = namedQueryId, Template = template }); - return RedirectToPage("/NamedQueries/Index"); + try + { + await mediator.Send(new UpdateNamedQuery(){ NamedQueryId = namedQueryId, Template = template }); + return Ok(); + } + catch (DlcsException dlcsEx) + { + return BadRequest(dlcsEx.Message); + } } [HttpPost] - public async Task Create([FromForm] string queryName, [FromForm] string template) + public async Task Create(string queryName, string queryTemplate) { try { - await mediator.Send(new CreateNamedQuery() { Name = queryName, Template = template }); - return RedirectToPage("/NamedQueries/Index", new { success = true }); + await mediator.Send(new CreateNamedQuery() { Name = queryName, Template = queryTemplate }); + return Ok(); } - catch (DlcsException) + catch (DlcsException dlcsEx) { - return RedirectToPage("/NamedQueries/Index",new { success = false }); + return BadRequest(dlcsEx.Message); } } } \ No newline at end of file diff --git a/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml b/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml index eb813941a..3f4591d38 100644 --- a/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml +++ b/src/protagonist/Portal/Pages/NamedQueries/Index.cshtml @@ -5,22 +5,6 @@ ViewData["Title"] = "Named Queries"; } -@if (Model.Success.HasValue) -{ - if (Model.Success.Value) - { - - } - else - { - - } -} -
    @if (!Model.NamedQueries.Any()) @@ -58,7 +42,7 @@ - + @@ -67,8 +51,8 @@
    - -