Skip to content

Commit 4e54b2a

Browse files
p-kaczynskip-kaczynski
committed
Multiple adjunct handling changes after PR feedback
1 parent 9266938 commit 4e54b2a

8 files changed

Lines changed: 91 additions & 51 deletions

File tree

‎src/protagonist/API.Tests/Integration/AdjunctTests.cs‎

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System;
2+
using System.Collections.Generic;
23
using System.Linq;
34
using System.Net;
45
using System.Net.Http;
@@ -85,6 +86,9 @@ public async Task PostAdjunct_CreatesExternalAdjunct()
8586

8687
hydra.Should().NotBeNull();
8788
hydra.Members.Should().HaveCount(1);
89+
hydra.PageSize.Should().Be(1);
90+
hydra.TotalItems.Should().Be(1);
91+
hydra.Type.Should().Be("Collection");
8892

8993
var adjunct = hydra.Members![0];
9094
adjunct.Id.Should()
@@ -629,7 +633,7 @@ await dbContext.Images.AddTestAsset(assetId)
629633
adjunct.Motivation.Should().Be("changed");
630634

631635
A.CallTo(() => DeliverableNotificationSender.SendDeliverableModifiedMessage(
632-
A<NotificationRecord<DLCS.Model.Assets.Adjunct>>.That.Matches(r => r.ChangeType == ChangeType.Update && r.After.AssetId == assetId),
636+
A<IReadOnlyCollection<NotificationRecord<DLCS.Model.Assets.Adjunct>>>.That.Matches(c => c.Count == 1 && c.Single().ChangeType == ChangeType.Update && c.Single().After.AssetId == assetId),
633637
A<CancellationToken>._)).MustHaveHappened();
634638
}
635639

@@ -686,7 +690,7 @@ public async Task PutAdjunct_CreatesAdjunct_WhenIdDoesNotExists()
686690
dbContext.Adjuncts.Any(a => a.AssetId == assetId && a.Id == adjunctId).Should()
687691
.BeTrue("Adjunct persisted to DB");
688692
A.CallTo(() => DeliverableNotificationSender.SendDeliverableModifiedMessage(
689-
A<NotificationRecord<DLCS.Model.Assets.Adjunct>>.That.Matches(r => r.ChangeType == ChangeType.Create && r.After.AssetId == assetId),
693+
A<IReadOnlyCollection<NotificationRecord<DLCS.Model.Assets.Adjunct>>>.That.Matches(c => c.Count == 1 && c.Single().ChangeType == ChangeType.Create && c.Single().After.AssetId == assetId),
690694
A<CancellationToken>._)).MustHaveHappened();
691695
}
692696

@@ -880,6 +884,12 @@ public async Task PostAdjunct_CreatesHostedAdjunct()
880884
response.Headers.Location.Should()
881885
.Be(
882886
$"http://localhost/customers/{assetId.Customer}/spaces/{assetId.Space}/images/{assetId.Asset}/adjuncts");
887+
888+
A.CallTo(() =>
889+
IngestNotificationSender.SendIngestAdjunctRequest(
890+
A<DLCS.Model.Assets.Adjunct>.That.Matches(a => a.Id == "someAdjunctId"), A<CancellationToken>._))
891+
.MustHaveHappened(1, Times.Exactly);
892+
883893
}
884894

885895
[Fact]
@@ -933,16 +943,22 @@ public async Task PostAdjunct_CreatesMultipleHostedAdjuncts_AsArray()
933943
for(var i = 0; i < result.Members.Length; i++)
934944
{
935945
var adjunct = result.Members[i];
946+
var adjunctId = $"someAdjunctId{i + 1}";
936947
adjunct.Id.Should()
937948
.Be(
938-
$"http://localhost/customers/{assetId.Customer}/spaces/{assetId.Space}/images/{assetId.Asset}/adjuncts/someAdjunctId{i+1}");
949+
$"http://localhost/customers/{assetId.Customer}/spaces/{assetId.Space}/images/{assetId.Asset}/adjuncts/{adjunctId}");
939950
adjunct.IIIFLink.Should().Be("seeAlso");
940951
adjunct.Label.First().Key.Should().Be("label");
941952
adjunct.Language.Should().Contain(l => l == "en").And.HaveCount(1);
942953
adjunct.Origin.Should().Be($"https://some-location.com/an-adjunct{i+1}");
943-
adjunct.PublicId.Should().Be($"https://dlcs.digirati.io/adjuncts/99/1/{assetId.Asset}/someAdjunctId{i+1}");
954+
adjunct.PublicId.Should().Be($"https://dlcs.digirati.io/adjuncts/99/1/{assetId.Asset}/{adjunctId}");
944955
adjunct.Ingesting.Should().Be(true, "the adjunct was sent to engine for ingestion");
945956
adjunct.Error.Should().BeNullOrEmpty("no errors yet");
957+
958+
A.CallTo(() =>
959+
IngestNotificationSender.SendIngestAdjunctRequest(
960+
A<DLCS.Model.Assets.Adjunct>.That.Matches(a =>a.AssetId == assetId && a.Id == adjunctId), A<CancellationToken>._))
961+
.MustHaveHappened(1, Times.Exactly);
946962
}
947963

948964
response.Headers.Location.Should()
@@ -965,7 +981,7 @@ public async Task PostAdjunct_CreatesMultipleHostedAdjuncts_AsHydraCollection()
965981
const string newAdjunctJson = """
966982
{
967983
"@type": "Collection",
968-
"members":
984+
"member":
969985
[
970986
{
971987
"id": "someAdjunctId1",
@@ -1005,16 +1021,23 @@ public async Task PostAdjunct_CreatesMultipleHostedAdjuncts_AsHydraCollection()
10051021
for(var i = 0; i < result.Members.Length; i++)
10061022
{
10071023
var adjunct = result.Members[i];
1024+
var adjunctId = $"someAdjunctId{i + 1}";
1025+
10081026
adjunct.Id.Should()
10091027
.Be(
1010-
$"http://localhost/customers/{assetId.Customer}/spaces/{assetId.Space}/images/{assetId.Asset}/adjuncts/someAdjunctId{i+1}");
1028+
$"http://localhost/customers/{assetId.Customer}/spaces/{assetId.Space}/images/{assetId.Asset}/adjuncts/{adjunctId}");
10111029
adjunct.IIIFLink.Should().Be("seeAlso");
10121030
adjunct.Label.First().Key.Should().Be("label");
10131031
adjunct.Language.Should().Contain(l => l == "en").And.HaveCount(1);
10141032
adjunct.Origin.Should().Be($"https://some-location.com/an-adjunct{i+1}");
1015-
adjunct.PublicId.Should().Be($"https://dlcs.digirati.io/adjuncts/99/1/{assetId.Asset}/someAdjunctId{i+1}");
1033+
adjunct.PublicId.Should().Be($"https://dlcs.digirati.io/adjuncts/99/1/{assetId.Asset}/{adjunctId}");
10161034
adjunct.Ingesting.Should().Be(true, "the adjunct was sent to engine for ingestion");
10171035
adjunct.Error.Should().BeNullOrEmpty("no errors yet");
1036+
1037+
A.CallTo(() =>
1038+
IngestNotificationSender.SendIngestAdjunctRequest(
1039+
A<DLCS.Model.Assets.Adjunct>.That.Matches(a =>a.AssetId == assetId && a.Id == adjunctId), A<CancellationToken>._))
1040+
.MustHaveHappened(1, Times.Exactly);
10181041
}
10191042

10201043
response.Headers.Location.Should()
@@ -1209,7 +1232,7 @@ public async Task PostAdjunct_FailsToCreateMultipleHostedAdjuncts_WhenEmptyColle
12091232
const string newAdjunctJson = """
12101233
{
12111234
"@type": "Collection",
1212-
"members": []
1235+
"member": []
12131236
}
12141237
""";
12151238

‎src/protagonist/API/Features/Adjuncts/AdjunctsController.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public async Task<IActionResult> GetAdjunct(int customerId, int spaceId, string
7272
[ProducesResponseType(201, Type = typeof(Adjunct))]
7373
[ProducesResponseType(404, Type = typeof(Error))]
7474
public async Task<IActionResult> PostAdjunct(int customerId, int spaceId, string imageId,
75-
[FromBody] ItemArrayOrHydraCollection<Adjunct> adjuncts,
75+
[FromBody] FlexCollection<Adjunct> adjuncts,
7676
[FromServices] HydraAdjunctValidator validator, CancellationToken cancellationToken = default)
7777
{
7878
if (adjuncts.Items is not { Length: > 0 } hydraAdjuncts)
@@ -141,7 +141,7 @@ private async Task<IActionResult> CreateOrUpdateAdjunct(int customerId, int spac
141141
return await HandleUpsert(
142142
createOrUpdateRequest,
143143
a => a.ToHydra(GetUrlRoots()),
144-
createOrUpdateRequest.Adjuncts[0].AssetId.ToString(),
144+
new AssetId(customerId, spaceId, imageId).ToString(),
145145
"Create or update adjunct failed", cancellationToken);
146146
}
147147
}

‎src/protagonist/API/Features/Adjuncts/Requests/CreateOrUpdateAdjunct.cs‎

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,9 @@ public async Task<ModifyEntityResult<Adjunct[]>> Handle(CreateOrUpdateAdjunct re
6262
{
6363
try
6464
{
65-
var existingAdjunct = !request.CreateOnly && existing.TryGetValue(adjunct.Id, out var e) ? e : null;
65+
var existingAdjunct = !request.CreateOnly && existing.TryGetValue(adjunct.Id, out var maybeAdjunct)
66+
? maybeAdjunct
67+
: null;
6668

6769
// flag remains true if it was true (tripped)
6870
anyUpdates = anyUpdates || existingAdjunct != null; // true if at least one is updating existing - this or previous
@@ -102,7 +104,6 @@ public async Task<ModifyEntityResult<Adjunct[]>> Handle(CreateOrUpdateAdjunct re
102104
.Where(a => a.AssetId == assetId && adjunctIds.Contains(a.Id))
103105
.ToDictionaryAsync(a => a.Id, cancellationToken: cancellationToken);
104106

105-
List<string> failed = [];
106107

107108
foreach (var adjunct in adjuncts)
108109
{
@@ -111,14 +112,10 @@ public async Task<ModifyEntityResult<Adjunct[]>> Handle(CreateOrUpdateAdjunct re
111112
// this is a bit clunky, but we don't want to query individually for each - there could be a lot
112113
adjunct.Processed = updated;
113114
}
114-
115-
var success = await TryIngestNotify(adjunct, cancellationToken);
116-
if (!success)
117-
{
118-
failed.Add(adjunct.Processed.Id);
119-
}
120115
}
121116

117+
var failed = await TryIngestNotify(adjuncts, cancellationToken);
118+
122119
if (failed.Count != 0)
123120
{
124121
return ModifyEntityResult<Adjunct[]>.Failure(
@@ -130,27 +127,34 @@ public async Task<ModifyEntityResult<Adjunct[]>> Handle(CreateOrUpdateAdjunct re
130127
anyUpdates ? WriteResult.Updated : WriteResult.Created);
131128
}
132129

133-
private async Task<bool> TryIngestNotify(AdjunctDocument adjunct, CancellationToken cancellationToken)
130+
private async Task<List<string>> TryIngestNotify(ICollection<AdjunctDocument> adjuncts, CancellationToken cancellationToken)
134131
{
135-
if (adjunct.ToBeIngested)
132+
List<string> failed = [];
133+
List<NotificationRecord<Adjunct>> notifications = [];
134+
135+
foreach (var adjunct in adjuncts)
136136
{
137-
var success = await notificationSender.SendIngestAdjunctRequest(adjunct.Processed, cancellationToken);
138-
if (!success)
137+
if (adjunct.ToBeIngested)
139138
{
140-
return false;
139+
var success = await notificationSender.SendIngestAdjunctRequest(adjunct.Processed, cancellationToken);
140+
if (!success)
141+
{
142+
failed.Add(adjunct.Processed.Id);
143+
continue;
144+
}
141145
}
142-
}
143146

144147

145-
var adjunctModificationRecord = adjunct.IsUpdate
146-
? NotificationRecord<Adjunct>.Update(adjunct.Original!, adjunct.Processed, adjunct.ToBeIngested)
147-
: NotificationRecord<Adjunct>.Create(adjunct.Processed);
148+
var adjunctModificationRecord = adjunct.IsUpdate
149+
? NotificationRecord<Adjunct>.Update(adjunct.Original!, adjunct.Processed, adjunct.ToBeIngested)
150+
: NotificationRecord<Adjunct>.Create(adjunct.Processed);
151+
152+
notifications.Add(adjunctModificationRecord);
153+
}
148154

149-
await deliverableNotificationSender.SendDeliverableModifiedMessage(adjunctModificationRecord,
150-
cancellationToken);
155+
await deliverableNotificationSender.SendDeliverableModifiedMessage(notifications, cancellationToken);
151156

152-
// all good
153-
return true;
157+
return failed;
154158
}
155159

156160
private async Task<AdjunctDocument> HandleAdjunct(Adjunct adjunct, Adjunct? dbAdjunct,

‎src/protagonist/API/Infrastructure/ControllerBaseX.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,9 +138,9 @@ public static CreatedResult HydraCreated(this ControllerBase controller, JsonLdB
138138
/// error and appropriate status code if failed.
139139
/// </summary>
140140
/// <param name="controller">Current controllerBase object</param>
141-
/// <param name="entity">entity that was modified or null if n/a</param>
142-
/// <param name="error">error if one happened, otherwise null</param>
143-
/// <param name="result">the result of the write operation</param>
141+
/// <param name="entity">Entity that was modified or null if n/a</param>
142+
/// <param name="error">Error if one happened, otherwise null</param>
143+
/// <param name="result">The result of the write operation</param>
144144
/// <param name="hydraBuilder">Delegate to transform ModifyEntityResult.Entity to Hydra representation</param>
145145
/// <param name="instance">The value for <see cref="Error.Instance" />.</param>
146146
/// <param name="errorTitle">

‎src/protagonist/API/Infrastructure/HydraController.cs‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,10 +82,11 @@ protected async Task<IActionResult> HandleUpsert<T>(
8282
return this.ModifyResultToHttpResult(result.Entity, result.Error, result.WriteResult, hydraBuilder, instance, errorTitle);
8383
}, errorTitle);
8484
}
85-
85+
8686
/// <summary>
8787
/// Handle an upsert request - this takes a IRequest which returns a ModifyEntityResult{T}.
88-
/// The request is sent and result is transformed to an http hydra result.
88+
/// The request is sent and result is transformed to an http hydra result.
89+
/// This is a collection variant that returns <see cref="HydraCollection{TLd}"/>
8990
/// </summary>
9091
/// <param name="request">IRequest to modify data</param>
9192
/// <param name="hydraBuilder">Delegate to transform returned entity to a Hydra representation</param>
@@ -96,6 +97,7 @@ protected async Task<IActionResult> HandleUpsert<T>(
9697
/// </param>
9798
/// <param name="cancellationToken">Current cancellation token</param>
9899
/// <typeparam name="T">Type of entity being upserted</typeparam>
100+
/// <typeparam name="TLd">The <see cref="JsonLdBase"/>-derived type that will be used for response</typeparam>
99101
/// <returns>
100102
/// ActionResult generated from ModifyEntityResult. This will be the Hydra model + 200/201 on success. Or a Hydra
101103
/// error and appropriate status code if failed.
@@ -113,13 +115,19 @@ protected async Task<IActionResult> HandleUpsert<T, TLd>(
113115
{
114116
var result = await Mediator.Send(request, cancellationToken);
115117

116-
var collectionId = Request.GetEncodedUrl();
118+
var collectionId = Request.GetJsonLdId();
117119

118120
return this.ModifyResultToHttpResult(result.Entity, result.Error, result.WriteResult, CollectionBuilder,
119121
instance, errorTitle);
120122

121123
HydraCollection<TLd> CollectionBuilder(T[] items) => new()
122-
{ Id = collectionId, Members = items.Select(hydraBuilder).ToArray() };
124+
{
125+
Id = collectionId,
126+
TotalItems = items.Length,
127+
PageSize = items.Length,
128+
WithContext = true,
129+
Members = items.Select(hydraBuilder).ToArray()
130+
};
123131
}, errorTitle);
124132
}
125133

‎src/protagonist/API/Startup.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ public void ConfigureServices(IServiceCollection services)
119119
.AddNewtonsoftJson(options =>
120120
{
121121
options.SerializerSettings.ApplyHydraSerializationSettings();
122-
ItemArrayOrHydraCollectionConverter<Adjunct>.Register(options.SerializerSettings);
122+
FlexCollectionConverter<Adjunct>.Register(options.SerializerSettings);
123123
});
124124

125125
services.AddSwaggerGenNewtonsoftSupport();

src/protagonist/DLCS.HydraModel/Converters/ItemArrayOrHydraCollectionConverter.cs renamed to src/protagonist/DLCS.HydraModel/Converters/FlexCollectionConverter.cs

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,30 @@
11
using System;
2-
using System.Text.Json;
32
using Hydra;
4-
using Hydra.Collections;
53
using Newtonsoft.Json;
64
using Newtonsoft.Json.Linq;
75
using JsonSerializer = Newtonsoft.Json.JsonSerializer;
86

97
namespace DLCS.HydraModel.Converters;
108

11-
public class ItemArrayOrHydraCollectionConverter<T> : JsonConverter<ItemArrayOrHydraCollection<T>> where T : JsonLdBase
9+
/// <summary>
10+
/// This converter, activated for <see cref="FlexCollection{T}"/>, checks whether the incoming JSON is a single
11+
/// <typeparamref name="T"/> object, a JSON Array of <typeparamref name="T"/> objects, or a HydraCollection where
12+
/// the 1..N <typeparamref name="T"/> objects exist as an array under `member` property
13+
/// </summary>
14+
/// <typeparam name="T"></typeparam>
15+
public class FlexCollectionConverter<T> : JsonConverter<FlexCollection<T>> where T : JsonLdBase
1216
{
1317
public static void Register(JsonSerializerSettings settings)
1418
{
15-
settings.Converters.Add(new ItemArrayOrHydraCollectionConverter<T>());
19+
settings.Converters.Add(new FlexCollectionConverter<T>());
1620
}
1721

1822
public override bool CanWrite => false;
1923

20-
public override void WriteJson(JsonWriter writer, ItemArrayOrHydraCollection<T>? value, JsonSerializer serializer)
24+
public override void WriteJson(JsonWriter writer, FlexCollection<T>? value, JsonSerializer serializer)
2125
=> throw new NotSupportedException();
2226

23-
public override ItemArrayOrHydraCollection<T>? ReadJson(JsonReader reader, Type objectType, ItemArrayOrHydraCollection<T>? existingValue,
27+
public override FlexCollection<T>? ReadJson(JsonReader reader, Type objectType, FlexCollection<T>? existingValue,
2428
bool hasExistingValue, JsonSerializer serializer)
2529
{
2630
// Sanity check -> fail fast
@@ -37,10 +41,10 @@ public override void WriteJson(JsonWriter writer, ItemArrayOrHydraCollection<T>?
3741
{
3842
case JTokenType.Array:
3943
var array = token.ToObject<T[]>(serializer);
40-
return new ItemArrayOrHydraCollection<T>(array ?? []);
44+
return new FlexCollection<T>(array ?? []);
4145
case JTokenType.Object:
4246
var items = ReadObjectCase((JObject)token, serializer);
43-
return new ItemArrayOrHydraCollection<T>(items);
47+
return new FlexCollection<T>(items);
4448
default:
4549
return null;
4650
}
@@ -56,15 +60,15 @@ private static T[] ReadObjectCase(JObject obj, JsonSerializer serializer)
5660
}
5761

5862
// Seems to be HydraCollection
59-
var members = obj["members"];
63+
var members = obj["member"];
6064

6165
if (members == null)
6266
{
6367
return [];
6468
}
6569

6670
return members.Type != JTokenType.Array
67-
? throw new JsonSerializationException("'members' must be an array.")
71+
? throw new JsonSerializationException("'member' must be an array.")
6872
: members.ToObject<T[]>(serializer) ?? [];
6973
}
7074

src/protagonist/DLCS.HydraModel/ItemArrayOrHydraCollection.cs renamed to src/protagonist/DLCS.HydraModel/FlexCollection.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,13 @@
66
namespace DLCS.HydraModel;
77

88
/// <summary>
9-
/// Used as a wrapper/guide for deserializer to accept either <typeparamref name="T"/>, or plain JSON array of
9+
/// With deserialization step in <see cref="FlexCollectionConverter{T}"/>, this collection allows for
10+
/// converting JSON input that's either a JSON representation of a single object <typeparamref name="T"/>, or plain JSON array of
1011
/// <typeparamref name="T"/>, or a <see cref="HydraCollection{T}"/>
1112
/// </summary>
1213
/// <param name="items">Used by deserializing converter to set item or items extracted from JSON payload</param>
1314
/// <typeparam name="T">Any valid <see cref="JsonLdBase"/> derrivative</typeparam>
14-
public class ItemArrayOrHydraCollection<T>(params T[] items)
15+
public class FlexCollection<T>(params T[] items)
1516
where T : JsonLdBase
1617
{
1718
public T[] Items { get; } = items;

0 commit comments

Comments
 (0)