Merge pull request #862 from dlcs/feature/delivery_channel_order

Ensure ImageDeliveryChannels are ordered

src/protagonist/API/Features/Image/Ingest/AssetProcessor.cs

@@ -49,10 +49,9 @@ public async Task<ProcessAssetResult> Process(AssetBeforeProcessing assetBeforeP
         Func<Asset, Task>? 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<ProcessAssetResult> Process(AssetBeforeProcessing assetBeforeP
                 return new ProcessAssetResult
                 {
                     Result = ModifyEntityResult<Asset>.Failure(
-                        "Delivery channels are required when updating an existing Asset via PUT",
+                        "Delivery channels are required when updating an existing Asset",
                         WriteResult.BadRequest
                     )
                 };

src/protagonist/API/Features/Image/Ingest/DeliveryChannelProcessor.cs

@@ -45,12 +45,9 @@ public async Task<bool> 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<bool> SetImageDeliveryChannels(Asset asset, DeliveryChannelsB
     private async Task<DeliveryChannelPolicy> 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)

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);
+    }
+}

src/protagonist/DLCS.Repository/Assets/AssetQueryX.cs

@@ -26,7 +26,7 @@ public static IQueryable<Asset> AsOrderedAssetQuery(this IQueryable<Asset> 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.
     /// </summary>
-    public static IQueryable<Asset> AsOrderedAssetQuery(this IQueryable<Asset> assetQuery, string? orderBy,
+    private static IQueryable<Asset> AsOrderedAssetQuery(this IQueryable<Asset> 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
@@ -115,11 +114,12 @@ public static IQueryable<Asset> ApplyAssetFilter(this IQueryable<Asset> queryabl
 
         return filtered;
     }
-    
+
     /// <summary>
     /// Include asset delivery channels and their associated policies.
     /// </summary>
     public static IQueryable<Asset> IncludeDeliveryChannelsWithPolicy(this IQueryable<Asset> assetQuery)
-        => assetQuery.Include(a => a.ImageDeliveryChannels)
+        => assetQuery
+            .Include(a => a.ImageDeliveryChannels.OrderBy(idc => idc.Channel))
             .ThenInclude(dc => dc.DeliveryChannelPolicy);
 }