Error during Batch Ingests of Compound Children before parent objects

[IAMlKeno]

ISLANDORA-2418: (https://jira.duraspace.org/browse/ISLANDORA-2418)

What does this Pull Request do?

This pull request will prevent the following PHP error caused when information about a compound child is requested using "islandora_compound_object_retrieve_compound_info".
Argument 2 passed to islandora_object_manage_access_callback() must be an instance of AbstractObject, boolean given, called in /var/www/drupal7/sites/all/modules/islandora_solution_pack_compound/islandora_compound_object.module on line 743 and defined in islandora_object_manage_access_callback() (line 930 of /var/www/drupal7/sites/all/modules/islandora/islandora.module).

This error can cause issues during batch ingests, where derivative generation is deferred and controlled by a third party tool such as Gearman. It was observed that this error prevented some servers from removing temporary files created during derivative generation.

This issue was brought up as a result of a recent change in the Islandora module. Commit a46936cf7b5fae9002b34c81f180509ce5af8744
Found here: Islandora/islandora@a46936c#diff-ab8875f550771e37425e399a32097fb0R930

What's new?

• The PR checks if the parent object is loaded
• If not, creates a standard object with NULL values to be used in the information array returned by the function.

How should this be tested?

--- Before merging code ---

• Create a batch ingest that creates at least one child compound object with a parent that does not exist.
  ** A module like the Islandora Spreadsheet Ingest module can be used.
• Once ingested, navigate to the object and the PHP error, "Argument 2 passed to islandora_object_manage_access_callback() must be an instance of AbstractObject..." should be produced.

--- After merging the code from this PR ---

• The error will not be seen and the object should load with no issues.

Additional Notes:

Note that when derivatives are deferred and a third party tool is used to generate derivatives, the PHP thread can die. This can lead to the system not removing temporary files created during derivative generation. Below is a sample of this error observed using Gearman
Apr 3 06:20:18 ingest gearman-worker.sh[14024]: WD php: TypeError: Argument 2 passed to [error] Apr 3 06:20:18 ingest gearman-worker.sh[14024]: islandora_object_manage_access_callback() must be an instance of Apr 3 06:20:18 ingest gearman-worker.sh[14024]: AbstractObject or null, boolean given, called in Apr 3 06:20:18 ingest gearman-worker.sh[14024]: /var/www/drupal7/sites/all/modules/islandora_solution_pack_compound/islandora_compound_object.module Apr 3 06:20:18 ingest gearman-worker.sh[14024]: on line 742 in islandora_object_manage_access_callback() (line 930 of Apr 3 06:20:18 ingest gearman-worker.sh[14024]: /var/www/drupal7/sites/all/modules/islandora/islandora.module). Apr 3 06:20:18 ingest gearman-worker.sh[14024]: Drush command terminated abnormally due to an unrecoverable error. [error] Apr 3 06:20:18 ingest gearman-worker.sh[14024]: gearman: gearman_worker_work : GEARMAN_UNKNOWN_STATE

Interested parties

@Islandora/7-x-1-x-committers
@DiegoPino
@jordandukart

[qadan]

I'd be okay with merging this as sort of a patch to prevent issues arising from this case; sounds like there's a rogue batch somewhere that's creating batches with compound parents but not setting the compound as their parent? Double checked to make sure it doesn't exist in this module; the zip importer appears to be fine 👍

[bondjimbond]

As far as I understand, there's no reason to create a compound child without a compound parent, is there? Instead of trying to accommodate such behaviour, might it not be better to throw an error and notify the user that they're a child cannot be ingested without a parent? I would think that the only reason someone might do this is by accident, so they would appreciate the notification.

[IAMlKeno]

@bondjimbond I took this approach because the issue came during a batch ingest.
In the particular use case, it was a large ingest where the process died before removing temporary files. This led to the server running out of space.

[DiegoPino]

@bondjimbond there is no way (in a large ingest process) to make sure that the parent is already there. Best case scenario, if ingest is happening parent first, then child, all ok, but objects can be ingested at different speeds? I see no harm in allowing that to happen, since if there is a parent ID, at least you know something in islandora already requested it.. or will

[DiegoPino]

Travis is failing on 5.3.3 but as @jonathangreen correctly stated here Islandora/islandora_solr_search#362 (comment) its a known fact (memes are real facts you know..) and i can live with it. Approving and waiting 24 hours to merge.

[DiegoPino]

Good to go. PHP 5.3.3 should not even be tested by now, abandonware...

[IAMlKeno]

Ready to be merged?

[DiegoPino]

Hi, sorry. Pull is good but there will be a TAG meeting this week to decide the faith of 5.3.3 so therr is agreement in waiting a little bit longer before merging pulls where that fails. Just a little bit longer!

[nhart]

ping @DiegoPino

[DiegoPino]

@nhart are you guys bringing the changes to allow 5.3.3 fail into this pull? (based on the groups post) Maybe i understood incorrectly but seems that the agreement was to deal with this on failing tests? @Islandora/7-x-1-x-committers anyone?

[DiegoPino]

re starting travis now that php 5.3.3 fix is in

[IAMlKeno]

@DiegoPino all checks are green to go

[DiegoPino]

@IAMlKeno perfect. Merged, good work!