Skip to content

Commit

Permalink
Merge pull request #830 from dlcs/feature/manifest_id_fix
Browse files Browse the repository at this point in the history 
Don't include query params in manifest ids
  • Loading branch information
@donaldgray
donaldgray committed Apr 26, 2024
2 parents 9b11e82 + a954464 commit b2fa464
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 24 deletions.
66 changes: 47 additions & 19 deletions src/protagonist/Orchestrator.Tests/Integration/ManifestHandlingTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
using Newtonsoft.Json.Linq;
using Orchestrator.Infrastructure.IIIF;
using Orchestrator.Tests.Integration.Infrastructure;
using Test.Helpers.Data;
using Test.Helpers.Integration;
using IIIF3 = IIIF.Presentation.V3;

Expand Down Expand Up @@ -125,7 +126,7 @@ public async Task Get_UnknownImage_Returns404(string path)
public async Task Get_NotForDelivery_Returns404()
{
// Arrange
var id = AssetId.FromString($"99/1/{nameof(Get_NotForDelivery_Returns404)}");
var id = AssetIdGenerator.GetAssetId();
await dbFixture.DbContext.Images.AddTestAsset(id, notForDelivery: true);
await dbFixture.DbContext.SaveChangesAsync();

Expand All @@ -144,7 +145,7 @@ public async Task Get_NotForDelivery_Returns404()
public async Task Get_NonImage_Returns404(AssetFamily family)
{
// Arrange
var id = AssetId.FromString($"99/1/{nameof(Get_NonImage_Returns404)}:{family}");
var id = AssetIdGenerator.GetAssetId(assetPostfix: $":{family}");
await dbFixture.DbContext.Images.AddTestAsset(id, family: family);
await dbFixture.DbContext.SaveChangesAsync();

Expand All @@ -161,7 +162,7 @@ public async Task Get_NonImage_Returns404(AssetFamily family)
public async Task Get_ManifestForImage_ReturnsManifest_CustomPathRules_Ignored()
{
// Arrange
var id = AssetId.FromString($"99/1/{nameof(Get_ManifestForImage_ReturnsManifest_CustomPathRules_Ignored)}");
var id = AssetIdGenerator.GetAssetId();
await dbFixture.DbContext.Images.AddTestAsset(id, origin: "testorigin", imageDeliveryChannels: imageDeliveryChannels);
await dbFixture.DbContext.SaveChangesAsync();

Expand All @@ -184,11 +185,38 @@ public async Task Get_ManifestForImage_ReturnsManifest_CustomPathRules_Ignored()
response.Headers.CacheControl.MaxAge.Should().BeGreaterThan(TimeSpan.FromSeconds(2));
}

[Fact]
public async Task Get_ManifestForImage_ReturnsManifest_IdIgnoresQueryString()
{
// Arrange
var id = AssetIdGenerator.GetAssetId();
await dbFixture.DbContext.Images.AddTestAsset(id, origin: "testorigin", imageDeliveryChannels: imageDeliveryChannels);
await dbFixture.DbContext.SaveChangesAsync();

var path = $"iiif-manifest/v2/{id}";

// Act
var request = new HttpRequestMessage(HttpMethod.Get, $"{path}?foo=bar");
request.Headers.Add("Host", "my-proxy.com");
var response = await httpClient.SendAsync(request);

// Assert
var jsonResponse = JObject.Parse(await response.Content.ReadAsStringAsync());
jsonResponse["@id"].ToString().Should().Be($"http://my-proxy.com/iiif-manifest/v2/{id}");
jsonResponse.SelectToken("sequences[0].canvases[0].thumbnail.@id").Value<string>()
.Should().StartWith($"http://my-proxy.com/thumbs/{id}/full");

response.StatusCode.Should().Be(HttpStatusCode.OK);
response.Headers.Should().ContainKey("x-asset-id").WhoseValue.Should().ContainSingle(id.ToString());
response.Headers.CacheControl.Public.Should().BeTrue();
response.Headers.CacheControl.MaxAge.Should().BeGreaterThan(TimeSpan.FromSeconds(2));
}

[Fact]
public async Task Get_ManifestForImage_ReturnsManifest()
{
// Arrange
var id = AssetId.FromString($"99/1/{nameof(Get_ManifestForImage_ReturnsManifest)}");
var id = AssetIdGenerator.GetAssetId();
await dbFixture.DbContext.Images.AddTestAsset(id, origin: "testorigin", imageDeliveryChannels: imageDeliveryChannels);
await dbFixture.DbContext.SaveChangesAsync();

Expand All @@ -213,7 +241,7 @@ public async Task Get_ManifestForImage_ReturnsManifest()
public async Task Get_V2ManifestForImage_ReturnsManifest_FromMetadata()
{
// Arrange
var id = AssetId.FromString($"99/1/{nameof(Get_ManifestForImage_ReturnsManifest)}");
var id = AssetIdGenerator.GetAssetId();
await dbFixture.DbContext.Images.AddTestAsset(id, origin: "testorigin", imageDeliveryChannels: imageDeliveryChannels)
.WithTestThumbnailMetadata();
await dbFixture.DbContext.SaveChangesAsync();
Expand All @@ -239,7 +267,7 @@ await dbFixture.DbContext.Images.AddTestAsset(id, origin: "testorigin", imageDel
public async Task Get_V2ManifestForImage_ReturnsManifestNoThumbnails_WhenNoThumbsChannel()
{
// Arrange
var id = AssetId.FromString($"99/1/{nameof(Get_ManifestForImage_ReturnsManifest)}");
var id = AssetIdGenerator.GetAssetId();
await dbFixture.DbContext.Images.AddTestAsset(id, origin: "testorigin").WithTestThumbnailMetadata();
await dbFixture.DbContext.SaveChangesAsync();

Expand All @@ -263,7 +291,7 @@ public async Task Get_V2ManifestForImage_ReturnsManifestNoThumbnails_WhenNoThumb
public async Task Get_ManifestForImage_ReturnsManifest_ByName()
{
// Arrange
var id = AssetId.FromString($"99/1/{nameof(Get_ManifestForImage_ReturnsManifest_ByName)}");
var id = AssetIdGenerator.GetAssetId();
var namedId = $"test/1/{nameof(Get_ManifestForImage_ReturnsManifest_ByName)}";
await dbFixture.DbContext.Images.AddTestAsset(id, origin: "testorigin", imageDeliveryChannels: imageDeliveryChannels);
await dbFixture.DbContext.SaveChangesAsync();
Expand All @@ -290,7 +318,7 @@ public async Task Get_V3ManifestForImage_ReturnsManifest_WithCustomFields()
{
// Arrange
const string defaultLanguage = "none";
var id = AssetId.FromString($"99/1/{nameof(Get_V3ManifestForImage_ReturnsManifest_WithCustomFields)}");
var id = AssetIdGenerator.GetAssetId();
var namedId = $"test/1/{nameof(Get_V3ManifestForImage_ReturnsManifest_WithCustomFields)}";
var asset = await dbFixture.DbContext.Images.AddTestAsset(id,
origin: "testorigin",
Expand Down Expand Up @@ -330,7 +358,7 @@ public async Task Get_V3ManifestForImage_ReturnsManifest_WithCustomFields()
public async Task Get_V3ManifestForImage_ReturnsManifest_FromMetadata()
{
// Arrange
var id = AssetId.FromString($"99/1/{nameof(Get_ManifestForImage_ReturnsManifest)}");
var id = AssetIdGenerator.GetAssetId();
await dbFixture.DbContext.Images.AddTestAsset(id, origin: "testorigin", imageDeliveryChannels: imageDeliveryChannels)
.WithTestThumbnailMetadata();
await dbFixture.DbContext.SaveChangesAsync();
Expand All @@ -356,7 +384,7 @@ await dbFixture.DbContext.Images.AddTestAsset(id, origin: "testorigin", imageDel
public async Task Get_V3ManifestForImage_ReturnsManifestNoThumbnails_WhenNoThumbsChannel()
{
// Arrange
var id = AssetId.FromString($"99/1/{nameof(Get_ManifestForImage_ReturnsManifest)}");
var id = AssetIdGenerator.GetAssetId();
await dbFixture.DbContext.Images.AddTestAsset(id, origin: "testorigin").WithTestThumbnailMetadata();
await dbFixture.DbContext.SaveChangesAsync();

Expand All @@ -380,7 +408,7 @@ public async Task Get_V3ManifestForImage_ReturnsManifestNoThumbnails_WhenNoThumb
public async Task Get_V2ManifestForImage_ReturnsManifest_WithCustomFields()
{
// Arrange
var id = AssetId.FromString($"99/1/{nameof(Get_V2ManifestForImage_ReturnsManifest_WithCustomFields)}");
var id = AssetIdGenerator.GetAssetId();
var namedId = $"test/1/{nameof(Get_V2ManifestForImage_ReturnsManifest_WithCustomFields)}";
var asset = await dbFixture.DbContext.Images.AddTestAsset(id,
origin: "testorigin",
Expand Down Expand Up @@ -420,7 +448,7 @@ public async Task Get_V2ManifestForImage_ReturnsManifest_WithCustomFields()
public async Task Get_V2ManifestForRestrictedImage_ReturnsManifest_WithoutAuthServices()
{
// Arrange
var id = AssetId.FromString($"99/1/{nameof(Get_V2ManifestForRestrictedImage_ReturnsManifest_WithoutAuthServices)}");
var id = AssetIdGenerator.GetAssetId();
await dbFixture.DbContext.Images.AddTestAsset(id, roles: "clickthrough", maxUnauthorised: 400,
origin: "testorigin", imageDeliveryChannels: imageDeliveryChannels);
await dbFixture.DbContext.SaveChangesAsync();
Expand Down Expand Up @@ -451,7 +479,7 @@ public async Task Get_V2ManifestForRestrictedImage_ReturnsManifest_WithoutAuthSe
public async Task Get_ReturnsV2Manifest_ViaConneg()
{
// Arrange
var id = AssetId.FromString($"99/1/{nameof(Get_ReturnsV2Manifest_ViaConneg)}");
var id = AssetIdGenerator.GetAssetId();
await dbFixture.DbContext.Images.AddTestAsset(id, origin: "testorigin", imageDeliveryChannels: imageDeliveryChannels);
await dbFixture.DbContext.SaveChangesAsync();
var path = $"iiif-manifest/{id}";
Expand All @@ -478,7 +506,7 @@ public async Task Get_ReturnsV2Manifest_ViaConneg()
public async Task Get_ReturnsV2Manifest_ViaDirectPath()
{
// Arrange
var id = AssetId.FromString($"99/1/{nameof(Get_ReturnsV2Manifest_ViaDirectPath)}");
var id = AssetIdGenerator.GetAssetId();
await dbFixture.DbContext.Images.AddTestAsset(id, origin: "testorigin", imageDeliveryChannels: imageDeliveryChannels);
await dbFixture.DbContext.SaveChangesAsync();
var path = $"iiif-manifest/v2/{id}";
Expand All @@ -502,7 +530,7 @@ public async Task Get_ReturnsV2Manifest_ViaDirectPath()
public async Task Get_ReturnsV3Manifest_ViaConneg()
{
// Arrange
var id = AssetId.FromString($"99/1/{nameof(Get_ReturnsV3Manifest_ViaConneg)}");
var id = AssetIdGenerator.GetAssetId();
await dbFixture.DbContext.Images.AddTestAsset(id, origin: "testorigin", imageDeliveryChannels: imageDeliveryChannels);
await dbFixture.DbContext.SaveChangesAsync();
var path = $"iiif-manifest/{id}";
Expand All @@ -529,7 +557,7 @@ public async Task Get_ReturnsV3Manifest_ViaConneg()
public async Task Get_ReturnsV3Manifest_ViaDirectPath()
{
// Arrange
var id = AssetId.FromString($"99/1/{nameof(Get_ReturnsV3Manifest_ViaDirectPath)}");
var id = AssetIdGenerator.GetAssetId();
await dbFixture.DbContext.Images.AddTestAsset(id, origin: "testorigin", imageDeliveryChannels: imageDeliveryChannels);
await dbFixture.DbContext.SaveChangesAsync();
var path = $"iiif-manifest/{id}";
Expand All @@ -553,7 +581,7 @@ public async Task Get_ReturnsV3Manifest_ViaDirectPath()
public async Task Get_ReturnsV3ManifestWithCorrectItemCount_AsCanonical()
{
// Arrange
var id = AssetId.FromString($"99/1/{nameof(Get_ReturnsV3ManifestWithCorrectItemCount_AsCanonical)}");
var id = AssetIdGenerator.GetAssetId();
await dbFixture.DbContext.Images.AddTestAsset(id, origin: "testorigin", imageDeliveryChannels: imageDeliveryChannels);
await dbFixture.DbContext.SaveChangesAsync();
var path = $"iiif-manifest/{id}";
Expand All @@ -576,7 +604,7 @@ public async Task Get_ReturnsV3ManifestWithCorrectItemCount_AsCanonical()
public async Task Get_ReturnsMultipleImageServices()
{
// Arrange
var id = AssetId.FromString($"99/1/{nameof(Get_ReturnsMultipleImageServices)}");
var id = AssetIdGenerator.GetAssetId();
await dbFixture.DbContext.Images.AddTestAsset(id, origin: "testorigin", imageDeliveryChannels: imageDeliveryChannels);
await dbFixture.DbContext.SaveChangesAsync();
var path = $"iiif-manifest/{id}";
Expand All @@ -603,7 +631,7 @@ public async Task Get_ReturnsMultipleImageServices()
public async Task Get_V3ManifestForRestrictedImage_ReturnsManifest_WithAuthServices()
{
// Arrange
var id = AssetId.FromString($"99/1/{nameof(Get_V3ManifestForRestrictedImage_ReturnsManifest_WithAuthServices)}");
var id = AssetIdGenerator.GetAssetId();
await dbFixture.DbContext.Images.AddTestAsset(id, roles: "clickthrough", maxUnauthorised: 400,
origin: "testorigin", imageDeliveryChannels: imageDeliveryChannels);
await dbFixture.DbContext.SaveChangesAsync();
Expand Down
37 changes: 37 additions & 0 deletions src/protagonist/Orchestrator.Tests/Integration/NamedQueryTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,11 +181,30 @@ public async Task Get_ReturnsV2ManifestWithCorrectCount_ViaDirectPath()
jsonResponse.SelectToken("sequences[0].canvases").Count().Should().Be(3);
}

[Fact]
public async Task Get_ReturnsV2Manifest_WithCorrectId_IgnoringQueryParam()
{
// Arrange
const string path = "iiif-resource/v2/99/test-named-query/my-ref/1";
const string iiif2 = "application/ld+json; profile=\"http://iiif.io/api/presentation/2/context.json\"";

// Act
var response = await httpClient.GetAsync($"{path}?foo=bar");

// Assert
response.StatusCode.Should().Be(HttpStatusCode.OK);
response.Headers.Vary.Should().Contain("Accept");
response.Content.Headers.ContentType.ToString().Should().Be(iiif2);
var jsonResponse = JObject.Parse(await response.Content.ReadAsStringAsync());
jsonResponse["@id"].ToString().Should().Be($"http://localhost/{path}");
}

[Fact]
public async Task Get_ReturnsV3ManifestWithCorrectCount_ViaConneg()
{
// Arrange
const string path = "iiif-resource/99/test-named-query/my-ref/1";

const string iiif2 = "application/ld+json; profile=\"http://iiif.io/api/presentation/3/context.json\"";

// Act
Expand Down Expand Up @@ -219,6 +238,24 @@ public async Task Get_ReturnsV3ManifestWithCorrectCount_ViaDirectPath()
jsonResponse.SelectToken("items").Count().Should().Be(3);
}

[Fact]
public async Task Get_ReturnsV3Manifest_WithCorrectId_IgnoringQueryParam()
{
// Arrange
const string path = "iiif-resource/v3/99/test-named-query/my-ref/1";
const string iiif3 = "application/ld+json; profile=\"http://iiif.io/api/presentation/3/context.json\"";

// Act
var response = await httpClient.GetAsync($"{path}?foo=bar");

// Assert
response.StatusCode.Should().Be(HttpStatusCode.OK);
response.Headers.Vary.Should().Contain("Accept");
response.Content.Headers.ContentType.ToString().Should().Be(iiif3);
var jsonResponse = JObject.Parse(await response.Content.ReadAsStringAsync());
jsonResponse["id"].ToString().Should().Be($"http://localhost/{path}");
}

[Fact]
public async Task Get_ReturnsV3ManifestWithCorrectCount_AsCanonical()
{
Expand Down
7 changes: 5 additions & 2 deletions src/protagonist/Orchestrator/Features/Manifests/IIIFNamedQueryProjector.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public IIIFNamedQueryProjector(IIIFManifestBuilder manifestBuilder)
CancellationToken cancellationToken)
{
var sequenceRootUrl = request.GetDisplayUrl("/iiif-query/");
var manifestId = request.GetDisplayUrl();
var manifestId = GetManifestId(request);
var label = GetManifestLabel(parsedNamedQuery);
var manifest =
await manifestBuilder.GenerateV2Manifest(results, customerPathElement, manifestId, label, sequenceRootUrl,
Expand All @@ -70,7 +70,7 @@ public IIIFNamedQueryProjector(IIIFManifestBuilder manifestBuilder)
CustomerPathElement customerPathElement, List<Asset> results, HttpRequest request,
CancellationToken cancellationToken)
{
var manifestId = request.GetDisplayUrl();
var manifestId = GetManifestId(request);
var label = GetManifestLabel(parsedNamedQuery);
var manifest =
await manifestBuilder.GenerateV3Manifest(results, customerPathElement, manifestId, label,
Expand All @@ -79,6 +79,9 @@ public IIIFNamedQueryProjector(IIIFManifestBuilder manifestBuilder)
return manifest;
}

private static string GetManifestId(HttpRequest request) =>
request.GetDisplayUrl(request.Path.Value, includeQueryParams: false);

private static string GetManifestLabel(IIIFParsedNamedQuery parsedNamedQuery)
=> $"Generated from '{parsedNamedQuery.NamedQueryName}' named query";
}
2 changes: 1 addition & 1 deletion src/protagonist/Orchestrator/Features/Manifests/Requests/GetManifestForAsset.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,5 +101,5 @@ public class GetManifestForAssetHandler : IRequestHandler<GetManifestForAsset, D
}

private string GetFullyQualifiedId(BaseAssetRequest baseAssetRequest)
=> assetPathGenerator.GetFullPathForRequest(baseAssetRequest, true);
=> assetPathGenerator.GetFullPathForRequest(baseAssetRequest, true, false);
}
5 changes: 3 additions & 2 deletions src/protagonist/Test.Helpers/Data/AssetIdGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ public static class AssetIdGenerator
/// <summary>
/// Generate new <see cref="AssetId"/> using calling function as "asset" part by default
/// </summary>
public static AssetId GetAssetId(int customer = 99, int space = 1, [CallerMemberName] string asset = "")
=> new(customer, space, asset);
public static AssetId GetAssetId(int customer = 99, int space = 1, [CallerMemberName] string asset = "",
string assetPostfix = "")
=> new(customer, space, $"{asset}{assetPostfix}");
}

0 comments on commit b2fa464

Please sign in to comment.