diff --git a/src/protagonist/API.Tests/Integration/ModifyAssetTests.cs b/src/protagonist/API.Tests/Integration/ModifyAssetTests.cs index be2ca3aa8..e857adcc4 100644 --- a/src/protagonist/API.Tests/Integration/ModifyAssetTests.cs +++ b/src/protagonist/API.Tests/Integration/ModifyAssetTests.cs @@ -39,7 +39,7 @@ public class ModifyAssetTests : IClassFixture> { private readonly DlcsContext dbContext; private readonly HttpClient httpClient; - private static readonly IAssetNotificationSender NotificationSender = A.Fake(); + private static readonly IAssetNotificationSender AssetNotificationSender = A.Fake(); private readonly IAmazonS3 amazonS3; private static readonly IEngineClient EngineClient = A.Fake(); @@ -57,7 +57,7 @@ public ModifyAssetTests( .WithLocalStack(storageFixture.LocalStackFixture) .WithTestServices(services => { - services.AddSingleton(_ => NotificationSender); + services.AddSingleton(_ => AssetNotificationSender); services.AddScoped(_ => EngineClient); services.AddAuthentication("API-Test") .AddScheme( @@ -149,6 +149,46 @@ public async Task Put_NewImageAsset_Creates_Asset_WithDeliveryChannelsSetToNone( i.DeliveryChannelPolicyId == KnownDeliveryChannelPolicies.None); } + [Fact] + public async Task Put_NewImageAsset_Creates_Asset_WithDeliveryChannelsSetToDefault() + { + var customerAndSpace = await CreateCustomerAndSpace(); + + var assetId = new AssetId(customerAndSpace.customer, customerAndSpace.space, nameof(Put_NewImageAsset_Creates_Asset_WithDeliveryChannelsSetToDefault)); + var hydraImageBody = $@"{{ + ""@type"": ""Image"", + ""origin"": ""https://example.org/{assetId.Asset}.tiff"", + ""family"": ""I"", + ""mediaType"": ""image/tiff"", + ""deliveryChannels"": [ + {{ + ""channel"": ""default"" + }}] + }}"; + A.CallTo(() => + EngineClient.SynchronousIngest( + A.That.Matches(r => r.Id == assetId), + A._)) + .Returns(HttpStatusCode.OK); + + // act + var content = new StringContent(hydraImageBody, Encoding.UTF8, "application/json"); + var response = await httpClient.AsCustomer(customerAndSpace.customer).PutAsync(assetId.ToApiResourcePath(), content); + + // assert + response.StatusCode.Should().Be(HttpStatusCode.Created); + response.Headers.Location.PathAndQuery.Should().Be(assetId.ToApiResourcePath()); + + var asset = dbContext.Images.Include(i => i.ImageDeliveryChannels).Single(x => x.Id == assetId); + asset.Id.Should().Be(assetId); + asset.MaxUnauthorised.Should().Be(-1); + asset.ImageDeliveryChannels + .Should().HaveCount(2).And.Contain(i => + i.Channel == AssetDeliveryChannels.Image && + i.DeliveryChannelPolicyId == KnownDeliveryChannelPolicies.ImageDefault).And + .Contain(i => i.Channel == AssetDeliveryChannels.Thumbnails); + } + [Fact] public async Task Put_NewImageAsset_Creates_Asset_WithCustomDefaultDeliveryChannel() { @@ -1060,10 +1100,32 @@ public async Task Put_Existing_Asset_ClearsError_AndMarksAsIngesting() } [Fact] - public async Task Put_Existing_Asset_Returns400_IfDeliveryChannelsNull() + public async Task Put_Existing_Asset_ReturnsPreviousDeliveryChannels_IfDeliveryChannelsAreNull() { var assetId = AssetIdGenerator.GetAssetId(); - await dbContext.Images.AddTestAsset(assetId); + + var thumbsPolicy = dbContext.DeliveryChannelPolicies.Single(dcp => dcp.Name == "example-thumbs-policy"); + + var deliveryChannels = new List() + { + new() + { + Channel = AssetDeliveryChannels.File, + DeliveryChannelPolicyId = KnownDeliveryChannelPolicies.FileNone + }, + new() + { + Channel = AssetDeliveryChannels.Thumbnails, + DeliveryChannelPolicyId = thumbsPolicy.Id + }, + new() + { + Channel = AssetDeliveryChannels.Image, + DeliveryChannelPolicyId = KnownDeliveryChannelPolicies.ImageDefault + } + }; + + await dbContext.Images.AddTestAsset(assetId, imageDeliveryChannels: deliveryChannels); await dbContext.SaveChangesAsync(); var hydraImageBody = $@"{{ @@ -1073,14 +1135,80 @@ public async Task Put_Existing_Asset_Returns400_IfDeliveryChannelsNull() ""mediaType"": ""image/tiff"" }}"; + A.CallTo(() => + EngineClient.SynchronousIngest( + A.That.Matches(r => r.Id == assetId), + A._)) + .Returns(HttpStatusCode.OK); + + // act + var content = new StringContent(hydraImageBody, Encoding.UTF8, "application/json"); + var response = await httpClient.AsCustomer(99).PutAsync(assetId.ToApiResourcePath(), content); + + // assert + response.StatusCode.Should().Be(HttpStatusCode.OK); + + var asset = dbContext.Images.Include(i => i.ImageDeliveryChannels).Single(x => x.Id == assetId); + asset.Id.Should().Be(assetId); + asset.MaxUnauthorised.Should().Be(-1); + asset.ImageDeliveryChannels.Should().HaveCount(3).And.Contain(i => + i.Channel == AssetDeliveryChannels.Image && + i.DeliveryChannelPolicyId == KnownDeliveryChannelPolicies.ImageDefault).And + .Contain(i => i.Channel == AssetDeliveryChannels.Thumbnails && i.DeliveryChannelPolicyId == thumbsPolicy.Id) + .And.Contain(i => + i.Channel == AssetDeliveryChannels.File && + i.DeliveryChannelPolicyId == KnownDeliveryChannelPolicies.FileNone); + } + + [Fact] + public async Task Put_Existing_Asset_ReturnsDefaultDeliveryChannels_IfDeliveryChannelIsDefault() + { + var assetId = AssetIdGenerator.GetAssetId(); + + var deliveryChannels = new List() + { + new() + { + Channel = AssetDeliveryChannels.File, + DeliveryChannelPolicyId = KnownDeliveryChannelPolicies.FileNone + } + }; + + await dbContext.Images.AddTestAsset(assetId, imageDeliveryChannels: deliveryChannels); + await dbContext.SaveChangesAsync(); + + var hydraImageBody = $@"{{ + ""@type"": ""Image"", + ""origin"": ""https://example.org/{assetId.Asset}.tiff"", + ""family"": ""I"", + ""mediaType"": ""image/tiff"", + ""deliveryChannels"": [ + {{ + ""channel"": ""default"" + }}] + }}"; + + A.CallTo(() => + EngineClient.SynchronousIngest( + A.That.Matches(r => r.Id == assetId), + A._)) + .Returns(HttpStatusCode.OK); + // act var content = new StringContent(hydraImageBody, Encoding.UTF8, "application/json"); var response = await httpClient.AsCustomer(99).PutAsync(assetId.ToApiResourcePath(), content); // assert - response.StatusCode.Should().Be(HttpStatusCode.BadRequest); - var body = await response.Content.ReadAsStringAsync(); - body.Should().Contain("Delivery channels are required when updating an existing Asset"); + response.StatusCode.Should().Be(HttpStatusCode.OK); + + var asset = dbContext.Images.Include(i => i.ImageDeliveryChannels).Single(x => x.Id == assetId); + asset.Id.Should().Be(assetId); + asset.MaxUnauthorised.Should().Be(-1); + asset.ImageDeliveryChannels + .Should().HaveCount(2).And.Contain(i => + i.Channel == AssetDeliveryChannels.Image && + i.DeliveryChannelPolicyId == KnownDeliveryChannelPolicies.ImageDefault).And + .Contain(i => i.Channel == AssetDeliveryChannels.Thumbnails); } [Fact] @@ -2331,7 +2459,7 @@ public async Task Delete_RemovesAssetAndAssociatedEntities_FromDb() ec.Customer == 99 && ec.Scope == "1" && ec.Type == KnownEntityCounters.SpaceImages); dbSpaceCounter.Next.Should().Be(currentSpaceImagesCounter - 1); - A.CallTo(() => NotificationSender.SendAssetModifiedMessage( + A.CallTo(() => AssetNotificationSender.SendAssetModifiedMessage( A.That.Matches(r => r.ChangeType == ChangeType.Delete && r.DeleteFrom == ImageCacheType.None), A._)).MustHaveHappened(); } @@ -2392,7 +2520,7 @@ public async Task Delete_NotifiesForCdnAndInternalCacheRemoval_FromAssetNotified ec.Customer == 99 && ec.Scope == "1" && ec.Type == KnownEntityCounters.SpaceImages); dbSpaceCounter.Next.Should().Be(currentSpaceImagesCounter - 1); - A.CallTo(() => NotificationSender.SendAssetModifiedMessage( + A.CallTo(() => AssetNotificationSender.SendAssetModifiedMessage( A.That.Matches(r => r.ChangeType == ChangeType.Delete && (int)r.DeleteFrom == 12), A._)).MustHaveHappened(); @@ -2450,7 +2578,7 @@ public async Task Delete_RemovesAssetWithoutImageLocation_FromDb() ec.Customer == 99 && ec.Scope == "1" && ec.Type == KnownEntityCounters.SpaceImages); dbSpaceCounter.Next.Should().Be(currentSpaceImagesCounter - 1); - A.CallTo(() => NotificationSender.SendAssetModifiedMessage( + A.CallTo(() => AssetNotificationSender.SendAssetModifiedMessage( A.That.Matches(r => r.ChangeType == ChangeType.Delete), A._)).MustHaveHappened(); } @@ -2486,7 +2614,7 @@ public async Task Delete_IncludesImageDeliveryChannels_InAssetModifiedMessage() // Assert response.StatusCode.Should().Be(HttpStatusCode.NoContent); - A.CallTo(() => NotificationSender.SendAssetModifiedMessage( + A.CallTo(() => AssetNotificationSender.SendAssetModifiedMessage( A.That.Matches(r => r.ChangeType == ChangeType.Delete && r.Before.ImageDeliveryChannels.Count == 3), diff --git a/src/protagonist/API/Features/Image/AssetBeforeProcessing.cs b/src/protagonist/API/Features/Image/AssetBeforeProcessing.cs index 6df058193..b01623d34 100644 --- a/src/protagonist/API/Features/Image/AssetBeforeProcessing.cs +++ b/src/protagonist/API/Features/Image/AssetBeforeProcessing.cs @@ -4,7 +4,7 @@ namespace API.Features.Image; public class AssetBeforeProcessing { - public AssetBeforeProcessing(Asset asset, DeliveryChannelsBeforeProcessing[] deliveryChannelsBeforeProcessing) + public AssetBeforeProcessing(Asset asset, DeliveryChannelsBeforeProcessing[]? deliveryChannelsBeforeProcessing) { Asset = asset; DeliveryChannelsBeforeProcessing = deliveryChannelsBeforeProcessing; @@ -12,7 +12,7 @@ public AssetBeforeProcessing(Asset asset, DeliveryChannelsBeforeProcessing[] del public Asset Asset { get; } - public DeliveryChannelsBeforeProcessing[] DeliveryChannelsBeforeProcessing { get; } + public DeliveryChannelsBeforeProcessing[]? DeliveryChannelsBeforeProcessing { get; set; } } /// diff --git a/src/protagonist/API/Features/Image/ImageController.cs b/src/protagonist/API/Features/Image/ImageController.cs index 7cbe2df72..3717b7739 100644 --- a/src/protagonist/API/Features/Image/ImageController.cs +++ b/src/protagonist/API/Features/Image/ImageController.cs @@ -288,7 +288,7 @@ private Task PutOrPatchAsset(int customerId, int spaceId, string // See https://github.com/dlcs/protagonist/issues/338 var method = hydraAsset is ImageWithFile ? "PUT" : Request.Method; - var deliveryChannelsBeforeProcessing = (hydraAsset.DeliveryChannels ?? Array.Empty()) + var deliveryChannelsBeforeProcessing = hydraAsset.DeliveryChannels? .Select(d => new DeliveryChannelsBeforeProcessing(d.Channel, d.Policy)).ToArray(); var assetBeforeProcessing = new AssetBeforeProcessing(asset, deliveryChannelsBeforeProcessing); diff --git a/src/protagonist/API/Features/Image/ImagesController.cs b/src/protagonist/API/Features/Image/ImagesController.cs index 8bf4f3c9c..943a669a5 100644 --- a/src/protagonist/API/Features/Image/ImagesController.cs +++ b/src/protagonist/API/Features/Image/ImagesController.cs @@ -125,7 +125,7 @@ public async Task PatchImages( { var asset = hydraImage.ToDlcsModel(customerId, spaceId); - var deliveryChannelsBeforeProcessing = (hydraImage.DeliveryChannels ?? Array.Empty()) + var deliveryChannelsBeforeProcessing = hydraImage.DeliveryChannels? .Select(d => new DeliveryChannelsBeforeProcessing(d.Channel, d.Policy)).ToArray(); var assetBeforeProcessing = new AssetBeforeProcessing(asset, deliveryChannelsBeforeProcessing); diff --git a/src/protagonist/API/Features/Image/Ingest/AssetProcessor.cs b/src/protagonist/API/Features/Image/Ingest/AssetProcessor.cs index 688dc1083..266b6784b 100644 --- a/src/protagonist/API/Features/Image/Ingest/AssetProcessor.cs +++ b/src/protagonist/API/Features/Image/Ingest/AssetProcessor.cs @@ -51,6 +51,7 @@ public async Task Process(AssetBeforeProcessing assetBeforeP try { var assetFromDatabase = await assetRepository.GetAsset(assetBeforeProcessing.Asset.Id, true, true); + var updatedDeliveryChannels = false; if (assetFromDatabase == null) { @@ -91,7 +92,15 @@ public async Task Process(AssetBeforeProcessing assetBeforeP counts.CustomerStorage.NumberOfStoredImages++; } - else if (assetBeforeProcessing.DeliveryChannelsBeforeProcessing.IsNullOrEmpty() && alwaysReingest) + else if (assetBeforeProcessing.DeliveryChannelsBeforeProcessing == null && alwaysReingest) + { + assetBeforeProcessing.DeliveryChannelsBeforeProcessing = assetFromDatabase.ImageDeliveryChannels + .Select(d => new DeliveryChannelsBeforeProcessing(d.Channel, d.DeliveryChannelPolicy.Name)) + .ToArray(); + + updatedDeliveryChannels = true; + } + else if (assetBeforeProcessing.DeliveryChannelsBeforeProcessing?.Length == 0 && alwaysReingest) { return new ProcessAssetResult { @@ -102,6 +111,7 @@ public async Task Process(AssetBeforeProcessing assetBeforeP }; } + var existingAsset = assetFromDatabase?.Clone(); var assetPreparationResult = AssetPreparer.PrepareAssetForUpsert(assetFromDatabase, assetBeforeProcessing.Asset, false, isBatchUpdate, @@ -120,7 +130,7 @@ public async Task Process(AssetBeforeProcessing assetBeforeP var requiresEngineNotification = assetPreparationResult.RequiresReingest || alwaysReingest; var deliveryChannelChanged = await deliveryChannelProcessor.ProcessImageDeliveryChannels(assetFromDatabase, - updatedAsset, assetBeforeProcessing.DeliveryChannelsBeforeProcessing); + updatedAsset, updatedDeliveryChannels, assetBeforeProcessing.DeliveryChannelsBeforeProcessing); if (deliveryChannelChanged) { requiresEngineNotification = true; diff --git a/src/protagonist/API/Features/Image/Ingest/DeliveryChannelProcessor.cs b/src/protagonist/API/Features/Image/Ingest/DeliveryChannelProcessor.cs index eb32c4648..0358a4578 100644 --- a/src/protagonist/API/Features/Image/Ingest/DeliveryChannelProcessor.cs +++ b/src/protagonist/API/Features/Image/Ingest/DeliveryChannelProcessor.cs @@ -33,10 +33,10 @@ public DeliveryChannelProcessor(IDefaultDeliveryChannelRepository defaultDeliver /// /// List of deliveryChannels submitted in body /// Boolean indicating whether asset requires processing Engine - public async Task ProcessImageDeliveryChannels(Asset? existingAsset, Asset updatedAsset, - DeliveryChannelsBeforeProcessing[] deliveryChannelsBeforeProcessing) + public async Task ProcessImageDeliveryChannels(Asset? existingAsset, Asset updatedAsset, + bool deliveryChannelsUpdated, DeliveryChannelsBeforeProcessing[]? deliveryChannelsBeforeProcessing) { - if (existingAsset == null || + if (existingAsset == null || deliveryChannelsUpdated || DeliveryChannelsRequireReprocessing(existingAsset, deliveryChannelsBeforeProcessing)) { try @@ -54,9 +54,8 @@ public async Task ProcessImageDeliveryChannels(Asset? existingAsset, Asset return false; } - private bool DeliveryChannelsRequireReprocessing(Asset originalAsset, DeliveryChannelsBeforeProcessing[] deliveryChannelsBeforeProcessing) + private bool DeliveryChannelsRequireReprocessing(Asset originalAsset, DeliveryChannelsBeforeProcessing[]? deliveryChannelsBeforeProcessing) { - // PUT prevents empty delivery channels from being passed here, but PATCH doesn't if (deliveryChannelsBeforeProcessing.IsNullOrEmpty()) return false; if (originalAsset.ImageDeliveryChannels.Count != deliveryChannelsBeforeProcessing.Length) return true; @@ -73,20 +72,20 @@ private bool DeliveryChannelsRequireReprocessing(Asset originalAsset, DeliveryCh return false; } - private async Task SetImageDeliveryChannels(Asset asset, DeliveryChannelsBeforeProcessing[] deliveryChannelsBeforeProcessing, bool isUpdate) + private async Task SetImageDeliveryChannels(Asset asset, DeliveryChannelsBeforeProcessing[]? deliveryChannelsBeforeProcessing, bool isUpdate) { var assetId = asset.Id; + var isDefaultChannel = deliveryChannelsBeforeProcessing?.Any(d => d.Channel == AssetDeliveryChannels.Default) ?? + false; - if (!isUpdate) + // if it's a new asset, or has "default" specified, figure out the delivery channels based on media type + if (!isUpdate || isDefaultChannel) { - logger.LogTrace("Asset {AssetId} is new, resetting ImageDeliveryChannels", assetId); + logger.LogTrace("Setting delivery channels for asset {AssetId}", assetId); asset.ImageDeliveryChannels = new List(); - - // Only valid for creation - set image delivery channels to default values for media type - if (deliveryChannelsBeforeProcessing.IsNullOrEmpty()) + + if (deliveryChannelsBeforeProcessing.IsNullOrEmpty() || isDefaultChannel) { - logger.LogDebug("Asset {AssetId} is new, no deliveryChannels specified. Assigning defaults for mediaType", - assetId); await AddDeliveryChannelsForMediaType(asset); return true; } @@ -103,16 +102,19 @@ private async Task SetImageDeliveryChannels(Asset asset, DeliveryChannelsB var changeMade = false; var handledChannels = new List(); var assetImageDeliveryChannels = asset.ImageDeliveryChannels; + foreach (var deliveryChannel in deliveryChannelsBeforeProcessing) { handledChannels.Add(deliveryChannel.Channel); var deliveryChannelPolicy = await GetDeliveryChannelPolicy(asset, deliveryChannel); - var currentChannel = assetImageDeliveryChannels.SingleOrDefault(idc => idc.Channel == deliveryChannel.Channel); + var currentChannel = + assetImageDeliveryChannels.SingleOrDefault(idc => idc.Channel == deliveryChannel.Channel); // No current ImageDeliveryChannel for channel so this is an addition if (currentChannel == null) { - logger.LogTrace("Adding new deliveryChannel {DeliveryChannel}, Policy {PolicyName} to Asset {AssetId}", + logger.LogTrace( + "Adding new deliveryChannel {DeliveryChannel}, Policy {PolicyName} to Asset {AssetId}", deliveryChannel.Channel, deliveryChannelPolicy.Name, assetId); assetImageDeliveryChannels.Add(new ImageDeliveryChannel diff --git a/src/protagonist/API/Features/Queues/CustomerQueueController.cs b/src/protagonist/API/Features/Queues/CustomerQueueController.cs index 3785264c5..6588537b0 100644 --- a/src/protagonist/API/Features/Queues/CustomerQueueController.cs +++ b/src/protagonist/API/Features/Queues/CustomerQueueController.cs @@ -424,7 +424,7 @@ private static List CreateAssetsBeforeProcessing(int cust { var assetsBeforeProcessing = images.Members! .Select(i => new AssetBeforeProcessing(i.ToDlcsModel(customerId), - (i.DeliveryChannels ?? Array.Empty()) + i.DeliveryChannels? .Select(d => new DeliveryChannelsBeforeProcessing(d.Channel, d.Policy)).ToArray())).ToList(); return assetsBeforeProcessing; } diff --git a/src/protagonist/DLCS.Model/Assets/AssetDeliveryChannels.cs b/src/protagonist/DLCS.Model/Assets/AssetDeliveryChannels.cs index 3f219fb98..7fee4d8e5 100644 --- a/src/protagonist/DLCS.Model/Assets/AssetDeliveryChannels.cs +++ b/src/protagonist/DLCS.Model/Assets/AssetDeliveryChannels.cs @@ -11,11 +11,12 @@ public static class AssetDeliveryChannels public const string Timebased = "iiif-av"; public const string File = "file"; public const string None = "none"; + public const string Default = "default"; /// /// All possible delivery channels /// - public static string[] All { get; } = { File, Timebased, Image, Thumbnails, None }; + public static string[] All { get; } = { File, Timebased, Image, Thumbnails, None, Default }; /// /// All possible delivery channels as a comma-delimited string @@ -72,6 +73,7 @@ public static bool IsChannelValidForMediaType(string deliveryChannel, string med Image => mediaType.StartsWith("image/"), Thumbnails => mediaType.StartsWith("image/"), Timebased => mediaType.StartsWith("video/") || mediaType.StartsWith("audio/"), + Default => true, // Default causes delivery channels to be selected based on media type File => true, // A file can be matched to any media type None => true, // Likewise for the 'none' channel _ when throwIfChannelUnknown => throw new ArgumentOutOfRangeException(nameof(deliveryChannel), deliveryChannel,