Skip to content

Fix error with empty workflow svg and error when saving empty workflow #6408

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
BartChris wants to merge 4 commits into from
Closed

Fix error with empty workflow svg and error when saving empty workflow #6408

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7974c68
Fix error with empty workflows
BartChris Feb 7, 2025
282f5fe
Further adjustments
BartChris Feb 7, 2025
a2d82c3
Ensure we always save an SVG file
BartChris Feb 10, 2025
cbdf11e
Add integration test for duplicate
BartChris Feb 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
147 changes: 113 additions & 34 deletions Kitodo/src/main/java/org/kitodo/production/forms/WorkflowForm.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,12 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.stream.Collectors;

import javax.enterprise.context.SessionScoped;
import javax.faces.context.ExternalContext;
import javax.faces.context.FacesContext;
import javax.faces.model.SelectItem;
import javax.faces.view.ViewScoped;
import javax.inject.Named;

import org.apache.commons.io.IOUtils;
Expand Down Expand Up @@ -59,7 +61,7 @@
import org.kitodo.production.workflow.model.Reader;

@Named("WorkflowForm")
@SessionScoped
@ViewScoped
public class WorkflowForm extends BaseForm {

private static final Logger logger = LogManager.getLogger(WorkflowForm.class);
Expand All @@ -75,16 +77,21 @@ public class WorkflowForm extends BaseForm {
private static final String SVG_EXTENSION = ".svg";
private static final String SVG_DIAGRAM_URI = "svgDiagramURI";
private static final String XML_DIAGRAM_URI = "xmlDiagramURI";
private Map<String, URI> activeDiagramsUris = new HashMap<>();
private String activeWorkflowTitle;
private final String workflowEditPath = MessageFormat.format(REDIRECT_PATH, "workflowEdit");
private Integer roleId;
private boolean migration;
private static final String MIGRATION_FORM_PATH = MessageFormat.format(REDIRECT_PATH,"system");
private ExternalContext externalContext;

/**
* Constructor.
*/
public WorkflowForm() {
super.setLazyDTOModel(new LazyDTOModel(ServiceManager.getWorkflowService()));
this.externalContext = FacesContext.getCurrentInstance() != null
? FacesContext.getCurrentInstance().getExternalContext() : null;
}

/**
Expand Down Expand Up @@ -114,25 +121,29 @@ public void setWorkflowStatus(WorkflowStatus workflowStatus) {
this.workflowStatus = workflowStatus;
}

public void setExternalContext(ExternalContext externalContext) {
this.externalContext = externalContext; // Allow test injection
}

/**
* Read XML for file chosen out of the select list.
*/
public void readXMLDiagram() {
URI xmlDiagramURI = new File(
ConfigCore.getKitodoDiagramDirectory() + encodeXMLDiagramName(this.workflow.getTitle())).toURI();
xmlDiagram = readFileContent(xmlDiagramURI);
}

try (InputStream inputStream = fileService.read(xmlDiagramURI);
BufferedReader bufferedReader = new BufferedReader(new InputStreamReader(inputStream))) {
StringBuilder sb = new StringBuilder();
String line = bufferedReader.readLine();
while (Objects.nonNull(line)) {
sb.append(line).append("\n");
line = bufferedReader.readLine();
private String readFileContent(URI fileUri) {
if (fileService.fileExist(fileUri)) {
try (InputStream inputStream = fileService.read(fileUri);
BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream))) {
return reader.lines().collect(Collectors.joining("\n"));
} catch (IOException e) {
Helper.setErrorMessage(e.getLocalizedMessage(), logger, e);
}
xmlDiagram = sb.toString();
} catch (IOException e) {
Helper.setErrorMessage(e.getLocalizedMessage(), logger, e);
}
return "";
}

/**
Expand All @@ -152,6 +163,11 @@ public String saveAndRedirect() {
if (!this.workflow.getTemplates().isEmpty()) {
updateTemplateTasks();
}
if (Objects.nonNull(activeWorkflowTitle)
&& !activeWorkflowTitle.equals(this.workflow.getTitle())) {
deleteOldWorkflowFiles(activeWorkflowTitle);
activeWorkflowTitle = this.workflow.getTitle();
}
if (migration) {
migration = false;
return MIGRATION_FORM_PATH + "&workflowId=" + workflow.getId();
Expand Down Expand Up @@ -245,20 +261,33 @@ public void delete() {
} else {
try {
ServiceManager.getWorkflowService().remove(this.workflow);
deleteOldWorkflowFiles(this.workflow.getTitle());

String diagramDirectory = ConfigCore.getKitodoDiagramDirectory();
URI svgDiagramURI = new File(
diagramDirectory + decodeXMLDiagramName(this.workflow.getTitle()) + SVG_EXTENSION).toURI();
URI xmlDiagramURI = new File(diagramDirectory + encodeXMLDiagramName(this.workflow.getTitle()))
.toURI();
} catch (DataException e) {
Helper.setErrorMessage(ERROR_DELETING, new Object[] {ObjectType.WORKFLOW.getTranslationSingular() },
logger, e);
}
}
}

private void deleteOldWorkflowFiles(String oldDiagramTitle) {
try {
String diagramDirectory = ConfigCore.getKitodoDiagramDirectory();
URI svgDiagramURI = new File(
diagramDirectory + decodeXMLDiagramName(oldDiagramTitle) + SVG_EXTENSION).toURI();
URI xmlDiagramURI = new File(diagramDirectory + encodeXMLDiagramName(oldDiagramTitle))
.toURI();
if (fileService.fileExist(svgDiagramURI)) {
fileService.delete(svgDiagramURI);
}
if (fileService.fileExist(xmlDiagramURI)) {
fileService.delete(xmlDiagramURI);
} catch (DataException | IOException e) {
Helper.setErrorMessage(ERROR_DELETING, new Object[] {ObjectType.WORKFLOW.getTranslationSingular() },
logger, e);
}
} catch (IOException e) {
Helper.setErrorMessage(ERROR_DELETING, new Object[] {ObjectType.WORKFLOW.getTranslationSingular() },
logger, e);
}

}

/**
Expand All @@ -277,19 +306,39 @@ private boolean saveFiles() throws IOException, WorkflowException {

xmlDiagram = requestParameterMap.get("editForm:workflowTabView:xmlDiagram");
if (Objects.nonNull(xmlDiagram)) {
svgDiagram = StringUtils.substringAfter(xmlDiagram, "kitodo-diagram-separator");
xmlDiagram = StringUtils.substringBefore(xmlDiagram, "kitodo-diagram-separator");
if (xmlDiagram.contains("kitodo-diagram-separator")) {
svgDiagram = StringUtils.substringAfter(xmlDiagram, "kitodo-diagram-separator");
xmlDiagram = StringUtils.substringBefore(xmlDiagram, "kitodo-diagram-separator");
}
if (xmlDiagram.isEmpty()) {
Helper.setErrorMessage("emptyWorkflow");
return false;
}
activeDiagramsUris.put(XML_DIAGRAM_URI, xmlDiagramURI);


Reader reader = new Reader(new ByteArrayInputStream(xmlDiagram.getBytes(StandardCharsets.UTF_8)));
reader.validateWorkflowTasks();

Converter converter = new Converter(new ByteArrayInputStream(xmlDiagram.getBytes(StandardCharsets.UTF_8)));
converter.validateWorkflowTaskList();

saveFile(svgDiagramURI, svgDiagram);
if (Objects.nonNull(svgDiagram)) {
saveFile(svgDiagramURI, svgDiagram);
activeDiagramsUris.put(SVG_DIAGRAM_URI, svgDiagramURI);
} else {
if (fileService.fileExist(activeDiagramsUris.get(SVG_DIAGRAM_URI))) {
try (InputStream svgInputStream = ServiceManager.getFileService().read(activeDiagramsUris.get(SVG_DIAGRAM_URI))) {
svgDiagram = IOUtils.toString(svgInputStream, StandardCharsets.UTF_8);
saveFile(svgDiagramURI, svgDiagram);
}
} else {
saveFile(svgDiagramURI, "");
}
activeDiagramsUris.put(SVG_DIAGRAM_URI, svgDiagramURI);
}
saveFile(xmlDiagramURI, xmlDiagram);
}

return fileService.fileExist(xmlDiagramURI) && fileService.fileExist(svgDiagramURI);
}

Expand Down Expand Up @@ -364,22 +413,28 @@ public String duplicate(Integer itemId) {
Map<String, URI> diagramsUris = getDiagramUris(baseWorkflow.getTitle());

URI xmlDiagramURI = diagramsUris.get(XML_DIAGRAM_URI);
URI svgDiagramURI = diagramsUris.get(SVG_DIAGRAM_URI);

this.workflow = ServiceManager.getWorkflowService().duplicateWorkflow(baseWorkflow);
setWorkflowStatus(WorkflowStatus.DRAFT);
Map<String, URI> diagramsCopyUris = getDiagramUris();

URI xmlDiagramCopyURI = diagramsCopyUris.get(XML_DIAGRAM_URI);

// Read XML diagram
try (InputStream xmlInputStream = ServiceManager.getFileService().read(xmlDiagramURI)) {
this.xmlDiagram = IOUtils.toString(xmlInputStream, StandardCharsets.UTF_8);
saveFile(xmlDiagramCopyURI, this.xmlDiagram);
} catch (IOException e) {
Helper.setErrorMessage("unableToDuplicateWorkflow", logger, e);
return this.stayOnCurrentPage;
}
return workflowEditPath;
} catch (DAOException e) {
// Read SVG diagram (use a separate input stream)
try (InputStream svgInputStream = ServiceManager.getFileService().read(svgDiagramURI)) {
this.svgDiagram = IOUtils.toString(svgInputStream, StandardCharsets.UTF_8);
}
// Store duplicated workflow in Flash scope
storeInFlashScope("duplicatedWorkflow", this.workflow);
storeInFlashScope("xmlDiagram", this.xmlDiagram);
storeInFlashScope("svgDiagram", this.svgDiagram);

return workflowEditPath + "&id=0";


} catch (IOException | DAOException e) {
Helper.setErrorMessage(ERROR_DUPLICATE, new Object[] {ObjectType.WORKFLOW.getTranslationSingular() },
logger, e);
return this.stayOnCurrentPage;
Expand Down Expand Up @@ -411,12 +466,30 @@ public void setWorkflowById(int id) {
*/
public void load(int id) {
try {
if (!Objects.equals(id, 0)) {
if (id > 0) {
// Normal case: Load workflow from database
Workflow workflow = ServiceManager.getWorkflowService().getById(id);
setWorkflow(workflow);
setWorkflowStatus(workflow.getStatus());
activeDiagramsUris = getDiagramUris(workflow.getTitle());
activeWorkflowTitle = workflow.getTitle();
readXMLDiagram();
this.dataEditorSettingsDefined = this.dataEditorSettingService.areDataEditorSettingsDefinedForWorkflow(workflow);
} else {
// Check if duplicated workflow is stored in Flash scope
ExternalContext externalContext = FacesContext.getCurrentInstance().getExternalContext();
Map<String, Object> flash = externalContext.getFlash();

if (flash.containsKey("duplicatedWorkflow")) {
this.workflow = (Workflow) flash.get("duplicatedWorkflow");
this.xmlDiagram = (String) flash.get("xmlDiagram");
this.svgDiagram = (String) flash.get("svgDiagram");
setWorkflowStatus(workflow.getStatus());
this.dataEditorSettingsDefined = this.dataEditorSettingService.areDataEditorSettingsDefinedForWorkflow(workflow);
}
if (this.workflow.getClient() == null) {
this.workflow.setClient(ServiceManager.getUserService().getSessionClientOfAuthenticatedUser());
}
}
setSaveDisabled(false);
} catch (DAOException e) {
Expand All @@ -425,6 +498,12 @@ public void load(int id) {
}
}

private void storeInFlashScope(String key, Object value) {
if (externalContext != null) {
externalContext.getFlash().put(key, value);
}
}

/**
* Get role id.
*
Expand Down
1 change: 1 addition & 0 deletions Kitodo/src/main/resources/messages/errors_de.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ docTypeNotFound=docType ''{0}'' wurde nicht im selektierten Regelsatz gefunden
duplicatedTitles=Es wurden dublette Ausgabe-Bezeichnungen gefunden. Dies erzeugt gegebenenfalls dublette Vorgangstitel!
duplicateWorkflowTitle=Ein Workflow mit dem Titel "{0}" existiert bereits.
emptySourceFolder=Der Quellordner zur Bildgenerierung ist leer
emptyWorkflow=Der Workflow ist leer und kann nicht gespeichert werden
errorDataIncomplete=Unvollst\u00E4ndige Daten\:
errorDatabaseReading=Fehler beim Datenbanklesen von ''{0}'' with ID {1}.
errorDeleting=Fehler beim L\u00F6schen von ''{0}''
Expand Down
1 change: 1 addition & 0 deletions Kitodo/src/main/resources/messages/errors_en.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ docTypeNotFound=docType ''{0}'' could not be found in selected ruleset
duplicatedTitles=Duplicate issue designations were found. This may produce duplicate process titles!
duplicateWorkflowTitle=Workflow with title "{0}" already exists.
emptySourceFolder=The source folder is empty
emptyWorkflow=The workflow is empty and cannot be saved
errorDataIncomplete=Incomplete data\:
errorDatabaseReading=Error on reading database for ''{0}'' with ID {1}.
errorDeleting=Error deleting ''{0}''
Expand Down
1 change: 1 addition & 0 deletions Kitodo/src/main/resources/messages/errors_es.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ docketTitleDuplicated=Ya existe una hoja de ruta con el mismo título.
docTypeNotFound=docType ''{0}'' o se encontró en el conjunto de reglas seleccionado
duplicatedTitles=Se han encontrado títulos de salida duplicados. ¡Esto puede crear títulos de proceso duplicados!
duplicateWorkflowTitle=El flujo de trabajo con el título "{0}" ya existe.
emptyWorkflow=El flujo de trabajo est� vac�o y no se puede guardar
emptySourceFolder=La carpeta de origen para la generación de imágenes está vacía
errorDataIncomplete=Datos incompletos\:
errorDatabaseReading=Error al leer ''{0}'' con ID {1} de la base de datos.
Expand Down
61 changes: 57 additions & 4 deletions Kitodo/src/test/java/org/kitodo/production/forms/WorkflowFormIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,19 @@

package org.kitodo.production.forms;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.io.File;
import java.io.IOException;
import java.net.URI;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.List;

import org.apache.commons.io.FileUtils;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.kitodo.MockDatabase;
import org.kitodo.config.ConfigCore;
import org.kitodo.data.database.beans.DataEditorSetting;
import org.kitodo.data.database.beans.Task;
import org.kitodo.data.database.beans.Template;
Expand All @@ -35,12 +36,25 @@
import org.kitodo.production.services.ServiceManager;
import org.kitodo.production.services.data.DataEditorSettingService;
import org.kitodo.production.services.data.TaskService;
import org.mockito.ArgumentCaptor;

import javax.faces.context.ExternalContext;
import javax.faces.context.FacesContext;
import javax.faces.context.Flash;

import static org.mockito.Mockito.*;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

public class WorkflowFormIT {

private WorkflowForm currentWorkflowForm = new WorkflowForm();
private static final TaskService taskService = ServiceManager.getTaskService();
private static final DataEditorSettingService dataEditorSettingService = ServiceManager.getDataEditorSettingService();
private ExternalContext mockExternalContext;
private Flash mockFlash;

/**
* Setup Database and start elasticsearch.
Expand Down Expand Up @@ -122,6 +136,45 @@ public void shouldUpdateTemplateTasksAndDeleteOnlyAffectedDataEditorSettings() t
assertEquals(0.6f, dataEditorSettingForTaskOfSecondTemplate.get(0).getGalleryWidth(), 0);
}

@Test
public void shouldDuplicateWorkflowAndStoreInFlash() throws Exception {
// Mock ExternalContext and Flash
mockExternalContext = mock(ExternalContext.class);
mockFlash = mock(Flash.class);
currentWorkflowForm.setExternalContext(mockExternalContext);

when(mockExternalContext.getFlash()).thenReturn(mockFlash); // Ensure Flash scope is returned

// Call the duplicate method
String resultUrl = currentWorkflowForm.duplicate(1);

// Ensure redirection to the edit page
assertTrue(resultUrl.contains("&id=0"));

// Capture arguments that were passed to Flash
ArgumentCaptor<Workflow> workflowCaptor = ArgumentCaptor.forClass(Workflow.class);
ArgumentCaptor<String> xmlCaptor = ArgumentCaptor.forClass(String.class);
ArgumentCaptor<String> svgCaptor = ArgumentCaptor.forClass(String.class);

// Verify and capture actual method calls
verify(mockFlash, times(1)).put(eq("duplicatedWorkflow"), workflowCaptor.capture());
verify(mockFlash, times(1)).put(eq("xmlDiagram"), xmlCaptor.capture());
verify(mockFlash, times(1)).put(eq("svgDiagram"), svgCaptor.capture());

// Assert the captured workflow is not null and has expected values
Workflow duplicatedWorkflow = workflowCaptor.getValue();
String xmlWorkflow = xmlCaptor.getValue();
assertNotNull(duplicatedWorkflow, "Duplicated workflow should not be null");
assertEquals("gateway-test1", duplicatedWorkflow.getTitle().replaceFirst("_[^_]*$", ""),
"Expected duplicated workflow title");

String diagramPath = ConfigCore.getKitodoDiagramDirectory() + "gateway-test1" + ".bpmn20.xml";
String data = FileUtils.readFileToString(new File(diagramPath), StandardCharsets.UTF_8);
assertEquals(data, xmlWorkflow, "Expected duplicated workflow title");
assertNotNull(xmlCaptor.getValue(), "XML Diagram should be stored");
assertNotNull(svgCaptor.getValue(), "SVG Diagram should be stored");
}

private Task createAndSaveTemplateTask(TaskStatus taskStatus, int ordering, Template template) throws DataException {
Task task = new Task();
task.setProcessingStatus(taskStatus);
Expand Down