Skip to content

Commit e2bd53e

Browse files
CLARIN-DSpace9.3/Issue 1360: allow to change license in workflow item, in PATCH operation (ufal#1365) (#1327) (#1382)
(cherry picked from commit 8320889 on dtq-dev) v9 adaptations: - WorkflowItemRestRepository.java: import conflict resolved keeping jakarta.servlet.http.HttpServletRequest + adding the commit's Jackson imports in v9 import order (com.fasterxml < jakarta < org). upload() keeps 'throws AuthorizeException' (legal: v9 DSpaceRestRepository.upload declares it). - ClarinWorkflowItemRestRepositoryIT: commit's new test code used javax.ws.rs.core.MediaType -> converted to jakarta.ws.rs.core.MediaType (7x). - TaskRestRepositoriesIT: conflict resolved to jakarta MediaType (HEAD) + isForbidden() (the commit's intended 422->403 claim-guard semantics change). - Gap fix per sync plan card step 5: patchUpdateClarinLicense extended with a bogus-workflowitem-id (999999) isNotFound() assertion covering the new 'source == null' 404 guard, which the original commit never tested. Fulfils CLARIN_V9_POST_SNAPSHOT_SYNC_ACCEPTANCE.md §5 / 8320889 (BE-4, Vlna 2). Co-authored-by: Ondřej Košarko <ko_ok@centrum.cz>
1 parent 198903b commit e2bd53e

3 files changed

Lines changed: 240 additions & 10 deletions

File tree

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

Lines changed: 74 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,17 @@
77
*/
88
package org.dspace.app.rest.repository;
99

10+
import static org.dspace.app.rest.repository.ClarinLicenseRestRepository.OPERATION_PATH_LICENSE_RESOURCE;
1011
import static org.dspace.xmlworkflow.state.actions.processingaction.ProcessingAction.SUBMIT_EDIT_METADATA;
1112

1213
import java.io.IOException;
1314
import java.sql.SQLException;
1415
import java.util.List;
16+
import java.util.Objects;
1517
import java.util.UUID;
1618

19+
import com.fasterxml.jackson.databind.JsonNode;
20+
import com.fasterxml.jackson.databind.node.ObjectNode;
1721
import jakarta.servlet.http.HttpServletRequest;
1822
import org.apache.logging.log4j.LogManager;
1923
import org.apache.logging.log4j.Logger;
@@ -24,9 +28,12 @@
2428
import org.dspace.app.rest.exception.UnprocessableEntityException;
2529
import org.dspace.app.rest.model.ErrorRest;
2630
import org.dspace.app.rest.model.WorkflowItemRest;
31+
import org.dspace.app.rest.model.patch.JsonValueEvaluator;
2732
import org.dspace.app.rest.model.patch.Operation;
2833
import org.dspace.app.rest.model.patch.Patch;
34+
import org.dspace.app.rest.model.patch.ReplaceOperation;
2935
import org.dspace.app.rest.submit.SubmissionService;
36+
import org.dspace.app.rest.submit.step.ClarinLicenseSubmissionUtils;
3037
import org.dspace.app.rest.utils.SolrOAIReindexer;
3138
import org.dspace.app.util.SubmissionConfigReaderException;
3239
import org.dspace.authorize.AuthorizeException;
@@ -199,12 +206,16 @@ public Class<WorkflowItemRest> getDomainClass() {
199206

200207
@Override
201208
public WorkflowItemRest upload(HttpServletRequest request, String apiCategory, String model, Integer id,
202-
MultipartFile file) throws SQLException {
209+
MultipartFile file) throws SQLException, AuthorizeException {
203210

204211
Context context = obtainContext();
205212
WorkflowItemRest wsi = findOne(context, id);
206213
XmlWorkflowItem source = wis.find(context, id);
207214

215+
if (source == null) {
216+
throw new ResourceNotFoundException("WorkflowItem with id " + id + " not found");
217+
}
218+
208219
this.checkIfEditMetadataAllowedInCurrentStep(context, source);
209220
List<ErrorRest> errors = submissionService.uploadFileToInprogressSubmission(context, request, wsi, source,
210221
file);
@@ -225,17 +236,27 @@ public void patch(Context context, HttpServletRequest request, String apiCategor
225236
WorkflowItemRest wsi = findOne(context, id);
226237
XmlWorkflowItem source = wis.find(context, id);
227238

239+
if (source == null) {
240+
throw new ResourceNotFoundException("WorkflowItem with id " + id + " not found");
241+
}
242+
228243
this.checkIfEditMetadataAllowedInCurrentStep(context, source);
229244

230245
for (Operation op : operations) {
231246
//the value in the position 0 is a null value
232247
String[] path = op.getPath().substring(1).split("/", 3);
233-
if (OPERATION_PATH_SECTIONS.equals(path[0])) {
248+
if (OPERATION_PATH_LICENSE_RESOURCE.equals(path[0])) {
249+
// Apply the CLARIN license change through the shared submission helper so the
250+
// workflow `/license` path behaves the same as the submission license paths.
251+
// A non-existing license surfaces as ClarinLicenseNotFoundException (404).
252+
ClarinLicenseSubmissionUtils.applyLicense(context, source.getItem(), extractLicenseName(op));
253+
} else if (OPERATION_PATH_SECTIONS.equals(path[0])) {
234254
String section = path[1];
235255
submissionService.evaluatePatchToInprogressSubmission(context, request, source, wsi, section, op);
236256
} else {
237257
throw new DSpaceBadRequestException(
238-
"Patch path operation need to starts with '" + OPERATION_PATH_SECTIONS + "'");
258+
"Patch path operation need to starts with '" +
259+
OPERATION_PATH_LICENSE_RESOURCE + "' or '" + OPERATION_PATH_SECTIONS + "'");
239260
}
240261
}
241262
wis.update(context, source);
@@ -277,20 +298,65 @@ protected void delete(Context context, Integer id) {
277298
}
278299
}
279300

301+
/**
302+
* Extract the CLARIN license name from a JSON Patch {@code replace} operation on the {@code /license}
303+
* path. The value is accepted either as a plain string or as an object wrapping a textual {@code value}
304+
* field; a non-replace operation or any other value shape is rejected as a bad request. A blank name is
305+
* passed through (the submission helper treats it as a request to clear the current license selection).
306+
* @param op the JSON Patch operation targeting the license path
307+
* @return the CLARIN license name to apply
308+
*/
309+
private String extractLicenseName(Operation op) {
310+
if (!(op instanceof ReplaceOperation)) {
311+
throw new DSpaceBadRequestException("The operation to update the license must be the 'replace' operation");
312+
}
313+
if (op.getValue() instanceof String) {
314+
return (String) op.getValue();
315+
}
316+
if (!(op.getValue() instanceof JsonValueEvaluator)) {
317+
throw wrongValueFormatException(op);
318+
}
319+
JsonValueEvaluator jsonValEvaluator = (JsonValueEvaluator) op.getValue();
320+
if (!(jsonValEvaluator.getValueNode() instanceof ObjectNode)) {
321+
throw wrongValueFormatException(op);
322+
}
323+
// a replace operation may wrap the value in an ObjectNode under the "value" key
324+
JsonNode jsonNodeValue = jsonValEvaluator.getValueNode().get("value");
325+
if (jsonNodeValue != null && jsonNodeValue.isTextual()) {
326+
return jsonNodeValue.asText();
327+
}
328+
throw wrongValueFormatException(op);
329+
}
330+
331+
private DSpaceBadRequestException wrongValueFormatException(Operation op) {
332+
return new DSpaceBadRequestException("Unsupported value format for operation '" + op.getOp()
333+
+ "'. Expected a string or an object with a textual 'value' field.");
334+
}
335+
280336
/**
281337
* Checks if @link{SUBMIT_EDIT_METADATA} is a valid option in the workflow step this task is currently at.
282338
* Patching and uploading is only allowed if this is the case.
283339
* @param context Context
284340
* @param xmlWorkflowItem WorkflowItem of the task
285341
*/
286-
private void checkIfEditMetadataAllowedInCurrentStep(Context context, XmlWorkflowItem xmlWorkflowItem) {
342+
private void checkIfEditMetadataAllowedInCurrentStep(Context context, XmlWorkflowItem xmlWorkflowItem)
343+
throws AuthorizeException {
287344
try {
288-
ClaimedTask claimedTask = claimedTaskService.findByWorkflowIdAndEPerson(context, xmlWorkflowItem,
289-
context.getCurrentUser());
290-
if (claimedTask == null) {
345+
List<ClaimedTask> claimTasks = claimedTaskService.findByWorkflowItem(context, xmlWorkflowItem);
346+
if (claimTasks.isEmpty()) {
291347
throw new UnprocessableEntityException("WorkflowItem with id " + xmlWorkflowItem.getID()
292-
+ " has not been claimed yet.");
348+
+ " has not been claimed yet.");
293349
}
350+
351+
ClaimedTask claimedTask = claimTasks.stream()
352+
.filter(ct -> Objects.equals(ct.getOwner(), context.getCurrentUser()))
353+
.findFirst()
354+
.orElse(null);
355+
if (claimedTask == null) {
356+
throw new AuthorizeException("The current user hasn't claimed the workflow item with id " +
357+
xmlWorkflowItem.getID() + ", so the user cannot patch this item");
358+
}
359+
294360
Workflow workflow = workflowFactory.getWorkflow(claimedTask.getWorkflowItem().getCollection());
295361
Step step = workflow.getStep(claimedTask.getStepID());
296362
WorkflowActionConfig currentActionConfig = step.getActionConfig(claimedTask.getActionID());

dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinWorkflowItemRestRepositoryIT.java

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,29 @@
1515
import static org.hamcrest.Matchers.not;
1616
import static org.junit.Assert.assertFalse;
1717
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
18+
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.patch;
1819
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
1920
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
2021
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
2122

23+
import java.util.ArrayList;
24+
import java.util.HashMap;
25+
import java.util.HashSet;
2226
import java.util.List;
27+
import java.util.Map;
28+
import java.util.Set;
2329
import java.util.UUID;
2430
import java.util.concurrent.atomic.AtomicReference;
2531

2632
import org.apache.commons.lang3.StringUtils;
33+
import org.dspace.app.rest.model.patch.AddOperation;
34+
import org.dspace.app.rest.model.patch.Operation;
35+
import org.dspace.app.rest.model.patch.ReplaceOperation;
36+
import org.dspace.app.rest.repository.ClarinLicenseRestRepository;
2737
import org.dspace.app.rest.test.AbstractControllerIntegrationTest;
38+
import org.dspace.builder.ClaimedTaskBuilder;
39+
import org.dspace.builder.ClarinLicenseBuilder;
40+
import org.dspace.builder.ClarinLicenseLabelBuilder;
2841
import org.dspace.builder.CollectionBuilder;
2942
import org.dspace.builder.CommunityBuilder;
3043
import org.dspace.builder.EPersonBuilder;
@@ -35,12 +48,18 @@
3548
import org.dspace.content.Item;
3649
import org.dspace.content.MetadataValue;
3750
import org.dspace.content.WorkspaceItem;
51+
import org.dspace.content.clarin.ClarinLicense;
52+
import org.dspace.content.clarin.ClarinLicenseLabel;
3853
import org.dspace.content.service.ItemService;
3954
import org.dspace.content.service.WorkspaceItemService;
55+
import org.dspace.content.service.clarin.ClarinLicenseLabelService;
56+
import org.dspace.content.service.clarin.ClarinLicenseService;
4057
import org.dspace.eperson.EPerson;
4158
import org.dspace.license.service.CreativeCommonsService;
4259
import org.dspace.services.ConfigurationService;
4360
import org.dspace.xmlworkflow.factory.XmlWorkflowFactory;
61+
import org.dspace.xmlworkflow.storedcomponents.ClaimedTask;
62+
import org.dspace.xmlworkflow.storedcomponents.XmlWorkflowItem;
4463
import org.dspace.xmlworkflow.storedcomponents.service.CollectionRoleService;
4564
import org.dspace.xmlworkflow.storedcomponents.service.XmlWorkflowItemService;
4665
import org.hamcrest.Matchers;
@@ -78,6 +97,11 @@ public class ClarinWorkflowItemRestRepositoryIT extends AbstractControllerIntegr
7897
@Autowired
7998
private ItemService itemService;
8099

100+
@Autowired
101+
private ClarinLicenseService clarinLicenseService;
102+
@Autowired
103+
private ClarinLicenseLabelService clarinLicenseLabelService;
104+
81105
Item item;
82106

83107
@Before
@@ -364,4 +388,143 @@ public void shouldCreateItemWithCustomTypeBindField() throws Exception {
364388
assertFalse(mvList.isEmpty());
365389
assertThat(mvList.get(0).getValue(), is(CITATION_VALUE));
366390
}
391+
392+
@Test
393+
public void patchUpdateClarinLicense() throws Exception {
394+
context.turnOffAuthorisationSystem();
395+
396+
// create Clarin License Label
397+
ClarinLicenseLabel clarinLicenseLabel = ClarinLicenseLabelBuilder.createClarinLicenseLabel(context).build();
398+
clarinLicenseLabel.setLabel("CC");
399+
clarinLicenseLabel.setExtended(false);
400+
clarinLicenseLabel.setTitle("CLL Title1");
401+
clarinLicenseLabelService.update(context, clarinLicenseLabel);
402+
403+
// create Clarin License
404+
ClarinLicense clarinLicense = ClarinLicenseBuilder.createClarinLicense(context).build();
405+
clarinLicense.setName("CL Name");
406+
clarinLicense.setConfirmation(ClarinLicense.Confirmation.NOT_REQUIRED);
407+
clarinLicense.setDefinition("CL Definition");
408+
clarinLicense.setRequiredInfo("CL Req");
409+
// add clarinLicenseLabel to clarinLicense
410+
Set<ClarinLicenseLabel> clarinLicenseLabels = new HashSet<>();
411+
clarinLicenseLabels.add(clarinLicenseLabel);
412+
clarinLicense.setLicenseLabels(clarinLicenseLabels);
413+
clarinLicenseService.update(context, clarinLicense);
414+
415+
// community with one collection.
416+
parentCommunity = CommunityBuilder.createCommunity(context)
417+
.withName("Parent Community")
418+
.build();
419+
Collection col = CollectionBuilder.createCollection(context, parentCommunity).withName("Collection 1")
420+
.withWorkflowGroup("editor", eperson).build();
421+
422+
// create a normal user to use as submitter
423+
EPerson submitter = EPersonBuilder.createEPerson(context)
424+
.withEmail("submitter@example.com")
425+
.withPassword("dspace")
426+
.build();
427+
428+
// claimed task with workflow item in edit step
429+
ClaimedTask claimedTask = ClaimedTaskBuilder.createClaimedTask(context, col, eperson)
430+
.withTitle("Workflow Item")
431+
.withIssueDate("2026-05-18")
432+
.withSubject("Extra Entry")
433+
.grantLicense()
434+
.build();
435+
claimedTask.setStepID("editstep");
436+
claimedTask.setActionID("editaction");
437+
XmlWorkflowItem wfItem = claimedTask.getWorkflowItem();
438+
439+
context.restoreAuthSystemState();
440+
441+
// prepare a patch targeting the clarin license resource path
442+
List<Operation> ops = new ArrayList<>();
443+
ops.add(new ReplaceOperation("/" + ClarinLicenseRestRepository.OPERATION_PATH_LICENSE_RESOURCE, "CL Name"));
444+
445+
String submitterToken = getAuthToken(submitter.getEmail(), "dspace");
446+
447+
// The submitter shouldn't be allowed to patch clarin license
448+
// because the workflow item was claimed by the other user (eperson),
449+
// and the submitter doesn't have permissions to edit it,
450+
// so the patch request should be rejected with error 403(Forbidden)
451+
getClient(submitterToken).perform(patch("/api/workflow/workflowitems/" + wfItem.getID())
452+
.content(getPatchContent(ops))
453+
.contentType(jakarta.ws.rs.core.MediaType.APPLICATION_JSON_PATCH_JSON))
454+
.andExpect(status().isForbidden());
455+
456+
String editorToken = getAuthToken(eperson.getEmail(), password);
457+
458+
ops.set(0, new ReplaceOperation("/" + ClarinLicenseRestRepository.OPERATION_PATH_LICENSE_RESOURCE, "Wrong CL"));
459+
460+
// The wrong clarin license name value should be rejected with 404 Not Found
461+
getClient(editorToken).perform(patch("/api/workflow/workflowitems/" + wfItem.getID())
462+
.content(getPatchContent(ops))
463+
.contentType(jakarta.ws.rs.core.MediaType.APPLICATION_JSON_PATCH_JSON))
464+
.andExpect(status().isNotFound());
465+
466+
// The valid clarin license name can be in the form of a simple string or
467+
// in the form of a map with "value" key, but it should be accepted in both cases
468+
ops.set(0, new ReplaceOperation("/" + ClarinLicenseRestRepository.OPERATION_PATH_LICENSE_RESOURCE, "CL Name"));
469+
470+
getClient(editorToken).perform(patch("/api/workflow/workflowitems/" + wfItem.getID())
471+
.content(getPatchContent(ops))
472+
.contentType(jakarta.ws.rs.core.MediaType.APPLICATION_JSON_PATCH_JSON))
473+
.andExpect(status().isOk());
474+
475+
XmlWorkflowItem updatedWfItem = xmlWorkflowItemService.find(context, wfItem.getID());
476+
assertThat(itemService.getMetadataFirstValue(updatedWfItem.getItem(), "dc", "rights", null, Item.ANY),
477+
is("CL Name"));
478+
479+
Map<String, String> wrappedValue = new HashMap<String, String>();
480+
wrappedValue.put("value", "CL Name");
481+
ops.set(0, new ReplaceOperation("/" + ClarinLicenseRestRepository.OPERATION_PATH_LICENSE_RESOURCE,
482+
wrappedValue));
483+
484+
getClient(editorToken).perform(patch("/api/workflow/workflowitems/" + wfItem.getID())
485+
.content(getPatchContent(ops))
486+
.contentType(jakarta.ws.rs.core.MediaType.APPLICATION_JSON_PATCH_JSON))
487+
.andExpect(status().isOk());
488+
489+
// The wrapped value should contain the "value" key, otherwise it is invalid
490+
Map<String, String> invalidWrappedValue1 = new HashMap<String, String>();
491+
ops.set(0, new ReplaceOperation("/" + ClarinLicenseRestRepository.OPERATION_PATH_LICENSE_RESOURCE,
492+
invalidWrappedValue1));
493+
494+
getClient(editorToken).perform(patch("/api/workflow/workflowitems/" + wfItem.getID())
495+
.content(getPatchContent(ops))
496+
.contentType(jakarta.ws.rs.core.MediaType.APPLICATION_JSON_PATCH_JSON))
497+
.andExpect(status().isBadRequest());
498+
499+
// The wrapped value should be in a map, not in a list
500+
List<String> invalidWrappedValue2 = new ArrayList<>();
501+
invalidWrappedValue2.add("CL Name");
502+
ops.set(0, new ReplaceOperation("/" + ClarinLicenseRestRepository.OPERATION_PATH_LICENSE_RESOURCE,
503+
invalidWrappedValue2));
504+
505+
getClient(editorToken).perform(patch("/api/workflow/workflowitems/" + wfItem.getID())
506+
.content(getPatchContent(ops))
507+
.contentType(jakarta.ws.rs.core.MediaType.APPLICATION_JSON_PATCH_JSON))
508+
.andExpect(status().isBadRequest());
509+
510+
// The only accepted operation for clarin license resource is "replace",
511+
// "add" operation should be rejected with 400 Bad Request even with the valid value
512+
ops.set(0, new AddOperation("/" + ClarinLicenseRestRepository.OPERATION_PATH_LICENSE_RESOURCE,
513+
"CL Name"));
514+
515+
getClient(editorToken).perform(patch("/api/workflow/workflowitems/" + wfItem.getID())
516+
.content(getPatchContent(ops))
517+
.contentType(jakarta.ws.rs.core.MediaType.APPLICATION_JSON_PATCH_JSON))
518+
.andExpect(status().isBadRequest());
519+
520+
// The reworked claim guard adds a 'source == null' check: a PATCH on a
521+
// nonexistent workflow item id must return 404 Not Found (pre-fix: NPE)
522+
ops.set(0, new ReplaceOperation("/" + ClarinLicenseRestRepository.OPERATION_PATH_LICENSE_RESOURCE,
523+
"CL Name"));
524+
525+
getClient(editorToken).perform(patch("/api/workflow/workflowitems/" + 999999)
526+
.content(getPatchContent(ops))
527+
.contentType(jakarta.ws.rs.core.MediaType.APPLICATION_JSON_PATCH_JSON))
528+
.andExpect(status().isNotFound());
529+
}
367530
}

dspace-server-webapp/src/test/java/org/dspace/app/rest/TaskRestRepositoriesIT.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2879,7 +2879,8 @@ public void patchTest_ClaimedTask_EditMetadataOptionNotAllowed() throws Exceptio
28792879
.andExpect(status().isCreated())
28802880
.andExpect(jsonPath("$", Matchers.allOf(hasJsonPath("$.type", is("claimedtask")))));
28812881

2882-
// try to patch a workspace item while it is in a step that does not have the edit_metadata option (review step)
2882+
// try to patch a workflow item by a user who does not have the edit metadata permission
2883+
// in the current step (review step)
28832884
String authToken = getAuthToken(eperson.getEmail(), password);
28842885

28852886
// a simple patch to update an existent metadata
@@ -2893,7 +2894,7 @@ public void patchTest_ClaimedTask_EditMetadataOptionNotAllowed() throws Exceptio
28932894
getClient(authToken).perform(patch("/api/workflow/workflowitems/" + witem.getID())
28942895
.content(patchBody)
28952896
.contentType(jakarta.ws.rs.core.MediaType.APPLICATION_JSON_PATCH_JSON))
2896-
.andExpect(status().isUnprocessableEntity());
2897+
.andExpect(status().isForbidden());
28972898
}
28982899

28992900
@Test

0 commit comments

Comments
 (0)