Prevent NPE when creating unknown metadata group and metadata #5603

solth · 2023-03-27T08:41:41Z

This PR fixes #5217 by preventing the NullPointerException that is currently being displayed to the user when the import mapping xslt file creates a metadata group that is undefined in the used ruleset.

BartChris · 2023-03-27T18:50:24Z

This might be about a special scenario but nevertheless: I tried the changes with the scenario described in the linked issue (importing an entry from K10 plus without having all keys defined in the ruleset). The first NPE does not happen anymore. I still get an NPE in this specific scenario because "Person" was not defined in my ruleset and the construction of the ATS does fail, since it identifies a Person key in the data, but not the other values which are required here.

java.lang.NullPointerException
at org.kitodo.production.forms.createprocess.ProcessTextMetadata.getMetadataID(ProcessTextMetadata.java:62)
at org.kitodo.production.services.data.ImportService.getListOfCreators(ImportService.java:827)

Edit: This seem to stem from the specific functionality attached to keys named as "Person", it does not happen if if define a "Person" key in the ruleset, even with different subkeys than the one coming from K10-plus. I have not tested it yet with other datasets yet where the ATS related NPE does not happen.

matthias-ronge

Seems neither wrong nor right, see comments.

matthias-ronge · 2023-04-06T09:53:25Z

Kitodo/src/main/java/org/kitodo/production/forms/createprocess/ProcessTextMetadata.java

@@ -43,7 +43,7 @@ public ProcessTextMetadata(ProcessTextMetadata template) {
    }

    private String addLeadingZeros(String value) {
-        if (Objects.equals(super.settings.getInputType(), InputType.INTEGER)) {
+        if (Objects.nonNull(super.settings) && InputType.INTEGER.equals(super.settings.getInputType())) {


I would like better:

Objects.nonNull(super.settings) && Objects.equals(super.settings.getInputType(), InputType.INTEGER)

Not a requirement, but I think it is better readable. And, Objects.equals() does null-checks.

In this particular case I disagree. The additional Objects.equals is not required and unnecessarily complicates this check, since InputType.INTEGER is not null and therefor we do not need a null check here (calling the equals method on constants or enums when comparing with variables is an encouraged way for avoiding null pointer exceptions). So if you do not mind I would leave this part unchanged.

matthias-ronge · 2023-04-06T09:54:25Z

Kitodo/src/main/java/org/kitodo/production/services/dataeditor/DataEditorService.java

-                            keyView instanceof SimpleMetadataViewInterface
-                                    ? ((SimpleMetadataViewInterface) keyView).getInputType().toString()
-                                    : "dataTable"));
+        if (Objects.nonNull(metadataView)) {


Wondering why metadataView can be null. This looks like a bug elsewhere.

Yes, I think you are correct. This pull request is not meant to fix the underlying problem of the metadata handling but is just intended to better deal with the resulting null pointer exception.

solth · 2023-08-01T13:01:34Z

Edit: This seem to stem from the specific functionality attached to keys named as "Person", it does not happen if if define a "Person" key in the ruleset, even with different subkeys than the one coming from K10-plus. I have not tested it yet with other datasets yet where the ATS related NPE does not happen.

@BartChris thanks for this clarification. I think "Person" is a special case and separate from the issue I tried to fix with this pull request.

Do you think the linked issue is sufficiently resolved with these proposed changes or are there other suggestions or change requests you would like to see? (e.g. as @matthias-ronge mentioned there seem to be other issues in the metadata handling which are not addressed by this pull request. It is merely meant to handle the exception that can occur when ruleset and xml transformation script do not match a little more gracefully. It does not fix the underlying problem.)

markusweigelt

Does not work for me because I still get an NPE.

What have I done?

Created a process with metadata group
Check if the group is available in metadata editor
Change the group name in meta.xml on filesystem
Open the metadata editor again -> NPE

java.lang.NullPointerException
	at org.kitodo.production.forms.createprocess.ProcessSimpleMetadata.isRequired(ProcessSimpleMetadata.java:96)
	at org.kitodo.production.forms.createprocess.ProcessTextMetadata.isRequired(ProcessTextMetadata.java:30)
...

solth · 2023-08-03T11:01:31Z

Does not work for me because I still get an NPE.

What have I done?

* Created a process with metadata group

* Check if the group is available in metadata editor

* Change the group name in `meta.xml` on filesystem

* Open the metadata editor again -> NPE

java.lang.NullPointerException
	at org.kitodo.production.forms.createprocess.ProcessSimpleMetadata.isRequired(ProcessSimpleMetadata.java:96)
	at org.kitodo.production.forms.createprocess.ProcessTextMetadata.isRequired(ProcessTextMetadata.java:30)
...

@markusweigelt thanks for testing the changes. Yes, I think the same problem exists in the metadata editor. I just tried to handle the issue described in the linked issue, which occurs during import, though. Since the whole topic is quite complex I would suggest to handle the problem in the metadata editor separately, if you think that's ok.

markusweigelt · 2023-08-03T11:22:38Z

Ok I have read over that. I will check this only for the import again.

markusweigelt · 2023-08-03T13:58:00Z

Not existing top level fields working

<kitodo:metadata name="notExistingField"><xsl:text>Lorem ipsum dolor sit amet</xsl:text></kitodo:metadata>

but I am not able to import without NPE with non-existent top-level groups.

<kitodo:metadataGroup name="notExistingGroup">
     <kitodo:metadata name="notExistingField"><xsl:text>Lorem ipsum dolor sit amet</xsl:text></kitodo:metadata>
</kitodo:metadataGroup>

or

     <kitodo:metadata name="TitleDocMain"><xsl:text>Lorem ipsum dolor sit amet</xsl:text></kitodo:metadata>
</kitodo:metadataGroup>

producing following NPE

java.lang.NullPointerException
	at org.kitodo.production.forms.createprocess.ProcessTextMetadata.addLeadingZeros(ProcessTextMetadata.java:46)
	at org.kitodo.production.forms.createprocess.ProcessTextMetadata.<init>(ProcessTextMetadata.java:37)
	at org.kitodo.production.forms.createprocess.ProcessFieldedMetadata.createMetadataEntryEdit(ProcessFieldedMetadata.java:375)
	at org.kitodo.production.forms.createprocess.ProcessFieldedMetadata.createUndefinedMetadataTable(ProcessFieldedMetadata.java:258)
	at org.kitodo.production.forms.createprocess.ProcessFieldedMetadata.createMetadataEntryEdit(ProcessFieldedMetadata.java:385)
	at org.kitodo.production.forms.createprocess.ProcessFieldedMetadata.createMetadataTable(ProcessFieldedMetadata.java:233)
	at org.kitodo.production.forms.createprocess.ProcessFieldedMetadata.buildTreeNodeAndCreateMetadataTable(ProcessFieldedMetadata.java:156)
	at org.kitodo.production.forms.createprocess.ProcessFieldedMetadata.<init>(ProcessFieldedMetadata.java:134)
	at org.kitodo.production.helper.ProcessHelper.initializeProcessDetails(ProcessHelper.java:102)
	at org.kitodo.production.helper.ProcessHelper.transformToProcessDetails(ProcessHelper.java:74)
	at org.kitodo.production.helper.ProcessHelper.generateAtstslFields(ProcessHelper.java:161)
	at org.kitodo.production.services.data.ImportService.importProcessHierarchy(ImportService.java:528)
	at org.kitodo.production.forms.createprocess.CatalogImportDialog.getRecordHierarchy(CatalogImportDialog.java:181)
	at org.kitodo.production.forms.createprocess.CatalogImportDialog.getRecordById(CatalogImportDialog.java:247)
	at org.kitodo.production.forms.createprocess.CatalogImportDialog.search(CatalogImportDialog.java:108)

markusweigelt · 2024-02-23T08:51:07Z

We could also provide a general error message here, indicating that most likely the Kitodo XML generated after the XSL transformation is faulty. The errors I have noticed include, for example, empty groups when fields imported from K10Plus are empty, and there is a condition for checking them in XSL but no condition for the group.

solth · 2024-03-04T08:18:41Z

We could also provide a general error message here, indicating that most likely the Kitodo XML generated after the XSL transformation is faulty. The errors I have noticed include, for example, empty groups when fields imported from K10Plus are empty, and there is a condition for checking them in XSL but no condition for the group.

That would be the optimal solution, in my opinion - show a warning dialog that something went wrong with the import, likeley related to the ruleset and the mapping file not being compatible, and that additional information can be found in the logs, where the exact error message (and perhaps stack trace) should be logged.

solth requested review from matthias-ronge and markusweigelt March 27, 2023 08:41

matthias-ronge reviewed Apr 6, 2023

View reviewed changes

IkramMaalej mentioned this pull request May 23, 2023

Remove unneccessary null-checks #5631

Closed

solth added the bug label Jul 8, 2023

solth force-pushed the unknown-metadata-group-npe branch from 0afcdba to 1f7b068 Compare August 1, 2023 12:50

markusweigelt suggested changes Aug 3, 2023

View reviewed changes

Prevent NPE when creating unknown metadata group and metadata

a56d6fc

solth force-pushed the unknown-metadata-group-npe branch from 1f7b068 to a56d6fc Compare March 4, 2024 09:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent NPE when creating unknown metadata group and metadata #5603

Prevent NPE when creating unknown metadata group and metadata #5603

solth commented Mar 27, 2023 •

edited

Loading

BartChris commented Mar 27, 2023 •

edited

Loading

matthias-ronge left a comment

matthias-ronge Apr 6, 2023 •

edited

Loading

solth Aug 1, 2023 •

edited

Loading

matthias-ronge Apr 6, 2023

solth Aug 1, 2023

solth commented Aug 1, 2023 •

edited

Loading

markusweigelt left a comment

solth commented Aug 3, 2023

markusweigelt commented Aug 3, 2023

markusweigelt commented Aug 3, 2023 •

edited

Loading

markusweigelt commented Feb 23, 2024

solth commented Mar 4, 2024 •

edited

Loading

Prevent NPE when creating unknown metadata group and metadata #5603

Are you sure you want to change the base?

Prevent NPE when creating unknown metadata group and metadata #5603

Conversation

solth commented Mar 27, 2023 • edited Loading

BartChris commented Mar 27, 2023 • edited Loading

matthias-ronge left a comment

Choose a reason for hiding this comment

matthias-ronge Apr 6, 2023 • edited Loading

Choose a reason for hiding this comment

solth Aug 1, 2023 • edited Loading

Choose a reason for hiding this comment

matthias-ronge Apr 6, 2023

Choose a reason for hiding this comment

solth Aug 1, 2023

Choose a reason for hiding this comment

solth commented Aug 1, 2023 • edited Loading

markusweigelt left a comment

Choose a reason for hiding this comment

solth commented Aug 3, 2023

markusweigelt commented Aug 3, 2023

markusweigelt commented Aug 3, 2023 • edited Loading

markusweigelt commented Feb 23, 2024

solth commented Mar 4, 2024 • edited Loading

solth commented Mar 27, 2023 •

edited

Loading

BartChris commented Mar 27, 2023 •

edited

Loading

matthias-ronge Apr 6, 2023 •

edited

Loading

solth Aug 1, 2023 •

edited

Loading

solth commented Aug 1, 2023 •

edited

Loading

markusweigelt commented Aug 3, 2023 •

edited

Loading

solth commented Mar 4, 2024 •

edited

Loading