From 5556ea577e5cb982c42c0789a8c4238d3837544c Mon Sep 17 00:00:00 2001 From: Donald Gray Date: Fri, 10 Jan 2025 10:54:26 +0000 Subject: [PATCH 1/2] Add 'batch' NQ parameter parsing --- src/protagonist/DLCS.Core/Strings/StringX.cs | 2 +- .../Assets/NamedQueries/ParsedNamedQuery.cs | 8 ++++- .../Parsing/IIIFNamedQueryParserTests.cs | 26 ++++++++++++-- .../Parsing/PdfNamedQueryParserTests.cs | 17 +++++++--- .../Parsing/BaseNamedQueryParser.cs | 34 +++++++++++-------- .../Parsing/StoredNamedQueryParser.cs | 2 +- 6 files changed, 65 insertions(+), 24 deletions(-) diff --git a/src/protagonist/DLCS.Core/Strings/StringX.cs b/src/protagonist/DLCS.Core/Strings/StringX.cs index 7d8a9a527..0a4ee012a 100644 --- a/src/protagonist/DLCS.Core/Strings/StringX.cs +++ b/src/protagonist/DLCS.Core/Strings/StringX.cs @@ -132,7 +132,7 @@ string DomainMapper(Match match) /// String to split /// String to split by. /// String split, or empty list. - public static IEnumerable SplitSeparatedString(this string str, string separator) + public static IEnumerable SplitSeparatedString(this string? str, string separator) => str?.Trim().Split(separator, StringSplitOptions.RemoveEmptyEntries) ?? Enumerable.Empty(); /// diff --git a/src/protagonist/DLCS.Model/Assets/NamedQueries/ParsedNamedQuery.cs b/src/protagonist/DLCS.Model/Assets/NamedQueries/ParsedNamedQuery.cs index 25470a2d8..7a4dc20b4 100644 --- a/src/protagonist/DLCS.Model/Assets/NamedQueries/ParsedNamedQuery.cs +++ b/src/protagonist/DLCS.Model/Assets/NamedQueries/ParsedNamedQuery.cs @@ -54,6 +54,11 @@ public class ParsedNamedQuery /// public long? Number3 { get; set; } + /// + /// Value of "batches" after parsing + /// + public int[]? Batches { get; set; } + /// /// The name of the namedQuery this object was parsed from. /// @@ -99,7 +104,8 @@ public enum QueryMapping String3, Number1, Number2, - Number3 + Number3, + Batch, } public enum OrderDirection diff --git a/src/protagonist/DLCS.Repository.Tests/NamedQueries/Parsing/IIIFNamedQueryParserTests.cs b/src/protagonist/DLCS.Repository.Tests/NamedQueries/Parsing/IIIFNamedQueryParserTests.cs index c5b750ad8..0802159bd 100644 --- a/src/protagonist/DLCS.Repository.Tests/NamedQueries/Parsing/IIIFNamedQueryParserTests.cs +++ b/src/protagonist/DLCS.Repository.Tests/NamedQueries/Parsing/IIIFNamedQueryParserTests.cs @@ -81,6 +81,9 @@ public void GenerateParsedNamedQueryFromRequest_ReturnsFaultParsedNQ_IfInvalidPa [Theory] [InlineData("n1=p1", "not-an-int")] [InlineData("n1=p1&#=not-an-int", "")] + [InlineData("batch=p1", "not-an-int")] + [InlineData("batch=p1", "1|2|3")] + [InlineData("batch=p1&#=not-an-int", "")] public void GenerateParsedNamedQueryFromRequest_ReturnsFaultParsedNQ_IfNonNumberPassedForNumberArg( string template, string args) @@ -126,6 +129,12 @@ public void GenerateParsedNamedQueryFromRequest_SuccessfullyParses(string templa "Space from template" }, new object[] + { + "batch=p1&#=10", "", + new IIIFParsedNamedQuery(Customer) { Batches = new[] { 10 }, NamedQueryName = "my-query" }, + "Single batch from template" + }, + new object[] { "manifest=s1&spacename=p1", "10", new IIIFParsedNamedQuery(Customer) @@ -138,7 +147,7 @@ public void GenerateParsedNamedQueryFromRequest_SuccessfullyParses(string templa new IIIFParsedNamedQuery(Customer) { String1 = "string-1", Number1 = 40, Space = 1, Manifest = ParsedNamedQuery.QueryMapping.String1, - AssetOrdering = new List{new(ParsedNamedQuery.QueryMapping.Number2)}, + AssetOrdering = new List { new(ParsedNamedQuery.QueryMapping.Number2) }, NamedQueryName = "my-query" }, "All params" @@ -149,7 +158,7 @@ public void GenerateParsedNamedQueryFromRequest_SuccessfullyParses(string templa new IIIFParsedNamedQuery(Customer) { String1 = "string-1", Number1 = 40, Space = 10, Manifest = ParsedNamedQuery.QueryMapping.String1, - AssetOrdering = new List{new(ParsedNamedQuery.QueryMapping.Number2)}, + AssetOrdering = new List { new(ParsedNamedQuery.QueryMapping.Number2) }, NamedQueryName = "my-query" }, "Extra args are ignored" @@ -160,10 +169,21 @@ public void GenerateParsedNamedQueryFromRequest_SuccessfullyParses(string templa new IIIFParsedNamedQuery(Customer) { String1 = "string-1", Number1 = 40, Space = 1, Manifest = ParsedNamedQuery.QueryMapping.String1, - AssetOrdering = new List{new(ParsedNamedQuery.QueryMapping.Number2)}, + AssetOrdering = new List { new(ParsedNamedQuery.QueryMapping.Number2) }, NamedQueryName = "my-query" }, "Incorrect template pairs are ignored" }, + new object[] + { + "manifest=s1&canvas=n2&s1=p1&n1=p2&batch=p3&space=p4&#=1", "string-1/40/10,20,30", + new IIIFParsedNamedQuery(Customer) + { + String1 = "string-1", Number1 = 40, Space = 1, Manifest = ParsedNamedQuery.QueryMapping.String1, + AssetOrdering = new List { new(ParsedNamedQuery.QueryMapping.Number2) }, + NamedQueryName = "my-query", Batches = new[] { 10, 20, 30 } + }, + "All params including multi Batch" + } }; } \ No newline at end of file diff --git a/src/protagonist/DLCS.Repository.Tests/NamedQueries/Parsing/PdfNamedQueryParserTests.cs b/src/protagonist/DLCS.Repository.Tests/NamedQueries/Parsing/PdfNamedQueryParserTests.cs index e3488efc9..b37cc92de 100644 --- a/src/protagonist/DLCS.Repository.Tests/NamedQueries/Parsing/PdfNamedQueryParserTests.cs +++ b/src/protagonist/DLCS.Repository.Tests/NamedQueries/Parsing/PdfNamedQueryParserTests.cs @@ -158,12 +158,21 @@ public void GenerateParsedNamedQueryFromRequest_SuccessfullyParses(string templa "Spacename from param" }, new object[] + { + "batch=p1", "10,20,40", + new PdfParsedNamedQuery(99) + { + NamedQueryName = "my-query", Batches = new[] { 10, 20, 40 }, Args = new List { "10,20,40" } + }, + "Batch from template" + }, + new object[] { "redactedmessage=you cannot view&canvas=s1&s1=p1&n1=p2&space=p3&#=1", "string-1/40", new PdfParsedNamedQuery(99) { String1 = "string-1", Number1 = 40, Space = 1, RedactedMessage = "you cannot view", - AssetOrdering = new List{new(ParsedNamedQuery.QueryMapping.String1)}, + AssetOrdering = new List { new(ParsedNamedQuery.QueryMapping.String1) }, Args = new List { "string-1", "40", "1" }, NamedQueryName = "my-query", }, "All params except format" @@ -174,7 +183,7 @@ public void GenerateParsedNamedQueryFromRequest_SuccessfullyParses(string templa new PdfParsedNamedQuery(99) { String1 = "string-1", Number1 = 40, Space = 10, - AssetOrdering = new List{new(ParsedNamedQuery.QueryMapping.Number2)}, + AssetOrdering = new List { new(ParsedNamedQuery.QueryMapping.Number2) }, Args = new List { "string-1", "40", "10", "100", "1" }, NamedQueryName = "my-query", }, "Extra args are ignored" @@ -184,8 +193,8 @@ public void GenerateParsedNamedQueryFromRequest_SuccessfullyParses(string templa "manifest=s1&&n3=&canvas=n2&=10&s1=p1&n1=p2&space=p3&#=1", "string-1/40", new PdfParsedNamedQuery(99) { - String1 = "string-1", Number1 = 40, Space = 1, - AssetOrdering = new List{new(ParsedNamedQuery.QueryMapping.Number2)}, + String1 = "string-1", Number1 = 40, Space = 1, + AssetOrdering = new List { new(ParsedNamedQuery.QueryMapping.Number2) }, Args = new List { "string-1", "40", "1" }, NamedQueryName = "my-query", }, "Incorrect template pairs are ignored" diff --git a/src/protagonist/DLCS.Repository/NamedQueries/Parsing/BaseNamedQueryParser.cs b/src/protagonist/DLCS.Repository/NamedQueries/Parsing/BaseNamedQueryParser.cs index c15bc56dd..96685ffca 100644 --- a/src/protagonist/DLCS.Repository/NamedQueries/Parsing/BaseNamedQueryParser.cs +++ b/src/protagonist/DLCS.Repository/NamedQueries/Parsing/BaseNamedQueryParser.cs @@ -3,6 +3,7 @@ using System.Linq; using DLCS.Core.Collections; using DLCS.Core.Guard; +using DLCS.Core.Strings; using DLCS.Model.Assets.NamedQueries; using Microsoft.Extensions.Logging; using QueryMapping = DLCS.Model.Assets.NamedQueries.ParsedNamedQuery.QueryMapping; @@ -12,7 +13,13 @@ namespace DLCS.Repository.NamedQueries.Parsing; /// -/// Basic NQ parser, supporting the following arguments: s1, s2, s3, n1, n2, n3, space, spacename, canvas, # and p* +/// Basic NQ parser; supporting the following arguments: +/// metadata-fields: s1, s2, s3, n1, n2, n3 +/// space-fields: space, spacename +/// ordering-fields: assetOrder, canvas +/// batch: batch +/// auto-added-values: # +/// parameter-values: p* /// public abstract class BaseNamedQueryParser : INamedQueryParser where T : ParsedNamedQuery @@ -35,6 +42,7 @@ public abstract class BaseNamedQueryParser : INamedQueryParser protected const string String3 = "s3"; protected const string AssetOrdering = "assetOrder"; protected const string PathReplacement = "%2F"; + protected const string Batch = "batch"; public BaseNamedQueryParser(ILogger logger) { @@ -73,9 +81,7 @@ protected virtual void PostParsingOperations(T parsedNamedQuery) private static List GetQueryArgsList(string? namedQueryArgs, string[] templatePairing) { // Get any arguments passed to the NQ from URL - var queryArgs = namedQueryArgs.IsNullOrEmpty() - ? new List() - : namedQueryArgs!.Split("/", StringSplitOptions.RemoveEmptyEntries).ToList(); + var queryArgs = namedQueryArgs.SplitSeparatedString("/").ToList(); // Iterate through any pairs that start with '#', as these are treated as additional args foreach (var additionalArg in templatePairing.Where(p => p.StartsWith(AdditionalArgMarker))) @@ -127,13 +133,17 @@ private T GenerateParsedNamedQuery(int customerId, string[] templatePairing, Lis ConvertToLongQueryArg(GetQueryArgumentFromTemplateElement(queryArgs, elements[1])); break; case Number2: - assetQuery.Number2 = + assetQuery.Number2 = ConvertToLongQueryArg(GetQueryArgumentFromTemplateElement(queryArgs, elements[1])); break; case Number3: - assetQuery.Number3 = + assetQuery.Number3 = ConvertToLongQueryArg(GetQueryArgumentFromTemplateElement(queryArgs, elements[1])); break; + case Batch: + assetQuery.Batches = + ConvertToIntArrayQueryArg(GetQueryArgumentFromTemplateElement(queryArgs, elements[1])); + break; } CustomHandling(queryArgs, elements[0], elements[1], assetQuery); @@ -148,15 +158,11 @@ private T GenerateParsedNamedQuery(int customerId, string[] templatePairing, Lis return assetQuery; } - private long? ConvertToLongQueryArg(string? argToConvert) - { - if (argToConvert.IsNullOrEmpty()) - { - return null; - } + private static long? ConvertToLongQueryArg(string? argToConvert) + => argToConvert.IsNullOrEmpty() ? null : long.Parse(argToConvert); - return long.Parse(argToConvert); - } + private static int[]? ConvertToIntArrayQueryArg(string? argToConvert) + => argToConvert.IsNullOrEmpty() ? null : argToConvert.SplitSeparatedString(",").Select(int.Parse).ToArray(); /// /// Adds handling for any custom key/value pairs, in addition to the core s1, s2, p1 etc diff --git a/src/protagonist/DLCS.Repository/NamedQueries/Parsing/StoredNamedQueryParser.cs b/src/protagonist/DLCS.Repository/NamedQueries/Parsing/StoredNamedQueryParser.cs index 8f83abc88..936a55cf8 100644 --- a/src/protagonist/DLCS.Repository/NamedQueries/Parsing/StoredNamedQueryParser.cs +++ b/src/protagonist/DLCS.Repository/NamedQueries/Parsing/StoredNamedQueryParser.cs @@ -36,7 +36,7 @@ protected override void CustomHandling(List queryArgs, string key, strin } /// - /// Get the template to use from specified object. + /// Get the template to use from specified object. /// /// Template to use containing {customer}, {queryname} + {args} replacements. protected abstract string GetTemplateFromSettings(NamedQueryTemplateSettings namedQuerySettings); From 4bfc054da228b945715b658a774d363f05537b66 Mon Sep 17 00:00:00 2001 From: Donald Gray Date: Fri, 10 Jan 2025 11:24:54 +0000 Subject: [PATCH 2/2] Filter NQ results by batch --- .../Assets/NamedQueries/ParsedNamedQuery.cs | 3 +- .../NamedQueries/NamedQueryRepositoryTests.cs | 97 ++++++++++++++++--- .../Assets/NamedQueryRepository.cs | 6 ++ 3 files changed, 89 insertions(+), 17 deletions(-) diff --git a/src/protagonist/DLCS.Model/Assets/NamedQueries/ParsedNamedQuery.cs b/src/protagonist/DLCS.Model/Assets/NamedQueries/ParsedNamedQuery.cs index 7a4dc20b4..97af76ad9 100644 --- a/src/protagonist/DLCS.Model/Assets/NamedQueries/ParsedNamedQuery.cs +++ b/src/protagonist/DLCS.Model/Assets/NamedQueries/ParsedNamedQuery.cs @@ -10,7 +10,7 @@ namespace DLCS.Model.Assets.NamedQueries; public class ParsedNamedQuery { /// - /// Collection of OrderBy clauses to apply to assets. + /// Collection of OrderBy clauses to apply to assets /// public List AssetOrdering { get; set; } = new() { new QueryOrder(QueryMapping.Unset) }; @@ -105,7 +105,6 @@ public enum QueryMapping Number1, Number2, Number3, - Batch, } public enum OrderDirection diff --git a/src/protagonist/DLCS.Repository.Tests/NamedQueries/NamedQueryRepositoryTests.cs b/src/protagonist/DLCS.Repository.Tests/NamedQueries/NamedQueryRepositoryTests.cs index e769dbac8..686670539 100644 --- a/src/protagonist/DLCS.Repository.Tests/NamedQueries/NamedQueryRepositoryTests.cs +++ b/src/protagonist/DLCS.Repository.Tests/NamedQueries/NamedQueryRepositoryTests.cs @@ -1,6 +1,6 @@ -using System.Linq; -using DLCS.Core.Caching; +using DLCS.Core.Caching; using DLCS.Core.Types; +using DLCS.Model.Assets; using DLCS.Model.Assets.NamedQueries; using DLCS.Repository.Assets; using LazyCache.Mocks; @@ -14,12 +14,11 @@ namespace DLCS.Repository.Tests.NamedQueries; [Collection(DatabaseCollection.CollectionName)] public class NamedQueryRepositoryTests { - private readonly DlcsContext dbContext; private readonly NamedQueryRepository sut; public NamedQueryRepositoryTests(DlcsDatabaseFixture dbFixture) { - dbContext = dbFixture.DbContext; + var dbContext = dbFixture.DbContext; sut = new NamedQueryRepository(dbFixture.DbContext, new MockCachingService(), Options.Create(new CacheSettings())); @@ -38,6 +37,26 @@ public NamedQueryRepositoryTests(DlcsDatabaseFixture dbFixture) dbContext.Images.AddTestAsset(AssetId.FromString("99/1/7"), space: 2); dbContext.Images.AddTestAsset(AssetId.FromString("99/1/8"), ref1: "foo", ref2: "bar", ref3: "baz", num1: 5, num2: 10, num3: 20); + + // Batch records - first with 3 and second with 2. 1 asset is in both + var batchedAsset1 = AssetId.FromString("99/2/1"); + var batchedAsset2 = AssetId.FromString("99/2/2"); + var batchedAsset3 = AssetId.FromString("99/2/3"); + var batchedAsset4 = AssetId.FromString("99/2/4"); + const int batchId1 = 101010; + const int batchId2 = 101011; + dbContext.Images.AddTestAsset(batchedAsset1, space: 2); + dbContext.Images.AddTestAsset(batchedAsset2, space: 2, num3: 1); + dbContext.Images.AddTestAsset(batchedAsset3, space: 2, num3: 1); + dbContext.Images.AddTestAsset(batchedAsset4, space: 2, num3: 1); + var batch = dbContext.Batches.AddTestBatch(batchId1).Result; + batch.Entity + .AddBatchAsset(batchedAsset1) + .AddBatchAsset(batchedAsset2, BatchAssetStatus.Completed) + .AddBatchAsset(batchedAsset3, BatchAssetStatus.Error); + + var batch2 = dbContext.Batches.AddTestBatch(batchId2).Result; + batch2.Entity.AddBatchAsset(batchedAsset1).AddBatchAsset(batchedAsset4); dbContext.SaveChanges(); } @@ -119,7 +138,7 @@ public async Task GetNamedQueryResults_ReturnsAllForCustomer_IfNoOtherCriteria() var result = await sut.GetNamedQueryResults(query).ToListAsync(); // Assert - result.Count().Should().Be(8); + result.Should().HaveCount(12); } [Fact] @@ -135,7 +154,7 @@ public async Task GetNamedQueryResults_FilterByString1() var result = await sut.GetNamedQueryResults(query).ToListAsync(); // Assert - result.Single().Id.Should().Be(AssetId.FromString("99/1/1")); + result.Should().ContainSingle(r => r.Id == AssetId.FromString("99/1/1")); } [Fact] @@ -151,7 +170,7 @@ public async Task GetNamedQueryResults_FilterByString2() var result = await sut.GetNamedQueryResults(query).ToListAsync(); // Assert - result.Single().Id.Should().Be(AssetId.FromString("99/1/2")); + result.Should().ContainSingle(r => r.Id == AssetId.FromString("99/1/2")); } [Fact] @@ -167,7 +186,7 @@ public async Task GetNamedQueryResults_FilterByString3() var result = await sut.GetNamedQueryResults(query).ToListAsync(); // Assert - result.Single().Id.Should().Be(AssetId.FromString("99/1/3")); + result.Should().ContainSingle(r => r.Id == AssetId.FromString("99/1/3")); } [Fact] @@ -183,7 +202,7 @@ public async Task GetNamedQueryResults_FilterByNumber1() var result = await sut.GetNamedQueryResults(query).ToListAsync(); // Assert - result.Single().Id.Should().Be(AssetId.FromString("99/1/4")); + result.Should().ContainSingle(r => r.Id == AssetId.FromString("99/1/4")); } [Fact] @@ -199,7 +218,7 @@ public async Task GetNamedQueryResults_FilterByNumber2() var result = await sut.GetNamedQueryResults(query).ToListAsync(); // Assert - result.Single().Id.Should().Be(AssetId.FromString("99/1/5")); + result.Should().ContainSingle(r => r.Id == AssetId.FromString("99/1/5")); } [Fact] @@ -215,7 +234,7 @@ public async Task GetNamedQueryResults_FilterByNumber3() var result = await sut.GetNamedQueryResults(query).ToListAsync(); // Assert - result.Single().Id.Should().Be(AssetId.FromString("99/1/6")); + result.Should().ContainSingle(r => r.Id == AssetId.FromString("99/1/6")); } [Fact] @@ -231,7 +250,7 @@ public async Task GetNamedQueryResults_FilterBySpace() var result = await sut.GetNamedQueryResults(query).ToListAsync(); // Assert - result.Count().Should().Be(7); + result.Should().HaveCount(7); } [Fact] @@ -247,7 +266,7 @@ public async Task GetNamedQueryResults_FilterBySpaceName() var result = await sut.GetNamedQueryResults(query).ToListAsync(); // Assert - result.Count().Should().Be(7); + result.Should().HaveCount(7); } [Fact] @@ -263,7 +282,7 @@ public async Task GetNamedQueryResults_FilterBySpaceAndSpaceName_SpaceTakesPrior var result = await sut.GetNamedQueryResults(query).ToListAsync(); // Assert - result.Count().Should().Be(7); + result.Should().HaveCount(7); } [Fact] @@ -280,6 +299,54 @@ public async Task GetNamedQueryResults_FilterByMultipleCriteria() var result = await sut.GetNamedQueryResults(query).ToListAsync(); // Assert - result.Single().Id.Should().Be(AssetId.FromString("99/1/8")); + result.Should().ContainSingle(r => r.Id == AssetId.FromString("99/1/8")); + } + + [Fact] + public async Task GetNamedQueryResults_FilterBySingleBatch() + { + // Arrange + var query = new ParsedNamedQuery(99) + { + Batches = new[] { 101010 } + }; + + // Act + var result = await sut.GetNamedQueryResults(query).ToListAsync(); + + // Assert + result.Should().HaveCount(3); + } + + [Fact] + public async Task GetNamedQueryResults_FilterByMultipleBatch() + { + // Arrange + var query = new ParsedNamedQuery(99) + { + Batches = new[] { 101010, 101011 } + }; + + // Act + var result = await sut.GetNamedQueryResults(query).ToListAsync(); + + // Assert + result.Should().HaveCount(4); + } + + [Fact] + public async Task GetNamedQueryResults_FilterByMultipleBatch_AndOtherCriteria() + { + // Arrange + var query = new ParsedNamedQuery(99) + { + Batches = new[] { 101010, 101011 }, Number3 = 1 + }; + + // Act + var result = await sut.GetNamedQueryResults(query).ToListAsync(); + + // Assert + result.Should().HaveCount(3); } } \ No newline at end of file diff --git a/src/protagonist/DLCS.Repository/Assets/NamedQueryRepository.cs b/src/protagonist/DLCS.Repository/Assets/NamedQueryRepository.cs index fcf11cc46..12267cd0b 100644 --- a/src/protagonist/DLCS.Repository/Assets/NamedQueryRepository.cs +++ b/src/protagonist/DLCS.Repository/Assets/NamedQueryRepository.cs @@ -2,6 +2,7 @@ using System.Linq; using System.Threading.Tasks; using DLCS.Core.Caching; +using DLCS.Core.Collections; using DLCS.Core.Strings; using DLCS.Model.Assets; using DLCS.Model.Assets.NamedQueries; @@ -101,6 +102,11 @@ public IQueryable GetNamedQueryResults(ParsedNamedQuery query) .Select(arg => arg.Image); } + if (!query.Batches.IsNullOrEmpty()) + { + imageFilter = imageFilter.Where(i => i.BatchAssets.Any(ba => query.Batches.Contains(ba.BatchId))); + } + return imageFilter; } } \ No newline at end of file