Skip to content

Commit 3505ee9

Browse files
KasinhouMatus Kasakclaude
authored
DSpace9.3/Fix CLARIN link rels returning 404: register link repositories under plural model names (#1404)
* Register CLARIN link repositories under plural model names (DSpace 9 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> * Add regression test for the CLARIN link-repository bean naming contract 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> * Report an actionable failure when a link repository bean is missing 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> --------- Co-authored-by: Matus Kasak <matus.kasak@dataquest.sk> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 854cd94 commit 3505ee9

7 files changed

Lines changed: 109 additions & 6 deletions

dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/CLRUAResourceMappingLinkRepository.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
/**
2929
* CLRUA = ClarinLicenseResourceUserAllowance
3030
*/
31-
@Component(ClarinLicenseResourceUserAllowanceRest.CATEGORY + "." + ClarinLicenseResourceUserAllowanceRest.NAME +
31+
@Component(ClarinLicenseResourceUserAllowanceRest.CATEGORY + "." + ClarinLicenseResourceUserAllowanceRest.PLURAL_NAME +
3232
"." + ClarinLicenseResourceUserAllowanceRest.RESOURCE_MAPPING)
3333
public class CLRUAResourceMappingLinkRepository extends AbstractDSpaceRestRepository
3434
implements LinkRestRepository {

dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/CLRUAUUserRegistrationLinkRepository.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
/**
3030
* CLRUA = ClarinLicenseResourceUserAllowance
3131
*/
32-
@Component(ClarinLicenseResourceUserAllowanceRest.CATEGORY + "." + ClarinLicenseResourceUserAllowanceRest.NAME +
32+
@Component(ClarinLicenseResourceUserAllowanceRest.CATEGORY + "." + ClarinLicenseResourceUserAllowanceRest.PLURAL_NAME +
3333
"." + ClarinLicenseResourceUserAllowanceRest.USER_REGISTRATION)
3434
public class CLRUAUUserRegistrationLinkRepository extends AbstractDSpaceRestRepository
3535
implements LinkRestRepository {

dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/CLRUAUserMetadataLinkRepository.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
/**
3333
* CLRUA = ClarinLicenseResourceUserAllowance
3434
*/
35-
@Component(ClarinLicenseResourceUserAllowanceRest.CATEGORY + "." + ClarinLicenseResourceUserAllowanceRest.NAME +
35+
@Component(ClarinLicenseResourceUserAllowanceRest.CATEGORY + "." + ClarinLicenseResourceUserAllowanceRest.PLURAL_NAME +
3636
"." + ClarinLicenseResourceUserAllowanceRest.USER_METADATA)
3737
public class CLRUAUserMetadataLinkRepository extends AbstractDSpaceRestRepository
3838
implements LinkRestRepository {

dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/CUserRegistrationCLicenseLinkRepository.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
import org.springframework.data.rest.webmvc.ResourceNotFoundException;
2828
import org.springframework.stereotype.Component;
2929

30-
@Component(ClarinUserRegistrationRest.CATEGORY + "." + ClarinUserRegistrationRest.NAME + "." +
30+
@Component(ClarinUserRegistrationRest.CATEGORY + "." + ClarinUserRegistrationRest.PLURAL_NAME + "." +
3131
ClarinUserRegistrationRest.CLARIN_LICENSES)
3232
public class CUserRegistrationCLicenseLinkRepository extends AbstractDSpaceRestRepository
3333
implements LinkRestRepository {

dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClarinResourceMappingCLicenseLinkRepository.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
import org.springframework.data.rest.webmvc.ResourceNotFoundException;
2525
import org.springframework.stereotype.Component;
2626

27-
@Component(ClarinLicenseResourceMappingRest.CATEGORY + "." + ClarinLicenseResourceMappingRest.NAME +
27+
@Component(ClarinLicenseResourceMappingRest.CATEGORY + "." + ClarinLicenseResourceMappingRest.PLURAL_NAME +
2828
"." + ClarinLicenseResourceMappingRest.CLARIN_LICENSE)
2929
public class ClarinResourceMappingCLicenseLinkRepository extends AbstractDSpaceRestRepository
3030
implements LinkRestRepository {

dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClarinUserRegistrationUserMetadataLinkRepository.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
import org.springframework.data.rest.webmvc.ResourceNotFoundException;
2626
import org.springframework.stereotype.Component;
2727

28-
@Component(ClarinUserRegistrationRest.CATEGORY + "." + ClarinUserRegistrationRest.NAME + "." +
28+
@Component(ClarinUserRegistrationRest.CATEGORY + "." + ClarinUserRegistrationRest.PLURAL_NAME + "." +
2929
ClarinUserRegistrationRest.USER_METADATA)
3030
public class ClarinUserRegistrationUserMetadataLinkRepository extends AbstractDSpaceRestRepository
3131
implements LinkRestRepository {
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
/**
2+
* The contents of this file are subject to the license and copyright
3+
* detailed in the LICENSE and NOTICE files at the root of the source
4+
* tree and available online at
5+
*
6+
* http://www.dspace.org/license/
7+
*/
8+
package org.dspace.app.rest;
9+
10+
import static org.junit.Assert.assertNotNull;
11+
import static org.junit.Assert.assertTrue;
12+
13+
import org.dspace.app.rest.exception.RepositoryNotFoundException;
14+
import org.dspace.app.rest.model.ClarinLicenseResourceMappingRest;
15+
import org.dspace.app.rest.model.ClarinLicenseResourceUserAllowanceRest;
16+
import org.dspace.app.rest.model.ClarinUserRegistrationRest;
17+
import org.dspace.app.rest.model.LinkRest;
18+
import org.dspace.app.rest.model.LinksRest;
19+
import org.dspace.app.rest.model.RestAddressableModel;
20+
import org.dspace.app.rest.repository.LinkRestRepository;
21+
import org.dspace.app.rest.test.AbstractControllerIntegrationTest;
22+
import org.dspace.app.rest.utils.Utils;
23+
import org.junit.Test;
24+
import org.springframework.beans.factory.annotation.Autowired;
25+
26+
/**
27+
* Guards the DSpace 9 link-repository bean naming contract for the CLARIN models.
28+
* <P>
29+
* DSpace 7 resolved a rel by singularizing the URL segment before the bean lookup
30+
* ({@code Utils.getLinkResourceRepository} called {@code makeSingular}). DSpace 9 removed that
31+
* step and looks the bean up under the plural segment verbatim, so every
32+
* {@link LinkRestRepository} must be registered as
33+
* {@code <category>.<typePlural>.<rel>}. A repository still registered under the singular name
34+
* is simply never found: the lookup raises {@code RepositoryNotFoundException}, the client gets
35+
* a 404, and because a missing route resolves BEFORE any authorization check the same defect
36+
* shows up as "404 instead of 200" for an admin and "404 instead of 401/403" for everyone else.
37+
* It therefore reads like an authorization bug while the object's own {@code _links} keep
38+
* advertising the dead rels.
39+
* <P>
40+
* This test asserts the invariant directly instead of going through HTTP, because the link
41+
* repositories also raise {@code ResourceNotFoundException} (another 404) when the linked data
42+
* simply does not exist -- an endpoint test could not tell the two apart without fixtures for
43+
* every entity type. It is deliberately driven off the {@link LinksRest} annotation rather than
44+
* a hardcoded list, so a rel added to any of these models is covered automatically.
45+
*/
46+
public class ClarinLinkRestRepositoryBeanNameIT extends AbstractControllerIntegrationTest {
47+
48+
@Autowired
49+
private Utils utils;
50+
51+
/**
52+
* Asserts that every rel declared via {@link LinksRest} on the given model resolves to a
53+
* registered {@link LinkRestRepository}, using the same lookup {@link RestResourceController}
54+
* performs when a client traverses the rel.
55+
*
56+
* @param modelClass the REST model whose declared rels should all be resolvable
57+
*/
58+
private void assertAllDeclaredRelsResolve(Class<? extends RestAddressableModel> modelClass)
59+
throws ReflectiveOperationException {
60+
RestAddressableModel model = modelClass.getDeclaredConstructor().newInstance();
61+
LinksRest linksRest = modelClass.getDeclaredAnnotation(LinksRest.class);
62+
assertNotNull(modelClass.getSimpleName() + " is expected to declare @LinksRest", linksRest);
63+
assertTrue(modelClass.getSimpleName() + " is expected to declare at least one @LinkRest",
64+
linksRest.links().length > 0);
65+
66+
for (LinkRest linkRest : linksRest.links()) {
67+
String expectedBeanName = model.getCategory() + "." + model.getTypePlural() + "." + linkRest.name();
68+
try {
69+
LinkRestRepository repository =
70+
utils.getLinkResourceRepository(model.getCategory(), model.getTypePlural(), linkRest.name());
71+
assertNotNull("No LinkRestRepository registered as '" + expectedBeanName + "'", repository);
72+
} catch (RepositoryNotFoundException e) {
73+
// Translate the lookup failure into an actionable assertion. RepositoryNotFoundException
74+
// reports only "<category>.<typePlural>" and never the rel, so on a model with several
75+
// rels its own message cannot say which one is unregistered. It is also misleading here:
76+
// it claims the repository *type* is missing when the main repository resolves fine and
77+
// only the link repository bean is absent.
78+
throw new AssertionError("No LinkRestRepository is registered as '" + expectedBeanName
79+
+ "'. On DSpace 9 link repositories are looked up under the plural model name, so"
80+
+ " the @Component of the repository serving this rel must be built from"
81+
+ " PLURAL_NAME, not NAME.", e);
82+
}
83+
}
84+
}
85+
86+
@Test
87+
public void clarinLicenseResourceUserAllowanceRelsResolve() throws Exception {
88+
// resourceMapping, userRegistration, userMetadata
89+
assertAllDeclaredRelsResolve(ClarinLicenseResourceUserAllowanceRest.class);
90+
}
91+
92+
@Test
93+
public void clarinUserRegistrationRelsResolve() throws Exception {
94+
// clarinLicenses, userMetadata
95+
assertAllDeclaredRelsResolve(ClarinUserRegistrationRest.class);
96+
}
97+
98+
@Test
99+
public void clarinLicenseResourceMappingRelsResolve() throws Exception {
100+
// clarinLicense
101+
assertAllDeclaredRelsResolve(ClarinLicenseResourceMappingRest.class);
102+
}
103+
}

0 commit comments

Comments
 (0)