Skip to content

Commit

Permalink
Merge pull request #872 from dlcs/feature/optionalNamedQuery
Browse files Browse the repository at this point in the history 
Adding support for optional named query parameters
  • Loading branch information
@JackLewis-digirati
JackLewis-digirati committed Jun 27, 2024
2 parents 5aa8eeb + 564ee53 commit f458a03
Show file tree
Hide file tree
Showing 7 changed files with 149 additions and 62 deletions.
15 changes: 14 additions & 1 deletion src/protagonist/API.Tests/Integration/CustomerResourcesTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,27 @@ public async Task Delete_PDF_Returns400_IfUnableToFind()
}

[Fact]
public async Task Delete_PDF_Returns400_IfArgsIncorrect()
public async Task Delete_PDF_Returns200_IfLessArgsThanQuery()
{
// Arrange
var path = "/customers/99/resources/pdf/cust-resource?args=too-little";

// Act
var response = await httpClient.AsCustomer().DeleteAsync(path);

// Assert
response.StatusCode.Should().Be(HttpStatusCode.OK);
}

[Fact]
public async Task Delete_PDF_Returns400_IfNoArgs()
{
// Arrange
var path = "/customers/99/resources/pdf/cust-resource";

// Act
var response = await httpClient.AsCustomer().DeleteAsync(path);

// Assert
response.StatusCode.Should().Be(HttpStatusCode.BadRequest);
}
Expand Down
23 changes: 18 additions & 5 deletions src/protagonist/DLCS.Repository.Tests/NamedQueries/Parsing/IIIFNamedQueryParserTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,33 @@ public void GenerateParsedNamedQueryFromRequest_Throws_IfTemplateEmptyOrWhiteSpa
.WithMessage("Value cannot be null. (Parameter 'namedQueryTemplate')");
}

[Fact]
public void GenerateParsedNamedQueryFromRequest_ReturnFaultyNQ_IfNoParametersPassed()
{
// Act
var result =
sut.GenerateParsedNamedQueryFromRequest<IIIFParsedNamedQuery>(Customer, "", "s1=p1&space=p2", "my-query");

// Assert
result.IsFaulty.Should().BeTrue();
result.ErrorMessage.Should().StartWith("Named query must have at least 1 argument");
}

[Theory]
[InlineData("space=p1", "")]
[InlineData("space=p1&s1=p2", "1")]
[InlineData("s1=p1&space=p2", "1")]
[InlineData("s1=p1&n1=p2", "1")]
[InlineData("s1=p1&n1=&n2=p2", "1/2")]
[InlineData("space=p1&s1=p2&#=1", "")]
public void GenerateParsedNamedQueryFromRequest_ReturnsFaultParsedNQ_IfTooFewParamsPassed(string template,
[InlineData("space=p1&s1=p2&#=1", "10")]
public void GenerateParsedNamedQueryFromRequest_ReturnsNQ_IfLessQueriesPassedThanParameters(string template,
string args)
{
// Act
var result =
sut.GenerateParsedNamedQueryFromRequest<IIIFParsedNamedQuery>(Customer, args, template, "my-query");

// Assert
result.IsFaulty.Should().BeTrue();
result.ErrorMessage.Should().StartWith("Not enough query arguments to satisfy template element parameter");
result.IsFaulty.Should().BeFalse();
}

[Theory]
Expand Down
23 changes: 18 additions & 5 deletions src/protagonist/DLCS.Repository.Tests/NamedQueries/Parsing/PdfNamedQueryParserTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,33 @@ public void GenerateParsedNamedQueryFromRequest_Throws_IfTemplateEmptyOrWhiteSpa
.WithMessage("Value cannot be null. (Parameter 'namedQueryTemplate')");
}

[Fact]
public void GenerateParsedNamedQueryFromRequest_ReturnFaultyNQ_IfNoParametersPassed()
{
// Act
var result =
sut.GenerateParsedNamedQueryFromRequest<PdfParsedNamedQuery>(99, "", "s1=p1&space=p2", "my-query");

// Assert
result.IsFaulty.Should().BeTrue();
result.ErrorMessage.Should().StartWith("Named query must have at least 1 argument");
}

[Theory]
[InlineData("space=p1", "")]
[InlineData("space=p1&s1=p2", "1")]
[InlineData("s1=p1&space=p2", "1")]
[InlineData("s1=p1&n1=p2", "1")]
[InlineData("s1=p1&n1=&n2=p2", "1/2")]
[InlineData("space=p1&s1=p2&#=1", "")]
public void GenerateParsedNamedQueryFromRequest_ReturnsFaultParsedNQ_IfTooFewParamsPassed(string template,
[InlineData("space=p1&s1=p2&#=1", "10")]
public void GenerateParsedNamedQueryFromRequest_ReturnsNQ_IfLessQueriesPassedThanParameters(string template,
string args)
{
// Act
var result =
sut.GenerateParsedNamedQueryFromRequest<PdfParsedNamedQuery>(99, args, template, "my-query");

// Assert
result.IsFaulty.Should().BeTrue();
result.ErrorMessage.Should().StartWith("Not enough query arguments to satisfy template element parameter");
result.IsFaulty.Should().BeFalse();
}

[Theory]
Expand Down
38 changes: 27 additions & 11 deletions src/protagonist/DLCS.Repository/NamedQueries/Parsing/BaseNamedQueryParser.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public abstract class BaseNamedQueryParser<T> : INamedQueryParser
protected const string String3 = "s3";
protected const string AssetOrdering = "assetOrder";
protected const string PathReplacement = "%2F";

public BaseNamedQueryParser(ILogger logger)
{
Logger = logger;
Expand Down Expand Up @@ -91,7 +91,7 @@ private static List<string> GetQueryArgsList(string? namedQueryArgs, string[] te
private T GenerateParsedNamedQuery(int customerId, string[] templatePairing, List<string> queryArgs)
{
var assetQuery = GenerateParsedQueryObject(customerId);

// Iterate through all of the pairs and generate the NQ model
try
{
Expand All @@ -107,7 +107,8 @@ private T GenerateParsedNamedQuery(int customerId, string[] templatePairing, Lis
assetQuery.AssetOrdering = GetAssetOrderingFromTemplateElement(elements[1]);
break;
case Space:
assetQuery.Space = int.Parse(GetQueryArgumentFromTemplateElement(queryArgs, elements[1]));
assetQuery.Space =
(int?)ConvertToLongQueryArg(GetQueryArgumentFromTemplateElement(queryArgs, elements[1]));
break;
case SpaceName:
assetQuery.SpaceName = GetQueryArgumentFromTemplateElement(queryArgs, elements[1]);
Expand All @@ -123,15 +124,15 @@ private T GenerateParsedNamedQuery(int customerId, string[] templatePairing, Lis
break;
case Number1:
assetQuery.Number1 =
long.Parse(GetQueryArgumentFromTemplateElement(queryArgs, elements[1]));
ConvertToLongQueryArg(GetQueryArgumentFromTemplateElement(queryArgs, elements[1]));
break;
case Number2:
assetQuery.Number2 =
long.Parse(GetQueryArgumentFromTemplateElement(queryArgs, elements[1]));
assetQuery.Number2 =
ConvertToLongQueryArg(GetQueryArgumentFromTemplateElement(queryArgs, elements[1]));
break;
case Number3:
assetQuery.Number3 =
long.Parse(GetQueryArgumentFromTemplateElement(queryArgs, elements[1]));
assetQuery.Number3 =
ConvertToLongQueryArg(GetQueryArgumentFromTemplateElement(queryArgs, elements[1]));
break;
}

Expand All @@ -147,6 +148,16 @@ private T GenerateParsedNamedQuery(int customerId, string[] templatePairing, Lis
return assetQuery;
}

private long? ConvertToLongQueryArg(string? argToConvert)
{
if (argToConvert.IsNullOrEmpty())
{
return null;
}

return long.Parse(argToConvert);
}

/// <summary>
/// Adds handling for any custom key/value pairs, in addition to the core s1, s2, p1 etc
/// </summary>
Expand All @@ -162,14 +173,19 @@ private T GenerateParsedNamedQuery(int customerId, string[] templatePairing, Lis
/// <remarks>Could use Activator.CreateInstance this avoids using reflection</remarks>
protected abstract T GenerateParsedQueryObject(int customerId);

protected string GetQueryArgumentFromTemplateElement(List<string> args, string element)
protected string? GetQueryArgumentFromTemplateElement(List<string> args, string element)
{
// Arg will be in format p1, p2, p3 etc. Get the index, then extract that element from args list
if (!element.StartsWith(ParameterPrefix) || element.Length <= 1)
{
// default to just return the element as a literal
return element;
}

if (args.Count == 0)
{
throw new ArgumentException("Named query must have at least 1 argument");
}

if (int.TryParse(element[1..], out int argNumber))
{
Expand All @@ -178,8 +194,8 @@ protected string GetQueryArgumentFromTemplateElement(List<string> args, string e
return args[argNumber - 1].Replace(PathReplacement, "/", StringComparison.OrdinalIgnoreCase);
}

throw new ArgumentOutOfRangeException(element,
"Not enough query arguments to satisfy template element parameter");
// parameter out of range of supplied arguments, assumed to be an optional param to the NQ
return null;
}

throw new ArgumentException($"Could not parse template element parameter '{element}'", element);
Expand Down
21 changes: 17 additions & 4 deletions src/protagonist/Orchestrator.Tests/Integration/NamedQueryTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,23 @@ public async Task Get_Returns404_IfCustomerNotFound(string path)
}

[Theory]
[InlineData("iiif-resource/99/test-named-query/too-little-params")]
[InlineData("iiif-resource/v2/99/test-named-query/too-little-params")]
[InlineData("iiif-resource/v3/99/test-named-query/too-little-params")]
public async Task Get_Returns400_IfNamedQueryParametersIncorrect(string path)
[InlineData("iiif-resource/99/test-named-query/my-ref")]
[InlineData("iiif-resource/v2/99/test-named-query/my-ref")]
[InlineData("iiif-resource/v3/99/test-named-query/my-ref")]
public async Task Get_Returns200_IfNamedQueryParametersLessThanMax(string path)
{
// Act
var response = await httpClient.GetAsync(path);

// Assert
response.StatusCode.Should().Be(HttpStatusCode.OK);
}

[Theory]
[InlineData("iiif-resource/99/test-named-query")]
[InlineData("iiif-resource/v2/99/test-named-query")]
[InlineData("iiif-resource/v3/99/test-named-query")]
public async Task Get_Returns400_IfNoNamedQueryParameters(string path)
{
// Act
var response = await httpClient.GetAsync(path);
Expand Down
39 changes: 32 additions & 7 deletions src/protagonist/Orchestrator.Tests/Integration/PdfTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ public PdfTests(ProtagonistAppFactory<Startup> factory, StorageFixture orchestra
maxUnauthorised: 10, roles: "clickthrough");
dbFixture.DbContext.Images.AddTestAsset(AssetId.FromString("99/1/not-for-delivery"), num1: 6, ref1: "my-ref",
notForDelivery: true);
dbFixture.DbContext.Images.AddTestAsset(AssetId.FromString("99/1/limited-projection"), num1: 2,
ref1: "limited-ref");
dbFixture.DbContext.SaveChanges();
}

Expand Down Expand Up @@ -110,10 +112,10 @@ public async Task GetPdf_Returns404_IfNQNotFound()
}

[Fact]
public async Task GetPdf_Returns400_IfParametersIncorrect()
public async Task GetPdf_Returns400_IfNoParameters()
{
// Arrange
const string path = "pdf/99/test-pdf/too-little-params";
const string path = "pdf/99/test-pdf";

// Act
var response = await httpClient.GetAsync(path);
Expand Down Expand Up @@ -150,6 +152,28 @@ await AddPdfControlFile("99/pdf/test-pdf/my-ref/1/1/tester.json",
response.StatusCode.Should().Be(HttpStatusCode.Accepted);
response.Headers.Should().ContainKey("Retry-After");
}

[Fact]
public async Task GetPdf_Returns200_IfParametersLessThanMax()
{
// Arrange
var fakePdfContent = nameof(GetPdf_Returns200_IfParametersLessThanMax);
const string path = "pdf/99/test-pdf/limited-ref";
const string pdfStorageKey = "99/pdf/test-pdf/limited-ref/tester";
await AddPdfControlFile("99/pdf/test-pdf/limited-ref/tester",
new ControlFile { Created = DateTime.UtcNow, InProcess = false });
pdfCreator.AddCallbackFor(pdfStorageKey, (query, assets) =>
{
AddPdf(pdfStorageKey, fakePdfContent).Wait();
return true;
});

// Act
var response = await httpClient.GetAsync(path);

// Assert
response.StatusCode.Should().Be(HttpStatusCode.OK);
}

[Fact]
public async Task GetPdf_Returns200_WithExistingPdf_IfPdfControlFileAndPdfExist()
Expand Down Expand Up @@ -398,10 +422,10 @@ public async Task GetPdfControlFile_Returns404_IfNQNotFound()
}

[Fact]
public async Task GetPdfControlFile_Returns404_IfParametersIncorrect()
public async Task GetPdfControlFile_Returns404_IfNoParameters()
{
// Arrange
const string path = "pdf-control/99/test-pdf/too-little-params";
const string path = "pdf-control/99/test-pdf";

// Act
var response = await httpClient.GetAsync(path);
Expand All @@ -410,11 +434,12 @@ public async Task GetPdfControlFile_Returns404_IfParametersIncorrect()
response.StatusCode.Should().Be(HttpStatusCode.NotFound);
}

[Fact]
public async Task GetPdfControlFile_Returns200_WithEmptyControlFile_IfNQValidButNoControlFile()
[Theory]
[InlineData("pdf-control/99/test-pdf/any-ref/1/2")]
[InlineData("pdf-control/99/test-pdf/any-ref")]
public async Task GetPdfControlFile_Returns200_WithEmptyControlFile_IfNQValidButNoControlFile(string path)
{
// Arrange
const string path = "pdf-control/99/test-pdf/any-ref/1/2";
var pdfControlFile = new PdfControlFile
{
Created = DateTime.MinValue, InProcess = false, Exists = false, Key = string.Empty, ItemCount = 0,
Expand Down
52 changes: 23 additions & 29 deletions src/protagonist/Orchestrator.Tests/Integration/ZipTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ public ZipTests(ProtagonistAppFactory<Startup> factory, StorageFixture orchestra
dbFixture.DbContext.Images.AddTestAsset(AssetId.FromString("99/1/matching-zip-5"), num1: 5, ref1: "my-ref");
dbFixture.DbContext.Images.AddTestAsset(AssetId.FromString("99/1/not-for-delivery"), num1: 6, ref1: "my-ref",
notForDelivery: true);
dbFixture.DbContext.Images.AddTestAsset(AssetId.FromString("99/1/limited-parameter-zip-1"), num1: 2,
ref1: "limited-ref");
dbFixture.DbContext.SaveChanges();
}

Expand Down Expand Up @@ -103,19 +105,6 @@ public async Task GetZip_Returns404_IfNQNotFound()
response.StatusCode.Should().Be(HttpStatusCode.NotFound);
}

[Fact]
public async Task GetZip_Returns400_IfParametersIncorrect()
{
// Arrange
const string path = "zip/99/test-zip/too-little-params";

// Act
var response = await httpClient.GetAsync(path);

// Assert
response.StatusCode.Should().Be(HttpStatusCode.BadRequest);
}

[Fact]
public async Task GetZip_Returns404_IfNoMatchingRecordsFound()
{
Expand Down Expand Up @@ -163,6 +152,23 @@ await AddControlFile("99/zip/test-zip/my-ref/1/1/tester.zip.json",
(await response.Content.ReadAsStringAsync()).Should().Be(fakeContent);
response.Content.Headers.ContentType.Should().Be(new MediaTypeHeaderValue("application/zip"));
}

[Fact]
public async Task GetZip_Returns200_IfLessParametersThanTotal()
{
// Arrange
var fakeContent = nameof(GetZip_Returns200_IfLessParametersThanTotal);
const string path = "zip/99/test-zip/limited-ref";
await AddControlFile("99/zip/test-zip/limited-ref/tester.zip.json",
new ControlFile { Created = DateTime.UtcNow, InProcess = false });
await AddZipArchive("99/zip/test-zip/limited-ref/tester.zip", fakeContent);

// Act
var response = await httpClient.GetAsync(path);

// Assert
response.StatusCode.Should().Be(HttpStatusCode.OK);
}

[Fact]
public async Task GetZip_Returns200_WithNewlyCreatedZip_IfControlFileExistsButZipDoesnt()
Expand Down Expand Up @@ -327,24 +333,12 @@ public async Task GetZipControlFile_Returns404_IfNQNotFound()
response.StatusCode.Should().Be(HttpStatusCode.NotFound);
}

[Fact]
public async Task GetZipControlFile_Returns404_IfParametersIncorrect()
{
// Arrange
const string path = "zip-control/99/test-zip/too-little-params";

// Act
var response = await httpClient.GetAsync(path);

// Assert
response.StatusCode.Should().Be(HttpStatusCode.NotFound);
}

[Fact]
public async Task GetZipControlFile_Returns200_WithEmptyControlFile_IfNQValidButNoControlFile()
[Theory]
[InlineData("zip-control/99/test-zip/any-ref/1/2")]
[InlineData("zip-control/99/test-zip/any-ref")]
public async Task GetZipControlFile_Returns200_WithEmptyControlFile_IfNQValidButNoControlFile(string path)
{
// Arrange
const string path = "zip-control/99/test-zip/any-ref/1/2";
var controlFileJson = JsonConvert.SerializeObject(ControlFile.Empty);

// Act
Expand Down

0 comments on commit f458a03

Please sign in to comment.