Skip to content
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.

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

[3.8] Fix NPE when importing newspaper course with metadata #6393

Open
wants to merge 1 commit into
base: 3.8.x
Choose a base branch
from

Conversation

matthias-ronge
Copy link
Collaborator

Backported #6390 to 3.8.x branch

Copy link
Collaborator

@henning-gerhardt henning-gerhardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Short note: This changes are difficult to review as there are no tests added nor I have some example data to test this changes with, so I need more time for reviewing this.

@solth solth changed the title [3.8.x] Fix NPE when importing newspaper course with metadata [3.8] Fix NPE when importing newspaper course with metadata Feb 6, 2025
@henning-gerhardt
Copy link
Collaborator

A team mate created a small newspaper course xml file for me which contains some meta data:

<?xml version="1.0" encoding="UTF-8"?>
<course>
   <description>Die Zeitung erschien vom 1. Februar 1757 bis zum 07. Februar 1757 regelmäßig an allen Montagen und Mittwochen.</description>
   <processes>
      <process>
         <title index="1">
            <appeared date="1757-02-02">
               <metadata metadataType="slub_script" value="Fraktur"/>
               <metadata metadataType="LegalNoteAndTermsOfUse" value="PDM1.0"/>
               <metadata metadataType="DocLanguage" value="ger"/>
               <metadata metadataType="slub_ownerDigi" value="SLUB Dresden"/>
            </appeared>
         </title>
      </process>
      <process>
         <title index="1">
            <appeared date="1757-02-07"/>
         </title>
      </process>
   </processes>
</course>

After uploading this file I got the following stack trace

java.lang.ClassCastException: class org.kitodo.production.forms.createprocess.ProcessSelectMetadata cannot be cast to class org.kitodo.production.forms.createprocess.ProcessTextMetadata (org.kitodo.production.forms.createprocess.ProcessSelectMetadata and org.kitodo.production.forms.createprocess.ProcessTextMetadata are in unnamed module of loader org.apache.catalina.loader.ParallelWebappClassLoader @1b6eb4d)
	at org.kitodo.production.model.bibliography.course.metadata.CountableMetadata.setStartValue(CountableMetadata.java:305)
	at org.kitodo.production.model.bibliography.course.Course.processRecoveredMetadata(Course.java:396)
	at org.kitodo.production.model.bibliography.course.Course.<init>(Course.java:315)
	at org.kitodo.production.forms.CalendarForm.upload(CalendarForm.java:625)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.apache.el.parser.AstValue.invoke(AstValue.java:252)
	at org.apache.el.MethodExpressionImpl.invoke(MethodExpressionImpl.java:266)
	....

While reading the code I found in the new introduced code a potential null pointer exception which I only see after the test execution was minimal adjusted (second parameter was set to null). I will comment this in the code change.

@@ -382,6 +387,12 @@ private void processRecoveredMetadata(List<RecoveredMetadata> recoveredMetadata)
}
CountableMetadata metadata = new CountableMetadata(foundBlock, Pair.of(metaDatum.getDate(), foundIssue));
metadata.setMetadataType(metaDatum.getMetadataType());
ProcessSimpleMetadata processSimpleMetadata = possibleProcessDetails.get(metaDatum.getMetadataType());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential null pointer exception if possibleProcessDetails ist set to null like in the test execution (see CourseTest.java line 34). The test code execution did not cover this peace of code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants