[Port to dtq-dev] Issue 1343: add PUT and DELETE endpoint methods to ClarinLicenseLabel REST repository#1325
Merged
milanmajchrak merged 3 commits intoJun 23, 2026
Conversation
… REST repository (#1357) * Issue 1343: add PUT and DELETE endpoint methods to ClarinLicenseLabelRest repository * resolve Copilot comments * fixed PUT request in ClarinLicenseLabelRestRepository * added check for Clarin License Label -> Label string to be shorter that 5 characters * implement coorrect put method in ClarinLicenseLabelRestRepository * add constraints to license_label table: made label UNIQUE, make license_label not deletable when used in clarin licenses * fixed IT test failure * change order of deleting objects in test cleanup(): delete license objects before license_label objects (to satisfy license_label constraints) * resolve PR Copilot comments, fixed failing ClarinWorkspaceItemRestRepositoryIT * prevent creating duplicate Clarin License Labels in REST API * not necessary to trim label twice * minor fixes, suggested by Copilot * Rename SQL migration files to use today's date (2026.06.01) --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> (cherry picked from commit b041c90)
…abel arg
The backport added a `label` String parameter to the createClarinLicense
test helper but left six 4-arg call sites unchanged, breaking testCompile
in dspace-server-webapp. Pass a label at each remaining call site
("lbl"; "lbl1"/"lbl2" for the paired-license test) to match the helper's
new signature, preserving the previously hard-coded "lbl" behaviour.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR adds label-based lookup capability for ClarinLicense and ClarinLicenseLabel entities across the data access, REST, and test layers. It introduces ChangesClarinLicense findByLabel and REST Validation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
dspace-api/src/main/java/org/dspace/content/dao/impl/clarin/ClarinLicenseLabelDAOImpl.java (1)
34-35: ⚡ Quick winAdd method Javadoc on the new public override.
For overridden public methods, a brief
/** {@inheritdoc} */is sufficient to satisfy documentation requirements.As per coding guidelines, "Provide Javadoc comments for all public classes and methods".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dspace-api/src/main/java/org/dspace/content/dao/impl/clarin/ClarinLicenseLabelDAOImpl.java` around lines 34 - 35, Add a Javadoc comment to the public overridden method findByLabel in class ClarinLicenseLabelDAOImpl: place a brief /** {`@inheritDoc`} */ directly above the method signature of public ClarinLicenseLabel findByLabel(Context context, String label) throws SQLException to satisfy the coding guideline requiring Javadoc on public methods.Source: Coding guidelines
dspace-api/src/main/java/org/dspace/content/dao/clarin/ClarinLicenseLabelDAO.java (1)
26-26: ⚡ Quick winDocument the new public DAO contract with Javadoc.
Please add method-level Javadoc for
findByLabel(...)to match API documentation requirements.As per coding guidelines, "Provide Javadoc comments for all public classes and methods".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dspace-api/src/main/java/org/dspace/content/dao/clarin/ClarinLicenseLabelDAO.java` at line 26, Add Javadoc for the public DAO method findByLabel in ClarinLicenseLabelDAO: describe the purpose (find a ClarinLicenseLabel by its label), document parameters (Context context, String label), document return value (ClarinLicenseLabel or null if not found), and list thrown exceptions (throws SQLException). Place the Javadoc immediately above the method declaration in the ClarinLicenseLabelDAO interface and use standard tags (`@param`, `@return`, `@throws`) to satisfy the API documentation requirement.Source: Coding guidelines
dspace-api/src/main/java/org/dspace/content/clarin/ClarinLicenseLabelServiceImpl.java (1)
80-83: ⚡ Quick winAdd Javadoc for the new public service method.
Please add method documentation (or
{@inheritdoc}) forfindByLabel(...).As per coding guidelines, "Provide Javadoc comments for all public classes and methods".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dspace-api/src/main/java/org/dspace/content/clarin/ClarinLicenseLabelServiceImpl.java` around lines 80 - 83, Add Javadoc to the public service method findByLabel in ClarinLicenseLabelServiceImpl: update the method declaration for public ClarinLicenseLabel findByLabel(Context context, String label) to include a Javadoc block (or {`@inheritDoc`}) describing the purpose, parameters (context, label), return value, and SQLException thrown; reference the DAO call clarinLicenseLabelDAO.findByLabel in the comment if helpful to explain behavior.Source: Coding guidelines
dspace-api/src/main/java/org/dspace/content/dao/clarin/ClarinLicenseDAO.java (1)
31-31: ⚡ Quick winAdd Javadoc to the new public DAO method.
findByLabel(...)is public API surface and should be documented for contract clarity.Suggested update
+ /** + * Find CLARIN licenses by associated label. + * + * `@param` context current DSpace context + * `@param` label license label + * `@return` matching licenses + * `@throws` SQLException if database access fails + */ List<ClarinLicense> findByLabel(Context context, String label) throws SQLException;As per coding guidelines, "Provide Javadoc comments for all public classes and methods".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dspace-api/src/main/java/org/dspace/content/dao/clarin/ClarinLicenseDAO.java` at line 31, Add Javadoc for the new public DAO method findByLabel in the ClarinLicenseDAO interface: document the method's purpose (lookup licenses by label), describe each parameter (Context context, String label), state the return value (List<ClarinLicense> matching the label, possibly empty), and document the SQLException thrown; ensure the Javadoc appears above the method signature in ClarinLicenseDAO and follows project tag conventions (`@param`, `@return`, `@throws`).Source: Coding guidelines
dspace-api/src/main/java/org/dspace/content/clarin/ClarinLicenseServiceImpl.java (1)
101-104: ⚡ Quick winDocument
findByLabel(...)with Javadoc.This new public method should include method-level Javadoc (or
{@inheritdoc}) to keep public API docs complete.As per coding guidelines, "Provide Javadoc comments for all public classes and methods".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dspace-api/src/main/java/org/dspace/content/clarin/ClarinLicenseServiceImpl.java` around lines 101 - 104, Add method-level Javadoc to the public method findByLabel in ClarinLicenseServiceImpl: either add a descriptive Javadoc block explaining purpose, parameters (Context context, String label), return value (List<ClarinLicense>), and thrown SQLException, or use {`@inheritDoc`} if the interface already documents it; ensure the Javadoc appears immediately above the findByLabel method and references the parameters and exception so the public API documentation remains complete.Source: Coding guidelines
dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClarinLicenseLabelRestRepository.java (1)
170-180: 💤 Low valueAdd Javadoc for the private helper method.
While less critical than public methods, documenting the validation logic in
checkLabelAndTitlewould improve maintainability.📝 Suggested Javadoc
+ /** + * Validates that the label and title are not blank and that the label length + * does not exceed MAX_LABEL_LENGTH. + * + * `@param` clarinLicenseLabelRest the REST object to validate + * `@throws` DSpaceBadRequestException if validation fails + */ private void checkLabelAndTitle(ClarinLicenseLabelRest clarinLicenseLabelRest) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClarinLicenseLabelRestRepository.java` around lines 170 - 180, Add Javadoc to the private helper checkLabelAndTitle in ClarinLicenseLabelRestRepository describing its purpose (validate non-null/empty trimmed label and title), the behavior (throws DSpaceBadRequestException on missing/blank fields or when label exceeds MAX_LABEL_LENGTH), and note the trimming of label via Optional and the use of MAX_LABEL_LENGTH; keep the comment concise and place it immediately above the checkLabelAndTitle method.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@dspace-api/src/main/java/org/dspace/content/dao/impl/clarin/ClarinLicenseDAOImpl.java`:
- Around line 62-76: The public method findByLabel in ClarinLicenseDAOImpl is
missing Javadoc; add a standard Javadoc block above the method explaining the
purpose of findByLabel(Context context, String label), describing the parameters
(context and label), the return value (List<ClarinLicense>), and the thrown
SQLException, and include any behavioural notes (e.g., matching semantics or
null/empty handling) so the documentation meets project guidelines and documents
the contract of the overridden DAO method.
In
`@dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/h2/V7.6_2026.06.01__license_label_constraint.sql`:
- Around line 17-18: The migration unconditionally drops/adds the UNIQUE
constraint on license_label.label which will fail if duplicate labels exist;
before adding license_label_label_unique run a preflight duplicate check (SELECT
label, COUNT(*) ... HAVING COUNT(*) > 1) and implement a deterministic
dedup/upsert step to remove or merge duplicates (e.g. keep the earliest id or
newest timestamp) for rows with the same label, then re-run the ALTER TABLE to
add the UNIQUE constraint; reference the table name license_label and the
constraint name license_label_label_unique when adding the preflight check and
deterministic cleanup migration.
In
`@dspace-api/src/test/java/org/dspace/builder/util/AbstractBuilderCleanupUtil.java`:
- Line 20: The cleanup map in AbstractBuilderCleanupUtil now includes
ClarinLicenseBuilder but is missing ClarinLicenseLabelBuilder, causing FK
deletion order issues; update the cleanup map in the AbstractBuilderCleanupUtil
class to add ClarinLicenseLabelBuilder immediately after the existing
ClarinLicenseBuilder entry so that ClarinLicenseBuilder (licenses) are removed
before ClarinLicenseLabelBuilder (labels), ensuring deletions obey the
foreign-key constraint referenced by ClarinLicenseLabelRestRepository; locate
the map population code (the entries involving ClarinLicenseBuilder) and insert
the ClarinLicenseLabelBuilder entry right after it.
In
`@dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClarinLicenseLabelRestRepository.java`:
- Around line 112-148: Add a Javadoc comment to the public method put in
ClarinLicenseLabelRestRepository: document the purpose of the endpoint, describe
each parameter (Context context, HttpServletRequest request, String apiCategory,
String model, Integer id, JsonNode jsonNode), explain the return value
(ClarinLicenseLabelRest via converter.toRest), and list thrown exceptions
(SQLException, AuthorizeException, DSpaceBadRequestException,
ClarinLicenseLabelNotFoundException). Keep the description concise and mention
relevant behavior such as parsing with objectMapper.treeToValue, validation via
checkLabelAndTitle, uniqueness check via clarinLicenseLabelService.findByLabel,
and that the entity is updated with updateClarinLicenseLabel and
clarinLicenseLabelService.update.
- Around line 150-168: Add Javadoc to the public method delete(Context context,
Integer id) in ClarinLicenseLabelRestRepository: document the purpose of the
method (deletes a ClarinLicenseLabel by id), describe parameters `@param` context
and `@param` id, and list thrown exceptions with `@throws` for AuthorizeException,
ClarinLicenseLabelNotFoundException, DSpaceBadRequestException and any
runtime/SQL-related exceptions that can propagate (e.g., RuntimeException
wrapping SQLException); also include a brief note about the
`@PreAuthorize`("hasAuthority('ADMIN')") security requirement. Ensure the Javadoc
sits immediately above the delete method signature.
---
Nitpick comments:
In
`@dspace-api/src/main/java/org/dspace/content/clarin/ClarinLicenseLabelServiceImpl.java`:
- Around line 80-83: Add Javadoc to the public service method findByLabel in
ClarinLicenseLabelServiceImpl: update the method declaration for public
ClarinLicenseLabel findByLabel(Context context, String label) to include a
Javadoc block (or {`@inheritDoc`}) describing the purpose, parameters (context,
label), return value, and SQLException thrown; reference the DAO call
clarinLicenseLabelDAO.findByLabel in the comment if helpful to explain behavior.
In
`@dspace-api/src/main/java/org/dspace/content/clarin/ClarinLicenseServiceImpl.java`:
- Around line 101-104: Add method-level Javadoc to the public method findByLabel
in ClarinLicenseServiceImpl: either add a descriptive Javadoc block explaining
purpose, parameters (Context context, String label), return value
(List<ClarinLicense>), and thrown SQLException, or use {`@inheritDoc`} if the
interface already documents it; ensure the Javadoc appears immediately above the
findByLabel method and references the parameters and exception so the public API
documentation remains complete.
In
`@dspace-api/src/main/java/org/dspace/content/dao/clarin/ClarinLicenseDAO.java`:
- Line 31: Add Javadoc for the new public DAO method findByLabel in the
ClarinLicenseDAO interface: document the method's purpose (lookup licenses by
label), describe each parameter (Context context, String label), state the
return value (List<ClarinLicense> matching the label, possibly empty), and
document the SQLException thrown; ensure the Javadoc appears above the method
signature in ClarinLicenseDAO and follows project tag conventions (`@param`,
`@return`, `@throws`).
In
`@dspace-api/src/main/java/org/dspace/content/dao/clarin/ClarinLicenseLabelDAO.java`:
- Line 26: Add Javadoc for the public DAO method findByLabel in
ClarinLicenseLabelDAO: describe the purpose (find a ClarinLicenseLabel by its
label), document parameters (Context context, String label), document return
value (ClarinLicenseLabel or null if not found), and list thrown exceptions
(throws SQLException). Place the Javadoc immediately above the method
declaration in the ClarinLicenseLabelDAO interface and use standard tags
(`@param`, `@return`, `@throws`) to satisfy the API documentation requirement.
In
`@dspace-api/src/main/java/org/dspace/content/dao/impl/clarin/ClarinLicenseLabelDAOImpl.java`:
- Around line 34-35: Add a Javadoc comment to the public overridden method
findByLabel in class ClarinLicenseLabelDAOImpl: place a brief /** {`@inheritDoc`}
*/ directly above the method signature of public ClarinLicenseLabel
findByLabel(Context context, String label) throws SQLException to satisfy the
coding guideline requiring Javadoc on public methods.
In
`@dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClarinLicenseLabelRestRepository.java`:
- Around line 170-180: Add Javadoc to the private helper checkLabelAndTitle in
ClarinLicenseLabelRestRepository describing its purpose (validate non-null/empty
trimmed label and title), the behavior (throws DSpaceBadRequestException on
missing/blank fields or when label exceeds MAX_LABEL_LENGTH), and note the
trimming of label via Optional and the use of MAX_LABEL_LENGTH; keep the comment
concise and place it immediately above the checkLabelAndTitle method.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 22405db3-31ea-40ca-aa5e-3395ae46e0db
📒 Files selected for processing (16)
dspace-api/src/main/java/org/dspace/content/clarin/ClarinLicenseLabelServiceImpl.javadspace-api/src/main/java/org/dspace/content/clarin/ClarinLicenseServiceImpl.javadspace-api/src/main/java/org/dspace/content/dao/clarin/ClarinLicenseDAO.javadspace-api/src/main/java/org/dspace/content/dao/clarin/ClarinLicenseLabelDAO.javadspace-api/src/main/java/org/dspace/content/dao/impl/clarin/ClarinLicenseDAOImpl.javadspace-api/src/main/java/org/dspace/content/dao/impl/clarin/ClarinLicenseLabelDAOImpl.javadspace-api/src/main/java/org/dspace/content/service/clarin/ClarinLicenseLabelService.javadspace-api/src/main/java/org/dspace/content/service/clarin/ClarinLicenseService.javadspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/h2/V7.6_2026.06.01__license_label_constraint.sqldspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/postgres/V7.6_2026.06.01__license_label_constraint.sqldspace-api/src/test/java/org/dspace/builder/util/AbstractBuilderCleanupUtil.javadspace-server-webapp/src/main/java/org/dspace/app/rest/exception/ClarinLicenseLabelNotFoundException.javadspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClarinLicenseLabelRestRepository.javadspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinLicenseLabelRestRepositoryIT.javadspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinWorkspaceItemRestRepositoryIT.javadspace-server-webapp/src/test/java/org/dspace/app/rest/ProvenanceServiceIT.java
There was a problem hiding this comment.
Pull request overview
This PR ports Issue 1343 to dtq-dev by expanding the CLARIN License Label REST support (adding PUT/DELETE), adding label-based lookup in the CLARIN License service/DAO layer, and enforcing stricter validation/constraints around license labels.
Changes:
- Add/update REST API behavior for
ClarinLicenseLabel(PUT + DELETE), including label/title validation, max label length enforcement, uniqueness checks, and “in-use” deletion protection. - Add DAO/service methods to find licenses by a given label (to support deletion checks and label-based retrieval).
- Add DB migration to enforce FK restrict semantics and a UNIQUE constraint on
license_label.label, plus expanded integration tests.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| dspace-server-webapp/src/test/java/org/dspace/app/rest/ProvenanceServiceIT.java | Updates test helpers to create licenses with explicit labels and adjusts cleanup order for FK constraints. |
| dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinWorkspaceItemRestRepositoryIT.java | Updates workspace item REST integration tests to create licenses with explicit label values. |
| dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinLicenseLabelRestRepositoryIT.java | Adds integration coverage for create/update/delete scenarios, including validation and “label in use” deletion behavior. |
| dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ClarinLicenseLabelRestRepository.java | Implements PUT/DELETE endpoints and centralizes validation + update logic for license labels. |
| dspace-server-webapp/src/main/java/org/dspace/app/rest/exception/ClarinLicenseLabelNotFoundException.java | Adds a dedicated 404 exception for missing license labels. |
| dspace-api/src/test/java/org/dspace/builder/util/AbstractBuilderCleanupUtil.java | Registers ClarinLicenseBuilder for automated builder cleanup in tests. |
| dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/postgres/V7.6_2026.06.01__license_label_constraint.sql | Adds/replaces FK + UNIQUE constraints for Postgres to enforce restrict semantics and label uniqueness. |
| dspace-api/src/main/resources/org/dspace/storage/rdbms/sqlmigration/h2/V7.6_2026.06.01__license_label_constraint.sql | Adds/replaces FK + UNIQUE constraints for H2 to enforce restrict semantics and label uniqueness. |
| dspace-api/src/main/java/org/dspace/content/service/clarin/ClarinLicenseService.java | Adds findByLabel() to the CLARIN license service API. |
| dspace-api/src/main/java/org/dspace/content/service/clarin/ClarinLicenseLabelService.java | Adds findByLabel() to the CLARIN license label service API. |
| dspace-api/src/main/java/org/dspace/content/dao/impl/clarin/ClarinLicenseLabelDAOImpl.java | Implements findByLabel() query for license labels via Criteria API. |
| dspace-api/src/main/java/org/dspace/content/dao/impl/clarin/ClarinLicenseDAOImpl.java | Implements findByLabel() query for licenses via join to license labels. |
| dspace-api/src/main/java/org/dspace/content/dao/clarin/ClarinLicenseLabelDAO.java | Adds findByLabel() to the DAO interface. |
| dspace-api/src/main/java/org/dspace/content/dao/clarin/ClarinLicenseDAO.java | Adds findByLabel() to the DAO interface. |
| dspace-api/src/main/java/org/dspace/content/clarin/ClarinLicenseServiceImpl.java | Wires the new findByLabel() through to the DAO. |
| dspace-api/src/main/java/org/dspace/content/clarin/ClarinLicenseLabelServiceImpl.java | Wires the new findByLabel() through to the DAO. |
milanmajchrak
requested changes
Jun 19, 2026
milanmajchrak
left a comment
milanmajchrak
left a comment
Collaborator
There was a problem hiding this comment.
Please check the copilot comments
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.
Port of ufal#1357 by @kuchtiak-ufal to
dtq-dev.Summary by CodeRabbit