Repository navigation
Conversation
Extract the index dispatch from Indexable's after_commit into enqueue_index_update. register_url now writes minted/updated with update_columns and re-indexes once, instead of a full update that re-fired update_url and registered the handle a second time. Co-authored-by: Joseph Rhoads <jrhoads@users.noreply.github.com>
Co-authored-by: Joseph Rhoads <jrhoads@users.noreply.github.com>
jrhoads
marked this pull request as ready for review
October 1, 2026 10:46
ProviderPrefix#prefix_id= looks prefixes up via Rails.cache (memcached, 24h). repository_type_spec and reference_repositories_spec both create and destroy prefix 10.17616 in before/after :all, so whichever runs second gets the destroyed record's id from the cache, ProviderPrefix fails validation, and Client#assign_prefix raises on a nil prefix. Pre-existing order dependency, surfaced when parallel_test regrouped spec files onto the same CI node. Co-authored-by: Joseph Rhoads <jrhoads@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The implementation addresses the duplicate registration path and includes focused regression coverage.
Review effort: Balanced
Findings: None
What changed in this PR
Prevents duplicate Handle registration when minting or publishing a DOI while preserving OpenSearch synchronization.
Changes:
- Persists minting timestamps without triggering callbacks, then explicitly re-indexes.
- Extracts reusable index dispatch logic.
- Adds regression tests and clears stale prefix cache entries in affected suites.
| File | Description |
|---|---|
app/models/concerns/helpable.rb |
Avoids nested saves after registration. |
app/models/concerns/indexable.rb |
Extracts index enqueue logic. |
spec/concerns/helpable_spec.rb |
Tests callback-free timestamp persistence. |
spec/requests/datacite_dois/post_spec.rb |
Verifies one Handle request per registration. |
spec/requests/reference_repositories_spec.rb |
Clears stale prefix cache state. |
spec/graphql/types/repository_type_spec.rb |
Isolates prefix cache state in GraphQL tests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
wendelfabianchinsamy
approved these changes
Oct 6, 2026
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.
Purpose
Closes #1295.
Creating a registered/findable DOI through the API (or publishing a draft with a
current_user) sent the handlePUTtwice. After the first successfulPUT,register_urlstoredmintedwith a fullupdate. That save opened a new transaction and fired everyafter_commitagain, includingupdate_url, which registered the handle a second time. The nested save also re-ran XML schema validation, bumpedversiona second time, and enqueued a second index job, all inside the API request.Approach
Write
minted/updatedwithout callbacks, then explicitly re-index once so OpenSearch still receives theregistereddate.Key Modifications
app/models/concerns/indexable.rb: move the index-dispatch body of the create/updateafter_commitinto a publicenqueue_index_updatemethod. The callback calls it, then keeps the Event Data message logic unchanged. Routing (IndexJobDoiRegistration,OtherDoibulk jobs, inactive-index sync, etc.) is unchanged for every model.app/models/concerns/helpable.rb: on a 200/201 with blankminted, persisted records useupdate_columns(minted:, updated:)followed byenqueue_index_update; unsaved records useassign_attributes(previouslyupdatesilently created the record).spec/requests/datacite_dois/post_spec.rb: WebMock-stubbed handle server; asserts exactly onePUTfor POST of a findable DOI and for PATCH of a draft withevent: "publish", plusregisteredin the response andmintedin the DB.spec/concerns/helpable_spec.rb: persisted DOI withminted: nilgetsminted/updatedset,versionunchanged, no secondupdate_url, oneenqueue_index_update; an unsaved DOI getsmintedassigned without being saved.spec/requests/reference_repositories_spec.rb,spec/graphql/types/repository_type_spec.rb: clear the cachedprefix_response/10.17616entry on entry and exit. Both files create and destroy prefix10.17616inbefore/after :all, andProviderPrefix#prefix_id=resolves prefixes viaRails.cache, so whichever file runs second got the destroyed prefix's id andClient#assign_prefixraised on a nil prefix. This order dependency already exists onmaster(reproduced there in both orders); the new specs above changedparallel_test's file-size grouping and put both files on the same CI node.Important Technical Details
Indexableis included beforeafter_commit :update_urlis declared, so the first index job is enqueued beforemintedis set. The explicitenqueue_index_updatereplaces the re-index the nested save used to provide; without itregisteredwould be missing in OpenSearch.versionnow increases by 1 on creation instead of 2. This is cosmetic, but worth knowing for anyone comparing versions before/after.HandleJob(fresh record, nocurrent_user) and later updates to an already-minted DOI (already onePUT).Doi#update_url(only register when URL/state changed) is left for a separate change.masterwith "expected to execute 1 time but it executed 2 times" and pass with this change.Types of changes
Reviewer, please remember our guidelines: