Create asset modified cleanup handler

docs/rfcs/017-asset-modified-cleanup.md

@@ -106,14 +106,16 @@ This is that the delivery channel stays the same, but the id of the policy has c
 
 - iiif-img changed
   - `info.json` needs removed
+  - If it moves to a `use-original` policy, the derivative asset can be removed
+  - If it moves away from `use-original`, then the `/original` asset can also be removed, provided there isn't a `file` channel
 - Thumbs changed
   - Thumbs need to be removed that are no longer required.
   - `s.json` and asset application metadata should be updated - `s.json` should be updated by the reingest
   - `info.json` removed
 - iiif-av changed
   - Old transcode derivative removed if the file extension is no longer required
 - File changed
-  - The asset at origin should be removed if there's an asset on the `/original` path - should only be removed if `iiif-img` is not using it
+  - do nothing
 
 #### Roles changed
 
@@ -126,15 +128,15 @@ The policy data being updated can be found from the date that the delivery chann
 - iiif-img changed
   - `info.json` needs removed
   - If it moves to a `use-original` policy, the derivative asset can be removed
-  - If it moves awa from `use-original`, then the `/original` asset can also be removed, provided there isn't a `file` channel
+  - If it moves away from `use-original`, then the `/original` asset can also be removed, provided there isn't a `file` channel
 - Thumbs changed
   - Thumbs need to be removed that are no longer required.
   - `s.json` and asset application metadata should be updated - `s.json` should be updated by the reingest
   - `info.json` removed
 - iiif-av changed
   - Old transcode derivative removed if the file extension is no longer required
 - File changed
-  - The asset at origin should be removed if there's an asset on the `/original` path - should only be removed if `iiif-img` is not using it
+  - do nothing
 
   ## General comments
 

src/protagonist/API.Client/DlcsClient.cs

@@ -118,7 +118,7 @@ public async Task<HydraCollection<Image>> GetSpaceImages(int page, int pageSize,
             var response = await httpClient.GetAsync(url);
             return await response.ReadAsHydraResponseAsync<CustomerStorage>(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<bool> 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)
@@ -226,7 +226,7 @@ public async Task<bool> 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)
@@ -317,7 +317,49 @@ public async Task<CustomerQueue> GetQueue()
         var queue = await response.ReadAsHydraResponseAsync<CustomerQueue>(jsonSerializerSettings);
         return queue;  
     }
+
+    public async Task<IEnumerable<NamedQuery>> GetNamedQueries(bool includeGlobal)
+    {
+        var url = $"customers/{currentUser.GetCustomerId()}/namedQueries";
+        var response = await httpClient.GetAsync(url);
+        var namedQueries = await response.ReadAsHydraResponseAsync<HydraCollection<NamedQuery>>(jsonSerializerSettings);
+
+        return includeGlobal
+            ? namedQueries.Members
+            : namedQueries.Members.Where(nq => nq.Global == false);
+    }
+
+    public async Task<bool> DeleteNamedQuery(string namedQueryId)
+    {
+        var url = $"customers/{currentUser.GetCustomerId()}/namedQueries/{namedQueryId}";
+        try
+        {
+            await httpClient.DeleteAsync(url);
+            return true;
+        }
+        catch (Exception ex)
+        {
+            logger.LogError(ex, "Error deleting named query '{NamedQueryId}'", namedQueryId);
+            return false;
+        }
+    }
 
+    public async Task<NamedQuery?> UpdateNamedQuery(string namedQueryId, string template)
+    {
+        var url = $"customers/{currentUser.GetCustomerId()}/namedQueries/{namedQueryId}";
+        var response = await httpClient.PutAsync(url, ApiBody(new NamedQuery(){ Template = template }));
+        var updatedNamedQuery = await response.ReadAsHydraResponseAsync<NamedQuery>(jsonSerializerSettings);
+        return updatedNamedQuery;
+    }
+
+    public async Task<NamedQuery> CreateNamedQuery(NamedQuery newNamedQuery)
+    {
+        var url = $"customers/{currentUser.GetCustomerId()}/namedQueries";
+        var response = await httpClient.PostAsync(url, ApiBody(newNamedQuery));
+        var namedQuery = await response.ReadAsHydraResponseAsync<NamedQuery>(jsonSerializerSettings);
+        return namedQuery;
+    }
+
     private HttpContent ApiBody(JsonLdBase apiObject)
     {
         var jsonString = JsonConvert.SerializeObject(apiObject, jsonSerializerSettings);

src/protagonist/API.Client/IDlcsClient.cs

@@ -34,4 +34,8 @@ Task<HydraCollection<Image>> GetSpaceImages(int page, int pageSize, int spaceId,
     Task<bool> TestBatch(int batchId);
     Task<HydraCollection<Image>> GetBatchImages(int batchId, int page, int pageSize);
     Task<CustomerQueue> GetQueue();
+    Task<IEnumerable<NamedQuery>> GetNamedQueries(bool includeGlobal);
+    Task<bool> DeleteNamedQuery(string namedQueryId);
+    Task<NamedQuery?> UpdateNamedQuery(string namedQueryId, string template);
+    Task<NamedQuery> CreateNamedQuery(NamedQuery newNamedQuery);
 }

src/protagonist/API.Tests/Infrastructure/Messaging/AssetModificationRecordTests.cs

@@ -37,19 +37,22 @@ public void Create_SetsCorrectFields()
         notification.Before.Should().BeNull();
     }
 
-    [Fact]
-    public void Update_SetsCorrectFields()
+    [Theory]
+    [InlineData(true)]
+    [InlineData(false)]
+    public void Update_SetsCorrectFields(bool engineNotified)
     {
         // Arrange
         var before = new Asset { Id = new AssetId(1, 2, "foo") };
         var after = new Asset { Id = new AssetId(1, 2, "foo"), MaxUnauthorised = 10 };
 
         // Act
-        var notification = AssetModificationRecord.Update(before, after);
+        var notification = AssetModificationRecord.Update(before, after, engineNotified);
 
         // Assert
         notification.ChangeType.Should().Be(ChangeType.Update);
         notification.Before.Should().Be(before);
         notification.After.Should().Be(after);
+        notification.EngineNotified.Should().Be(engineNotified);
     }
 }

src/protagonist/API.Tests/Infrastructure/Messaging/AssetNotificationSenderTests.cs

@@ -1,7 +1,6 @@
 using System.Collections.Generic;
 using System.Linq;
 using System.Threading;
-using System.Threading.Tasks;
 using API.Infrastructure.Messaging;
 using DLCS.AWS.SNS;
 using DLCS.Core.Types;
@@ -33,7 +32,7 @@ public async Task SendAssetModifiedMessage_Single_SendsNotification_IfUpdate()
     {
         // Arrange
         var assetModifiedRecord =
-            AssetModificationRecord.Update(new Asset(new AssetId(1, 2, "foo")), new Asset(new AssetId(1, 2, "bar")));
+            AssetModificationRecord.Update(new Asset(new AssetId(1, 2, "foo")), new Asset(new AssetId(1, 2, "bar")), true);
 
         // Act
         await sut.SendAssetModifiedMessage(assetModifiedRecord, CancellationToken.None);
@@ -76,7 +75,7 @@ public async Task SendAssetModifiedMessage_Single_SendsNotification_IfDelete()
         A.CallTo(() =>
             topicPublisher.PublishToAssetModifiedTopic(
                 A<IReadOnlyList<AssetModifiedNotification>>.That.Matches(n =>
-                    n.Single().ChangeType == ChangeType.Delete && n.Single().MessageContents.Contains(customerName)),
+                    n.Single().Attributes.Values.Contains(ChangeType.Delete.ToString()) && n.Single().MessageContents.Contains(customerName)),
                 A<CancellationToken>._)).MustHaveHappened();
     }
 
@@ -102,7 +101,7 @@ public async Task SendAssetModifiedMessage_Multiple_SendsNotification_IfDelete()
             topicPublisher.PublishToAssetModifiedTopic(
                 A<IReadOnlyList<AssetModifiedNotification>>.That.Matches(n =>
                     n.Count == 2 && n.All(m =>
-                        m.ChangeType == ChangeType.Delete && m.MessageContents.Contains(customerName))),
+                        n.First().Attributes.Values.Contains(ChangeType.Delete.ToString()) && m.MessageContents.Contains(customerName))),
                 A<CancellationToken>._)).MustHaveHappened();
     }
 }

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<DLCS.HydraModel.CustomerQueue>();
+        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()
     {

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

@@ -54,14 +54,27 @@ 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";
 
         // 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);
     }

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);
 
     /// <summary>
     /// Converts from legacy format to new format
     /// </summary>
     /// <param name="image">The image to convert should be emulated and translated into delivery channels</param>
     /// <returns>A converted image</returns>
-    public static T VerifyAndConvertToModernFormat<T>(T image)
+    public static T VerifyAndConvertToModernFormat<T>(T image, ILogger? logger = null)
         where T : Image
     {
         if (image.Origin.IsNullOrEmpty())
@@ -30,6 +34,7 @@ public static T VerifyAndConvertToModernFormat<T>(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>(T image)
 
         if (image.MaxUnauthorised is null or 0 && image.Roles.IsNullOrEmpty())
         {
+            logger?.LogLegacyUsage("MaxUnauthorised");
             image.MaxUnauthorised = -1;
         }
 

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

@@ -44,16 +44,16 @@ public AssetProcessor(
     /// </param>
     /// <param name="requiresReingestPreSave">Optional delegate for modifying asset prior to saving</param>
     /// <param name="cancellationToken">Current cancellation token</param>
+    /// <param name="isPriorityQueue">Whether the request is for the priority queue or not</param>
     public async Task<ProcessAssetResult> Process(AssetBeforeProcessing assetBeforeProcessing, bool mustExist, bool alwaysReingest, bool isBatchUpdate, 
         Func<Asset, Task>? requiresReingestPreSave = null, 
         CancellationToken cancellationToken = default)
     {
-        Asset? existingAsset;
         try
         {
-            existingAsset = await assetRepository.GetAsset(assetBeforeProcessing.Asset.Id, true);
+            var assetFromDatabase = await assetRepository.GetAsset(assetBeforeProcessing.Asset.Id, true, true);
 
-            if (existingAsset == null)
+            if (assetFromDatabase == null)
             {
                 if (mustExist)
                 {
@@ -102,9 +102,10 @@ public async Task<ProcessAssetResult> Process(AssetBeforeProcessing assetBeforeP
                     )
                 };
             }
-
+
+            var existingAsset = assetFromDatabase?.Clone();
             var assetPreparationResult =
-                AssetPreparer.PrepareAssetForUpsert(existingAsset, assetBeforeProcessing.Asset, false, isBatchUpdate,
+                AssetPreparer.PrepareAssetForUpsert(assetFromDatabase, assetBeforeProcessing.Asset, false, isBatchUpdate,
                     settings.RestrictedAssetIdCharacters);
 
             if (!assetPreparationResult.Success)
@@ -115,11 +116,11 @@ public async Task<ProcessAssetResult> Process(AssetBeforeProcessing assetBeforeP
                         WriteResult.FailedValidation)
                 };
             }
-
+            
             var updatedAsset = assetPreparationResult.UpdatedAsset!; // this is from Database
             var requiresEngineNotification = assetPreparationResult.RequiresReingest || alwaysReingest;
 
-            var deliveryChannelChanged = await deliveryChannelProcessor.ProcessImageDeliveryChannels(existingAsset,
+            var deliveryChannelChanged = await deliveryChannelProcessor.ProcessImageDeliveryChannels(assetFromDatabase,
                 updatedAsset, assetBeforeProcessing.DeliveryChannelsBeforeProcessing);
             if (deliveryChannelChanged)
             {
@@ -140,14 +141,14 @@ public async Task<ProcessAssetResult> Process(AssetBeforeProcessing assetBeforeP
                 updatedAsset.MarkAsFinished();
             }
 
-            var assetAfterSave = await assetRepository.Save(updatedAsset, existingAsset != null, cancellationToken);
+            var assetAfterSave = await assetRepository.Save(updatedAsset, assetFromDatabase != null, cancellationToken);
 
             return new ProcessAssetResult
             {
                 ExistingAsset = existingAsset,
                 RequiresEngineNotification = requiresEngineNotification,
                 Result = ModifyEntityResult<Asset>.Success(assetAfterSave,
-                    existingAsset == null ? WriteResult.Created : WriteResult.Updated)
+                    assetFromDatabase == null ? WriteResult.Created : WriteResult.Updated)
             };
         }
         catch (APIException apiEx)
@@ -175,4 +176,4 @@ public class ProcessAssetResult
     public bool RequiresEngineNotification { get; set; }
 
     public bool IsSuccess => Result.IsSuccess;
-}
+}