DSpace9.3/Fix CLARIN link rels returning 404: register link repositories under plural model names - #1404
Merged
milanmajchrak merged 3 commits intoAug 10, 2026
Conversation
…contract)
Six CLARIN LinkRestRepository beans are still registered under the singular model
name, so all six advertised sub-resources return 404 on the v9 base while they
return 200 on 7.6.5.
DSpace 7 singularized the URL segment before the bean lookup --
Utils.getLinkResourceRepository() called makeSingular(modelPlural). DSpace 9
removed that step and looks the bean up under the plural segment verbatim, so a
link repository must now be registered as <category>.<typePlural>.<rel>. The
upstream migration renamed its own 71 link repositories accordingly and added
PLURAL_NAME to the REST models, but these six CLARIN ones were missed.
Note ClarinLicenseResourceUserAllowanceRestRepository (the MAIN repository) *was*
migrated to PLURAL_NAME, which is why GET /core/clarinlruallowances/242 returns
200 while every one of its rels 404s. Migrating a main repository without its
link repositories leaves all its sub-resources dead.
Why this is easy to misread: a missing bean raises RepositoryNotFoundException,
and a missing route resolves BEFORE any authorization check. The one defect
therefore surfaces as "404 != 200" for an admin, "404 != 401" for anonymous and
"404 != 403" for a non-owner -- it reads like an authorization problem, and the
404 body names the plural type that is not how the bean is registered:
{"status":404,"message":"The repository type core.clarinlruallowances was not found"}
Meanwhile the allowance JSON keeps advertising all three _links, so the API
describes endpoints it cannot serve.
Measured, admin token, dev-6.pc:8603 (9.3) vs dev-5.pc:88 (7.6.5):
core/clarinlruallowances/242 200 200
core/clarinlruallowances/242/userMetadata *404* 200
core/clarinlruallowances/242/userRegistration *404* 200
core/clarinlruallowances/242/resourceMapping *404* 200
core/clarinuserregistrations/1/userMetadata *404* 200
core/clarinuserregistrations/1/clarinLicenses *404* 200
core/clarinlicenseresourcemappings/1383/clarinLicense *404* 200
Audit backing the "six and only six" claim: of 77 LinkRestRepository
implementations on this branch, 71 already use PLURAL_NAME, these 6 used NAME,
and none uses a literal bean-name string. Every main (non-link) repository is
already plural. So this closes the gap completely.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Nothing in the suite caught the previous commit's bug: no IT traverses any of the
six rels as a URL sub-path, so all six could 404 in production while CI stayed
green. It surfaced only in the external dspace-rest-test suite, as 8 failing
test_endpoints tests whose messages looked like an authorization problem.
The test asserts the invariant directly through
Utils.getLinkResourceRepository() -- the same lookup RestResourceController
performs when a client traverses a rel -- rather than over HTTP. Going over HTTP
could not isolate this defect: these link repositories also raise
ResourceNotFoundException, another 404, when the linked data simply does not
exist, so telling "route missing" from "no data" would need fixtures for a
bitstream, a licence, a resource mapping, a user registration and user metadata
per rel, and would still conflate the two on failure.
It is driven off the @LinksRest annotation instead of a hardcoded rel list, so a
rel added to any of these three models is covered automatically.
Covers all six rels: clarinlruallowances {resourceMapping, userRegistration,
userMetadata}, clarinuserregistrations {clarinLicenses, userMetadata},
clarinlicenseresourcemappings {clarinLicense}.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes CLARIN REST sub-resource (_links) endpoints returning 404 on DSpace 9.3 by registering the affected LinkRestRepository beans under the plural model name, matching the DSpace 9 bean-lookup contract (<category>.<typePlural>.<rel>). It also adds an integration test to prevent regressions where main repositories are migrated to plural routing but their link repositories are not.
Changes:
- Updated 6 CLARIN
LinkRestRepository@Componentbean names fromNAMEtoPLURAL_NAMEso link repository lookup succeeds on DSpace 9. - Added
ClarinLinkRestRepositoryBeanNameITto assert that all@LinksRestrelations for the CLARIN models resolve to registered link repositories viaUtils.getLinkResourceRepository().
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinLinkRestRepositoryBeanNameIT.java | Adds an IT asserting CLARIN @LinksRest rels resolve to registered link repositories using the same lookup path as the controller. |
| dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/CUserRegistrationCLicenseLinkRepository.java | Registers the CLARIN user-registration → licenses link repository under the plural model name. |
| dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/CLRUAUUserRegistrationLinkRepository.java | Registers the allowance → userRegistration link repository under the plural model name. |
| dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/CLRUAUserMetadataLinkRepository.java | Registers the allowance → userMetadata link repository under the plural model name. |
| dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/CLRUAResourceMappingLinkRepository.java | Registers the allowance → resourceMapping link repository under the plural model name. |
| dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClarinUserRegistrationUserMetadataLinkRepository.java | Registers the userRegistration → userMetadata link repository under the plural model name. |
| dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClarinResourceMappingCLicenseLinkRepository.java | Registers the resourceMapping → clarinLicense link repository under the plural model name. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Addresses Copilot review feedback on #1404. utils.getLinkResourceRepository() throws RepositoryNotFoundException rather than returning null, so the assertNotNull message was unreachable: the test would have failed with a raw exception instead. That mattered more than it looks, because RepositoryNotFoundException.getMessage() formats only "<apiCategory>.<model>": "The repository type core.clarinlruallowances was not found" It never names the rel, so on ClarinLicenseResourceUserAllowanceRest -- which has three -- the failure could not say whether resourceMapping, userRegistration or userMetadata was the unregistered one. The wording is also misleading in this context: it claims the repository *type* is missing when the main repository resolves fine and only the link repository bean is absent, which is exactly the confusion this test exists to prevent. The lookup failure is now caught and rethrown as an AssertionError naming the expected bean and the required fix, with the original exception chained as the cause so nothing is lost (Assert.fail would have discarded it). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
References
test_endpointstests indataquest-dev/dspace-rest-test(DSPACE_TEST_CUSTOMER=dtq-dev-9-base).Description
Six CLARIN
LinkRestRepositorybeans are still registered under the singular model name,so all six advertised sub-resources return 404 on the v9 base while the same requests
return 200 on 7.6.5. The fix is
NAME→PLURAL_NAMEin six@Componentannotations.Root cause
DSpace 7 singularized the URL segment before the bean lookup —
Utils.getLinkResourceRepository()calledmakeSingular(modelPlural). DSpace 9 removed thatstep and looks the bean up under the plural segment verbatim:
So a link repository must now be registered as
<category>.<typePlural>.<rel>. These six wereleft as
<category>.<name>.<rel>, and are simply never found.ClarinLicenseResourceUserAllowanceRestRepository— the main repository — was migrated toPLURAL_NAME. That is exactly whyGET /core/clarinlruallowances/242returns 200 while everyone of its rels 404s: migrating a main repository without its link repositories leaves all its
sub-resources dead. Worth remembering for any further v9 ports.
Why this looks like an authorization bug
A missing bean raises
RepositoryNotFoundException, and a missing route resolves before anyauthorization check. One defect therefore surfaces three different ways —
404 != 200for anadmin,
404 != 401for anonymous,404 != 403for a non-owner — which is why the failing RESTtests read as a permissions problem. The 404 body even names the plural type that is not how
the bean is registered:
{"status":404,"error":"Not Found","message":"The repository type core.clarinlruallowances was not found"}Meanwhile the allowance JSON keeps advertising all three
_links, so the API describes endpointsit cannot serve.
Measured before/after
Admin token,
dev-6.pc:8603(9.3, this branch) vsdev-5.pc:88(7.6.5):core/clarinlruallowances/242core/clarinlruallowances/242/userMetadatacore/clarinlruallowances/242/userRegistrationcore/clarinlruallowances/242/resourceMappingcore/clarinuserregistrations/1/userMetadatacore/clarinuserregistrations/1/clarinLicensescore/clarinlicenseresourcemappings/1383/clarinLicenseThe original failure report named only the first three. A sweep of the branch found three more
broken rels that no test touches — doubling the real scope.
Audit backing "six and only six"
Of 77
LinkRestRepositoryimplementations on this branch: 71 already usePLURAL_NAME,these 6 used
NAME, and 0 use a literal bean-name string. Every main (non-link)repository is already plural (
ldn/messagesusesNAME_PLURALS;ContentReportRestRepositoryextends
AbstractDSpaceRestRepositoryand is not plural-routed at all). This closes the gapcompletely rather than fixing only the reported symptoms.
Instructions for Reviewers
List of changes in this PR:
NAME→PLURAL_NAMEin the@Componentof 6 link repositories. One line each;.NAMEwasverified to occur nowhere else in those files. The three REST models already declared
PLURAL_NAMEandgetTypePlural(), so nothing else was needed.ClarinLinkRestRepositoryBeanNameIT— see below.About the test
Nothing in the suite caught this. No IT traverses any of the six rels as a URL sub-path, so
all six could 404 in production with CI green. It surfaced only in the external
dspace-rest-testsuite.The test asserts the invariant through
Utils.getLinkResourceRepository()— the same lookupRestResourceControllerperforms — rather than over HTTP. That is deliberate: these linkrepositories also raise
ResourceNotFoundException(another 404) when the linked data does notexist, so an HTTP test could not separate "route missing" from "no data" without fixtures for a
bitstream, a licence, a resource mapping, a user registration and user metadata per rel — and
would still conflate the two on failure. It is driven off the
@LinksRestannotation instead of ahardcoded list, so any rel added to these models is covered automatically.
Note
CI is this test's first execution. There is no
testEnvironment.zipor test database in myenvironment, so I could not run the IT locally — I am not claiming I did. What I did verify:
mvn -pl dspace-server-webapp test-compile checkstyle:check→ BUILD SUCCESS, 0 Checkstyleviolations, and all six endpoints measured live before/after as above. If the IT trips on
context setup, that is on me and I will fix it.
How to test manually
Then the three the report missed:
The real acceptance criterion is that the three auth cases become distinguishable — admin 200,
anonymous 401, non-owning user 403. Right now all three are 404.
Expected effect on the REST test suite
This should clear 6 of the 8 failing
test_endpointstests. The remaining 2 failures + 2errors are client-side:
libs/dspace-rest-pythonstill calls the singular/core/clarinlruallowance/search/byBitstreamAndUser(404 on 9; the plural form returns 200 andworks on 7.6.5 too, so no version switch is needed).
POST /core/clarinusermetadata/managemust stay singular. It is an explicit
@RequestMappingon a plain@RestController, byteidentical on both branches and not routed through the plural repository lookup. Verified live —
singular POST returns 400 (route exists, param missing), plural returns 415. Pluralizing it would
break a working route.
Checklist
dtq-dev-9-base) — v9-base-specific fix.0 violations)._linkswork again, so no RestContract PR is needed.🤖 Generated with Claude Code