ZCU-DATA/Add bitstream-level access status/embargo date REST endpoint - #1380
Conversation
Ports the intent of upstream commits ed02a60, 43e41e0, 6a229f1, and 0a0b575 (dataquest-dev/DSpace 9.x line) onto zcu-pub's DSpace 7.6.1 codebase. A raw cherry-pick was not viable: upstream uses java.time.LocalDate + jakarta.* and refactors the helper around a Pair<String, LocalDate>/AccessStatus object with a current-user-vs- anonymous calculation type, none of which match zcu-pub's simpler joda-time + javax.* shape. Instead, re-derive the minimal intent by generalizing the existing item-only helper methods to also accept a Bitstream, reusing the already-DSpaceObject-generic calculateAccessStatusForDso() and the bitstream-scoped retrieveShortestEmbargo(). - AccessStatusHelper / DefaultAccessStatusHelper / AccessStatusServiceImpl: add getAccessStatusFromBitstream / getEmbargoFromBitstream and their AccessStatusService-level counterparts. - AccessStatusRest: add the embargoDate field (previously only used at item level; zcu-pub's item-level link repository never actually populated it despite the service already computing it, so ItemAccessStatusLinkRepository now also sets it for consistency). - BitstreamRest: add the ACCESS_STATUS link. - New BitstreamAccessStatusLinkRepository, mirroring ItemAccessStatusLinkRepository and BitstreamFormatLinkRepository's existing METADATA_READ permission convention. - Tests: extend AccessStatusServiceTest, DefaultAccessStatusHelperTest, AccessStatusRestTest, and BitstreamRestRepositoryIT for the new bitstream-level behavior. Deliberately does not port the AccessStatus-object/Pair refactor or the current-user-vs-anonymous calculation type from the later upstream commits (6a229f1, 0a0b575) -- that's 9.x-specific complexity not present in zcu-pub's simpler anonymous-only model. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Sibling PR for ZCU-PUB (same fix): #1377. Tracking issue: dataquest-dev/dspace-customers#823. |
The ZCU-DATA Item View file listing doesn't use the standard
/bitstreams REST resource (and its new accessStatus link from this
same PR) at all - it goes through a separate, CLARIN-specific
endpoint (metadatabitstreams/search/byHandle -> MetadataBitstreamWrapperRest)
that returns a flat, lightweight DTO per file. Extend that DTO
directly instead, reusing the same AccessStatusService methods added
earlier in this PR.
- MetadataBitstreamWrapperRest gains status/embargoDate fields.
- MetadataBitstreamWrapperConverter populates them from the same
Bitstream object it already has on hand, via
AccessStatusService.getAccessStatus()/getEmbargoFromBitstream().
Wrapped in try/catch so a failure here degrades to no status
rather than 500ing the whole file listing.
Also fixes a real pre-existing bug surfaced while testing this against
an embargoed bitstream: MetadataBitstreamRestRepository.findOutCanPreview()
only caught MissingLicenseAgreementException, not the general
AuthorizeException thrown when the requesting user (e.g. anonymous)
lacks READ on the bitstream - e.g. exactly the embargo case. That
propagated uncaught and 500'd the entire file listing for the item
today, independent of this change. Fixed by catching it the same way
as the license case (same "can't preview" outcome).
Test evidence:
mvn checkstyle:check -pl dspace-server-webapp -> BUILD SUCCESS
mvn verify -pl dspace-server-webapp -DskipIntegrationTests=false
-Dit.test=MetadataBitstreamRestRepositoryIT
-> Tests run: 6, Failures: 0, Errors: 0
New test (findByHandleEmbargoedBitstream) exercises exactly the path
that used to 500: an item with an embargoed ORIGINAL bitstream,
requested anonymously.
|
Follow-up commit (4b4ce9f): the ZCU-DATA Item View doesn't actually use the standard `/bitstreams` REST resource this PR originally extended - it goes through a separate, CLARIN-specific endpoint (`metadatabitstreams/search/byHandle`). Extended that DTO directly (`status`/`embargoDate` on `MetadataBitstreamWrapperRest`, populated by the same `AccessStatusService` methods). Also fixes a real pre-existing bug, found while testing against an embargoed bitstream: `MetadataBitstreamRestRepository.findOutCanPreview()` only caught `MissingLicenseAgreementException`, not the general `AuthorizeException` thrown when the requesting user lacks READ - i.e. exactly the embargo case. That was propagating uncaught and 500ing the entire file listing for any item with an embargoed bitstream, anonymously, today - independent of this PR. Fixed the same way the license case is already handled. New test `findByHandleEmbargoedBitstream` exercises exactly this path. Full suite: 6/6 passing. |
|
Companion frontend PR: dataquest-dev/dspace-angular#1394 |
There was a problem hiding this comment.
Pull request overview
This PR adds a bitstream-level REST endpoint to expose access status and embargo end date, enabling the frontend to display embargo badges with the correct end date for restricted bitstreams (aligning customer/zcu-data behavior with the already-fixed sibling branch).
Changes:
- Add bitstream-level access status + embargo date support in
dspace-api(AccessStatusService/helper APIs and implementations). - Expose
GET /api/core/bitstreams/{id}/accessStatusvia a new link repository and add anembargoDatefield toAccessStatusRest(plus wiring the link fromBitstreamRest). - Extend metadata bitstream listing DTOs/tests to include
statusandembargoDate, and add/adjust REST and integration tests.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| dspace-server-webapp/src/test/java/org/dspace/app/rest/model/AccessStatusRestTest.java | Adds unit coverage for new embargoDate field on REST model. |
| dspace-server-webapp/src/test/java/org/dspace/app/rest/MetadataBitstreamRestRepositoryIT.java | Extends integration assertions for metadata bitstream listing to include status/embargoDate. |
| dspace-server-webapp/src/test/java/org/dspace/app/rest/matcher/BitstreamMatcher.java | Updates embed/link expectations to include the new accessStatus link. |
| dspace-server-webapp/src/test/java/org/dspace/app/rest/BitstreamRestRepositoryIT.java | Adds integration tests for the bitstream accessStatus endpoint and edge cases. |
| dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/MetadataBitstreamRestRepository.java | Prevents embargoed/no-READ bitstreams from breaking preview listing by handling AuthorizeException. |
| dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ItemAccessStatusLinkRepository.java | Populates embargoDate for item-level accessStatus responses (DTO consistency). |
| dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/BitstreamAccessStatusLinkRepository.java | Introduces bitstream-level accessStatus link repository (including embargoDate). |
| dspace-server-webapp/src/main/java/org/dspace/app/rest/model/MetadataBitstreamWrapperRest.java | Adds status and embargoDate fields to the metadata bitstream wrapper DTO. |
| dspace-server-webapp/src/main/java/org/dspace/app/rest/model/BitstreamRest.java | Adds accessStatus link definition/constant to bitstream REST resource. |
| dspace-server-webapp/src/main/java/org/dspace/app/rest/model/AccessStatusRest.java | Adds embargoDate field to access status REST resource. |
| dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/MetadataBitstreamWrapperConverter.java | Populates status/embargoDate onto metadata bitstream wrapper responses. |
| dspace-api/src/test/java/org/dspace/access/status/DefaultAccessStatusHelperTest.java | Adds helper-level tests for bitstream access status/embargo retrieval. |
| dspace-api/src/test/java/org/dspace/access/status/AccessStatusServiceTest.java | Adds service-level tests for bitstream access status/embargo retrieval. |
| dspace-api/src/main/java/org/dspace/access/status/service/AccessStatusService.java | Adds Bitstream overloads for access status and embargo retrieval. |
| dspace-api/src/main/java/org/dspace/access/status/DefaultAccessStatusHelper.java | Implements bitstream access status and embargo retrieval in the default helper. |
| dspace-api/src/main/java/org/dspace/access/status/AccessStatusServiceImpl.java | Wires new Bitstream methods through to the configured AccessStatusHelper plugin. |
| dspace-api/src/main/java/org/dspace/access/status/AccessStatusHelper.java | Extends helper SPI to support bitstream-level access status/embargo retrieval. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
getEmbargoFromItem/getEmbargoFromBitstream returned Date#toString(), a locale/timezone-dependent format (e.g. "Tue Jan 01 00:00:00 UTC 2050"), instead of the plain ISO yyyy-MM-dd format used elsewhere in the REST API (see ResourcePolicyRest / ResourcePolicyRestRepositoryIT). Since this PR is what first exposes embargoDate over REST, format it consistently so the frontend can reliably parse it. Updated DefaultAccessStatusHelperTest assertions to match. (Same fix as sibling PR #1377, which shares this code.) Also swap the EMBARGO.equals(status) comparisons in Item/BitstreamAccessStatusLinkRepository to be constant-first, so a null status from a custom AccessStatusHelper plugin can't NPE, as flagged directly by Copilot review on this PR. Addresses Copilot review comments on PR #1380. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…isting (#1394) * Show a per-bitstream embargo-date badge in the CLARIN file listing Fixes dataquest-dev/dspace-customers#823 (ZCU-DATA portion). ZCU-DATA's Item View file listing doesn't go through the standard file-download-link/AccessStatusBadgeComponent path used on ZCU-PUB (#1378) - it's a separate, CLARIN-specific component chain (clarin-files-section -> preview-section -> file-description) backed by a completely different REST endpoint (metadatabitstreams/search/byHandle) that returns a flat DTO, not a HAL resource with resolvable links. This is new implementation, not a port. Change set: - MetadataBitstream model gains status/embargoDate fields, matching what the backend companion PR (dataquest-dev/DSpace#1380) now embeds directly in that endpoint's response - no separate resolution step needed, unlike the HAL-link approach on ZCU-PUB. - file-description.component.html renders the same visual badge (badge badge-secondary, embargo.listelement.badge i18n key) used on ZCU-PUB, for a consistent look, next to the file name. - New item.bitstream.showAccessStatuses config flag (defaults to false, same reasoning as ZCU-PUB: flip only once the backend PR is deployed). - i18n copy matches ZCU-PUB/vanilla: "Embargo until {{ date }}" (en) / "Embargo do {{ date }}" (cs). Test evidence: npx ng lint --quiet -> All files pass linting. npx ng build --configuration production -> build succeeded, no budget warnings npx ng test (file-description.component.spec.ts) -> 5/5 SUCCESS, including coverage for: badge hidden by default, badge shown when embargoed + flag on, badge hidden when embargoed but flag off. * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Show lock icon next to embargoed bitstream name Matches the existing lock-icon pattern used on ZCU-PUB's file-download-link component for restricted bitstreams. * Make embargoed file name a clickable link, matching ZCU-PUB visual style 1:1 The lock icon now inherits link color (blue) by living inside an anchor that triggers downloadFile(), exactly mirroring the file-download-link component's <a><lock/>content</a> structure used on ZCU-PUB. Non-embargoed file names are unaffected (still plain text). --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Problem
The bitstream-level embargo badge on the frontend (dataquest-dev/dspace-angular#1378 for ZCU-PUB; ZCU-DATA frontend work still pending) needs to know a restricted bitstream's embargo end date. This endpoint doesn't exist on
customer/zcu-data(same gap ascustomer/zcu-pub, fixed there by #1377 - this is the same fix applied to the sibling customer branch).Backend portion of dataquest-dev/dspace-customers#823 (ZCU-DATA).
Root cause
Same as #1377: never ported from the DSpace 9.x line (where it already exists, e.g.
customer/mendelu) to the 7.x line.zcu-dataandzcu-pubshare the same DSpace 7.6.1 version family and, for the touched files, byte-identical source - confirmed via diff before making this change.Change set
Identical in shape to #1377 (cherry-picked cleanly onto
customer/zcu-data- the underlying files matched exactly, no adaptation needed beyond the base branch):dspace-api:AccessStatusHelper/DefaultAccessStatusHelper/AccessStatusServiceImpl/AccessStatusService—getAccessStatusFromBitstream/getEmbargoFromBitstream.dspace-server-webapp:AccessStatusRestgainsembargoDate;BitstreamRestgains theACCESS_STATUSlink; newBitstreamAccessStatusLinkRepository;ItemAccessStatusLinkRepositoryalso populatesembargoDatenow (DTO consistency, same as ZCU-PUB/Add bitstream-level access status/embargo date REST endpoint #1377).GET /server/api/core/bitstreams/{id}/accessStatus->{status, embargoDate}.Test evidence
Risk & rollback
Low risk, same as #1377: additive REST surface only. Revert is a straight
git revert.Notes / assumptions
customer/zcu-pub).clarin-files-section/file-description) that doesn't reusefile-download-link/AccessStatusBadgeComponentat all. That's tracked separately as its own frontend task; this backend endpoint is ready whenever that lands.