Skip to content

Commit

Permalink
Merge pull request #6326 from BartChris/fix_duplicate_titles
Browse files Browse the repository at this point in the history
Prevent duplicate and invalid titles
  • Loading branch information
solth authored Jan 24, 2025
2 parents 1202bf5 + d2eb10a commit 9185945
Show file tree
Hide file tree
Showing 6 changed files with 145 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ public enum ParameterCore implements ParameterInterface {
/**
* Validation of process title via regular expression.
*/
VALIDATE_PROCESS_TITLE_REGEX(new Parameter<>("validateProzessTitelRegex", "[\\w-]*")),
VALIDATE_PROCESS_TITLE_REGEX(new Parameter<>("validateProzessTitelRegex", "^[\\w-]+$")),

/**
* Validation of the identifier via regular expression.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,7 @@ static boolean setChildCount(Process parent, RulesetManagementInterface ruleset,
/**
* Create process hierarchy.
*/
private void createProcessHierarchy()
public void createProcessHierarchy()
throws DataException, ProcessGenerationException, IOException {
// discard all processes in hierarchy except the first if parent process in
// title record link tab is selected!
Expand All @@ -523,7 +523,7 @@ private void createProcessHierarchy()
&& !this.titleRecordLinkTab.getSelectedInsertionPosition().isEmpty()) {
this.processes = new LinkedList<>(Collections.singletonList(this.processes.get(0)));
}
ProcessService.checkTasks(this.getMainProcess(), processDataTab.getDocType());
processTempProcess(this.processes.get(0));
processAncestors();
processChildren();
// main process and it's ancestors need to be saved, so they have IDs before creating their process directories
Expand Down Expand Up @@ -612,21 +612,25 @@ private void processAncestors() throws ProcessGenerationException {
ProcessService.setParentRelations(processes.get(index + 1).getProcess(), process);
}
if (Objects.nonNull(tempProcess.getMetadataNodes())) {
try {
tempProcess.getProcessMetadata().preserve();
ImportService.processTempProcess(tempProcess, rulesetManagement, acquisitionStage, priorityList, null);
} catch (InvalidMetadataValueException | NoSuchMetadataFieldException e) {
throw new ProcessGenerationException("Error creating process hierarchy: invalid metadata found!");
} catch (RulesetNotFoundException e) {
throw new ProcessGenerationException(
"Ruleset not found:" + tempProcess.getProcess().getRuleset().getTitle());
} catch (IOException e) {
throw new ProcessGenerationException("Error reading Ruleset: " + tempProcess.getProcess().getRuleset().getTitle());
}
processTempProcess(tempProcess);
}
}
}

private void processTempProcess(TempProcess tempProcess) throws ProcessGenerationException {
try {
tempProcess.getProcessMetadata().preserve();
ImportService.processTempProcess(tempProcess, rulesetManagement, acquisitionStage, priorityList, null);
} catch (InvalidMetadataValueException | NoSuchMetadataFieldException e) {
throw new ProcessGenerationException("Error creating process hierarchy: invalid metadata found!");
} catch (RulesetNotFoundException e) {
throw new ProcessGenerationException(
"Ruleset not found:" + tempProcess.getProcess().getRuleset().getTitle());
} catch (IOException e) {
throw new ProcessGenerationException("Error reading Ruleset: " + tempProcess.getProcess().getRuleset().getTitle());
}
}

private void saveProcessHierarchyMetadata() {
// save ancestor processes meta.xml files
for (TempProcess tempProcess : this.processes) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1782,7 +1782,8 @@ private List<PropertyDTO> filterForCorrectionSolutionMessages(List<PropertyDTO>
* @return amount as Long
*/
public Long findNumberOfProcessesWithTitle(String title) throws DataException {
return count(createSimpleQuery(ProcessTypeField.TITLE.getKey(), title, true, Operator.AND));
return countDocumentsAcrossProjects(createSimpleQuery(ProcessTypeField.TITLE.getKey(), title, true,
Operator.AND));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,4 +106,15 @@ public List<S> findByQueryInAllProjects(QueryBuilder query, boolean related) thr
public Long countDocuments(QueryBuilder query) throws DataException {
return super.countDocuments(queryForProjects(query));
}

/**
* Execute a count query without filtering
* for projects of the current user.
* @param query
* as QueryBuilder object
* @return amount of objects according to given query or 0 if query is null
*/
public Long countDocumentsAcrossProjects(QueryBuilder query) throws DataException {
return super.countDocuments(query);
}
}
2 changes: 1 addition & 1 deletion Kitodo/src/main/resources/kitodo_config.properties
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ numberOfMetaBackups=8
useMetadatenvalidierung=true

# Validierung der Vorgangstitel ueber regulaeren Ausdruck
validateProzessTitelRegex=[\\w-]*
validateProzessTitelRegex=^[\\w-]+$

# Validierung des Identifiers ueber regulaeren Ausdruck
# Regular Expression for validating the identifier
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@

import static org.awaitility.Awaitility.await;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.io.File;
Expand All @@ -23,6 +25,7 @@

import org.apache.commons.lang3.SystemUtils;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.kitodo.ExecutionPermission;
Expand All @@ -33,6 +36,7 @@
import org.kitodo.config.enums.ParameterCore;
import org.kitodo.data.database.beans.Process;
import org.kitodo.data.database.beans.User;
import org.kitodo.exceptions.ProcessGenerationException;
import org.kitodo.production.forms.createprocess.CreateProcessForm;
import org.kitodo.production.helper.TempProcess;
import org.kitodo.production.services.ServiceManager;
Expand All @@ -48,6 +52,7 @@ public class CreateProcessFormIT {
private static final ProcessService processService = ServiceManager.getProcessService();

private static final String firstProcess = "First process";
private Process createdProcess;

/**
* Is running before the class runs.
Expand Down Expand Up @@ -75,65 +80,132 @@ public static void cleanDatabase() throws Exception {
MockDatabase.cleanDatabase();
}

@Test
public void shouldCreateNewProcess() throws Exception {
CreateProcessForm underTest = new CreateProcessForm();
underTest.getProcessDataTab().setDocType("Monograph");
Process newProcess = new Process();
Workpiece newWorkPiece = new Workpiece();
TempProcess tempProcess = new TempProcess(newProcess, newWorkPiece);
underTest.setProcesses(new LinkedList<>(Collections.singletonList(tempProcess)));
underTest.getMainProcess().setProject(ServiceManager.getProjectService().getById(1));
underTest.getMainProcess().setRuleset(ServiceManager.getRulesetService().getById(1));
underTest.getMainProcess().setTitle("title");
@AfterEach
public void cleanUpAfterEach() throws Exception {
if (createdProcess != null && createdProcess.getId() != null) {
processService.remove(createdProcess.getId());
fileService.delete(URI.create(createdProcess.getId().toString()));
}
createdProcess = null;
setScriptPermissions(false);
}

// Helper to create and initialize a CreateProcessForm with common properties
private CreateProcessForm setupCreateProcessForm(String docType) throws Exception {
CreateProcessForm form = new CreateProcessForm();
form.getProcessDataTab().setDocType(docType);

Process process = new Process();
Workpiece workPiece = new Workpiece();
TempProcess tempProcess = new TempProcess(process, workPiece);
form.setProcesses(new LinkedList<>(Collections.singletonList(tempProcess)));

form.getMainProcess().setProject(ServiceManager.getProjectService().getById(1));
form.getMainProcess().setRuleset(ServiceManager.getRulesetService().getById(1));

form.updateRulesetAndDocType(tempProcess.getProcess().getRuleset());
return form;
}

// Helper to manage script permissions
private void setScriptPermissions(boolean enable) throws Exception {
File script = new File(ConfigCore.getParameter(ParameterCore.SCRIPT_CREATE_DIR_META));
if (!SystemUtils.IS_OS_WINDOWS) {
ExecutionPermission.setExecutePermission(script);
if (enable) {
ExecutionPermission.setExecutePermission(script);
} else {
ExecutionPermission.setNoExecutePermission(script);
}
}
}

@Test
public void shouldCreateNewProcess() throws Exception {
CreateProcessForm underTest = setupCreateProcessForm("Monograph");
underTest.getMainProcess().setTitle("title");
setScriptPermissions(true);
long before = processService.count();
underTest.createNewProcess();
if (!SystemUtils.IS_OS_WINDOWS) {
ExecutionPermission.setNoExecutePermission(script);
}
setScriptPermissions(false);

long after = processService.count();
createdProcess = underTest.getMainProcess();
assertEquals(before + 1, after, "No process was created!");

// clean up database, index and file system
Integer processId = newProcess.getId();
processService.remove(processId);
fileService.delete(URI.create(processId.toString()));
}

/**
* tests creation of processes without workflow.
*/
@Test
public void shouldCreateNewProcessWithoutWorkflow() throws Exception {
CreateProcessForm underTest = new CreateProcessForm();
underTest.getProcessDataTab().setDocType("MultiVolumeWork");
Process newProcess = new Process();
Workpiece newWorkPiece = new Workpiece();
TempProcess tempProcess = new TempProcess(newProcess, newWorkPiece);
underTest.setProcesses(new LinkedList<>(Collections.singletonList(tempProcess)));
underTest.getMainProcess().setProject(ServiceManager.getProjectService().getById(1));
underTest.getMainProcess().setRuleset(ServiceManager.getRulesetService().getById(1));
CreateProcessForm underTest = setupCreateProcessForm("MultiVolumeWork");
underTest.getMainProcess().setTitle("title");

File script = new File(ConfigCore.getParameter(ParameterCore.SCRIPT_CREATE_DIR_META));
ExecutionPermission.setExecutePermission(script);
setScriptPermissions(true);
long before = processService.count();
underTest.createNewProcess();
ExecutionPermission.setNoExecutePermission(script);
setScriptPermissions(false);
long after = processService.count();
createdProcess = underTest.getMainProcess();
assertEquals(before + 1, after, "No process was created!");
assertTrue(underTest.getMainProcess().getTasks().isEmpty(), "Process should not have tasks");
assertNull(underTest.getMainProcess().getSortHelperStatus(), "Process should not have sortHelperStatus");
}

@Test
public void shouldThrowExceptionForInvalidTitle() throws Exception {
// Attempt to create a process with an invalid title
CreateProcessForm underTest = setupCreateProcessForm("Monograph");
underTest.getMainProcess().setTitle("title with whitespaces");
long before = processService.count();
assertThrows(ProcessGenerationException.class, underTest::createProcessHierarchy,
"Expected a ProcessGenerationException to be thrown for an invalid title, but it was not.");
long after = processService.count();
// Ensure no process was created
assertEquals(before, after, "A process with an invalid title was created!");
}


@Test
public void shouldNotAllowDuplicateProcessTitles() throws Exception {
assertDuplicateTitleNotAllowed(1, 1, false);
}

@Test
public void shouldNotAllowProcessTitlesInProjectsTheUserDoesNotBelongTo() throws Exception {
assertDuplicateTitleNotAllowed(2, 1, true);
}

assertTrue(newProcess.getTasks().isEmpty(), "Process should not have tasks");
assertNull(newProcess.getSortHelperStatus(), "process should not have sortHelperStatus");
private void assertDuplicateTitleNotAllowed(int firstProjectId, int secondProjectId, boolean switchUserContext) throws Exception {
// First process creation
CreateProcessForm underTest = setupCreateProcessForm("Monograph");
underTest.getMainProcess().setTitle("title");
underTest.getMainProcess().setProject(ServiceManager.getProjectService().getById(firstProjectId));

setScriptPermissions(true);
long before = processService.count();
underTest.createProcessHierarchy();
setScriptPermissions(false);

// clean up database, index and file system
Integer processId = newProcess.getId();
processService.remove(processId);
fileService.delete(URI.create(processId.toString()));
long after = processService.count();
createdProcess = underTest.getMainProcess();
assertEquals(before + 1, after, "First process creation failed. No process was created!");

// Switch user context to check with a user which does not has access to project 2
if (switchUserContext) {
User userTwo = ServiceManager.getUserService().getById(2);
SecurityTestUtils.addUserDataToSecurityContext(userTwo, 1);
// Assert that the user 2 is NOT associated with project 2
assertFalse(ServiceManager.getProjectService()
.getById(firstProjectId)
.getUsers()
.contains(userTwo), "User 2 should not have access to project 2");
}
// Second process creation with duplicate title
CreateProcessForm underTestTwo = setupCreateProcessForm("Monograph");
underTestTwo.getMainProcess().setTitle("title");
underTestTwo.getMainProcess().setProject(ServiceManager.getProjectService().getById(secondProjectId));

long beforeDuplicate = processService.count();
assertThrows(ProcessGenerationException.class, underTestTwo::createProcessHierarchy,
"Expected a ProcessGenerationException to be thrown for duplicate title, but it was not.");
long afterDuplicate = processService.count();
assertEquals(beforeDuplicate, afterDuplicate, "A duplicate process with the same title was created!");
}
}

0 comments on commit 9185945

Please sign in to comment.