Skip to content

Handle access controlled assets in ManifestMerger - #684

Open
JackLewis-digirati wants to merge 1 commit into
developfrom
feature/handleAccessControlledAssets
Open

JackLewis-digirati wants to merge 1 commit into
developfrom
feature/handleAccessControlledAssets

Conversation

@JackLewis-digirati

@JackLewis-digirati JackLewis-digirati commented Sep 18, 2026 •

Copy link
Copy Markdown
Collaborator

What does this change?

Resolves #569

This PR modifies manifestMerger to correctly copy the manifest.services block when assets have access control services.

Additionally, this fixes an issue with audio/video assets not correctly copying service blocks

@JackLewis-digirati
JackLewis-digirati requested a review from a team as a code owner September 18, 2026 11:52
@JackLewis-digirati

Copy link
Copy Markdown
Collaborator Author

Explaining the approach for reviewers:

Manifest-level access services (ApplyAccessServices in ManifestMerger.cs)

Protagonist's NamedQuery response carries access services at two levels:

  • Full AuthAccessService2 definitions (with token/logout services, labels etc) in the Manifest's top-level "services" block
  • Each protected asset's service list carries an AuthProbeService2 that references those full definitions by id only (a stub {id, type}), sometimes nested inside an ImageService2/ImageService3, sometimes as a sibling entry

To build the merged manifest's "services" block, we:

  1. Walk every PaintingAnnotation body actually built into the final manifest (via the existing Canvas.GetPaintingAnnotations() helper), and collect the ids referenced by any AuthProbeService2 found - recursing into nested services (ServiceListX.GetReferencedAuthServiceIds)
  2. Filter the NamedQuery's full "services" list down to just those referenced ids
  3. Append them onto the base manifest's "services", deduping by id via the existing AddDistinctById helper (so a previously-set value is never overwritten)

Only walking the built canvases (not the raw NamedQuery items) is what satisfies "referenced by at least one asset" - it guards against the NQ returning assets that aren't actually in this manifest's CanvasPaintings.

Sound/Video fix

Separately found and fixed a bug: when cloning painting-annotation bodies (GetSafePaintable), Image bodies had their .Service copied across but Sound and Video didn't, so an AuthProbeService2 on an audio/video asset was silently dropped during merge. Now all three preserve it.

@context

The auth context (http://iiif.io/api/auth/2/context.json) requirement was already satisfied by the existing EnsureContext logic - no change needed there, confirmed via a real end-to-end manifest during testing.

@JackLewis-digirati
JackLewis-digirati force-pushed the feature/handleAccessControlledAssets branch 2 times, most recently from 63c9e2f to 316f83e Compare September 23, 2026 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Handle access controlled assets

1 participant