From 8120525e0f15358e39c6de587948ed4cf0bd84ca Mon Sep 17 00:00:00 2001 From: griffri Date: Fri, 22 Mar 2024 10:28:51 +0000 Subject: [PATCH 1/4] Remove InitialOrigin support from CSV uploader, update sample.csv --- .../Batches/Requests/IngestFromCsv.cs | 21 +++++++++---------- src/protagonist/Portal/wwwroot/csv/sample.csv | 10 ++++----- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/protagonist/Portal/Features/Batches/Requests/IngestFromCsv.cs b/src/protagonist/Portal/Features/Batches/Requests/IngestFromCsv.cs index 38edfa05b..f4206a377 100644 --- a/src/protagonist/Portal/Features/Batches/Requests/IngestFromCsv.cs +++ b/src/protagonist/Portal/Features/Batches/Requests/IngestFromCsv.cs @@ -197,7 +197,7 @@ public class ImageIngestModel { public static readonly string[] FieldNames = { - "Type", "Line", "Space", "ID", "Origin", "InitialOrigin", "Reference1", "Reference2", "Reference3", "Tags", + "Type", "Line", "Space", "ID", "Origin", "Reference1", "Reference2", "Reference3", "Tags", "Roles", "MaxUnauthorised", "NumberReference1", "NumberReference2", "NumberReference3" }; @@ -207,14 +207,13 @@ public class ImageIngestModel [Index(2)] public int Space { get; set; } [Index(3)] public string Id { get; set; } [Index(4)] public string Origin { get; set; } - [Index(5), NullValues("")] public string? InitialOrigin { get; set; } - [Index(6)] public string String1 { get; set; } - [Index(7)] public string String2 { get; set; } - [Index(8)] public string String3 { get; set; } - [Index(9)] public string Tags { get; set; } - [Index(10)] public string Roles { get; set; } - [Index(11)] public int? MaxUnauthorized { get; set; } - [Index(12)] public int? Number1 { get; set; } - [Index(13)] public int? Number2 { get; set; } - [Index(14)] public int? Number3 { get; set; } + [Index(5)] public string String1 { get; set; } + [Index(6)] public string String2 { get; set; } + [Index(7)] public string String3 { get; set; } + [Index(8)] public string Tags { get; set; } + [Index(9)] public string Roles { get; set; } + [Index(10)] public int? MaxUnauthorized { get; set; } + [Index(11)] public int? Number1 { get; set; } + [Index(12)] public int? Number2 { get; set; } + [Index(13)] public int? Number3 { get; set; } } diff --git a/src/protagonist/Portal/wwwroot/csv/sample.csv b/src/protagonist/Portal/wwwroot/csv/sample.csv index 1e9aeb54b..fa916fcce 100644 --- a/src/protagonist/Portal/wwwroot/csv/sample.csv +++ b/src/protagonist/Portal/wwwroot/csv/sample.csv @@ -1,5 +1,5 @@ -"Type","Line","Space","ID","Origin","InitialOrigin","Reference1","Reference2","Reference3","Tags","Roles","MaxUnauthorised","NumberReference1","NumberReference2","NumberReference3" -"Image","0","2","8b9be371-3e28-4b36-972b-29809224d3a6","https://wellcomelibrary.github.io/dlcsdemo/b18771476_0_0_6/assets/8b9be371-3e28-4b36-972b-29809224d3a6.jp2","","b18771476","","","","","-1","0","0","0" -"Image","1","2","714b54d9-a14e-4afb-aace-1941ee0ed2b6","https://wellcomelibrary.github.io/dlcsdemo/b18771476_0_0_6/assets/714b54d9-a14e-4afb-aace-1941ee0ed2b6.jp2","","b18771476","","","","","-1","0","1","0" -"Image","2","2","125911d4-c79c-4e85-9d67-1131033aec56","https://wellcomelibrary.github.io/dlcsdemo/b18771476_0_0_6/assets/125911d4-c79c-4e85-9d67-1131033aec56.jp2","","b18771476","","","","","-1","0","2","0" -"Image","3","2","0e3f6ada-276b-4e21-815c-127f738d0cfa","https://wellcomelibrary.github.io/dlcsdemo/b18771476_0_0_6/assets/0e3f6ada-276b-4e21-815c-127f738d0cfa.jp2","","b18771476","","","","","-1","0","3","0" \ No newline at end of file +"Type","Line","Space","ID","Origin","Reference1","Reference2","Reference3","Tags","Roles","MaxUnauthorised","NumberReference1","NumberReference2","NumberReference3" +"Image","0","2","8b9be371-3e28-4b36-972b-29809224d3a6","https://wellcomelibrary.github.io/dlcsdemo/b18771476_0_0_6/assets/8b9be371-3e28-4b36-972b-29809224d3a6.jp2","b18771476","","","","","-1","0","0","0" +"Image","1","2","714b54d9-a14e-4afb-aace-1941ee0ed2b6","https://wellcomelibrary.github.io/dlcsdemo/b18771476_0_0_6/assets/714b54d9-a14e-4afb-aace-1941ee0ed2b6.jp2","b18771476","","","","","-1","0","1","0" +"Image","2","2","125911d4-c79c-4e85-9d67-1131033aec56","https://wellcomelibrary.github.io/dlcsdemo/b18771476_0_0_6/assets/125911d4-c79c-4e85-9d67-1131033aec56.jp2","b18771476","","","","","-1","0","2","0" +"Image","3","2","0e3f6ada-276b-4e21-815c-127f738d0cfa","https://wellcomelibrary.github.io/dlcsdemo/b18771476_0_0_6/assets/0e3f6ada-276b-4e21-815c-127f738d0cfa.jp2","b18771476","","","","","-1","0","3","0" From 7601d3fbf661ee6302d61338745477ee064a9de4 Mon Sep 17 00:00:00 2001 From: griffri Date: Fri, 22 Mar 2024 11:11:54 +0000 Subject: [PATCH 2/4] Strip legacy Engine payload support --- .../DLCS.Core/Settings/DlcsSettings.cs | 5 - .../Messaging/EngineClientTests.cs | 94 ++------ .../DLCS.Repository/Messaging/EngineClient.cs | 6 - .../Messaging/LegacyJsonMessageHelpers.cs | 203 ------------------ .../Ingest/Handlers/IngestHandlerTests.cs | 67 ------ .../Models/LegacyIngestEventConverterTests.cs | 88 -------- .../Ingest/Models/LegacyIngestEventTests.cs | 67 ------ .../Integration/IngestResponseTests.cs | 63 +----- .../Engine/Ingest/AssetIngester.cs | 29 +-- .../Engine/Ingest/IngestController.cs | 21 +- .../Engine/Ingest/IngestHandler.cs | 26 +-- .../Engine/Ingest/Models/LegacyIngestEvent.cs | 51 ----- .../Models/LegacyIngestEventConverter.cs | 99 --------- 13 files changed, 26 insertions(+), 793 deletions(-) delete mode 100644 src/protagonist/DLCS.Repository/Messaging/LegacyJsonMessageHelpers.cs delete mode 100644 src/protagonist/Engine.Tests/Ingest/Models/LegacyIngestEventConverterTests.cs delete mode 100644 src/protagonist/Engine.Tests/Ingest/Models/LegacyIngestEventTests.cs delete mode 100644 src/protagonist/Engine/Ingest/Models/LegacyIngestEvent.cs delete mode 100644 src/protagonist/Engine/Ingest/Models/LegacyIngestEventConverter.cs diff --git a/src/protagonist/DLCS.Core/Settings/DlcsSettings.cs b/src/protagonist/DLCS.Core/Settings/DlcsSettings.cs index d58641ac5..a79837154 100644 --- a/src/protagonist/DLCS.Core/Settings/DlcsSettings.cs +++ b/src/protagonist/DLCS.Core/Settings/DlcsSettings.cs @@ -37,9 +37,4 @@ public class DlcsSettings /// URL format for generating manifests for single assets /// public string SingleAssetManifestTemplate { get; set; } - - /// - /// If true, the legacy/Deliverator message format is used for requests to Engine - /// - public bool UseLegacyEngineMessage { get; set; } } \ No newline at end of file diff --git a/src/protagonist/DLCS.Repository.Tests/Messaging/EngineClientTests.cs b/src/protagonist/DLCS.Repository.Tests/Messaging/EngineClientTests.cs index ebbc4e3ca..eb09b9d39 100644 --- a/src/protagonist/DLCS.Repository.Tests/Messaging/EngineClientTests.cs +++ b/src/protagonist/DLCS.Repository.Tests/Messaging/EngineClientTests.cs @@ -24,7 +24,8 @@ public class EngineClientTests private readonly IQueueLookup queueLookup; private readonly IQueueSender queueSender; private readonly HttpClient httpClient; - + private readonly EngineClient sut; + public EngineClientTests() { httpHandler = new ControllableHttpMessageHandler(); @@ -35,37 +36,17 @@ public EngineClientTests() queueLookup = A.Fake(); queueSender = A.Fake(); - } - - [Theory] - [InlineData(AssetFamily.File, 'F')] - [InlineData(AssetFamily.Image, 'I')] - [InlineData(AssetFamily.Timebased, 'T')] - public void SynchronousIngest_FailsToCallEngineWithLegacyModel_IfUseLegacyEngineMessageTrue( - AssetFamily family, char expected) - { - // Arrange - var asset = new Asset(AssetId.FromString("99/1/ingest-asset")) + + var engineClientOptions = Options.Create(new DlcsSettings { - Family = family - }; - - var ingestRequest = new IngestAssetRequest(asset.Id, DateTime.UtcNow); - HttpRequestMessage message = null; - httpHandler.RegisterCallback(r => message = r); - httpHandler.GetResponseMessage("{ \"engine\": \"hello\" }", HttpStatusCode.OK); - - var sut = GetSut(true); - - // Act - Action action = () => sut.SynchronousIngest(asset).Wait(); - - // Assert - action.Should().Throw().WithMessage("Legacy ingest events are no longer supported"); + EngineRoot = new Uri("http://engine.dlcs/") + }); + + sut = new EngineClient(queueLookup, queueSender, httpClient, engineClientOptions, new NullLogger()); } [Fact] - public async Task SynchronousIngest_CallsEngineWithCurrentModel_IfUseLegacyEngineMessageFalse() + public async Task SynchronousIngest_CallsEngine() { // Arrange var asset = new Asset(AssetId.FromString("99/1/ingest-asset")) @@ -81,8 +62,6 @@ public async Task SynchronousIngest_CallsEngineWithCurrentModel_IfUseLegacyEngin httpHandler.RegisterCallback(r => message = r); httpHandler.GetResponseMessage("{ \"engine\": \"hello\" }", HttpStatusCode.OK); - var sut = GetSut(false); - // Act var statusCode = await sut.SynchronousIngest(asset); @@ -102,32 +81,7 @@ public async Task SynchronousIngest_CallsEngineWithCurrentModel_IfUseLegacyEngin } [Fact] - public void AsynchronousIngest_FailsToQueueLegacyModel_IfUseLegacyEngineMessageTrue() - { - // Arrange - var asset = new Asset(AssetId.FromString("99/1/ingest-asset")) - { - Family = AssetFamily.Image - }; - - var ingestRequest = new IngestAssetRequest(asset.Id, DateTime.UtcNow); - - var sut = GetSut(true); - string jsonString = string.Empty; - A.CallTo(() => queueLookup.GetQueueNameForFamily(AssetFamily.Image, false)).Returns("test-queue"); - A.CallTo(() => queueSender.QueueMessage("test-queue", A._, A._)) - .Invokes((string _, string message, CancellationToken _) => jsonString = message) - .Returns(true); - - // Act - Action action = () => sut.AsynchronousIngest(asset).Wait(); - - // Assert - action.Should().Throw().WithMessage("Legacy ingest events are no longer supported"); - } - - [Fact] - public async Task AsynchronousIngest_QueuesMessageWithCurrentModel_IfUseLegacyEngineMessageFalse() + public async Task AsynchronousIngest_QueuesMessage() { // Arrange var asset = new Asset(AssetId.FromString("99/1/ingest-asset")) @@ -139,9 +93,8 @@ public async Task AsynchronousIngest_QueuesMessageWithCurrentModel_IfUseLegacyEn }; var ingestRequest = new IngestAssetRequest(asset.Id, DateTime.UtcNow); - var sut = GetSut(false); - - string jsonString = string.Empty; + + var jsonString = string.Empty; A.CallTo(() => queueLookup.GetQueueNameForFamily(AssetFamily.Image, false)).Returns("test-queue"); A.CallTo(() => queueSender.QueueMessage("test-queue", A._, A._)) .Invokes((string _, string message, CancellationToken _) => jsonString = message) @@ -163,14 +116,12 @@ public async Task AsynchronousIngest_QueuesMessageWithCurrentModel_IfUseLegacyEn [Fact] public async Task GetAllowedAvOptions_RetrievesAllowedAvPolicies() { - // Act - var sut = GetSut(false); - + // Arrange HttpRequestMessage message = null; httpHandler.RegisterCallback(r => message = r); httpHandler.GetResponseMessage("[\"video-mp4-480p\",\"video-webm-720p\",\"audio-mp3-128k\"]", HttpStatusCode.OK); - // Assert + // Act var returnedAvPolicyOptions = await sut.GetAllowedAvPolicyOptions(); // Assert @@ -183,14 +134,12 @@ public async Task GetAllowedAvOptions_RetrievesAllowedAvPolicies() [Fact] public async Task GetAllowedAvOptions_ReturnsNull_IfEngineAvPolicyEndpointUnreachable() { - // Act - var sut = GetSut(false); - + // Arrange HttpRequestMessage message = null; httpHandler.RegisterCallback(r => message = r); httpHandler.GetResponseMessage("Not found", HttpStatusCode.NotFound); - // Assert + // Act var returnedAvPolicyOptions = await sut.GetAllowedAvPolicyOptions(); // Assert @@ -198,15 +147,4 @@ public async Task GetAllowedAvOptions_ReturnsNull_IfEngineAvPolicyEndpointUnreac message.Method.Should().Be(HttpMethod.Get); returnedAvPolicyOptions.Should().BeNull(); } - - private EngineClient GetSut(bool useLegacyMessageFormat) - { - var options = Options.Create(new DlcsSettings - { - EngineRoot = new Uri("http://engine.dlcs/"), - UseLegacyEngineMessage = useLegacyMessageFormat - }); - - return new EngineClient(queueLookup, queueSender, httpClient, options, new NullLogger()); - } } \ No newline at end of file diff --git a/src/protagonist/DLCS.Repository/Messaging/EngineClient.cs b/src/protagonist/DLCS.Repository/Messaging/EngineClient.cs index cb75250e7..3c59fce95 100644 --- a/src/protagonist/DLCS.Repository/Messaging/EngineClient.cs +++ b/src/protagonist/DLCS.Repository/Messaging/EngineClient.cs @@ -155,12 +155,6 @@ public async Task SynchronousIngest(Asset asset, CancellationTok private string GetJsonString(Asset asset) { - // If running in legacy mode, the payload should contain the full Legacy JSON string - if (dlcsSettings.UseLegacyEngineMessage) - { - throw new InvalidOperationException("Legacy ingest events are no longer supported"); - } - var ingestAssetRequest = new IngestAssetRequest(asset.Id, DateTime.UtcNow); // Otherwise, it should contain only the Asset ID - for now, this is an Asset object containing just the ID diff --git a/src/protagonist/DLCS.Repository/Messaging/LegacyJsonMessageHelpers.cs b/src/protagonist/DLCS.Repository/Messaging/LegacyJsonMessageHelpers.cs deleted file mode 100644 index ed0d7abff..000000000 --- a/src/protagonist/DLCS.Repository/Messaging/LegacyJsonMessageHelpers.cs +++ /dev/null @@ -1,203 +0,0 @@ -using System; -using System.Collections.Generic; -using System.IO; -using System.Threading.Tasks; -using DLCS.Model.Assets; -using DLCS.Model.Messaging; -using Newtonsoft.Json; - -namespace DLCS.Repository.Messaging; - -/// -/// This is for temporary compatibility with legacy Engine, we need to send this request body to engine in -/// the format legacy engine expects. This signal doesn't have to look like this though in new Protagonist. -/// -internal static class LegacyJsonMessageHelpers -{ - public static async Task GetLegacyJsonString(Asset asset, bool derivativesOnly) - { - var stringParams = new Dictionary - { - ["id"] = asset.Id.ToString(), - ["customer"] = asset.Customer.ToString(), - ["space"] = asset.Space.ToString(), - ["image"] = AsJsonStringForMessaging(asset) - }; - - // we'll never set initialorigin in our limited first port of this - if (derivativesOnly) - { - stringParams["operation"] = "derivatives-only"; - } - - await using var stringWriter = new StringWriter(); - using (var jsonWriter = new JsonTextWriter(stringWriter)) - { - ToLegacyMessageJson(jsonWriter, "event::image-ingest", stringParams); - } - - var jsonString = stringWriter.ToString(); - return jsonString; - } - - private static string AsJsonStringForMessaging(Asset asset) - { - using var stringWriter = new StringWriter(); - using (var jsonWriter = new JsonTextWriter(stringWriter)) - { - jsonWriter.WriteStartObject(); - WriteJsonProperties(jsonWriter, asset); - jsonWriter.WriteEndObject(); - } - return stringWriter.ToString(); - } - - private static void ToLegacyMessageJson(JsonWriter json, string message, Dictionary stringParams) - { - // This replicates the payload created by the Inversion MessagingEvent class. - json.WriteStartObject(); - json.WritePropertyName("_type"); - json.WriteValue("event"); - json.WritePropertyName("_created"); - json.WriteValue(DateTime.UtcNow.ToString("o")); - json.WritePropertyName("message"); - json.WriteValue(message); - json.WritePropertyName("params"); - json.WriteStartObject(); - foreach (KeyValuePair keyValuePair in (IEnumerable>) stringParams) - { - json.WritePropertyName(keyValuePair.Key); - json.WriteValue(keyValuePair.Value); - } - json.WriteEndObject(); - json.WriteEndObject(); - } - - static void WriteJsonProperties(JsonWriter writer, Asset asset) - { - bool asLinkedData = false; - - // what follows is copied exactly from deliverator. - // We do not need to preserve this message format but obvs we need to change this and Engine together. - if (!asLinkedData) - { - writer.WritePropertyName("id"); - writer.WriteValue(asset.Id.ToString()); - writer.WritePropertyName("customer"); - writer.WriteValue(asset.Customer); - writer.WritePropertyName("space"); - writer.WriteValue(asset.Space); - writer.WritePropertyName("rawId"); - writer.WriteValue(asset.Id.Asset); - } - - writer.WritePropertyName("created"); - writer.WriteValue(asset.Created); - writer.WritePropertyName("origin"); - writer.WriteValue(asset.Origin); - writer.WritePropertyName("tags"); - writer.WriteStartArray(); - foreach (string tag in asset.TagsList) - { - writer.WriteValue(tag); - } - writer.WriteEndArray(); - writer.WritePropertyName("roles"); - writer.WriteStartArray(); - foreach (string role in asset.RolesList) - { - if (!asLinkedData) - { - writer.WriteValue(role); - } - else if (role.ToLowerInvariant().StartsWith("http")) - { - writer.WriteValue(role); - } - else - { - // ignore this for now - // writer.WriteValue(String.Format("{0}/customers/{1}/roles/{2}", Context.BaseURL, this.Customer, role)); - } - } - writer.WriteEndArray(); - writer.WritePropertyName("preservedUri"); - writer.WriteValue(asset.PreservedUri); - writer.WritePropertyName("string1"); - writer.WriteValue(asset.Reference1); - writer.WritePropertyName("string2"); - writer.WriteValue(asset.Reference2); - writer.WritePropertyName("string3"); - writer.WriteValue(asset.Reference3); - writer.WritePropertyName("maxUnauthorised"); - writer.WriteValue(asset.MaxUnauthorised); - writer.WritePropertyName("number1"); - writer.WriteValue(asset.NumberReference1); - writer.WritePropertyName("number2"); - writer.WriteValue(asset.NumberReference2); - writer.WritePropertyName("number3"); - writer.WriteValue(asset.NumberReference3); - writer.WritePropertyName("width"); - writer.WriteValue(asset.Width); - writer.WritePropertyName("height"); - writer.WriteValue(asset.Height); - writer.WritePropertyName("duration"); - writer.WriteValue(asset.Duration); - writer.WritePropertyName("error"); - writer.WriteValue(asset.Error); - writer.WritePropertyName("batch"); - writer.WriteValue(asset.Batch); - - writer.WritePropertyName("finished"); - if (asset.Finished == DateTime.MinValue) - { - writer.WriteNull(); - } - else - { - writer.WriteValue(asset.Finished); - } - - writer.WritePropertyName("ingesting"); - writer.WriteValue(asset.Ingesting); - - writer.WritePropertyName("imageOptimisationPolicy"); - if (!asLinkedData) - { - writer.WriteValue(asset.ImageOptimisationPolicy); - } - else - { - // writer.WriteValue(String.Format("{0}/imageOptimisationPolicies/{1}", - // Context.BaseURL, - // this.ImageOptimisationPolicy)); - } - - writer.WritePropertyName("thumbnailPolicy"); - if (!asLinkedData) - { - writer.WriteValue(asset.ThumbnailPolicy); - } - else - { - // writer.WriteValue(String.Format("{0}/thumbnailPolicies/{1}", Context.BaseURL, this.ThumbnailPolicy)); - } - - writer.WritePropertyName("family"); - writer.WriteValue((char)(asset.Family ?? AssetFamily.Image)); - - writer.WritePropertyName("mediaType"); - writer.WriteValue(asset.MediaType); - - if (asLinkedData) - { - // writer.WritePropertyName("storage"); - // writer.WriteValue(String.Format("{0}/customers/{1}/spaces/{2}/images/{3}/storage", Context.BaseURL, - // this.Customer, this.Space, this.GetUniqueName())); - // - // writer.WritePropertyName("metadata"); - // writer.WriteValue(String.Format("{0}/customers/{1}/spaces/{2}/images/{3}/metadata", - // Context.BaseURL, this.Customer, this.Space, this.GetUniqueName())); - } - } -} \ No newline at end of file diff --git a/src/protagonist/Engine.Tests/Ingest/Handlers/IngestHandlerTests.cs b/src/protagonist/Engine.Tests/Ingest/Handlers/IngestHandlerTests.cs index acee80516..bda1bbc8e 100644 --- a/src/protagonist/Engine.Tests/Ingest/Handlers/IngestHandlerTests.cs +++ b/src/protagonist/Engine.Tests/Ingest/Handlers/IngestHandlerTests.cs @@ -23,73 +23,6 @@ public IngestHandlerTests() sut = new IngestHandler(assetIngester, customerQueueRepository, new NullLogger()); } - [Fact] - public async Task HandleMessage_ReturnsFalse_IfInvalidJsonType_LegacyMessage() - { - // Arrange - var body = new JsonObject - { - ["_type"] = "type", - ["_created"] = "not-a-date" - }; - var queueMessage = new QueueMessage { Body = body }; - - // Act - var success = await sut.HandleMessage(queueMessage, CancellationToken.None); - - // Assert - A.CallTo(() => assetIngester.Ingest(A._, A._)).MustNotHaveHappened(); - success.Should().BeFalse(); - } - - [Theory] - [InlineData(IngestResultStatus.Failed)] - [InlineData(IngestResultStatus.Unknown)] - public async Task HandleMessage_ReturnsTrue_IfFailedOrUnknown_LegacyMessage(IngestResultStatus result) - { - // Arrange - var body = new JsonObject - { - ["_type"] = "type" - }; - var queueMessage = new QueueMessage { Body = body, QueueName = "test" }; - A.CallTo(() => assetIngester.Ingest(A._, A._)) - .Returns(new IngestResult(new AssetId(1 , 2, "fake"), result)); - - // Act - var success = await sut.HandleMessage(queueMessage, CancellationToken.None); - - // Assert - A.CallTo(() => assetIngester.Ingest(A._, A._)).MustHaveHappened(); - A.CallTo(() => customerQueueRepository.DecrementSize(A._, A._, A._, A._)) - .MustHaveHappened(); - success.Should().BeTrue(); - } - - [Theory] - [InlineData(IngestResultStatus.Success)] - [InlineData(IngestResultStatus.QueuedForProcessing)] - public async Task HandleMessage_ReturnsTrue_IfSuccessOrQueued_LegacyMessage(IngestResultStatus result) - { - // Arrange - var body = new JsonObject - { - ["_type"] = "type" - }; - var queueMessage = new QueueMessage { Body = body, QueueName = "test" }; - A.CallTo(() => assetIngester.Ingest(A._, A._)) - .Returns(new IngestResult(new AssetId(1 , 2, "fake"), result)); - - // Act - var success = await sut.HandleMessage(queueMessage, CancellationToken.None); - - // Assert - A.CallTo(() => assetIngester.Ingest(A._, A._)).MustHaveHappened(); - A.CallTo(() => customerQueueRepository.DecrementSize(A._, A._, A._, A._)) - .MustHaveHappened(); - success.Should().BeTrue(); - } - [Fact] public async Task HandleMessage_ReturnsFalse_IfInvalidJsonType() { diff --git a/src/protagonist/Engine.Tests/Ingest/Models/LegacyIngestEventConverterTests.cs b/src/protagonist/Engine.Tests/Ingest/Models/LegacyIngestEventConverterTests.cs deleted file mode 100644 index d88fed38d..000000000 --- a/src/protagonist/Engine.Tests/Ingest/Models/LegacyIngestEventConverterTests.cs +++ /dev/null @@ -1,88 +0,0 @@ -using DLCS.Core.Types; -using DLCS.Model.Assets; -using Engine.Ingest.Models; - -namespace Engine.Tests.Ingest.Models; - -public class LegacyIngestEventConverterTests -{ - [Fact] - public void ConvertToInternalRequest_Throws_IfIncomingRequestNull() - { - // Arrange - LegacyIngestEvent? request = null; - - // Act - Action action = () => request.ConvertToAssetRequest(); - - // Assert - action.Should() - .Throw() - .WithMessage("Value cannot be null. (Parameter 'incomingRequest')"); - } - - [Fact] - public void ConvertToInternalRequest_Throws_IfIncomingRequestDoesNotContainAssetJson() - { - // Arrange - var request = Create(new Dictionary()); - - // Act - Action action = () => request.ConvertToAssetRequest(); - - // Assert - action.Should() - .Throw() - .WithMessage("Cannot convert LegacyIngestEvent that has no Asset Json"); - } - - [Fact] - public void ConvertToInternalRequest_Throws_IfIncomingRequestContainsAssetJson_InInvalidFormat() - { - // Arrange - const string assetJson = "i-am-not-json{}"; - var paramsDict = new Dictionary { ["image"] = assetJson }; - var request = Create(paramsDict); - - // Act - Action action = () => request.ConvertToAssetRequest(); - - // Assert - action.Should() - .Throw() - .WithMessage("Unable to deserialize Asset Json from LegacyIngestEvent"); - } - - [Theory] - [InlineData( - "{\"id\": \"2/1/engine-9\",\"customer\": 2,\"space\": 1,\"rawId\": \"engine-9\",\"created\": \"2020-04-09T00:00:00\",\"origin\": \"https://burst.shopifycdn.com/photos/chrome-engine-close-up.jpg\",\"tags\": [\"one\"],\"roles\": [\"https://api.dlcs.digirati.io/customers/2/roles/clickthrough\" ], \"preservedUri\": \"\", \"string1\": \"foo\", \"string2\": \"bar\", \"string3\": \"baz\", \"maxUnauthorised\": 300, \"number1\": 10, \"number2\": 20, \"number3\": 30, \"width\": 100, \"height\": 200, \"duration\": 90,\"error\": \"\",\"batch\": 999,\"finished\": null,\"ingesting\": false,\"imageOptimisationPolicy\": \"fast-higher\",\"thumbnailPolicy\": \"default\",\"family\": \"I\",\"mediaType\": \"image/jp2\"}")] - [InlineData( - "{\r\n \"id\": \"2/1/engine-9\",\r\n \"customer\": 2,\r\n \"space\": 1,\r\n \"rawId\": \"engine-9\",\r\n \"created\": \"2020-04-09T00:00:00\",\r\n \"origin\": \"https://burst.shopifycdn.com/photos/chrome-engine-close-up.jpg\",\r\n \"tags\": [\r\n \"one\"],\r\n \"roles\": [\r\n \"https://api.dlcs.digirati.io/customers/2/roles/clickthrough\"\r\n ],\r\n \"preservedUri\": \"\",\r\n \"string1\": \"foo\",\r\n \"string2\": \"bar\",\r\n \"string3\": \"baz\",\r\n \"maxUnauthorised\": 300,\r\n \"number1\": 10,\r\n \"number2\": 20,\r\n \"number3\": 30,\r\n \"width\": 100,\r\n \"height\": 200,\r\n \"duration\": 90,\r\n \"error\": \"\",\r\n \"batch\": 999,\r\n \"finished\": null,\r\n \"ingesting\": false,\r\n \"imageOptimisationPolicy\": \"fast-higher\",\r\n \"thumbnailPolicy\": \"default\",\r\n \"family\": \"I\",\r\n \"mediaType\": \"image/jp2\"\r\n}")] - public void ConvertToInternalRequest_ReturnsExpected(string assetJson) - { - // Arrange - var paramsDict = new Dictionary { ["image"] = assetJson }; - var request = Create(paramsDict); - var created = new DateTime(2020, 04, 09); - DateTime.SpecifyKind(created, DateTimeKind.Utc); - var expected = new Asset - { - Id = AssetId.FromString("2/1/engine-9"), Customer = 2, Space = 1, Created = created, - Origin = "https://burst.shopifycdn.com/photos/chrome-engine-close-up.jpg", Tags = "one", - Roles = "https://api.dlcs.digirati.io/customers/2/roles/clickthrough", PreservedUri = "", Reference1 = "foo", - Reference2 = "bar", Reference3 = "baz", MaxUnauthorised = 300, NumberReference1 = 10, NumberReference2 = 20, - NumberReference3 = 30, Width = 100, Height = 200, Duration = 90, Error = string.Empty, Batch = 999, - Finished = null, Ingesting = false, ImageOptimisationPolicy = "fast-higher", ThumbnailPolicy = "default", - Family = AssetFamily.Image, MediaType = "image/jp2" - }; - - // Act - var result = request.ConvertToAssetRequest(); - - // Assert - result.Id.Should().BeEquivalentTo(expected.Id); - } - - private LegacyIngestEvent Create(Dictionary paramsDict) - => new("test", DateTime.Now, "test::type", paramsDict); -} \ No newline at end of file diff --git a/src/protagonist/Engine.Tests/Ingest/Models/LegacyIngestEventTests.cs b/src/protagonist/Engine.Tests/Ingest/Models/LegacyIngestEventTests.cs deleted file mode 100644 index 2a16d4546..000000000 --- a/src/protagonist/Engine.Tests/Ingest/Models/LegacyIngestEventTests.cs +++ /dev/null @@ -1,67 +0,0 @@ -using Engine.Ingest.Models; - -namespace Engine.Tests.Ingest.Models; - -public class LegacyIngestEventTests -{ - [Fact] - public void AssetJson_Null_IfDictionaryNull() - { - // Arrange - var evt = Create(null); - - // Act - var assetJson = evt.AssetJson; - - // Assert - assetJson.Should().BeNullOrEmpty(); - } - - [Fact] - public void AssetJson_Null_IfDictionaryEmpty() - { - // Arrange - var evt = Create(new Dictionary()); - - // Act - var assetJson = evt.AssetJson; - - // Assert - assetJson.Should().BeNullOrEmpty(); - } - - [Fact] - public void AssetJson_Null_IfDictionaryDoesNotContainCorrectElement() - { - // Arrange - var paramsDict = new Dictionary { ["foo"] = "bar" }; - var evt = Create(paramsDict); - - // Act - var assetJson = evt.AssetJson; - - // Assert - assetJson.Should().BeNullOrEmpty(); - } - - [Theory] - [InlineData(null)] - [InlineData("")] - [InlineData(" ")] - [InlineData("something")] - public void AssetJson_ReturnsExpected_IfDictionaryContainCorrectElement(string value) - { - // Arrange - var paramsDict = new Dictionary { ["image"] = value }; - var evt = Create(paramsDict); - - // Act - var assetJson = evt.AssetJson; - - // Assert - assetJson.Should().Be(value); - } - - private LegacyIngestEvent Create(Dictionary paramsDict) - => new("test", DateTime.Now, "test::type", paramsDict); -} \ No newline at end of file diff --git a/src/protagonist/Engine.Tests/Integration/IngestResponseTests.cs b/src/protagonist/Engine.Tests/Integration/IngestResponseTests.cs index 949f7894f..1c0eb7cd1 100644 --- a/src/protagonist/Engine.Tests/Integration/IngestResponseTests.cs +++ b/src/protagonist/Engine.Tests/Integration/IngestResponseTests.cs @@ -54,36 +54,6 @@ public async Task IngestAsset_ReturnsExpectedCode_ForIngestResult(IngestResultSt result.StatusCode.Should().Be(expected); } - [Theory] - [InlineData(IngestResultStatus.Unknown, HttpStatusCode.InternalServerError)] - [InlineData(IngestResultStatus.Failed, HttpStatusCode.InternalServerError)] - [InlineData(IngestResultStatus.Success, HttpStatusCode.OK)] - [InlineData(IngestResultStatus.QueuedForProcessing, HttpStatusCode.Accepted)] - [InlineData(IngestResultStatus.StorageLimitExceeded, HttpStatusCode.InsufficientStorage)] - public async Task IngestImage_ReturnsExpectedCode_ForIngestResult_Legacy(IngestResultStatus ingestResult, HttpStatusCode expected) - { - // Arrange - var assetId = $"1/2/{ingestResult}"; - var message = new LegacyIngestEvent( - assetId, - DateTime.UtcNow, - "message", - new Dictionary - { - ["asset"] = "test" - }); - - A.CallTo(() => - assetIngester.Ingest(A.That.Matches(r => r.Type == assetId), - A._)).Returns(new IngestResult(null, ingestResult)); - - // Act - var result = await httpClient.PostAsync("image-ingest", GetJsonContent(message)); - - // Assert - result.StatusCode.Should().Be(expected); - } - [Theory] [InlineData(IngestResultStatus.Unknown, HttpStatusCode.InternalServerError)] [InlineData(IngestResultStatus.Failed, HttpStatusCode.InternalServerError)] @@ -106,38 +76,7 @@ public async Task IngestImage_ReturnsExpectedCode_ForIngestResult_Legacy(IngestR // Assert result.StatusCode.Should().Be(expected); } - - [Theory] - [InlineData(IngestResultStatus.Unknown, HttpStatusCode.InternalServerError)] - [InlineData(IngestResultStatus.Failed, HttpStatusCode.InternalServerError)] - [InlineData(IngestResultStatus.Success, HttpStatusCode.OK)] - [InlineData(IngestResultStatus.QueuedForProcessing, HttpStatusCode.Accepted)] - [InlineData(IngestResultStatus.StorageLimitExceeded, HttpStatusCode.InsufficientStorage)] - public async Task IngestImage_ReturnsExpectedCode_ForIngestResult_Legacy_ByteArray(IngestResultStatus ingestResult, - HttpStatusCode expected) - { - // Arrange - var assetId = $"1/2/{ingestResult}"; - var message = new LegacyIngestEvent( - assetId, - DateTime.UtcNow, - "message", - new Dictionary - { - ["asset"] = "test" - }); - - A.CallTo(() => - assetIngester.Ingest(A.That.Matches(r => r.Type == assetId), - A._)).Returns(new IngestResult(null, ingestResult)); - - // Act - var result = await httpClient.PostAsync("image-ingest", GetByteArrayContent(message)); - - // Assert - result.StatusCode.Should().Be(expected); - } - + private StringContent GetJsonContent(object message) { var jsonString = JsonSerializer.Serialize(message, settings); diff --git a/src/protagonist/Engine/Ingest/AssetIngester.cs b/src/protagonist/Engine/Ingest/AssetIngester.cs index 2c73cc2cc..b29c80491 100644 --- a/src/protagonist/Engine/Ingest/AssetIngester.cs +++ b/src/protagonist/Engine/Ingest/AssetIngester.cs @@ -10,13 +10,6 @@ namespace Engine.Ingest; public interface IAssetIngester { - /// - /// Run ingest based on . - /// - /// Result of ingest operations - /// This is to comply with message format sent by Deliverator API. - Task Ingest(LegacyIngestEvent request, CancellationToken cancellationToken = default); - /// /// Run ingest based on . /// @@ -48,27 +41,7 @@ public class AssetIngester : IAssetIngester this.executor = executor; this.engineAssetRepository = engineAssetRepository; } - - /// - /// Run ingest based on . - /// - /// Result of ingest operations - /// This is to comply with message format sent by Deliverator API. - public Task Ingest(LegacyIngestEvent request, CancellationToken cancellationToken = default) - { - IngestAssetRequest? internalIngestRequest = null; - try - { - internalIngestRequest = request.ConvertToAssetRequest(); - return Ingest(internalIngestRequest, cancellationToken); - } - catch (Exception e) - { - logger.LogError(e, "Exception ingesting IncomingIngest - {Message}", request.Message); - return Task.FromResult(new IngestResult(internalIngestRequest?.Id, IngestResultStatus.Failed)); - } - } - + /// /// Run ingest based on . /// diff --git a/src/protagonist/Engine/Ingest/IngestController.cs b/src/protagonist/Engine/Ingest/IngestController.cs index 3f846b91b..8319d3a69 100644 --- a/src/protagonist/Engine/Ingest/IngestController.cs +++ b/src/protagonist/Engine/Ingest/IngestController.cs @@ -1,7 +1,6 @@ using System.Net; using System.Text.Json; using DLCS.Model.Messaging; -using Engine.Ingest.Models; using Engine.Settings; using Microsoft.AspNetCore.Mvc; using Microsoft.Extensions.Options; @@ -20,25 +19,7 @@ public IngestController(IAssetIngester ingester, IOptions engine this.ingester = ingester; timebasedIngestSettings = engineSettings.Value.TimebasedIngest; } - - /// - /// Synchronously ingest an asset using legacy model - /// - [HttpPost] - [Route("image-ingest")] - public async Task IngestImage(CancellationToken cancellationToken) - { - // TODO - throw if this is a 'T' request - var message = - await JsonSerializer.DeserializeAsync(Request.Body, - JsonSerializerOptions, cancellationToken); - - // TODO - throw if this is a 'T' request - var result = await ingester.Ingest(message, cancellationToken); - - return ConvertToStatusCode(message, result.Status); - } - + /// /// Synchronously ingest an asset /// diff --git a/src/protagonist/Engine/Ingest/IngestHandler.cs b/src/protagonist/Engine/Ingest/IngestHandler.cs index 0c061cb1f..5e4c7b38c 100644 --- a/src/protagonist/Engine/Ingest/IngestHandler.cs +++ b/src/protagonist/Engine/Ingest/IngestHandler.cs @@ -25,24 +25,15 @@ public class IngestHandler : IMessageHandler public async Task HandleMessage(QueueMessage message, CancellationToken cancellationToken) { - IngestResult ingestResult; - if (IsLegacyMessageType(message)) - { - var legacyEvent = DeserializeBody(message); - if (legacyEvent == null) return false; - ingestResult = await ingester.Ingest(legacyEvent, cancellationToken); - } - else - { - var ingestEvent = DeserializeBody(message); - if (ingestEvent == null) return false; - ingestResult = await ingester.Ingest(ingestEvent, cancellationToken); - } - + var ingestEvent = DeserializeBody(message); + + if (ingestEvent == null) return false; + + var ingestResult = await ingester.Ingest(ingestEvent, cancellationToken); + logger.LogDebug("Message {MessageId} handled with result {IngestResult}", message.MessageId, ingestResult.Status); - await UpdateCustomerQueue(message, cancellationToken, ingestResult); - + // return true so that the message is deleted from the queue in all instances. // This shouldn't be the case and can be revisited at a later date as it will need logic of how Batch.Errors is // calculated @@ -83,7 +74,4 @@ public async Task HandleMessage(QueueMessage message, CancellationToken ca return null; } } - - // If the message contains "_type" field then it is the legacy version from Deliverator/Inversion - private bool IsLegacyMessageType(QueueMessage message) => message.Body.ContainsKey("_type"); } \ No newline at end of file diff --git a/src/protagonist/Engine/Ingest/Models/LegacyIngestEvent.cs b/src/protagonist/Engine/Ingest/Models/LegacyIngestEvent.cs deleted file mode 100644 index 1dad70edb..000000000 --- a/src/protagonist/Engine/Ingest/Models/LegacyIngestEvent.cs +++ /dev/null @@ -1,51 +0,0 @@ -using System.Text.Json.Serialization; -using DLCS.Model.Assets; - -namespace Engine.Ingest.Models; - -/// -/// Serialized Inversion MessagingEvent passed to the Engine by DLCS API. -/// -/// Legacy fields from the Inversion framework. -public class LegacyIngestEvent -{ - private const string AssetDictionaryKey = "image"; - - /// - /// Gets the type of MessagingEvent. - /// - [JsonPropertyName("_type")] - public string Type { get; } - - /// - /// Gets the date this message was created. - /// - [JsonPropertyName("_created")] - public DateTime? Created { get; } - - /// - /// Gets the type of this message. - /// - [JsonPropertyName("message")] - public string Message { get; } - - /// - /// A collection of additional parameters associated with event. - /// - [JsonPropertyName("params")] - public Dictionary Params { get; } - - /// - /// Serialized as JSON. - /// - public string? AssetJson => Params.TryGetValue(AssetDictionaryKey, out var image) ? image : null; - - [JsonConstructor] - public LegacyIngestEvent(string type, DateTime? created, string message, Dictionary? @params) - { - Type = type; - Created = created; - Message = message; - Params = @params ?? new Dictionary(); - } -} \ No newline at end of file diff --git a/src/protagonist/Engine/Ingest/Models/LegacyIngestEventConverter.cs b/src/protagonist/Engine/Ingest/Models/LegacyIngestEventConverter.cs deleted file mode 100644 index 6835d265b..000000000 --- a/src/protagonist/Engine/Ingest/Models/LegacyIngestEventConverter.cs +++ /dev/null @@ -1,99 +0,0 @@ -using System.Text.Json; -using System.Text.Json.Nodes; -using DLCS.Core.Guard; -using DLCS.Core.Types; -using DLCS.Model.Assets; -using DLCS.Model.Messaging; - -namespace Engine.Ingest.Models; - -public static class LegacyIngestEventConverter -{ - /// - /// Convert to IngestAssetRequest object. - /// - /// Event to convert - /// IngestAssetRequest - /// Thrown if IncomingIngestEvent doesn't contain any Asset data - public static IngestAssetRequest ConvertToAssetRequest(this LegacyIngestEvent incomingRequest) - { - incomingRequest.ThrowIfNull(nameof(incomingRequest)); - - if (string.IsNullOrEmpty(incomingRequest.AssetJson)) - { - throw new InvalidOperationException("Cannot convert LegacyIngestEvent that has no Asset Json"); - } - - try - { - var formattedJson = incomingRequest.AssetJson.Replace("\r\n", string.Empty); - var asset = ConvertJsonToAsset(formattedJson); - return new IngestAssetRequest(asset.Id, incomingRequest.Created); - } - catch (JsonException e) - { - var ex = new InvalidOperationException("Unable to deserialize Asset Json from LegacyIngestEvent", e); - ex.Data.Add("AssetJson", incomingRequest.AssetJson); - throw ex; - } - } - - // This is very temporary and should be removed asap, only included for backwards compat - private static Asset ConvertJsonToAsset(string assetJsonString) - { - var parsedJson = JsonObject.Parse(assetJsonString).AsObject(); - - var asset = new Asset(); - asset.Id = parsedJson.TryGetPropertyValue("id", out var id) ? AssetId.FromString(id.GetValue()) : null; - asset.Customer = parsedJson.TryGetPropertyValue("customer", out var customer) ? customer.GetValue() : 0; - asset.Space = parsedJson.TryGetPropertyValue("space", out var space) ? space.GetValue() : 0; - asset.Created = parsedJson.TryGetPropertyValue("created", out var created) ? created.GetValue() : null; - asset.Origin = parsedJson.TryGetPropertyValue("origin", out var origin) ? origin.GetValue() : null; - asset.Reference1 = parsedJson.TryGetPropertyValue("string1", out var string1) ? string1.GetValue() : null; - asset.Reference2 = parsedJson.TryGetPropertyValue("string2", out var string2) ? string2.GetValue() : null; - asset.Reference3 = parsedJson.TryGetPropertyValue("string3", out var string3) ? string3.GetValue() : null; - asset.PreservedUri = parsedJson.TryGetPropertyValue("preservedUri", out var preservedUri) - ? preservedUri.GetValue() - : null; - asset.MaxUnauthorised = parsedJson.TryGetPropertyValue("maxUnauthorised", out var maxUnauthorised) - ? maxUnauthorised.GetValue() - : 0; - asset.NumberReference1 = parsedJson.TryGetPropertyValue("number1", out var number1) ? number1.GetValue() : 0; - asset.NumberReference2 = parsedJson.TryGetPropertyValue("number2", out var number2) ? number2.GetValue() : 0; - asset.NumberReference3 = parsedJson.TryGetPropertyValue("number3", out var number3) ? number3.GetValue() : 0; - asset.Width = parsedJson.TryGetPropertyValue("width", out var width) ? width.GetValue() : 0; - asset.Height = parsedJson.TryGetPropertyValue("height", out var height) ? height.GetValue() : 0; - asset.Duration = parsedJson.TryGetPropertyValue("duration", out var duration) ? duration.GetValue() : 0; - asset.Error = parsedJson.TryGetPropertyValue("error", out var error) ? error.GetValue() : null; - asset.Batch = parsedJson.TryGetPropertyValue("batch", out var batch) ? batch.GetValue() : 0; - asset.Finished = parsedJson.TryGetPropertyValue("finished", out var finished) && finished != null - ? finished.GetValue() - : null; - asset.Ingesting = parsedJson.TryGetPropertyValue("ingesting", out var ingesting) - ? ingesting.GetValue() - : null; - asset.MediaType = parsedJson.TryGetPropertyValue("mediaType", out var mediaType) - ? mediaType.GetValue() - : null; - asset.TagsList = parsedJson.TryGetPropertyValue("tags", out var tags) ? tags.Deserialize() : null; - asset.RolesList = parsedJson.TryGetPropertyValue("roles", out var roles) ? roles.Deserialize() : null; - asset.ImageOptimisationPolicy = parsedJson.TryGetPropertyValue("imageOptimisationPolicy", out var imageOptimisationPolicy) - ? imageOptimisationPolicy.GetValue() - : null; - asset.ThumbnailPolicy = parsedJson.TryGetPropertyValue("thumbnailPolicy", out var thumbnailPolicy) - ? thumbnailPolicy.GetValue() - : null; - - if (parsedJson.TryGetPropertyValue("family", out var family)) - { - var familyChar = family.GetValue(); - asset.Family = (AssetFamily)familyChar; - } - else - { - asset.Family = AssetFamily.Image; - } - - return asset; - } -} \ No newline at end of file From 4d76a9f3b5d015a81a729a1a05e28697379ba010 Mon Sep 17 00:00:00 2001 From: griffri Date: Fri, 22 Mar 2024 13:28:04 +0000 Subject: [PATCH 3/4] Remove legacy ingest route info from readme --- src/protagonist/Engine/readme.md | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/protagonist/Engine/readme.md b/src/protagonist/Engine/readme.md index 543e273d9..91eb9fd29 100644 --- a/src/protagonist/Engine/readme.md +++ b/src/protagonist/Engine/readme.md @@ -6,10 +6,7 @@ Engine is responsible for ingesting assets; either synchronously via an API call ### API (Synchronous) -The engine has 2 routes for synchronous processing: - -* `/asset-ingest` - Process incoming `IngestAssetRequest` - generating derivatives for asset delivery. -* `/image-ingest` - As above but takes `LegacyIngestEvent`, which is Deliverator notification model. The `LegacyIngestEvent` is converted to `IngestAssetRequest` and follows exact same process as above. _The intention is that this endpoint will be removed when Deliverator engine is retired_ +For synchronous processing, the engine takes incoming`IngestAssetRequest` at `/asset-ingest`, generating derivatives for asset delivery. ### Queue (Asynchronous) From 52709571841db420c0718ffc750d4289cdd06643 Mon Sep 17 00:00:00 2001 From: griffri Date: Fri, 22 Mar 2024 13:44:20 +0000 Subject: [PATCH 4/4] Add `/allowed-av` summary to engine readme --- src/protagonist/Engine/readme.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/protagonist/Engine/readme.md b/src/protagonist/Engine/readme.md index 91eb9fd29..c5fe3c84a 100644 --- a/src/protagonist/Engine/readme.md +++ b/src/protagonist/Engine/readme.md @@ -61,6 +61,8 @@ The process for each asset delivery-channel is outlined below, the same process * Input file is removed. * "Images" database record updated with dimensions and marked as complete, "ImageStorage" is updated with size of bytes stored +A list of transcode policies supported by Engine (as a JSON string array) can be retrieved the `/allowed-av` route. + #### File (file channel) * If asset is stored at optimised origin this is a no-op (we will server from origin). Else,