Store and serve the session-level data container - #3
Open
jalexspringer wants to merge 1 commit into
Open
jalexspringer wants to merge 1 commit into
jalexspringer wants to merge 1 commit into
Conversation
The specification defines a session-level `data` container in 5.1.3, added on 13 August 2026. `SessionCreateRequest` was written on 6 August 2026 and enumerates named fields, none of them `data`; serde ignores unknown fields, so a conformant session document carrying `access_context` was accepted with a 201 and the container was discarded without a log line or a stored trace. Migration 0004 adds `session_data` to `sessions`. The container is bound on both session paths, stored as given, and materialised at the document root, so a round trip returns it unchanged. Nothing inside is normalised: consumers MUST tolerate unknown fields within the container and unknown `access_context` identifier schemes, so a scheme outside the core `ror`, `saml_entity_id` and `isni` is stored and served like any other. `access_context` is in core because COUNTER usage reporting asked for it: it names the institution whose entitlement the session used, and reporting usage by institution is already normal in scholarly publishing. Ingest checks its shape only - `identifiers` an array of objects each carrying a string `scheme` and `value`, which is what the schema requires and what the standard's two invalid fixtures exercise. The second change is what would have caught this on the day the specification moved. Both request types now capture top-level members the server does not define instead of letting serde drop them: ingest logs the names, migration 0004 stores them in `unrecognised_fields`, and materialisation returns them under `extensions.unrecognised_fields`. Recording rather than rejecting is what the consumer rules allow - a conforming consumer MUST tolerate unknown fields without error (5.7.4), so refusing the document is not open to us, and the session root is not an extension point (5.1.3), so the members are kept but never interpreted. `BulkSessionRequest::session_create` replaces the hand-written field copy in the bulk handler, so there is one mapping from the document format onto the stored session rather than a second list to forget to update. Tests: the standard's own `session-access-context.json` round-trips through ingest and materialisation with its container intact, its two invalid access-context fixtures are refused, an unknown identifier scheme survives the round trip, and the smoke script checks the same path over HTTP. The fixtures are copied byte-for-byte into `tests/spec/`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rid5BdMHf2W3umEWtiQj6F
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The specification defines a session-level
datacontainer (§5.1.3), added on 13 August.SessionCreateRequestwas written on 6 August and enumerates named fields, none of themdata. Serde ignores unknown fields, so a conformant session document carryingaccess_contextwas accepted with a 201 and the container was discarded with no log line and no stored trace. The README says the specification wins where prose and code disagree and that a disagreement is a bug; this is one.Changes
session_datatosessions. The container is bound on both session paths, stored as given and materialised at the document root, so a round trip returns it unchanged. Nothing inside is normalised: consumers MUST tolerate unknown fields within the container and unknownaccess_contextidentifier schemes, so a scheme outsideror,saml_entity_idandisniis stored and served like any other. Ingest checks shape only:identifiersis an array of objects each carrying a stringschemeandvalue, which is what the schema requires and what the standard's two invalid fixtures exercise.unrecognised_fields, and materialisation returns them underextensions.unrecognised_fields. Recording rather than rejecting is what the consumer rules allow: a conforming consumer MUST tolerate unknown fields without error (§5.7.4), and the session root is not an extension point (§5.1.3), so the members are kept but never interpreted. This is the change that would have caught the gap on the day the specification moved.BulkSessionRequest::session_createreplaces the hand-written field copy in the bulk handler, so there is one mapping from the document format onto the stored session rather than a second list to forget to update.Why
access_contextmattersIt is in core because COUNTER usage reporting asked for it: it names the institution whose entitlement the session used, and reporting usage by institution is already normal in scholarly publishing. The reference implementation is what a publisher tries first, so it must not lose the field they asked for.
Tests
The standard's own
session-access-context.jsonround-trips through ingest and materialisation with its container intact; its two invalid access-context fixtures are refused; an unknown identifier scheme survives the round trip; the smoke script checks the same path over HTTP. Fixtures are copied byte-for-byte intotests/spec/.Related
oa-telemetry-serverand the NarrativAI server have the same gap and also carry auser_contextcontainer that the specification does not define. They sit under separate governance and are not touched here.🤖 Generated with Claude Code