Skip to content

Hierarchical consistent PUT/POST - #641

Open
JackLewis-digirati wants to merge 11 commits into
developfrom
feature/consistentPutPost
Open

Hierarchical consistent PUT/POST#641
JackLewis-digirati wants to merge 11 commits into
developfrom
feature/consistentPutPost

Conversation

@JackLewis-digirati

@JackLewis-digirati JackLewis-digirati commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

What does this change?

Resolves #464
Resolves #291

This PR is a reimplemented version of the stale #503 which had gotten very out of date.

It has 2 main goals:

  • Refactor write collections to share more code
  • Implement hierarchical PUT and change the hierarchical POST to be more consistent for collections
  • Implement PUT/POST for collections

In order to complete this goal, 3 new handlers have been introduced and postHierarchical is modified extensively. Additionallly, a new central class for collection requests has been introduced to mirror the ManifestWriteService called the CollectionWriteService to reduce duplication.

Finally, several helpers have been introduced to help validate and respond to the new/modified hierarchical requests.

Note

The original PR implemented a hierarchical DELTE endpoint which has not been reimplemented here to reduce the size/scope of an already lrge PR

Note

The ticket notes a breaking change in the stale branch. This has been fixed so that Post works with the into logic, requiring a publicId, id and/or slug property to correctly derive the slug (with id being ignored if the host doesn't match the value from settings)

@JackLewis-digirati JackLewis-digirati changed the title Initial commit adding hierarchical collection PUT/POST hierarchical collection consistent PUT/POST Jul 30, 2026
@JackLewis-digirati JackLewis-digirati changed the title hierarchical collection consistent PUT/POST hierarchical consistent PUT/POST Jul 30, 2026
@JackLewis-digirati

Copy link
Copy Markdown
Collaborator Author

Summary

Implements #464 (consistent PUT/POST behaviour). Old PR #503 was used purely as a design reference (unmergeable/stale) — this is a fresh implementation that extends the existing ParentSlugParser/flat-endpoint machinery rather than replacing everything with a per-type mega-handler.

New/changed capabilities:

  • Hierarchical PUT now exists for both Collections and Manifests (PutHierarchicalCollection, PutHierarchicalManifest) — previously only hierarchical POST existed, and only for Collections.
  • Hierarchical POST now exists for Manifests (PostHierarchicalManifest).
  • id/parent/slug can now be supplied consistently from the URL, the body's parent/slug/publicId, or the body's id (own-host, resolved via new RequestIdResolver) across all 6 endpoint combinations (flat/hierarchical × POST/PUT × Collection/Manifest). Conflicting sources are rejected (400) rather than one silently winning.
  • New CollectionWriteService/ICollectionWrite, mirroring the existing ManifestWriteService shape (Create/Upsert → shared internals). CreateCollection/UpsertCollection are now thin MediatR wrappers, matching CreateManifest/UpsertManifest.
  • Common hierarchical prologue (deserialize → validate → resolve id → reconcile parent/slug → resolve internal id for PUT) extracted into HierarchicalRequestHelper.PrepareForPost/PrepareForPut, shared by all 4 hierarchical handlers so future validation/rule changes only need to be made once.

Bugs fixed (pre-existing, on develop)

The old PostHierarchicalCollection handler had several bugs that are fixed here, not just extended:

  • It treated the entire POSTed-to URL path as the new resource's own slug, instead of as the parent it should be created inside (so POST /1/foo/bar created a resource at slug bar under foo — correct by accident — but POST /1/foo created a resource named foo, colliding with/replacing the parent's own address, instead of creating a child of foo).
  • It ignored the body's slug/parent/publicId entirely, and deserialized the body as a bare IIIF Collection, not PresentationCollection.
  • It hardcoded IsStorageCollection = false regardless of what the client sent — storage collections could not be created via hierarchical POST at all. (Confirmed against Consistent POST/PUT behaviour #503's own reference implementation that this was unintended — it computes IsStorageCollection from the body the same way flat endpoints do.)
  • No ETag/update handling — every hierarchical POST minted a brand-new id, so there was no way to hit an existing resource.

⚠️ Breaking change

Collection PUT-create now returns 201 Created instead of 200 OK. Previously, UpsertCollectionHandler had a bug where both the create-via-PUT and update-via-PUT branches fell through to one shared return with the default WriteResult.Updated (200) — so a PUT that created a brand-new collection incorrectly returned 200, unlike POST-create (correctly 201) and unlike Manifests (ManifestWriteService already returned 201 correctly for both paths). This is now consistent: any PUT that creates returns 201, for both resource types. Any consumer relying on the old 200-on-create-via-PUT behaviour for Collections will see a status code change.

Notes on hierarchical response shape

  • Hierarchical POST/PUT responses are always plain IIIF (no Prefer/show-extras support) — out of scope for this ticket per earlier discussion.
  • For Manifests, the response is built via PresentationIIIFCleaner.OnlyIIIFProperties — a reflection-based copy of only base-IIIF properties off the enriched PresentationManifest, since manifests' enrich step never rewrites Behavior.
  • For Collections, the enrich step (EnrichPresentationCollection) does unconditionally regenerate Behavior from just two DB booleans (IsPublic/IsStorageCollection — the Collection DB entity has no column for arbitrary behaviour strings), which would silently drop custom values like auto-advance. So non-storage-collection hierarchical responses instead re-parse the client's raw request body directly (new shared HierarchicalCollectionResponse.Build, used by both PostHierarchicalCollection/PutHierarchicalCollection) to preserve those. This is a real, pre-existing asymmetry between Collections and Manifests, not something introduced here — flagged in case it's worth a follow-up ticket to let Collections persist arbitrary behaviours too.

Test coverage added

  • Full flat/hierarchical × POST/PUT × id/parent/slug-source matrix (ManifestPaths_ResolveIdParentSlug_FromAllDocumentedSources, 15 cases).
  • Hierarchical PUT create/update, multi-segment path (parent-splitting), body-id-vs-URL conflict rejection, and body-id-honoured-on-create — previously only exercised by isolated unit tests on HierarchicalRequestHelper's now-private SplitPath/ReconcileId/ResolveIdForPut methods; those unit tests were removed once caller-level coverage closed the gap.
  • Storage collections created via hierarchical POST and PUT (previously untested at the hierarchical level).

Full solution test suite (1561 tests) passes.

}

[Fact]
public async Task PostHierarchical_WithAsset_IngestsAsset_WithoutShowExtrasHeader()

@JackLewis-digirati JackLewis-digirati Jul 31, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure about this, but erred on the side it should be allowed - this test is basically proving you can get asset ingestion from a hierarchical path (and it will do it without the show-extras header, but the [Authorize] means you need to be authed)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tomcrane flagging this for opinion.

@JackLewis-digirati
JackLewis-digirati marked this pull request as ready for review July 31, 2026 16:11
@JackLewis-digirati
JackLewis-digirati requested a review from a team as a code owner July 31, 2026 16:11
Comment thread src/IIIFPresentation/API/Features/Manifest/Requests/PostHierarchicalManifest.cs Outdated
Comment thread src/IIIFPresentation/API/Features/Storage/StorageController.cs Outdated
Comment thread src/IIIFPresentation/API/Features/Storage/StorageController.cs
Comment thread src/IIIFPresentation/API/Helpers/HierarchicalRequestHelper.cs Outdated
Comment thread src/IIIFPresentation/API/Features/Storage/CollectionController.cs Outdated
}

[Fact]
public async Task PostHierarchical_WithAsset_IngestsAsset_WithoutShowExtrasHeader()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tomcrane flagging this for opinion.

@JackLewis-digirati JackLewis-digirati changed the title hierarchical consistent PUT/POST Hierarchical consistent PUT/POST Aug 4, 2026
@tomcrane

tomcrane commented Aug 5, 2026

Copy link
Copy Markdown

ShowExtras is not required on a PUT or POST, because the server can tell whether the request payload is vanilla or extras. ShowExtras is not an auth header, it's an instruction about what to include in responses.

BUT - if you make a PUT or POST that eventually returns a body, showExtras on the original PUT or POST determines whether or not extras are included in that body.

This means that a "vanilla" authed manifest editor that only submits vanilla IIIF doesn't get an extras response it won't understand.

This applies to flat or hierarchical PUT and POST. (needs new ticket)

@tomcrane

tomcrane commented Aug 5, 2026

Copy link
Copy Markdown

Handoff from the DLCS docs/API hygiene sprint (cross-cutting session, 2026-08-06) — two conventions were ratified today that this PR's PUT/POST standardisation is the natural landing point for on the iiif-presentation side:

  1. Create-POST returns 201 Created + Location. The only sanctioned exceptions are action POSTs returning an ephemeral payload with no canonical URL (in protagonist: API-key creation and application setup, which return 200). Any new exception must be named explicitly, never copied silently.

  2. PUT upsert: 201 on create, 200 on replace — the handler must report Updated vs Created truthfully (never hard-code Created on a known-update path; protagonist had exactly one such bug, live since 2023, fixed today in Hygiene: DELETE returns 204 (breaking) + origin-strategy Hydra cleanups protagonist#1236). A true-upsert PUT annotates/documents both 200 and 201; an update-only PUT (404s rather than creates) annotates exactly what it does, e.g. 200/400/404.

Also ratified, for completeness: every DELETE returns 204 No Content with an empty body (404/409/500 as error bodies) — protagonist's two legacy 200-with-body deletes are migrating in the same PR (breaking change, signposted there).

No action requested beyond alignment — the room's intent is that this PR carries the convention on this repo rather than a parallel ruling being minted. Register trail: scratch/hygiene-sprint/session-0-cross-cutting.md in dlcs/public-docs (cards XC-01/02/03).

🤖 Generated with Claude Code

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.

Consistent API PUT and POST behaviour Refactor Collection write requests

3 participants