Regression: Hierarchy export did not work anymore

[henning-gerhardt]

Describe the bug
After the merge of pull request #5482 the hierarchy export did not work any more like in the 3.5.0 release: Only the process itself get exported. If the exported process is a hierarchy process this process did not contain any references in the logical struct map to its children and if the the process is a child then the parent process did not get exported additional any more.

To Reproduce

1. Export a hierarchical process with a version after the pull request Fix async export #5482 get merged (around 15th February 2023): the exported data did not contain any reference to the child processes in the logical structural map
2. Export a child process: the child process get exported but without the corresponding parent process.

Expected behavior

1. If a hierarchical process get exported it must contain the references to its children in the logical structural map
2. If a children process get exported the children process itself and its corresponding parent process must be exported with references to this children process and maybe other already exported children processes.

Release
Export of hierarchical processes was working in 3.5.0 but not after February 15 2023 after merging the pull request #5482.

Additional context
I looked into the changes of pull request #5482 and after reverting the change in line 139 from false back to true in the ExportDMS class (https://github.com/kitodo/kitodo-production/pull/5482/files#diff-ef37fd2392d47f53a42fe639cf809b1cd47934294a4cedc3662adaca7f7441a2R139) the hierarchical export was working again like before.

Edit: i did not check what is happening if this export is broken after I changed the value back to true. /Edit

I can understand that the return value of this method was changed to false to stop the asynchronous export if an error occurred in this exported process but changing this value to false is stopping the hierarchical export too independent if an error occur or not. The return value in this method and line should be set to this value depending on how this async export was but this could be complicated as the export is done later in a separate thread and not at the moment where the code is executed.

[BartChris]

You are right, this leads to problems here :/:

kitodo-production/Kitodo/src/main/java/org/kitodo/export/ExportMets.java

Line 67 in f1b7ed4

if (exportSuccessful) {

kitodo-production/Kitodo/src/main/java/org/kitodo/export/ExportDms.java

Line 114 in f1b7ed4

if (exportSuccessful) {

I will see if i can find ways to adress that or we have to revert the change.

Edit: I am wondering wether we really need to check if the export of the child was succesful before exporting the parent here. If the child process cannot be exported the parent might be exported but it would not have any consequences since the workflow step will not be closed. The only consequence would be the generation of a parent process export file which will be overridden if the child gets exported again, or?
So my suggestion would be to not require a succesful child export before exporting the parent, but to just start the export for the parent. The main thing is, that the task, where the export happens is not closed, when something with the child process (which owns the task) is wrong.

kitodo-production/Kitodo/src/main/java/org/kitodo/export/ExportDms.java

Line 114 in f1b7ed4

if (exportSuccessful) {

[henning-gerhardt]

Thank you @BartChris for your investigation.

If the child process cannot be exported the parent might be exported but it would not have any consequences since the workflow step will not be closed.

This should be the case, that the workflow export task remain open if the child export is aborted.

The only consequence would be the generation of a parent process export file which will be overridden if the child gets exported again, or?

What is happening with the exported data depends on how it is used after. As long as the exported data itself is correct and belong all data then it is independent if the parent process is exported to many times.

So my suggestion would be to not require a succesful child export before exporting the parent, but to just start the export for the parent. The main thing is, that the task, where the export happens is not closed, when something with the child process (which owns the task) is wrong.

I can go with this.

[BartChris]

A stricter variation, which would require more changes, would be to consider an export only as valid if the child and every parent is exported succesfully. I am not sure how to implement that, but this would maybe also be a way to address the problem that a process can pass validation even if the parent process is invalid: #5280.

[henning-gerhardt]

An even stricter variation, which would require more changes, would be to consider an export only as valid if the child and every parent is exported succesfully.

You are right. But this is difficult to implement. For me it would be fine if every hierarchical level could be exported. If one or more exports are failing is or should not be part of handling the export in first place.

I am not sure how to implement that, but this would maybe also be a way to address the problem that a process can pass validation even if the parent process is invalid: #5280.

Maybe this should be done in a second task or after thinking about exporting processes in general like:
a) did we really need a synchronous and asynchronous export or would an asynchronous export not enough
b) which kind of validation should be done or not before an export is starting
c) maybe more things to discuss

We at SLUB need first a good solution without many changes as we would go productive in September 2023 and can not wait for a solution until a few days before we start the migration. Currently we are checking and maybe go live with the master branch from March 1st with some additional changes which are merged after March 1st.