Added Dataset.getGlobalIdForFileStorage for direct upload URLs that support legacy lowercase DOIs#12268
Merged
stevenwinship merged 5 commits intoApr 8, 2026
Conversation
…the original casing, so it does not uppercase DOIs. It is called when generating the upload URLs for direct upload instead of using dataset.getGlobalId().asString(). The latter gave a mismatch between the complete URL and the actual object keys in object store when DOIs contained lowercase letters (which is possible when they were created before Dataverse started enforcing uppercase DOIs).
janvanmansum
changed the title
janvanmansum
marked this pull request as ready for review
March 31, 2026 12:47
Clarified the issue addressed with S3 direct upload regarding PID authority and alternative identifiers.
qqmyers
approved these changes
Mar 31, 2026
qqmyers
left a comment
qqmyers
left a comment
Member
There was a problem hiding this comment.
Looks good. Note that DANS tested this on DOIs using lowercase from their early migrations.
qqmyers
added
Size: 3
scolapasta
moved this from Ready for Triage
to Ready for Review ⏩
in IQSS Dataverse Project
…FileStorageAsString() to clarify that it does not return a GlobalId object
…s' into direct-upload-to-mixed-case-pids
github-project-automation
Bot
moved this from In Review 🔎
to Ready for QA ⏩
in IQSS Dataverse Project
github-project-automation
Bot
moved this from QA ✅
to Merged 🚀
in IQSS Dataverse Project
Contributor
|
tested develop and this branch. get upload urls and"abort" upload. Was able to see the errors in develop branch and verify that this fix worked. I wasn't able to test "complete" |
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
5 participants
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 this PR does / why we need it:
Adds
Dataset.getGlobalIdForFileStorage()and uses it in the calls tos3io.generateTemporaryS3UploadUrls()instead ofDataset.getGlobalId().toString(). When Dataverse contains datasets with DOIs which were added before Dataverse started enforcing uppercase DOIs, the upload URLs didn't match the actually keys in the object store for those datasets, causing direct upload to fail at the "completion" step (with multi-part uploads, not sure if single part uploads failed). This changed fixes the problem.Which issue(s) this PR closes:
N/a
Special notes for your reviewer:
I have tested this with new datasets (all uppercase DOIs) and with legacy DOIs from our system, which contain lowercase letters. Both work. I have only been able to test through the API, although the UI-code has also been updated.
Suggestions on how to test this:
(Jim:) Minimally, one could edit the db to set a dataset identifier to lower/mixed case. (Not sure if using a lower case shoulder still results in a lowercase entry in the db.) One could also find/configure a dataset with an alternate identifier and configure it for use in file storage. In either case, direct uploads via the UI should work. Same for the API but in that case one could also inspect the results of the /api/datasets/{id}/uploadurls call and verify that the case used in the abort and complete URLs match the URLs for parts w.r.t. case.
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
No
Is there a release notes update needed for this change?:
See: https://github.com/IQSS/dataverse/pull/12268/changes#diff-3187604c28e828f652f43814550e9271409b81a62db28527c71fdd3c08cf9344
Additional documentation:
N/a