From e63201f9f9e8457ac84a0e8884c276962a5f2391 Mon Sep 17 00:00:00 2001 From: qadan Date: Fri, 8 Jul 2016 13:17:30 -0300 Subject: [PATCH 1/4] update API to include DS modified params --- includes/derivatives.inc | 9 +++++++-- includes/tuque_wrapper.inc | 2 +- islandora.api.php | 20 ++++++++++++-------- islandora.module | 3 ++- 4 files changed, 22 insertions(+), 12 deletions(-) diff --git a/includes/derivatives.inc b/includes/derivatives.inc index ccd8c0d3..9b44b9af 100644 --- a/includes/derivatives.inc +++ b/includes/derivatives.inc @@ -62,6 +62,9 @@ function islandora_run_derivatives(AbstractObject $object, $dsid) { * from or NULL if it's the object itself. * - destination_dsid: (Optional) String of the datastream id that is being * created. To be used in the UI. + * - ds_modified_params: (Optional, typically internally used) The array of + * parameters originally sent to FedoraDatastream::modifyDatastream(). Used + * to refine the derivative list; typically should't be passed in manually. * * @return array * An array of messages describing the outcome of the derivative events. @@ -287,6 +290,7 @@ function islandora_remove_defer_derivatives_flag(AbstractObject $object) { function islandora_get_derivative_list(AbstractObject $object, &$options) { module_load_include('inc', 'islandora', 'includes/utilities'); + $ds_modified_params = isset($options['ds_modified_params']) ? $options['ds_modified_params'] : array(); $options += array( 'force' => FALSE, ); @@ -294,11 +298,12 @@ function islandora_get_derivative_list(AbstractObject $object, &$options) { $derivatives = islandora_invoke_hook_list( ISLANDORA_DERIVATIVE_CREATION_HOOK, $object->models, - array($object) + array($object), + $ds_modified_params ); foreach (islandora_build_hook_list(ISLANDORA_DERIVATIVE_CREATION_HOOK, $object->models) as $hook) { - drupal_alter($hook, $derivatives, $object); + drupal_alter($hook, $derivatives, $object, $ds_modified_params); } uasort($derivatives, 'drupal_sort_weight'); diff --git a/includes/tuque_wrapper.inc b/includes/tuque_wrapper.inc index 6a38ad4a..7d1d348d 100644 --- a/includes/tuque_wrapper.inc +++ b/includes/tuque_wrapper.inc @@ -594,7 +594,7 @@ class IslandoraFedoraDatastream extends FedoraDatastream { protected function modifyDatastream(array $args) { try { parent::modifyDatastream($args); - islandora_invoke_datastream_hooks(ISLANDORA_DATASTREAM_MODIFIED_HOOK, $this->parent->models, $this->id, $this->parent, $this); + islandora_invoke_datastream_hooks(ISLANDORA_DATASTREAM_MODIFIED_HOOK, $this->parent->models, $this->id, $this->parent, $this, $args); if ($this->state == 'D') { islandora_invoke_datastream_hooks(ISLANDORA_DATASTREAM_PURGED_HOOK, $this->parent->models, $this->id, $this->parent, $this->id); } diff --git a/islandora.api.php b/islandora.api.php index 1d0d9872..31c6a6d5 100644 --- a/islandora.api.php +++ b/islandora.api.php @@ -334,12 +334,12 @@ function hook_cmodel_pid_dsid_islandora_datastream_ingested(AbstractObject $obje * @param AbstractObject $object * The object the datastream belongs to. * @param AbstractDatastream $datastream - * The datastream that was ingested. - * - * @todo We should also include what changes were made in a additional - * parameter. + * The datastream that was modified. + * @param array $params + * The parameters from FedoraDatastream::modifyDatastream() used to modify the + * datastream. */ -function hook_islandora_datastream_modified(AbstractObject $object, AbstractDatastream $datastream) { +function hook_islandora_datastream_modified(AbstractObject $object, AbstractDatastream $datastream, array $params) { } @@ -348,7 +348,7 @@ function hook_islandora_datastream_modified(AbstractObject $object, AbstractData * * @see hook_islandora_datastream_modified() */ -function hook_cmodel_pid_islandora_datastream_modified(AbstractObject $object, AbstractDatastream $datastream) { +function hook_cmodel_pid_islandora_datastream_modified(AbstractObject $object, AbstractDatastream $datastream, array $params) { } @@ -660,6 +660,10 @@ function hook_cmodel_pid_islandora_overview_object_alter(AbstractObject &$object * * @param AbstractObject $object * Optional object to which derivatives will be added + * @param array $ds_modified_params + * An array that will contain the properties changed on the datastream if + * derivatives were triggered from a datastream_modified hook. Can be + * populated manually, but likely empty otherwise. * * @return array * An array containing an entry for each derivative to be created. Each entry @@ -693,7 +697,7 @@ function hook_cmodel_pid_islandora_overview_object_alter(AbstractObject &$object * - file: A string denoting the path to the file where the function * is being called from. */ -function hook_islandora_derivative(AbstractObject $object = NULL) { +function hook_islandora_derivative(AbstractObject $object = NULL, $ds_modified_params = array()) { $derivatives[] = array( 'source_dsid' => 'OBJ', 'destination_dsid' => 'DERIV', @@ -736,7 +740,7 @@ function hook_cmodel_pid_islandora_derivative() { /** * Allows for the altering of defined derivative functions. */ -function hook_islandora_derivative_alter(&$derivatives, AbstractObject $object) { +function hook_islandora_derivative_alter(&$derivatives, AbstractObject $object, $ds_modified_params = array()) { foreach ($derivatives as $key => $derivative) { if ($derivative['destination_dsid'] == 'TN') { unset($derivatives[$key]); diff --git a/islandora.module b/islandora.module index b3c4263c..670b2b66 100644 --- a/islandora.module +++ b/islandora.module @@ -1798,11 +1798,12 @@ function islandora_islandora_datastream_ingested(AbstractObject $object, Abstrac * equal to the current ingested datastream's id. Force is set to TRUE such that * existing derivatives will be updated to reflect the change in the source. */ -function islandora_islandora_datastream_modified(AbstractObject $object, AbstractDatastream $datastream) { +function islandora_islandora_datastream_modified(AbstractObject $object, AbstractDatastream $datastream, $params) { module_load_include('inc', 'islandora', 'includes/derivatives'); $logging_results = islandora_do_derivatives($object, array( 'source_dsid' => $datastream->id, 'force' => TRUE, + 'ds_modified_params' => $params, )); islandora_derivative_logging($logging_results); islandora_conditionally_clear_cache(); From 0f4ae4043febe780fb709d2709ab4fdb73e3821f Mon Sep 17 00:00:00 2001 From: qadan Date: Tue, 12 Jul 2016 10:56:44 -0300 Subject: [PATCH 2/4] better documentation, tests --- islandora.api.php | 22 +++++++++++-- islandora.module | 3 ++ tests/derivatives.test | 16 +++++++++ tests/hooks.test | 12 +++++++ tests/islandora_derivatives_test.module | 43 +++++++++++++++++++++++++ tests/islandora_hooks_test.module | 6 +++- 6 files changed, 98 insertions(+), 4 deletions(-) diff --git a/islandora.api.php b/islandora.api.php index 31c6a6d5..3ec7fa74 100644 --- a/islandora.api.php +++ b/islandora.api.php @@ -340,7 +340,12 @@ function hook_cmodel_pid_dsid_islandora_datastream_ingested(AbstractObject $obje * datastream. */ function hook_islandora_datastream_modified(AbstractObject $object, AbstractDatastream $datastream, array $params) { - + // Sample of sanitizing a label. + $datastream->label = trim($datastream->label); + // Sample of using modifyDatastream parameters. + if (isset($params['mimetype'])) { + $datastream->label .= " ({$params['mimetype']})"; + } } /** @@ -662,8 +667,9 @@ function hook_cmodel_pid_islandora_overview_object_alter(AbstractObject &$object * Optional object to which derivatives will be added * @param array $ds_modified_params * An array that will contain the properties changed on the datastream if - * derivatives were triggered from a datastream_modified hook. Can be - * populated manually, but likely empty otherwise. + * derivatives were triggered from a datastream_modified hook, as well as a + * 'dsid' key naming the datastream that was modified. Can be populated + * manually, but likely empty otherwise. * * @return array * An array containing an entry for each derivative to be created. Each entry @@ -746,6 +752,16 @@ function hook_islandora_derivative_alter(&$derivatives, AbstractObject $object, unset($derivatives[$key]); } } + // Example of altering out derivative generation if only a specified set of + // datastream parameters have been modified. + $mask = array( + 'label' => NULL, + 'dateLastModified' => NULL, + 'dsid' => NULL, + ); + if (empty(array_diff_key($ds_modified_params, $mask))) { + $derivatives = array(); + } } /** diff --git a/islandora.module b/islandora.module index 670b2b66..b3f20e21 100644 --- a/islandora.module +++ b/islandora.module @@ -1800,6 +1800,9 @@ function islandora_islandora_datastream_ingested(AbstractObject $object, Abstrac */ function islandora_islandora_datastream_modified(AbstractObject $object, AbstractDatastream $datastream, $params) { module_load_include('inc', 'islandora', 'includes/derivatives'); + $params += array( + 'dsid' => $datastream->id, + ); $logging_results = islandora_do_derivatives($object, array( 'source_dsid' => $datastream->id, 'force' => TRUE, diff --git a/tests/derivatives.test b/tests/derivatives.test index b7eeae9b..95377a75 100644 --- a/tests/derivatives.test +++ b/tests/derivatives.test @@ -248,6 +248,22 @@ class IslandoraDerivativesTestCase extends IslandoraWebTestCase { $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) . '.'); } + /** + * Tests that a derivative can be conditional on the source_dsid modification. + */ + public function testConditionalDerivative() { + $object = $this->constructBaseObject(); + // Derivatives should fire. + $ds = $object->constructDatastream('SOMEOTHERDATASTREAM'); + $ds->label = 'Some Label'; + $ds->mimetype = 'some/mime'; + $object->ingestDatastream($ds); + $this->assertEqual(1, count($object['SOMEWEIRDDERIV']), 'Expected one version of SOMEWEIDDERIV when unconditionally creating derivatives, got ' . count($object['SOMEWEIRDDERIV'])); + // Derivatives should not fire. + $object['SOMEOTHERDATASTREAM']->label = 'Changed Label'; + $this->assertEqual(1, count($object['SOMEWEIRDDERIV']), 'Expected the version count of SOMEWEIRDDERIV to remain the same when only changing the label of its source, got ' . count($object['SOMEWEIRDDERIV'])); + } + /** * Helper function that will construct a base object. */ diff --git a/tests/hooks.test b/tests/hooks.test index b8846c52..073d0349 100644 --- a/tests/hooks.test +++ b/tests/hooks.test @@ -214,6 +214,18 @@ class IslandoraHooksTestCase extends IslandoraWebTestCase { } $this->repository->purgeObject($object->id); + // Test modifying a datastream conditionally on the modification parameters. + $object = $this->repository->constructObject('test:testModifiedDatastreamHook'); + $this->repository->ingestObject($object); + $ds = $object->constructDatastream('TEST'); + $object->ingestDatastream($ds); + $_SESSION['islandora_hooks']['hook'][ISLANDORA_DATASTREAM_MODIFIED_HOOK] = FALSE; + $_SESSION['islandora_hooks']['iteration'][ISLANDORA_DATASTREAM_MODIFIED_HOOK] = 0; + $_SESSION['islandora_hooks']['alter'][ISLANDORA_DATASTREAM_MODIFIED_HOOK] = FALSE; + $object->label = "New Label!"; + $ds->mimetype = "mr/mime"; + $this->assertEqual('New Label! (Mr. Mime)', $object->label, 'Modified object conditionally on datastream modification.'); + // Test purging with FedoraRepository::purgeObject(). $object = $this->repository->constructObject('test:testPurgedDatastreamHook'); $this->repository->ingestObject($object); diff --git a/tests/islandora_derivatives_test.module b/tests/islandora_derivatives_test.module index 2e27c5c7..4a46eb66 100644 --- a/tests/islandora_derivatives_test.module +++ b/tests/islandora_derivatives_test.module @@ -34,9 +34,36 @@ function islandora_derivatives_test_some_cmodel_islandora_derivative() { 'islandora_derivatives_test_create_nosource_datastream', ), ), + array( + 'source_dsid' => 'SOMEOTHERDATASTREAM', + 'destination_dsid' => 'SOMEWEIRDDERIV', + 'weight' => '0', + 'function' => array( + 'islandora_derivatives_test_derive_other_datastream', + ), + ), ); } +/** + * Implements hook_islandora_CMODEL_PID_derivative_alter(). + */ +function islandora_derivatives_test_some_cmodel_islandora_derivative_alter(&$derivatives, AbstractObject $object, $ds_modified_params) { + // Use a mask to determine if only the label has been modified. + $diff = array_diff_key($ds_modified_params, array( + 'label' => NULL, + 'dateLastModified' => NULL, + )); + // If that's the case, don't proceed. + if (empty($diff)) { + foreach ($derivatives as $key => $derivative) { + if ($derivative['source_dsid'] == 'SOMEOTHERDATASTREAM') { + unset($derivatives[$key]); + } + } + } +} + /** * Creates the DERIV datastream for use in testing. * @@ -140,6 +167,22 @@ function islandora_derivatives_test_create_nosource_datastream(AbstractObject $o } } +/** + * Derives the SOMEWEIRDDERIV datastream. + * + * @param AbstractObject $object + * An AbstractObject representing a Fedora object. + * @param bool $force + * Whether or not derivative generation is to be forced. + */ +function islandora_derivatives_test_derive_other_datastream(AbstractObject $object, $force = FALSE) { + global $_islandora_derivative_test_derivative_functions; + $_islandora_derivative_test_derivative_functions[] = 'islandora_derivatives_test_derive_other_datastream'; + if (!isset($object['SOMEWEIRDDERIV']) || isset($object['SOMEWEIRDDERIV']) && $force === TRUE) { + islandora_derivatives_test_add_datastream($object, 'SOMEWEIRDDERIV', 'wat'); + } +} + /** * Helper function that adds/modifies the datastream to the object in testing. * diff --git a/tests/islandora_hooks_test.module b/tests/islandora_hooks_test.module index cceb4dbc..092a7041 100644 --- a/tests/islandora_hooks_test.module +++ b/tests/islandora_hooks_test.module @@ -140,13 +140,17 @@ function islandora_hooks_test_islandora_datastream_ingested(AbstractObject $obje /** * Implements hook_islandora_datastream_modified(). */ -function islandora_hooks_test_islandora_datastream_modified(AbstractObject $object, AbstractDatastream $datastream) { +function islandora_hooks_test_islandora_datastream_modified(AbstractObject $object, AbstractDatastream $datastream, array $params) { if ($object->id == 'test:testModifiedDatastreamHook' && $datastream->id == "TEST") { $_SESSION['islandora_hooks']['hook'][ISLANDORA_DATASTREAM_MODIFIED_HOOK] = TRUE; if ($_SESSION['islandora_hooks']['iteration'][ISLANDORA_DATASTREAM_MODIFIED_HOOK]++ < 3) { $new_label = 'New Label! + ' . $_SESSION['islandora_hooks']['iteration'][ISLANDORA_DATASTREAM_MODIFIED_HOOK]; $datastream->label = $new_label; } + if (isset($params['mimetype'] && $params['mimetype'] == 'mr/mime')) { + $new_label = 'New Label! (Mr. Mime)'; + $object->label = $new_label; + } } } From 69005d0dd34f0740aeafad73ae939bab46ef88b7 Mon Sep 17 00:00:00 2001 From: qadan Date: Tue, 12 Jul 2016 13:22:56 -0300 Subject: [PATCH 3/4] tested one of the tests but forgot to test the other test --- tests/islandora_hooks_test.module | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/islandora_hooks_test.module b/tests/islandora_hooks_test.module index 092a7041..d602cbfd 100644 --- a/tests/islandora_hooks_test.module +++ b/tests/islandora_hooks_test.module @@ -147,7 +147,7 @@ function islandora_hooks_test_islandora_datastream_modified(AbstractObject $obje $new_label = 'New Label! + ' . $_SESSION['islandora_hooks']['iteration'][ISLANDORA_DATASTREAM_MODIFIED_HOOK]; $datastream->label = $new_label; } - if (isset($params['mimetype'] && $params['mimetype'] == 'mr/mime')) { + if (isset($params['mimeType']) && $params['mimeType'] == 'mr/mime') { $new_label = 'New Label! (Mr. Mime)'; $object->label = $new_label; } From 592623f0c32823fe54457549261f98de2b4b7f11 Mon Sep 17 00:00:00 2001 From: qadan Date: Thu, 14 Jul 2016 13:42:19 -0300 Subject: [PATCH 4/4] accounting for PHP weirdness --- islandora.api.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/islandora.api.php b/islandora.api.php index 3ec7fa74..a50a0c24 100644 --- a/islandora.api.php +++ b/islandora.api.php @@ -759,7 +759,8 @@ function hook_islandora_derivative_alter(&$derivatives, AbstractObject $object, 'dateLastModified' => NULL, 'dsid' => NULL, ); - if (empty(array_diff_key($ds_modified_params, $mask))) { + $param_diff = array_diff_key($ds_modified_params, $mask); + if (empty($param_diff)) { $derivatives = array(); } }