diff --git a/includes/derivatives.inc b/includes/derivatives.inc index 78f00aab..d25789cc 100644 --- a/includes/derivatives.inc +++ b/includes/derivatives.inc @@ -41,20 +41,7 @@ function islandora_do_derivatives(AbstractObject $object, array $options) { $hooks = islandora_invoke_hook_list(ISLANDORA_DERVIATIVE_CREATION_HOOK, $object->models, array($object)); uasort($hooks, 'drupal_sort_weight'); $results = array(); - - if (array_key_exists('source_dsid', $options)) { - $hooks = array_filter($hooks, function($filter_hook) use($options) { - return array_key_exists('source_dsid', $filter_hook) && - $filter_hook['source_dsid'] == $options['source_dsid']; - }); - } - - if (array_key_exists('destination_dsid', $options)) { - $hooks = array_filter($hooks, function($filter_hook) use($options) { - return array_key_exists('destination_dsid', $filter_hook) && - $filter_hook['destination_dsid'] == $options['destination_dsid']; - }); - } + $hooks = islandora_filter_derivatives($hooks, $options, $object); foreach ($hooks as $hook) { if (isset($hook['file'])) { @@ -148,20 +135,7 @@ function islandora_do_batch_derivatives(AbstractObject $object, array $options) uasort($hooks, 'drupal_sort_weight'); $operations = array(); - if (array_key_exists('source_dsid', $options)) { - $hooks = array_filter($hooks, function($filter_hook) use($options) { - return array_key_exists('source_dsid', $filter_hook) && - $filter_hook['source_dsid'] == $options['source_dsid']; - }); - } - - if (array_key_exists('destination_dsid', $options)) { - $hooks = array_filter($hooks, function($filter_hook) use($options) { - return array_key_exists('destination_dsid', $filter_hook) && - $filter_hook['destination_dsid'] == $options['destination_dsid']; - }); - } - + $hooks = islandora_filter_derivatives($hooks, $options, $object); foreach ($hooks as $hook) { $file = FALSE; if (isset($hook['file'])) { @@ -178,3 +152,50 @@ function islandora_do_batch_derivatives(AbstractObject $object, array $options) } return $operations; } + +/** + * Filter the derivative functions to only call those which are valid. + * + * @param array $hooks + * An array of hooks to be filtered depending on options. + * @param array $options + * An array of options for the derivative generation. + * @param AbstractObject $object + * An AbstractObject representing an object within Fedora. + * + * @return array + * Returns the filtered array of hooks to be ran. + */ +function islandora_filter_derivatives($hooks, $options, AbstractObject $object) { + if (array_key_exists('source_dsid', $options)) { + $hooks = array_filter($hooks, function ($filter_hook) use ($options) { + return array_key_exists('source_dsid', $filter_hook) && + $filter_hook['source_dsid'] == $options['source_dsid']; + }); + } + if (array_key_exists('destination_dsid', $options)) { + $hooks = array_filter($hooks, function ($filter_hook) use ($options) { + return array_key_exists('destination_dsid', $filter_hook) && + $filter_hook['destination_dsid'] == $options['destination_dsid']; + }); + } + // Do a final filtering to make sure that the source DSID exists on the object + // where needed. Using a defined function as opposed to the way above as + // it seems to break PHPCS as of 1.4.8. + $filter_function = function ($filter_hook) use ($object) { + $to_return = FALSE; + if (array_key_exists('source_dsid', $filter_hook)) { + if ($filter_hook['source_dsid'] != NULL) { + if (isset($object[$filter_hook['source_dsid']])) { + $to_return = TRUE; + } + } + else { + $to_return = TRUE; + } + } + return $to_return; + }; + $hooks = array_filter($hooks, $filter_function); + return $hooks; +} diff --git a/includes/object_properties.form.inc b/includes/object_properties.form.inc index be31719c..f9c8e23b 100644 --- a/includes/object_properties.form.inc +++ b/includes/object_properties.form.inc @@ -28,7 +28,10 @@ function islandora_object_properties_form(array $form, array &$form_state, Abstr } $regenerate_derivatives_access = FALSE; if (islandora_object_access(ISLANDORA_REGENERATE_DERIVATIVES, $object)) { - if (count(islandora_invoke_hook_list(ISLANDORA_DERVIATIVE_CREATION_HOOK, $object->models, array($object))) > 0) { + module_load_include('inc', 'islandora', 'includes/derivatives'); + $hooks = islandora_invoke_hook_list(ISLANDORA_DERVIATIVE_CREATION_HOOK, $object->models, array($object)); + $hooks = islandora_filter_derivatives($hooks, array('force' => TRUE), $object); + if (count($hooks) > 1) { $regenerate_derivatives_access = TRUE; } } diff --git a/islandora.module b/islandora.module index 8173df52..8e480baa 100644 --- a/islandora.module +++ b/islandora.module @@ -1838,11 +1838,13 @@ function islandora_islandora_datastream_access($op, AbstractDatastream $datastre $result = islandora_namespace_accessible($datastream->parent->id) && user_access($op, $user); if ($result && $op == ISLANDORA_REGENERATE_DERIVATIVES) { + module_load_include('inc', 'islandora', 'includes/derivatives'); $applicable_hook = FALSE; $object = $datastream->parent; $hooks = islandora_invoke_hook_list(ISLANDORA_DERVIATIVE_CREATION_HOOK, $object->models, array($object)); + $hooks = islandora_filter_derivatives($hooks, array('force' => TRUE), $object); foreach ($hooks as $hook) { - if ($hook['destination_dsid'] == $datastream->id && ((isset($hook['source_dsid']) && isset($object[$hook['source_dsid']]) && islandora_datastream_access(ISLANDORA_VIEW_OBJECTS, $object[$hook['source_dsid']], $user)) || (array_key_exists('source_dsid', $hook) && $hook['source_dsid'] == NULL))) { + if ($hook['destination_dsid'] == $datastream->id && islandora_datastream_access(ISLANDORA_VIEW_OBJECTS, $object[$hook['source_dsid']], $user)) { $applicable_hook = TRUE; break; } diff --git a/tests/derivatives.test b/tests/derivatives.test index 13014880..b7eeae9b 100644 --- a/tests/derivatives.test +++ b/tests/derivatives.test @@ -159,7 +159,8 @@ class IslandoraDerivativesTestCase extends IslandoraWebTestCase { public function testDerivativeFilteringOnSourceDSID() { global $_islandora_derivative_test_derivative_functions; $_islandora_derivative_test_derivative_functions = array(); - $this->constructBaseObject(); + $object = $this->constructBaseObject(); + $this->constructSOMEWEIRDDATASTREAMDatastream($object); $object = islandora_object_load($this->pid); islandora_do_derivatives($object, array( 'source_dsid' => 'OBJ', @@ -207,7 +208,8 @@ class IslandoraDerivativesTestCase extends IslandoraWebTestCase { public function testNoSourceDSIDNoForce() { global $_islandora_derivative_test_derivative_functions; $_islandora_derivative_test_derivative_functions = array(); - $this->constructBaseObject(); + $object = $this->constructBaseObject(); + $object = $this->constructSOMEWEIRDDATASTREAMDatastream($object); $object = islandora_object_load($this->pid); islandora_do_derivatives($object, array()); $this->assertDatastreams($object, array( @@ -216,6 +218,8 @@ class IslandoraDerivativesTestCase extends IslandoraWebTestCase { 'OBJ', 'DERIV', 'NOSOURCE', + 'SOMEWEIRDDATASTREAM', + 'STANLEY', )); $this->assertEqual(3, count($_islandora_derivative_test_derivative_functions), 'Expected 3 derivative functions when there is no source_dsid, got ' . count($_islandora_derivative_test_derivative_functions) . '.'); } @@ -226,7 +230,8 @@ class IslandoraDerivativesTestCase extends IslandoraWebTestCase { public function testNoSourceDSIDForce() { global $_islandora_derivative_test_derivative_functions; $_islandora_derivative_test_derivative_functions = array(); - $this->constructBaseObject(); + $object = $this->constructBaseObject(); + $this->constructSOMEWEIRDDATASTREAMDatastream($object); $object = islandora_object_load($this->pid); islandora_do_derivatives($object, array( 'force' => TRUE, @@ -236,6 +241,8 @@ class IslandoraDerivativesTestCase extends IslandoraWebTestCase { 'RELS-EXT', 'OBJ', 'DERIV', + 'SOMEWEIRDDATASTREAM', + 'STANLEY', 'NOSOURCE', )); $this->assertEqual(3, count($_islandora_derivative_test_derivative_functions), 'Expected 3 derivative functions when there is no source_dsid, got ' . count($_islandora_derivative_test_derivative_functions) . '.'); @@ -293,4 +300,22 @@ class IslandoraDerivativesTestCase extends IslandoraWebTestCase { $object->ingestDatastream($ds); return $object; } + + /** + * Helper function to construct the SWD datastream without firing hooks. + * + * @param AbstractObject $object + * An AbstractObject representing a FedoraObject. + * + * @return AbstractObject + * The modified AbstractObject. + */ + public function constructSOMEWEIRDDATASTREAMDatastream(AbstractObject $object) { + $dsid = 'SOMEWEIRDDATASTREAM'; + $ds = $object->constructDatastream($dsid); + $ds->label = 'Test'; + $ds->content = 'Omnomnom'; + $object->ingestDatastream($ds); + return $object; + } } diff --git a/tests/islandora_derivatives_test.module b/tests/islandora_derivatives_test.module index 80530296..2e27c5c7 100644 --- a/tests/islandora_derivatives_test.module +++ b/tests/islandora_derivatives_test.module @@ -23,7 +23,7 @@ function islandora_derivatives_test_some_cmodel_islandora_derivative() { 'destination_dsid' => 'STANLEY', 'weight' => '-1', 'function' => array( - 'islandora_derivatives_test_create_some_weird_datastream', + 'islandora_derivatives_test_create_stanley_datastream', ), ), array( @@ -81,17 +81,20 @@ function islandora_derivatives_test_create_deriv_datastream(AbstractObject $obje } /** - * Stub function that used only for datastream filtering counts. + * Creates the STANLEY datastream for use in testing. * * @param AbstractObject $object * An AbstractObject representing a Fedora object. * @param bool $force * Whether the derivatives are being forcefully generated or not. */ -function islandora_derivatives_test_create_some_weird_datastream(AbstractObject $object, $force = FALSE) { +function islandora_derivatives_test_create_stanley_datastream(AbstractObject $object, $force = FALSE) { global $_islandora_derivative_test_derivative_functions; // Add to the global that we got to this function. $_islandora_derivative_test_derivative_functions[] = 'islandora_derivatives_test_create_some_weird_datastream'; + if (!isset($object['STANLEY']) || (isset($object['STANLEY']) && $force === TRUE)) { + islandora_derivatives_test_add_datastream($object, 'STANLEY', 'yum'); + } } /**