Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Fix NullPointerException in VersionHistoryService.getVersion() method#11677

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
Paurikova2 wants to merge4 commits intoDSpace:main
base:main
Choose a base branch
Loading
fromdataquest-dev:get-version-null-exception-fix

Conversation

@Paurikova2
Copy link

@Paurikova2Paurikova2 commentedDec 11, 2025
edited by tdonohue
Loading

Description

Fixes#11670

Fixed a bug where getVersion() threw a NullPointerException when called for an item without a version. The method now safely checks for null before accessing version history and returns null instead of throwing an error. Added a unit test to cover this case and prevent future regressions.

Instructions for Reviewers

Verify null check in VersionHistoryServiceImpl.getVersion() prevents NPE
Confirm method returns null gracefully for items without versions
Check new test testGetVersionWithNullPointerException() covers the bug scenario
Run new unit test (should pass)

Checklist

This checklist provides a reminder of what we are going to look for when reviewing your PR. You need not complete this checklist prior to creating your PR (draft PRs are always welcome).
However, reviewers may request that you complete any actions in this list if you have not done so. If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!

  • My PR iscreated against themain branch of code (unless it is a backport or is fixing an issue specific to an older branch).
  • My PR issmall in size (e.g. less than 1,000 lines of code, not including comments & integration tests). Exceptions may be made if previously agreed upon.
  • My PRpasses Checkstyle validation based on theCode Style Guide.
  • My PRincludes Javadoc forall new (or modified) public methods and classes. It also includes Javadoc for large or complex private methods.
  • My PRpasses all tests and includes new/updated Unit or Integration Tests based on theCode Testing Guide.
  • My PRincludes details on how to test it. I've provided clear instructions to reviewers on how to successfully test this fix or feature.
  • If my PR includes new libraries/dependencies (in anypom.xml), I've made sure their licenses align with theDSpace BSD License based on theLicensing of Contributions documentation.
  • If my PR modifies REST API endpoints, I've opened a separateREST Contract PR related to this change.
  • If my PR includes new configurations, I've provided basic technical documentation in the PR itself.
  • If my PR fixes an issue ticket, I'velinked them together.

@tdonohuetdonohue added bug code taskCode cleanup task component: versioningRelated to item-level versioning system 1 APPROVALpull request only requires a single approval to merge. labelsDec 11, 2025
@tdonohuetdonohue moved this to🙋 Needs Reviewers Assigned inDSpace 10.0 ReleaseDec 11, 2025
@tdonohuetdonohue added port to dspace-7_xThis PR needs to be ported to `dspace-7_x` branch for next bug-fix release port to dspace-8_xThis PR needs to be ported to `dspace-8_x` branch for next bug-fix release port to dspace-9_xThis PR needs to be ported to `dspace-9_x` branch for next bug-fix release labelsDec 11, 2025
@saschaszott
Copy link
Contributor

@Paurikova2 , the change looks good. However, there are additional calls toversioningService in classVersionHistoryService where nonull check is performed. Should these places also be hardened against potential NPEs?

A more general question: is anull value at these points an unexpected result from theversioningService? If so, we should at least record this situation in the application log.

@saschaszottsaschaszott self-requested a reviewDecember 15, 2025 08:23
@Paurikova2
Copy link
Author

@saschaszott Hi, I’ll identify the relevant places, analyze them, and then we can discuss.

saschaszott reacted with thumbs up emoji

@Paurikova2
Copy link
Author

@saschaszott I reviewed the additional calls.

From the current implementation, it appears that anull return value is possible
(e.g. ingetFirstVersion() andgetLatestVersion() when no versions exist).

After the recent changes, I think we should also throw an exception in the following place
to fail fast (as before). Currently, ifnull is returned, the failure will only occur later,
which makes the root cause harder to identify:
https://github.com/DSpace/DSpace/blob/main/dspace-api/src/main/java/org/dspace/identifier/VersionedDOIIdentifierProvider.java#L287

In addition, I believe there is a bug in the following place: the return value ofgetVersion()
is never assigned to theversion variable, so theif condition is never true:
https://github.com/DSpace/DSpace/blob/main/dspace-api/src/main/java/org/dspace/identifier/VersionedHandleIdentifierProvider.java#L149

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@saschaszottsaschaszottAwaiting requested review from saschaszott

At least 1 approving review is required to merge this pull request.

Assignees

@Paurikova2Paurikova2

Labels

1 APPROVALpull request only requires a single approval to merge.bugcode taskCode cleanup taskcomponent: versioningRelated to item-level versioning systemport to dspace-7_xThis PR needs to be ported to `dspace-7_x` branch for next bug-fix releaseport to dspace-8_xThis PR needs to be ported to `dspace-8_x` branch for next bug-fix releaseport to dspace-9_xThis PR needs to be ported to `dspace-9_x` branch for next bug-fix release

Projects

Status: 🙋 Needs Reviewers Assigned

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Should getVersion() return null or throw an exception when version does not exist?

3 participants

@Paurikova2@saschaszott@tdonohue

[8]ページ先頭

©2009-2025 Movatter.jp