Embed classic MDS protocol into Lupo - #1556
Conversation
Scaffold settings for the embedded Metadata Store protocol (MDS_ENABLED, MDS_HOSTS, MDS_URL, MDS_REALM) used by host-constrained routes. Enable by default in development and test only.
Introduce Mds:: controllers for auth/heartbeat/login, DoiOperations for list/get/put/delete of classic /doi paths, and host-constrained routes matching the former Poodle MDS surface.
Port classic /metadata and /media MDS behavior in-process: format detection and auto-mint for metadata, line-oriented media bodies, mapped to DataciteDoi and Media models with source=mds events.
Add MDS API notes to the README and a dedicated nginx server_name block for mds.* hosts that redirects / to the MDS support guide.
Cover host-constrained routing, DOI/metadata/media contract behaviors, auth failures, heartbeat/login, and Mds helper unit tests.
Stop setting MDS_ENABLED/MDS_HOSTS in application.rb before env files load. Restrict test/dev hosts to mds.local only so REST on default Rack hosts is not swallowed by the MDS catch-all. Tighten isolation specs.
Point to lib/mds.rb DEFAULT_HOSTS instead of a non-existent after_initialize.
Introduce a single exception-based error model and one upsert/find/URL helper concern so MDS can stop forking domain logic per resource.
Delete the parallel *Operations/Result stack. Controllers raise Mds::Error, share one DOI upsert, reuse REST-style auth parsing, Bolognese format detection, and Handle get_url resolution. Stop mapping NoMethodError to 422.
Add uses_stored_landing_url? and resolved_landing_url on Helpable so REST and MDS share one stored-vs-Handle decision instead of forked controller logic.
Extract RequestCredentials so both ApplicationController and MDS auth use the same Authorization header split, User construction, and JWT blacklist check.
Replace kitchen-sink DoiSupport with DoiLookup/DoiWriter includes only where needed, move minting to Mds::DoiMinter, use authorize! only in upsert, and drop the Rails.env.test? uniqueness bypass.
Apply indented_internal_methods style for private/protected sections and drop trailing blank lines so CI rubocop is clean for the MDS embed.
Update Gemfile.lock to patched versions resolving GHSA/CVE findings reported by bundler-audit (crass 1.0.7, css_parser 3.0.0, msgpack 1.8.3).
Render metadata XML with an explicit application/xml content type because Lupo unregisters the default :xml MIME type. Seed a landing URL when forcing findable state so hide's handle update_url callback does not raise on a blank URL.
Map ActionController::BadRequest to plain-text MDS 400 responses so handle/url invariant failures do not 500. Extract DoiMinting from Helpable and use it from Mds::DoiMinter without the full model concern. Fix REST head :no_content early returns, nil-safe get_dois client lookup, drop unused render_mds 204 branch, and tighten MDS GET /doi specs to deterministic stored-URL cases.
Reuse HeartbeatController on MDS hosts instead of an always-OK stub, and remove the unused session-cookie login endpoint.
Silences the Node.js 20 deprecation annotation on GitHub Actions runners.
Parse mediaType=url before persist so malformed bodies do not fall through to Media defaults or partial records. Cover blank type, blank URL, and missing pair in request specs.
DoiMinter now compares path and DataCite XML identifier before upsert, matching the PUT /doi consistency rule, so mismatched metadata cannot be stored under the wrong DOI.
Reject empty or malformed mint input with a deterministic IdentifierError instead of relying on generate_random_dois internals. Cover blank and invalid mint requests on POST /metadata.
Cache only prefix primary keys and re-verify uid on load so destroyed and recreated prefixes cannot leave orphan FKs. Make assign_prefix use create! and the known Prefix uid instead of provider_prefix.prefix.uid, which flaked shard 14 (repository_type_spec) on this PR.
CI flake fix (shard 14)Latest push ( Not MDS-related — it showed up in Root cause: Fix:
This is shared infrastructure (same code path on master); this long-lived PR hit the flake more often via shard packing / random order. |
Share path/body DOI consistency via Mds.assert_path_matches_body!, centralize classic MDS 404 copy constants, list DOIs through DoiLookup, always register rescue_from handlers, and return plain-text MDS errors for blank DOI/URL on PUT /doi.
Move full Handle vs stored URL policy into Doi#resolve_landing_url via LandingUrlResolution so REST and MDS map the same domain outcomes. Define Authenticable.api_key_token? once and reuse it from RequestCredentials. Align metadata create blank-DOI 404 copy with DOI_UNKNOWN_TO_MDS.
The class method was nested inside included, so it attached to User instead of Authenticable and RequestCredentials could not call it.
Gate GET metadata/media with not_allowed_by_doi_and_user (404, not Ability :read) so clients cannot read another repository's drafts. Extend path/body DOI checks to non-DataCite bodies via Bolognese, surface Handle failures on GET /doi instead of collapsing to 204, align media URL schemes with landing URLs, limit DOI/media routes, and restore a pessimistic graphql constraint.
Client create runs assign_prefix/check_prefix; with prefix_pool_size 1 the first client consumes the only free prefix and DATACITE.OTHER fails validation.
There was a problem hiding this comment.
Pull request overview
This PR embeds the classic DataCite MDS (legacy Metadata Store) protocol surface into Lupo, enabling plain-text MDS endpoints (/doi, /metadata, /media, /heartbeat) on host-constrained routes while keeping Lupo as the system of record.
Changes:
- Adds an MDS routing constraint (
Mds.host_match?) and introduces dedicated MDS controllers, shared helpers, and protocol error handling. - Refactors/shared logic for DOI minting and landing-URL resolution; improves API-key handling and observability tagging via a shared
RequestCredentialsconcern. - Adds MDS configuration defaults for dev/test, updates nginx template, and adds extensive routing/request/service specs plus documentation.
Reviewed changes
Copilot reviewed 37 out of 39 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| vendor/docker/webapp.conf.template | Adds an nginx server block for MDS hostnames and keeps the default REST server block. |
| README.md | Documents the embedded MDS protocol surface and configuration flags. |
| .env.example | Adds environment variables for enabling and configuring MDS hosts/realm/url. |
| Gemfile | Bumps graphql minimum patch version. |
| Gemfile.lock | Updates locked graphql version accordingly. |
| config/routes.rb | Adds host-constrained MDS routes and a catch-all for MDS-style 404s. |
| config/environments/test.rb | Enables MDS by default in tests for mds.local only. |
| config/environments/development.rb | Enables MDS by default in development for mds.local only. |
| config/application.rb | Loads MDS helper module and provides default MDS URL/realm env fallbacks. |
| lib/mds/error.rb | Introduces an MDS protocol-level error type carrying HTTP status. |
| lib/mds.rb | Adds MDS enablement/host matching helpers and shared protocol constants. |
| app/services/mds/doi_minter.rb | Adds metadata-body DOI extraction + minting logic for MDS metadata registration. |
| app/models/landing_url_resolution.rb | Introduces a domain object representing landing URL resolution outcomes. |
| app/models/concerns/helpable.rb | Refactors DOI minting + landing URL resolution into reusable methods and domain outcomes. |
| app/models/concerns/doi_minting.rb | Extracts random DOI generation into a shared concern. |
| app/models/concerns/cacheable.rb | Hardens prefix caching by caching/verifying IDs to avoid stale-orphan behavior. |
| app/models/concerns/authenticable.rb | Centralizes DC.* API-key detection and tightens API-key Basic auth behavior. |
| app/models/user.rb | Prefers API-key parsing ahead of JWT parsing and avoids JWT blacklist interaction for keys. |
| app/models/client.rb | Makes prefix assignment logic clearer and uses bang-creates for invariant records. |
| app/models/ability.rb | Grants get_urls ability for appropriate client roles (used by MDS GET /doi). |
| app/controllers/concerns/request_credentials.rb | Adds shared auth header parsing, blacklist handling (JWT-only), and Sentry tagging. |
| app/controllers/application_controller.rb | Switches to the shared RequestCredentials authentication flow. |
| app/controllers/datacite_dois_controller.rb | Uses shared landing-URL resolution and hardens get_dois nil handling. |
| app/controllers/concerns/mds/doi_lookup.rb | Adds shared DOI lookup + visibility behavior for MDS controllers. |
| app/controllers/concerns/mds/doi_writer.rb | Adds a shared MDS upsert path for DOI writes with CanCan checks. |
| app/controllers/mds/application_controller.rb | Adds MDS base controller with auth, plain-text rendering, and protocol error mapping. |
| app/controllers/mds/dois_controller.rb | Implements classic MDS /doi endpoints with plain-text semantics. |
| app/controllers/mds/metadata_controller.rb | Implements classic MDS /metadata endpoints and metadata registration flow. |
| app/controllers/mds/media_controller.rb | Implements classic MDS /media endpoints with plain-text semantics. |
| spec/lib/mds_spec.rb | Adds unit specs for MDS enablement/hosts and path-vs-body DOI matching. |
| spec/services/mds/doi_minter_spec.rb | Adds service specs for resolving DOI IDs from path/body formats. |
| spec/routing/mds_routing_spec.rb | Adds routing specs validating MDS host isolation and path routing. |
| spec/requests/mds/misc_spec.rb | Adds request specs for MDS heartbeat and unknown-path behavior. |
| spec/requests/mds/dois_spec.rb | Adds request specs for MDS /doi behavior, auth, host isolation, and Handle errors. |
| spec/requests/mds/metadata_spec.rb | Adds request specs for MDS /metadata behavior, validation, and visibility rules. |
| spec/requests/mds/media_spec.rb | Adds request specs for MDS /media behavior and visibility rules. |
| spec/requests/mds/api_keys_spec.rb | Adds request specs for Bearer and Basic API-key auth variants on MDS endpoints. |
| spec/models/ability_spec.rb | Updates ability expectations to include get_urls access. |
| spec/concerns/authenticable_spec.rb | Updates/extends auth decoding specs for invalid API keys and clearer error payloads. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| resources :dois, path: "/doi", only: %i[index show update destroy], | ||
| constraints: { id: /.+/ } do | ||
| resources :media, only: %i[index show create destroy] | ||
| end |
There was a problem hiding this comment.
Good catch on the nested param name.
Poodle uses the same nesting (resources :dois + nested resources :media) with only the parent :id constrained. Top-level /media/:doi_id already had doi_id: /.+/.
Fixed in 9437d471: nested media now has constraints: { doi_id: /.+/ } so slashy DOIs match (routing spec covers GET/POST /doi/10.14454/abc/media). Protocol paths unchanged — routing hardening only.
| doi: doi_id, | ||
| xml: xml_b64, | ||
| should_validate: true, | ||
| source: "mds", | ||
| event: "show", | ||
| client_id: client_symbol, |
There was a problem hiding this comment.
Agreed that show can move registered → findable in Lupo AASM — and that is intentional Poodle parity, not an oversight.
Classic Poodle Metadatable#create_metadata sends exactly this to REST:
"event" => "show",
"source" => "mds",
"should_validate" => "true",(and delete metadata uses "event" => "hide", PUT /doi uses "event" => "publish" — same as this embed).
We are moving Poodle in-process, not redesigning MDS state transitions. Changing to register would diverge from production MDS/Poodle behavior (including the historical “metadata resubmit can unhide” path). Left as show with an in-code comment pointing at Poodle in 9437d471.
Nested /doi/:doi_id/media needs doi_id: /./+ so slashy DOIs match, matching top-level media and Poodle intent. Keep metadata event show as in Poodle Metadatable#create_metadata.
Summary
mds.datacite.org/ test/stage/local), so MDS URLs keep working while Lupo remains the system of record./doi,/metadata,/media, and/heartbeatwhenMDS_ENABLEDis on and the request host is inMDS_HOSTS; production stays off unless explicitly enabled; test/dev usemds.localonly so REST specs are not captured.DataciteDoi, CanCan, ParamsSanitizer, Bolognese format detection, sharedRequestCredentials, domain landing-URL helpers). Single DOI upsert path for PUT/doiand PUT/metadata; no parallel Operations/Result stack.Configuration
MDS_ENABLED(default false)MDS_HOSTS(comma-separated; falls back to production-like defaults when blank and enabled)MDS_URL,MDS_REALMserver_namefor MDS hostsTest plan
bundle exec rspec spec/lib/mds_spec.rb spec/routing/mds_routing_spec.rb spec/requests/mds//doisand does not return MDS plain-text bodies for/doimds.localfor GET/PUT/doi, PUT/GET/DELETE/metadata, POST/GET/mediaMDS_ENABLED=trueis setGET .../dois/:id/get-urlstill resolves landing URLs (shared domain policy)