UoE/WP2-New features + bug fixes - #7
Merged
dspeed2 merged 29 commits intoJul 27, 2026
Merged
Conversation
…DSpace#761) UoE/datashare: keep embargo when moving an item with inherit policies (DSpace#761)
… admins
Reject accessConditions JSON-patch operations on the submission
upload section (/sections/upload/files/*/accessConditions*) for
non-admin users with HTTP 403.
- new shared guard BitstreamResourcePolicyUtils
.requireAdminForAccessConditions -> RESTAuthorizationException
- guard applied in BitstreamResourcePolicy{Add,Remove,Replace}
PatchOperation
- SubmissionService rethrows RESTAuthorizationException instead of
wrapping it into PatchException (would surface as 500)
- ITs: new patchUploadAccessConditionNonAdminForbiddenTest; 20
existing tests now perform the accessConditions PATCH with an
admin token (deliberate fork divergence from vanilla tests);
item-level defaultAC patch intentionally left as submitter
Server-side enforcement for dataquest-dev/dspace-customers#801
(frontend counterpart: uoe-dspace-datashare-angular#21)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ustBeDownloadableTest tokenAdmin declaration was at 8-space indent while sibling lines in the same try block use 12-space, left over from the submitter->admin token switch in the access-condition admin-only patch.
patchUploadAccessConditionNonAdminForbiddenTest only covered add and remove; the replace patch-op (BitstreamResourcePolicyReplacePatchOperation) was untested. Seed one condition as admin, then assert a submitter's replace attempt is 403 and the seeded condition is unchanged.
…kflowitems The access-condition guard (BitstreamResourcePolicyUtils.requireAdminForAccessConditions) is invoked from the same three patch-op classes on both the workspaceitem and workflowitem PATCH routes, so a non-admin workflow reviewer editing their own claimed task is blocked too, same as a non-admin submitter. This was previously unverified by any test.
Lower-level statistics tables (File visits, Top countries/cities, visits per month) were starved by backend caps, so the UI's client-side pagination had nothing to page through. - new usage-statistics.topDownloadsLimit (bitstream rows were hardcoded to 10); topCountriesLimit/topCitiesLimit defaults 100 -> -1; non-positive maps to Integer.MAX_VALUE because SolrLogger treats only -1 as "skip facet limit" and Solr then caps at facet.limit=100 - widen startDateInterval -6 -> -60 so the month report exposes 5 years of browsable history - month report DSO axis max 10 -> -1: takes the direct date-range facet path, which zero-fills months even when the statistics core has no matching docs; also fixes the order-dependent usageReportsSearch_Collection_NotVisited IT - IT month helper now derives the window from config; new ITs for unlimited/capped downloads, configurable window, and unlimited country/city limits Refs dataquest-dev/dspace-customers#807 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Emit month points most-recent-first so the current month lands on the first pagination page and users page backwards through history. Refs dataquest-dev/dspace-customers#807 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address Copilot review on PR #37. - wrap the four new tests that set topDownloadsLimit / topCountriesLimit / topCitiesLimit in try/finally so the values are restored; config is not reset between ITs and leaking them can make later tests order-dependent - rename getListOfVisitsPerMonthsPoints param viewsLastMonth -> viewsCurrentMonth (it applies to the current month, i == 0) Refs dataquest-dev/dspace-customers#807 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Ports the health-report and report-diff DSpace scripts from the LINDAT/UFAL dtq-dev branch into the UoE DataShare backend, following the same approach as the JCU port (dataquest-dev#1363). Both run via the CLI (dspace health-report, dspace report-diff) and the Processes UI. Backend-only change (the scripts surface through the generic Processes UI). - Persistence: ReportResult entity + service/DAO, Spring wiring (core-services, core-dao-services), Hibernate mapping, report_result migration (postgres+h2). - Health-check framework: refactored org.dspace.health checks to emit JSON; added EmbargoInfoCheck and DateFormatConstants; removed legacy Report CLI (healthcheck launcher entry), superseded by health-report. - Scripts: HealthReport + ReportDiff (+ ScriptConfigurations), registered in scripts.xml (main + test override). - Deps/resources: zjsonpatch 0.4.16 (matches dspace-server-webapp) and report-diff-fields.json. - Tests: HealthReportIT (9) and ReportDiffIT (28). Adaptations for this branch (DSpace 8.3, Java 17, jakarta): - Kept jakarta.* (persistence/mail) as-is. - EmbargoInfoCheck: ResourcePolicy start/end dates are java.util.Date on 8.3 (not LocalDate); converted at collection time for display. - ChecksumCheck: CheckerCommand/SimpleDispatcher take Date on 8.3 (not Instant). - LogAnalyserCheck and ReportInfo kept at branch original (JCU diffs there were pure 9.3 API adaptations, no feature content). - Migration numbered V8.0_2026.07.20 (above the latest applied V8.0_2025.* so Flyway runs it). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- ItemCheck: drop the wrapSql RuntimeException wrapper so a failing count query surfaces as SQLException and is handled by the existing catch(SQLException)->error() block instead of aborting the whole report. - EmbargoInfoCheck: populate reportJson (counts + embargoed-object lists) so the "Embargo check" section is present in the stored JSON and picked up by report-diff (previously an empty object). - scripts.xml (main + test): grammar "Get a report about DSpace health". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…n existing DBs Flyway parses V8.0_2026.07.20 as version 8.0.2026.07.20, which sorts BEFORE the base branch's V8.3_2026.04.12 (create_dataset_table). On a DB that already applied 8.3.2026.04.12, the 8.0.x migration is out-of-order-lower and, with ignoreIgnoredMigrations(true) + outOfOrder=false, gets marked IGNORED and never runs -> report_result is never created -> runtime failure in ReportResultDAOImpl. Fresh installs and the H2 test DB run migrations low->high so CI stayed green, masking the issue until deploy. Rename both dialect files to V8.3_2026.07.20 (8.3.2026.07.20 > 8.3.2026.04.12), making it the highest version. Content unchanged; no repair needed as it has not been merged/run anywhere yet. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ResourcePolicy start/end dates are DATE columns that Hibernate returns as java.sql.Date. java.sql.Date.toInstant() throws UnsupportedOperationException by design (no time component), so the report crashed with "java.lang.UnsupportedOperationException at java.sql.Date.toInstant" whenever a policy actually had a start/end date set. Convert via Instant.ofEpochMilli( date.getTime()) instead, which is supported by every java.util.Date subclass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds the LINDAT/CLARIN access-control-list mechanism for submission form
fields, so a field can be restricted to site administrators by declaring
<acl>policy=deny,action=read,grantee-type=user,grantee-id=*</acl>
Fields the current user may neither read nor write are dropped from
/api/config/submissionforms/*, so the client never learns they exist.
ACE and ACL are ported from the LINDAT/CLARIAH-CZ fork with the attribution
kept. Two deliberate differences: a null Context now denies a guarded field
instead of reaching authorizeService.isAdmin(null) and throwing, and the
SQLException path logs which action was being evaluated.
DCInputsReader needs no change - it already copies every child element of
<field> into the field map, so <acl> arrives there for free.
This is display filtering only. It hides a field; it does not stop anyone
writing the underlying metadata by other means, so any field whose value has
security consequences still needs a server-side check of its own.
Note the DTD comment: a field that is both ACL-hidden and <required> makes
the submission permanently undepositable for users who cannot see it,
because metadata validation is ACL-unaware.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ield Registers local.bitstream.redirectToURL in the existing local schema and adds a non-repeatable onebox row for it to metadatapageone, guarded by an <acl> so only site administrators see it. <required> is deliberately left empty. Metadata validation is ACL-unaware, so a required field that ordinary users cannot see would give them a validation error they can neither satisfy nor dismiss. The hint deviates from the upstream fork's wording, which is wrong for this build: it promised HTTP URLs, which are refused here, and quoted a 4GB limit, while this deployment already allows 20GB multipart uploads. It now says what is true - an absolute path, inside a directory the administrator has allow-listed, ingested on save. SubmissionFormAclIT covers the field being returned to a site administrator and withheld from both an ordinary user and a collection administrator, and that unguarded fields are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A site administrator can put an absolute path into
local.bitstream.redirectToURL; on save the file is streamed from the server's
own filesystem into the item as a bitstream. This is for datasets already
staged on the server, which never have to travel through the browser.
Works for workspace items and for workflow items. The upstream fork only
hooked into WorkspaceItemRestRepository, so the field silently stopped
working once an item entered the workflow; UploadFromPathIT now covers the
edit and final-edit steps. It remains unavailable at the review step, where
ReviewAction offers no submit_edit_metadata option - the same restriction
that already applies to an ordinary file upload during review.
Security. The path is supplied by a user and read with the servlet
container's privileges, so:
- the feature is off by default, and refuses everything when its
allow-list is empty, rather than defaulting to the whole filesystem;
- the path is canonicalised with toRealPath() before it is tested against
the allow-list, which is what defeats ".." and a planted symlink;
- the file is opened once with NOFOLLOW_LINKS and its identity rechecked
across the window, so it cannot be swapped between check and open, and
the same handle is used for size and content;
- only site administrators may trigger an ingest, and only site
administrators may write the field at all. The write guard matters
because WorkflowItemRestRepository.patch has no @PreAuthorize: without
it, a reviewer could plant a path for an administrator to ingest, and a
stray value would lock its owner out of saving;
- URLs are refused with their own message rather than becoming a confusing
"no such file";
- delete-after-upload is off by default, only ever unlinks a path already
proved to be inside the allow-list, and runs after the commit so a failed
save cannot destroy the source.
A step that reports a problem now throws rather than returning an error that
the caller discarded. UploadStep signals a failed store by returning an
ErrorRest instead of raising, so the previous behaviour was HTTP 200 with no
bitstream and the typed path already cleared - nothing to retry from.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rce page
Utils throws PaginationException when the requested page offset exceeds the
subresource total. findAllInternal (top-level lists) already catches it and
returns an empty page, but findRelInternal (paged link subresources such as
/bundles/{id}/bitstreams) let it propagate to the generic exception handler,
producing HTTP 500 with an ERROR stack trace. Catch it in the paged branch and
return an empty page with the true total, mirroring findAllInternal. Applies to
every paged subresource, not just bundle bitstreams.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… replaced The ingest clears local.bitstream.redirectToURL after a successful upload, but a browser whose form still shows the typed path sends a `replace` on that field on its next save. DescribeStep's ItemMetadataValueReplacePatchOperation asserts the value already exists (the misleading "No metadata fields match ..."), so the whole PATCH failed with HTTP 500 and the newly typed path was lost. UploadFromPathService.normalizePendingPathOperations now rewrites a `replace` on the field to a plain `add` whenever the item currently holds no value for it, which both avoids the crash and honours the intent - ingest the new path. When the field genuinely holds a value the `replace` is untouched, so an ordinary edit of an existing value is unaffected. Applied at both submission PATCH call sites (workspace and workflow). UploadFromPathIT.replaceOnClearedFieldIngestsInsteadOfFailing pins it; reverting the normalisation makes it 500. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses Copilot review: add BundleRestRepositoryIT case requesting a bundle bitstreams page past the end (page=999) and asserting HTTP 200 with an empty embedded list and page.totalElements equal to the real total, locking in the findRelInternal PaginationException fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…page Expert review: the subresource EmbeddedPage serializes an empty page as an empty `_embedded.bitstreams` array (not omitted, unlike the top-level PagedResourcesAssembler path), verified against the live REST API. Assert Matchers.empty() instead of doesNotExist() so the IT matches actual output. Also gitignore JVM hs_err/replay crash dumps so they can't be committed again. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Copilot review: the same InputStream was passed to createBitstream twice; the second read starts at EOF. Use a distinct stream per bitstream so the fixture is deterministic. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address Copilot review: ContextUtil.obtainCurrentRequestContext() was called once per field inside the nested row/field loops of getPage(). The context is the same for the whole form, so resolve it once and reuse it - fewer RequestService lookups and a clearer authorization decision. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…h field Replace the reworded hint with the original dtq-dev wording verbatim, in both the production and the integration-test submission-forms.xml. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- correct the field hint (drop the 4GB/URL claims that are false here) - rename UploadFromPathPathValidator -> UploadFromPathValidator - shorten the verbose comments across the feature Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Still fail-closed: without allowed-paths configured it refuses everything, so turning the switch on does not expose anything until an allow-list is set. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Remove the hs_err_pid*.log / replay_pid*.log ignore rules that slipped into this PR; they are unrelated to the pagination fix and shouldn't be part of it. .gitignore now matches the base branch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dspeed2
merged commit Jul 27, 2026
62e291d
into
UoEMainLibrary:datashare-UoEMainLibrary-dspace-8_x
10 checks passed
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
3 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.
This PR includes:
New feature:
Bug fixes: