From 52d309238cdd60e8e7caf55c1dbfec6abbe2fa70 Mon Sep 17 00:00:00 2001 From: Matthias Ronge Date: Fri, 31 Jan 2025 11:44:16 +0100 Subject: [PATCH 1/2] Fix NPE when importing newspaper course with metadata --- .../kitodo/production/forms/CalendarForm.java | 12 ++++++++++-- .../createprocess/ProcessBooleanMetadata.java | 2 +- .../createprocess/ProcessDateMetadata.java | 2 +- .../createprocess/ProcessSelectMetadata.java | 2 +- .../createprocess/ProcessSimpleMetadata.java | 4 ++-- .../createprocess/ProcessTextMetadata.java | 2 +- .../model/bibliography/course/Course.java | 17 ++++++++++++++--- .../model/bibliography/course/CourseTest.java | 2 +- 8 files changed, 31 insertions(+), 12 deletions(-) diff --git a/Kitodo/src/main/java/org/kitodo/production/forms/CalendarForm.java b/Kitodo/src/main/java/org/kitodo/production/forms/CalendarForm.java index 5cb3b7b98a9..730c854e6f2 100644 --- a/Kitodo/src/main/java/org/kitodo/production/forms/CalendarForm.java +++ b/Kitodo/src/main/java/org/kitodo/production/forms/CalendarForm.java @@ -28,6 +28,8 @@ import java.util.Map; import java.util.NoSuchElementException; import java.util.Objects; +import java.util.function.Function; +import java.util.stream.Collectors; import javax.faces.context.FacesContext; import javax.faces.view.ViewScoped; @@ -48,6 +50,7 @@ import org.kitodo.exceptions.DoctypeMissingException; import org.kitodo.exceptions.ProcessGenerationException; import org.kitodo.production.forms.createprocess.ProcessDetail; +import org.kitodo.production.forms.createprocess.ProcessSimpleMetadata; import org.kitodo.production.forms.createprocess.ProcessTextMetadata; import org.kitodo.production.helper.Helper; import org.kitodo.production.helper.XMLUtils; @@ -614,12 +617,17 @@ public void upload() { return; } Document xml = XMLUtils.load(uploadedFile.getInputStream()); - course = new Course(xml); + List processDetails = CalendarService + .getAddableMetadataTable(ServiceManager.getProcessService().getById(parentId)); + Map processDetailsByMetadataID = processDetails.stream() + .filter(ProcessSimpleMetadata.class::isInstance).map(ProcessSimpleMetadata.class::cast) + .collect(Collectors.toMap(ProcessDetail::getMetadataID, Function.identity())); + course = new Course(xml, processDetailsByMetadataID); Helper.removeManagedBean("GranularityForm"); navigate(course.get(0)); } catch (SAXException e) { Helper.setErrorMessage(UPLOAD_ERROR, "errorSAXException", logger, e); - } catch (IOException e) { + } catch (IOException | DataException | DAOException e) { Helper.setErrorMessage(UPLOAD_ERROR, e.getLocalizedMessage(), logger, e); } catch (IllegalArgumentException e) { Helper.setErrorMessage("calendar.upload.overlappingDateRanges", logger, e); diff --git a/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessBooleanMetadata.java b/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessBooleanMetadata.java index 342aa095643..2e04d5fc4d5 100644 --- a/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessBooleanMetadata.java +++ b/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessBooleanMetadata.java @@ -57,7 +57,7 @@ private ProcessBooleanMetadata(ProcessBooleanMetadata template) { } @Override - ProcessBooleanMetadata getClone() { + public ProcessBooleanMetadata getClone() { return new ProcessBooleanMetadata(this); } diff --git a/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessDateMetadata.java b/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessDateMetadata.java index a8486b35a65..460ea1d56aa 100644 --- a/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessDateMetadata.java +++ b/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessDateMetadata.java @@ -41,7 +41,7 @@ private ProcessDateMetadata(ProcessDateMetadata template) { } @Override - ProcessTextMetadata getClone() { + public ProcessTextMetadata getClone() { return new ProcessDateMetadata(this); } diff --git a/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessSelectMetadata.java b/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessSelectMetadata.java index bc326244824..13cdb20b0aa 100644 --- a/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessSelectMetadata.java +++ b/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessSelectMetadata.java @@ -87,7 +87,7 @@ private ProcessSelectMetadata(ProcessSelectMetadata template) { } @Override - ProcessSelectMetadata getClone() { + public ProcessSelectMetadata getClone() { return new ProcessSelectMetadata(this); } diff --git a/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessSimpleMetadata.java b/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessSimpleMetadata.java index 1b29dbefc90..d69fce47cfe 100644 --- a/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessSimpleMetadata.java +++ b/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessSimpleMetadata.java @@ -23,7 +23,7 @@ import org.kitodo.api.dataformat.LogicalDivision; import org.kitodo.api.dataformat.PhysicalDivision; -abstract class ProcessSimpleMetadata extends ProcessDetail implements Serializable { +public abstract class ProcessSimpleMetadata extends ProcessDetail implements Serializable { static final List>> PARENT_CLASSES = Arrays.asList(LogicalDivision.class, PhysicalDivision.class); @@ -50,7 +50,7 @@ protected ProcessSimpleMetadata(ProcessFieldedMetadata container, SimpleMetadata * * @return an independently mutable copy */ - abstract ProcessSimpleMetadata getClone(); + public abstract ProcessSimpleMetadata getClone(); /** * Returns a simpler string representation of the Metadata. diff --git a/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessTextMetadata.java b/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessTextMetadata.java index b0218e17845..5b4ba1e92f9 100644 --- a/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessTextMetadata.java +++ b/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessTextMetadata.java @@ -48,7 +48,7 @@ private String addLeadingZeros(String value) { } @Override - ProcessTextMetadata getClone() { + public ProcessTextMetadata getClone() { return new ProcessTextMetadata(this); } diff --git a/Kitodo/src/main/java/org/kitodo/production/model/bibliography/course/Course.java b/Kitodo/src/main/java/org/kitodo/production/model/bibliography/course/Course.java index 6bab6583e4c..ebd510f03fa 100644 --- a/Kitodo/src/main/java/org/kitodo/production/model/bibliography/course/Course.java +++ b/Kitodo/src/main/java/org/kitodo/production/model/bibliography/course/Course.java @@ -34,6 +34,8 @@ import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.tuple.Pair; +import org.kitodo.exceptions.MetadataException; +import org.kitodo.production.forms.createprocess.ProcessSimpleMetadata; import org.kitodo.production.helper.XMLUtils; import org.kitodo.production.model.bibliography.course.metadata.CountableMetadata; import org.kitodo.production.model.bibliography.course.metadata.RecoveredMetadata; @@ -279,6 +281,8 @@ public Course() { * * @param xml * XML document data structure + * @param possibleProcessDetails + * possible process details from the process creation form * @throws NoSuchElementException * if ELEMENT_COURSE or ELEMENT_PROCESSES cannot be found * @throws IllegalArgumentException @@ -286,7 +290,7 @@ public Course() { * @throws NullPointerException * if a mandatory element is absent */ - public Course(Document xml) { + public Course(Document xml, Map possibleProcessDetails) { super(); processesAreVolatile = false; Element rootNode = XMLUtils.getFirstChildWithTagName(xml, ELEMENT_COURSE); @@ -308,7 +312,7 @@ public Course(Document xml) { } initialCapacity = processProcessNode(initialCapacity, lastIssueForDate, recoveredMetadata, processNode); } - processRecoveredMetadata(recoveredMetadata); + processRecoveredMetadata(recoveredMetadata, possibleProcessDetails); recalculateRegularityOfIssues(); processesAreVolatile = true; } @@ -362,7 +366,8 @@ private void processBlockNode(Map lastIssueForDate, } } - private void processRecoveredMetadata(List recoveredMetadata) { + private void processRecoveredMetadata(List recoveredMetadata, + Map possibleProcessDetails) { Map, CountableMetadata> last = new HashMap<>(); for (RecoveredMetadata metaDatum : recoveredMetadata) { Block foundBlock = null; @@ -382,6 +387,12 @@ private void processRecoveredMetadata(List recoveredMetadata) } CountableMetadata metadata = new CountableMetadata(foundBlock, Pair.of(metaDatum.getDate(), foundIssue)); metadata.setMetadataType(metaDatum.getMetadataType()); + ProcessSimpleMetadata processSimpleMetadata = possibleProcessDetails.get(metaDatum.getMetadataType()); + if (Objects.isNull(processSimpleMetadata)) { + throw new MetadataException("Cannot add metadata key " + metaDatum.getMetadataType(), + new NoSuchElementException(metaDatum.getMetadataType())); + } + metadata.setMetadataDetail(processSimpleMetadata.getClone()); metadata.setStartValue(metaDatum.getValue()); metadata.setStepSize(metaDatum.getStepSize()); foundBlock.addMetadata(metadata); diff --git a/Kitodo/src/test/java/org/kitodo/production/model/bibliography/course/CourseTest.java b/Kitodo/src/test/java/org/kitodo/production/model/bibliography/course/CourseTest.java index 4efe59f686e..91a9d83ce9c 100644 --- a/Kitodo/src/test/java/org/kitodo/production/model/bibliography/course/CourseTest.java +++ b/Kitodo/src/test/java/org/kitodo/production/model/bibliography/course/CourseTest.java @@ -31,7 +31,7 @@ public class CourseTest { public void testCloneMethod() throws IOException, ParserConfigurationException, SAXException { String xmlString = new String(Files.readAllBytes(new File("src/test/resources/newspaper-course.xml").toPath())); Document xmlDocument = XMLUtils.parseXMLString(xmlString); - Course course = new Course(xmlDocument); + Course course = new Course(xmlDocument, null); // assert / check some data from the xml file assertEquals(23L, course.getIndividualIssues().size()); From dc909c472c6ed5c2d0da691064dd343908003588 Mon Sep 17 00:00:00 2001 From: Matthias Ronge Date: Thu, 13 Feb 2025 16:09:28 +0100 Subject: [PATCH 2/2] Fix issues found in review --- .../forms/createprocess/ProcessBooleanMetadata.java | 5 +++++ .../forms/createprocess/ProcessSelectMetadata.java | 12 ++++++++++++ .../forms/createprocess/ProcessSimpleMetadata.java | 8 ++++++++ .../forms/createprocess/ProcessTextMetadata.java | 7 +------ .../course/metadata/CountableMetadata.java | 4 ++-- .../model/bibliography/course/CourseTest.java | 3 ++- 6 files changed, 30 insertions(+), 9 deletions(-) diff --git a/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessBooleanMetadata.java b/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessBooleanMetadata.java index 2e04d5fc4d5..066c82a9fc6 100644 --- a/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessBooleanMetadata.java +++ b/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessBooleanMetadata.java @@ -121,4 +121,9 @@ public boolean isValid() { public void setActive(boolean active) { this.active = active; } + + @Override + public void setValue(String value) { + setActive(StringUtils.isNotBlank(value)); + } } diff --git a/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessSelectMetadata.java b/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessSelectMetadata.java index 13cdb20b0aa..8e990cf79ee 100644 --- a/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessSelectMetadata.java +++ b/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessSelectMetadata.java @@ -19,6 +19,7 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.NoSuchElementException; import java.util.Objects; import javax.faces.model.SelectItem; @@ -31,6 +32,7 @@ import org.kitodo.api.dataeditor.rulesetmanagement.Domain; import org.kitodo.api.dataeditor.rulesetmanagement.SimpleMetadataViewInterface; import org.kitodo.exceptions.InvalidMetadataValueException; +import org.kitodo.exceptions.MetadataException; public class ProcessSelectMetadata extends ProcessSimpleMetadata implements Serializable { private static final Logger logger = LogManager.getLogger(ProcessSelectMetadata.class); @@ -226,4 +228,14 @@ public void setSelectedItem(String selectedItem) { public String getMetadataID() { return settings.getId(); } + + @Override + public void setValue(String value) { + if (!items.parallelStream().anyMatch(selectItem -> Objects.equals(value, selectItem.getValue()))) { + throw new MetadataException( + "Invalid value \"" + value + "\" for select-type metadata \"" + super.label + '"', + new NoSuchElementException(value)); + } + setSelectedItem(value); + } } diff --git a/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessSimpleMetadata.java b/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessSimpleMetadata.java index d69fce47cfe..31ba1cb45f7 100644 --- a/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessSimpleMetadata.java +++ b/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessSimpleMetadata.java @@ -92,4 +92,12 @@ public boolean isRequired() { public int getMinOccurs() { return settings.getMinOccurs(); } + + /** + * Sets the contents of the input field of this process metadata. + * + * @param value + * value to be set + */ + public abstract void setValue(String value); } diff --git a/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessTextMetadata.java b/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessTextMetadata.java index 5b4ba1e92f9..27eeac831d8 100644 --- a/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessTextMetadata.java +++ b/Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessTextMetadata.java @@ -122,12 +122,7 @@ public String getValue() { return value; } - /** - * Sets the contents of the text input field of this process metadata. - * - * @param value - * value to be set - */ + @Override public void setValue(String value) { this.value = addLeadingZeros(value); } diff --git a/Kitodo/src/main/java/org/kitodo/production/model/bibliography/course/metadata/CountableMetadata.java b/Kitodo/src/main/java/org/kitodo/production/model/bibliography/course/metadata/CountableMetadata.java index be3d436bf72..1afea2da0d3 100644 --- a/Kitodo/src/main/java/org/kitodo/production/model/bibliography/course/metadata/CountableMetadata.java +++ b/Kitodo/src/main/java/org/kitodo/production/model/bibliography/course/metadata/CountableMetadata.java @@ -29,7 +29,7 @@ import org.kitodo.data.exceptions.DataException; import org.kitodo.exceptions.InvalidMetadataValueException; import org.kitodo.production.forms.createprocess.ProcessDetail; -import org.kitodo.production.forms.createprocess.ProcessTextMetadata; +import org.kitodo.production.forms.createprocess.ProcessSimpleMetadata; import org.kitodo.production.helper.Helper; import org.kitodo.production.helper.metadata.pagination.Paginator; import org.kitodo.production.model.bibliography.course.Block; @@ -302,7 +302,7 @@ public void setStepSize(Granularity stepSize) { * the start value to set */ public void setStartValue(String startValue) { - ((ProcessTextMetadata) metadataDetail).setValue(startValue); + ((ProcessSimpleMetadata) metadataDetail).setValue(startValue); this.startValue = startValue; } diff --git a/Kitodo/src/test/java/org/kitodo/production/model/bibliography/course/CourseTest.java b/Kitodo/src/test/java/org/kitodo/production/model/bibliography/course/CourseTest.java index 91a9d83ce9c..e155bd7c7c4 100644 --- a/Kitodo/src/test/java/org/kitodo/production/model/bibliography/course/CourseTest.java +++ b/Kitodo/src/test/java/org/kitodo/production/model/bibliography/course/CourseTest.java @@ -17,6 +17,7 @@ import java.io.File; import java.io.IOException; import java.nio.file.Files; +import java.util.Collections; import javax.xml.parsers.ParserConfigurationException; @@ -31,7 +32,7 @@ public class CourseTest { public void testCloneMethod() throws IOException, ParserConfigurationException, SAXException { String xmlString = new String(Files.readAllBytes(new File("src/test/resources/newspaper-course.xml").toPath())); Document xmlDocument = XMLUtils.parseXMLString(xmlString); - Course course = new Course(xmlDocument, null); + Course course = new Course(xmlDocument, Collections.emptyMap()); // assert / check some data from the xml file assertEquals(23L, course.getIndividualIssues().size());