From ee3f39d09ddb2fc33684550c7a788495fbc4298b Mon Sep 17 00:00:00 2001 From: Donald Gray Date: Wed, 26 Jun 2024 14:56:14 +0100 Subject: [PATCH] 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