Merge pull request #857 from dlcs/develop

Update main with bugfixes from develop

src/protagonist/API.Tests/Features/DeliveryChannels/Validation/DeliveryChannelPolicyDataValidatorTests.cs

@@ -24,12 +24,14 @@ public DeliveryChannelPolicyDataValidatorTests()
             .Returns(fakedAvPolicies);
         sut = new DeliveryChannelPolicyDataValidator(avChannelPolicyOptionsRepository);
     }
-
+    
     [Theory]
     [InlineData("[\"400,\",\"200,\",\"100,\"]")]
-    [InlineData("[\"!400,\",\"!200,\",\"!100,\"]")]
     [InlineData("[\",400\",\",200\",\",100\"]")]
-    [InlineData("[\"!,400\",\"!,200\",\"!,100\"]")]
+    [InlineData("[\"^400,\",\"^200,\",\"^100,\"]")]
+    [InlineData("[\"^,400\",\"^,200\",\"^,100\"]")]
+    [InlineData("[\"!400,400\",\"!200,200\",\"!100,100\"]")]
+    [InlineData("[\"^!400,400\",\"^!200,200\",\"^!100,100\"]")]
     public async Task PolicyDataValidator_ReturnsTrue_ForValidThumbParameters(string policyData)
     {
         // Arrange and Act
@@ -88,6 +90,8 @@ public async Task PolicyDataValidator_ReturnsFalse_ForEmptyThumbSizes(string pol
     [InlineData("[\"^pct:41.6,7.5\"]")]
     [InlineData("[\"10,50\"]")]
     [InlineData("[\",\"]")]
+    [InlineData("[\"!10,\"]")]
+    [InlineData("[\"!,10\"]")]
     public async Task PolicyDataValidator_ReturnsFalse_ForInvalidThumbParameters(string policyData)
     {
         // Arrange and Act

src/protagonist/API.Tests/Integration/DeliveryChannelTests.cs

@@ -785,42 +785,21 @@ public async Task Get_DeliveryChannelPolicyCollection_400_IfChannelInvalid()
             @"[\""400,\"",\""200,\"",\""100,\""]"
         },
         new object[]
-        {
-            "my-thumbs-policy-1-b",
-            @"[\""!400,\"",\""!200,\"",\""!100,\""]"
-        },
-        new object[]
         {
             "my-thumbs-policy-1-c",
             @"[\"",400\"",\"",200\"",\"",100\""]"
         },
         new object[]
-        {
-            "my-thumbs-policy-1-d",
-            @"[\""!,400\"",\""!,200\"",\""!,100\""]"
-
-        },
-        new object[]
         {
             "my-thumbs-policy-1-e",
             @"[\""^400,\"",\""^200,\"",\""^100,\""]"
         },
         new object[]
-        {
-            "my-thumbs-policy-1-f",
-            @"[\""^!400,\"",\""^!200,\"",\""^!100,\""]"
-        },
-        new object[]
         {
             "my-thumbs-policy-1-g",
             @"[\""^,400\"",\""^,200\"",\""^,100\""]"
         },
         new object[]
-        {
-            "my-thumbs-policy-1-h",
-            @"[\""^!,400\"",\""^!,200\"",\""^!,100\""]"
-        },
-        new object[]
         {
             "my-thumbs-policy-1-i",
             @"[\""!400,400\"",\""!200,200\"",\""!100,100\""]"

src/protagonist/API/Features/DeliveryChannels/Validation/DeliveryChannelPolicyDataValidator.cs

@@ -68,10 +68,15 @@ private bool ValidateThumbnailPolicyData(string policyDataJson)
         return true;
     }
 
-    private bool IsValidThumbnailParameter(SizeParameter param)
-        => !(param.Max || param.PercentScale.HasValue ||
-             (param.Width.HasValue && param.Height.HasValue && !param.Confined) ||
-             (!param.Width.HasValue && !param.Height.HasValue));
+    private bool IsValidThumbnailParameter(SizeParameter param) => param switch
+        {
+            { Max: true } => false,
+            { PercentScale: not null } => false,
+            { Confined: false, Width: not null, Height: not null } => false,
+            { Confined: true } and ({ Width: null } or { Height : null }) => false,
+            { Width: null, Height: null } => false,
+            _ => true,
+        };
 
     private async Task<bool> ValidateTimeBasedPolicyData(string policyDataJson)
     {

src/protagonist/DLCS.Model.Tests/Assets/AssetDeliveryChannelsTests.cs

@@ -108,4 +108,35 @@ public void HasSingleDeliveryChannel_True(string channel)
 
         asset.HasSingleDeliveryChannel(channel).Should().BeTrue();
     }
+
+    [Theory]
+    [InlineData(AssetDeliveryChannels.File)]
+    [InlineData(AssetDeliveryChannels.Image)]
+    [InlineData(AssetDeliveryChannels.Timebased)]
+    public void HasAnyDeliveryChannel_True(string channel)
+    {
+        var asset = new Asset { ImageDeliveryChannels = new List<ImageDeliveryChannel>()
+        {
+            new()
+            {
+                Channel = channel,
+            }
+        } };
+
+        asset.HasAnyDeliveryChannel(AssetDeliveryChannels.All).Should().BeTrue();
+    }
+
+    [Fact]
+    public void HasAnyDeliveryChannel_False()
+    {
+        var asset = new Asset { ImageDeliveryChannels = new List<ImageDeliveryChannel>()
+        {
+            new()
+            {
+                Channel = "not-a-channel",
+            }
+        } };
+
+        asset.HasAnyDeliveryChannel(AssetDeliveryChannels.All).Should().BeFalse();
+    }
 }

src/protagonist/DLCS.Model/Assets/AssetDeliveryChannels.cs

@@ -21,22 +21,28 @@ public static class AssetDeliveryChannels
     /// All possible delivery channels as a comma-delimited string
     /// </summary>
     public static readonly string AllString = string.Join(',', All);
-
+    
     /// <summary>
-    /// Check if asset has specified deliveryChannel
+    /// Checks if an asset has any delivery channel specified in a list
     /// </summary>
-    public static bool HasDeliveryChannel(this Asset asset, string deliveryChannel)
+    public static bool HasAnyDeliveryChannel(this Asset asset, params string[] deliveryChannels)
     {
-        if (asset.ImageDeliveryChannels.IsNullOrEmpty()) return false;
-        if (!All.Contains(deliveryChannel))
+        if (asset.ImageDeliveryChannels.IsNullOrEmpty() || deliveryChannels.IsNullOrEmpty()) return false;
+        if (deliveryChannels.Any(dc => !All.Contains(dc)))
         {
-            throw new ArgumentOutOfRangeException(nameof(deliveryChannel), deliveryChannel,
+            throw new ArgumentOutOfRangeException(nameof(deliveryChannels), deliveryChannels,
                 $"Acceptable delivery-channels are: {AllString}");
         }
-
-        return asset.ImageDeliveryChannels.Any(i => i.Channel == deliveryChannel);
+        
+        return asset.ImageDeliveryChannels.Any(dc => deliveryChannels.Contains(dc.Channel));
     }
-
+
+    /// <summary>
+    /// Check if asset has specified deliveryChannel
+    /// </summary>
+    public static bool HasDeliveryChannel(this Asset asset, string deliveryChannel)
+        => HasAnyDeliveryChannel(asset, deliveryChannel);
+
     /// <summary>
     /// Checks if asset has specified deliveryChannel only (e.g. 1 channel and it matches specified value
     /// </summary>

src/protagonist/Engine.Tests/Ingest/Image/Completion/ImageIngestPostProcessingTests.cs

@@ -37,7 +37,8 @@ private ImageIngestPostProcessing GetSut(bool orchestrateAfterIngest)
             ImageIngest = new ImageIngestSettings
             {
                 SourceTemplate = "{customer}_{space}_{image}",
-                OrchestrateImageAfterIngest = orchestrateAfterIngest
+                OrchestrateImageAfterIngest = orchestrateAfterIngest,
+                ScratchRoot = "scratch/"
             },
         };
         var optionsMonitor = OptionsHelpers.GetOptionsMonitor(engineSettings);
@@ -132,11 +133,15 @@ public async Task PostIngest_DeletesWorkingFolder_IfSuccessOrFailure(bool succes
         var assetId = AssetId.FromString("2/1/avalon");
         var asset = new Asset(assetId);
         var sut = GetSut(true);
-
+        var context = new IngestionContext(asset);
+
         // Act
-        await sut.CompleteIngestion(new IngestionContext(asset), success);
+        await sut.CompleteIngestion(context, success);
 
         // Assert
-        fileSystem.DeletedDirectories.Should().Contain("2_1_avalon");
+        var expectedWorkingFolder = $"scratch/{context.IngestId}/";
+        fileSystem.DeletedDirectories.Should().Contain(d => 
+            // Account for backslashes being used to separate directories when running on Windows
+            d.Replace("\\", "/") == expectedWorkingFolder);
     }
 }

src/protagonist/Engine.Tests/Ingest/Image/ImageServer/ImageServerClientTests.cs

@@ -91,10 +91,13 @@ public async Task ProcessImage_ChangesFileSavedLocationBasedOnImageIdWithBracket
         await sut.ProcessImage(context);
 
         // Assert
-        A.CallTo(() => fileSystem.CreateDirectory( "scratch/1/2/some_id_/output")).MustHaveHappenedOnceExactly();
+        var expectedDirectory = $"scratch/{context.IngestId}/1/2/some_id_/output";
+        A.CallTo(() => fileSystem.CreateDirectory( 
+            // Account for backslashes being used to separate directories when running on Windows
+            A<string>.That.Matches(x => x.Replace("\\", "/") == expectedDirectory ))).MustHaveHappenedOnceExactly();
         A.CallTo(() => fileSystem.DeleteDirectory(A<string>._, true, true)).MustHaveHappenedTwiceExactly();
     }
-
+    
     [Fact]
     public async Task ProcessImage_False_IfImageProcessorCallFails()
     {
@@ -178,13 +181,13 @@ public async Task ProcessImage_UpdatesAssetDimensions()
     {
         // Arrange
         var imageProcessorResponse = new AppetiserResponseModel();
-
+        var context = IngestionContextFactory.GetIngestionContext("/1/2/test");
+
         A.CallTo(() => appetiserClient.GenerateJP2(A<IngestionContext>._, A<AssetId>._, A<CancellationToken>._))
             .Returns(Task.FromResult(imageProcessorResponse as IAppetiserResponse));
-        A.CallTo(() => appetiserClient.GetJP2FilePath(A<AssetId>._, A<bool>._))
-            .Returns("scratch/1/2/test/outputtest.jp2");
-
-        var context = IngestionContextFactory.GetIngestionContext("/1/2/test");
+        A.CallTo(() => appetiserClient.GetJP2FilePath(A<AssetId>._, context.IngestId, A<bool>._))
+            .Returns($"scratch/{context.IngestId}/1/2/test/outputtest.jp2");
+
         context.AssetFromOrigin.CustomerOriginStrategy = new CustomerOriginStrategy
         {
             Optimised = optimised,
@@ -201,7 +204,7 @@ public async Task ProcessImage_UpdatesAssetDimensions()
         // Assert
         bucketWriter
             .ShouldHaveKey("1/2/test")
-            .WithFilePath("scratch/1/2/test/outputtest.jp2")
+            .WithFilePath($"scratch/{context.IngestId}/1/2/test/outputtest.jp2")
             .WithContentType("image/jp2");
         context.ImageLocation.S3.Should().Be(expected);
         context.StoredObjects.Should().NotBeEmpty();

src/protagonist/Engine.Tests/Ingest/Workers/ImageIngesterWorkerTests.cs

@@ -28,7 +28,8 @@ public ImageIngesterWorkerTests()
             ImageIngest = new ImageIngestSettings
             {
                 SourceTemplate = "{root}",
-                OrchestrateImageAfterIngest = true
+                OrchestrateImageAfterIngest = true,
+                ScratchRoot = "scratch/"
             },
         };
         var optionsMonitor = OptionsHelpers.GetOptionsMonitor(engineSettings);

src/protagonist/Engine/Ingest/Image/Completion/ImageIngestPostProcessing.cs

@@ -86,7 +86,7 @@ private bool ShouldOrchestrate(int customerId)
 
     private void DeleteWorkingFolder(IngestionContext ingestionContext)
     {
-        var sourceTemplate = ImageIngestionHelpers.GetSourceFolder(ingestionContext.Asset, engineSettings);
+        var sourceTemplate = ImageIngestionHelpers.GetWorkingFolder(ingestionContext.IngestId, engineSettings.ImageIngest);
         fileSystem.DeleteDirectory(sourceTemplate, true);
     }
 }

src/protagonist/Engine/Ingest/Image/ImageIngesterWorker.cs

@@ -44,7 +44,7 @@ public class ImageIngesterWorker : IAssetIngesterWorker, IAssetIngesterPostProce
     {
         bool ingestSuccess;
         var asset = ingestionContext.Asset;
-        var sourceTemplate = ImageIngestionHelpers.GetSourceFolder(asset, engineSettings);
+        var sourceTemplate = ImageIngestionHelpers.GetSourceFolder(ingestionContext, engineSettings);
 
         try
         {

src/protagonist/Engine/Ingest/Image/ImageIngestionHelpers.cs

@@ -7,19 +7,27 @@ namespace Engine.Ingest.Image;
 
 internal static class ImageIngestionHelpers
 {
+    /// <summary>
+    /// Get the top level working directory for an ingest request
+    /// </summary>
+    public static string GetWorkingFolder(string ingestId, ImageIngestSettings imageIngestSettings, bool forImageProcessor = false)
+    {
+        return $"{Path.Combine(imageIngestSettings.GetRoot(forImageProcessor), ingestId)}{Path.DirectorySeparatorChar}";
+    }
+
     /// <summary>
     /// Get folder location where working assets are to be saved to
     /// </summary>
-    public static string GetSourceFolder(Asset asset, EngineSettings engineSettings)
+    public static string GetSourceFolder(IngestionContext ingestionContext, EngineSettings engineSettings)
     {
         var imageIngest = engineSettings.ImageIngest;
-        var root = imageIngest.GetRoot();
-            
+        var workingFolder = GetWorkingFolder(ingestionContext.IngestId, imageIngest);
+
         // source is the main folder for storing downloaded image
-        var assetId = new AssetId(asset.Id.Customer, asset.Id.Space,
-            asset.Id.Asset.Replace("(", imageIngest.OpenBracketReplacement)
+        var assetId = new AssetId(ingestionContext.AssetId.Customer, ingestionContext.AssetId.Space,
+            ingestionContext.AssetId.Asset.Replace("(", imageIngest.OpenBracketReplacement)
                 .Replace(")", imageIngest.CloseBracketReplacement));
-        var source = TemplatedFolders.GenerateFolderTemplate(imageIngest.SourceTemplate, assetId, root: root);
+        var source = TemplatedFolders.GenerateFolderTemplate(imageIngest.SourceTemplate, assetId, root: workingFolder);
         return source;
     }
 }

src/protagonist/Engine/Ingest/Image/ImageServer/Clients/AppetiserClient.cs

@@ -23,7 +23,7 @@ public class AppetiserClient : IAppetiserClient
 
     public async Task<IAppetiserResponse> GenerateJP2(
         IngestionContext context, 
-        AssetId modifiedAssetId, 
+        AssetId modifiedAssetId,
         CancellationToken cancellationToken = default)
     {
         var requestModel = CreateModel(context, modifiedAssetId);
@@ -58,7 +58,7 @@ private AppetiserRequestModel CreateModel(IngestionContext context, AssetId modi
     {
         var requestModel = new AppetiserRequestModel
         {
-            Destination = GetJP2FilePath(modifiedAssetId, true),
+            Destination = GetJP2FilePath(modifiedAssetId, context.IngestId, true),
             Operation = "image-only",
             Optimisation = "kdu_max",
             Origin = context.Asset.Origin,
@@ -76,23 +76,24 @@ private string GetRelativeLocationOnDisk(IngestionContext context, AssetId modif
         var extension = assetOnDisk.EverythingAfterLast('.');
 
         // this is to get it working nice locally as appetiser/tizer root needs to be unix + relative to it
-        var imageProcessorRoot = engineSettings.ImageIngest.GetRoot(true);
+
+        var imageProcessorRoot = ImageIngestionHelpers.GetWorkingFolder(context.IngestId, engineSettings.ImageIngest, true);
         var unixPath = TemplatedFolders.GenerateTemplateForUnix(engineSettings.ImageIngest.SourceTemplate, modifiedAssetId,
             root: imageProcessorRoot);
 
         unixPath += $"/{modifiedAssetId.Asset}.{extension}";
         return unixPath;
     }
 
-    public string GetJP2FilePath(AssetId assetId, bool forImageProcessor)
+    public string GetJP2FilePath(AssetId assetId, string ingestId, bool forImageProcessor)
     {
         // Appetiser/Tizer want unix paths relative to mount share.
         // This logic allows handling when running locally on win/unix and when deployed to unix
         var destFolder = forImageProcessor
             ? TemplatedFolders.GenerateTemplateForUnix(engineSettings.ImageIngest.DestinationTemplate,
-                assetId, root: engineSettings.ImageIngest.GetRoot(true))
+                assetId, root: ImageIngestionHelpers.GetWorkingFolder(ingestId, engineSettings.ImageIngest, true))
             : TemplatedFolders.GenerateFolderTemplate(engineSettings.ImageIngest.DestinationTemplate,
-                assetId, root: engineSettings.ImageIngest.GetRoot());
+                assetId, root: ImageIngestionHelpers.GetWorkingFolder(ingestId, engineSettings.ImageIngest));
 
         return $"{destFolder}{assetId.Asset}.jp2";
     }

src/protagonist/Engine/Ingest/Image/ImageServer/Clients/IAppetiserClient.cs

@@ -21,7 +21,8 @@ public interface IAppetiserClient
     /// Retrieves a JP2 filepath for an image
     /// </summary>
     /// <param name="assetId">The asset id used to retrieve the JP2 filepath</param>
+    /// <param name="ingestId">The id for the ingest operation associated with this image</param>
     /// <param name="forImageProcessor">Whether this is for the image processor or not</param>
     /// <returns></returns>
-    public string GetJP2FilePath(AssetId assetId, bool forImageProcessor);
+    public string GetJP2FilePath(AssetId assetId, string ingestId, bool forImageProcessor);
 }

src/protagonist/Engine/Ingest/Image/ImageServer/ImageServerClient.cs

@@ -53,11 +53,11 @@ public async Task<bool> ProcessImage(IngestionContext context)
         var modifiedAssetId = new AssetId(context.AssetId.Customer, context.AssetId.Space,
             context.AssetId.Asset.Replace("(", engineSettings.ImageIngest.OpenBracketReplacement)
                 .Replace(")", engineSettings.ImageIngest.CloseBracketReplacement));
-        var (dest, thumb) = CreateRequiredFolders(modifiedAssetId);
+        var (dest, thumb) = CreateRequiredFolders(modifiedAssetId, context.IngestId);
 
         try
         {
-            var flags = new ImageProcessorFlags(context, appetiserClient.GetJP2FilePath(modifiedAssetId, false));
+            var flags = new ImageProcessorFlags(context, appetiserClient.GetJP2FilePath(modifiedAssetId, context.IngestId, false));
             logger.LogDebug("Got flags '{@Flags}' for {AssetId}", flags, context.AssetId);
             var responseModel = await appetiserClient.GenerateJP2(context, modifiedAssetId);
 
@@ -89,16 +89,16 @@ public async Task<bool> ProcessImage(IngestionContext context)
         }
     }
 
-    private (string dest, string thumb) CreateRequiredFolders(AssetId assetId)
+    private (string dest, string thumb) CreateRequiredFolders(AssetId assetId, string ingestId)
     {
         var imageIngest = engineSettings.ImageIngest;
-        var root = imageIngest.GetRoot();
+        var workingFolder = ImageIngestionHelpers.GetWorkingFolder(ingestId, imageIngest);
 
         // dest is the folder where appetiser will copy output to
-        var dest = TemplatedFolders.GenerateFolderTemplate(imageIngest.DestinationTemplate, assetId, root: root);
+        var dest = TemplatedFolders.GenerateFolderTemplate(imageIngest.DestinationTemplate, assetId, root: workingFolder);
 
         // thumb is the folder where generated thumbnails will be output to
-        var thumb = TemplatedFolders.GenerateFolderTemplate(imageIngest.ThumbsTemplate, assetId, root: root);
+        var thumb = TemplatedFolders.GenerateFolderTemplate(imageIngest.ThumbsTemplate, assetId, root: workingFolder);
 
         fileSystem.CreateDirectory(dest);
         fileSystem.CreateDirectory(thumb);