From 28b3deedccf63a3b66de38699d4ce807122a95ad Mon Sep 17 00:00:00 2001 From: Jordan Dukart Date: Fri, 20 Dec 2013 22:24:50 +0000 Subject: [PATCH 01/16] Add derivative regeneration to the UI. --- includes/datastream.inc | 2 +- includes/object_properties.form.inc | 24 ++++ includes/regenerate_derivatives.form.inc | 163 +++++++++++++++++++++++ islandora.module | 30 ++++- theme/theme.inc | 31 ++++- 5 files changed, 247 insertions(+), 3 deletions(-) create mode 100644 includes/regenerate_derivatives.form.inc diff --git a/includes/datastream.inc b/includes/datastream.inc index 1d506e3d..c9976cad 100644 --- a/includes/datastream.inc +++ b/includes/datastream.inc @@ -312,7 +312,7 @@ function islandora_edit_datastream(AbstractDatastream $datastream) { case 0: // No edit implementations. drupal_set_message(t('There are no edit methods specified for this datastream.')); - drupal_goto("islandora/object/{$object->id}/manage/datastreams"); + drupal_goto("islandora/object/{$datastream->parent->id}/manage/datastreams"); break; case 1: diff --git a/includes/object_properties.form.inc b/includes/object_properties.form.inc index 676f0d0c..be31719c 100644 --- a/includes/object_properties.form.inc +++ b/includes/object_properties.form.inc @@ -26,6 +26,12 @@ function islandora_object_properties_form(array $form, array &$form_state, Abstr if (!empty($temp)) { $related_objects_pids = array_merge_recursive($related_objects_pids, $temp); } + $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) { + $regenerate_derivatives_access = TRUE; + } + } return array( 'pid' => array( '#type' => 'hidden', @@ -84,6 +90,12 @@ function islandora_object_properties_form(array $form, array &$form_state, Abstr '#submit' => array('islandora_object_properties_form_delete'), '#limit_validation_errors' => array(array('pid')), ), + 'regenerate' => array( + '#type' => 'submit', + '#access' => $regenerate_derivatives_access, + '#value' => t("Regenerate all derivatives"), + '#submit' => array('islandora_object_properties_regenerate_derivatives'), + ), ); } @@ -188,3 +200,15 @@ function islandora_update_object_properties($pid, $update_states, $state, $updat } } } + +/** + * Callback function for object properties regenerate all derivatives. + * + * @param array $form + * The Drupal form. + * @param array $form_state + * The Drupal form state. + */ +function islandora_object_properties_regenerate_derivatives(array $form, array &$form_state) { + drupal_goto("islandora/object/{$form_state['object']}/regenerate"); +} diff --git a/includes/regenerate_derivatives.form.inc b/includes/regenerate_derivatives.form.inc new file mode 100644 index 00000000..917154ea --- /dev/null +++ b/includes/regenerate_derivatives.form.inc @@ -0,0 +1,163 @@ + $datastream->id)), + "islandora/object/{$datastream->parent->id}/manage/datastreams", + t('This will create a new version of the datastream. Please wait while this happens.'), + t('Regenerate'), + t('Cancel') + ); +} + +/** + * Submit handler for the regenerate datastream derivative form. + * + * @param array $form + * The Drupal form. + * @param array $form_state + * The Drupal form state. + */ +function islandora_regenerate_datastream_derivative_form_submit(array $form, array &$form_state) { + module_load_include('inc', 'islandora', 'includes/derivatives'); + $datastream = $form_state['datastream']; + $batch = islandora_regenerate_datastream_derivative_batch($datastream); + batch_set($batch); + $form_state['redirect'] = "islandora/object/{$datastream->parent->id}/manage/datastreams"; +} + +/** + * Regenerate all derivatives on an object. + * + * @param array $form + * The Drupal form. + * @param array $form_state + * The Drupal form state. + * @param AbstractObject $object + * The object that is having its derivatives regenerated. + * + * @return array + * The Drupal form definition. + */ +function islandora_regenerate_object_derivatives_form(array $form, array &$form_state, AbstractObject $object) { + $form_state['object'] = $object; + return confirm_form($form, + t('Are you sure you want to regenerate all the derivatives for %title?', array('%title' => $object->label)), + "islandora/object/{$object}/manage/properties", + t('This will create a new version for every datastream on the object. Please wait while this happens.'), + t('Regenerate'), + t('Cancel') + ); +} + +/** + * Submit handler for the regenerate object derivativse form. + * + * @param array $form + * The Drupal form. + * @param array $form_state + * The Drupal form state. + */ +function islandora_regenerate_object_derivatives_form_submit(array $form, array &$form_state) { + module_load_include('inc', 'islandora', 'includes/derivatives'); + $object = $form_state['object']; + $batch = islandora_regenerate_object_derivatives_batch($object); + batch_set($batch); + $form_state['redirect'] = "islandora/object/{$object}/manage/properties"; +} + +/** + * Creates a batch to go out and re-create all of the derivatives for an object. + * + * @param AbstractObject $object + * A AbstractObject representing an object within Fedora. + * + * @return array + * An array specifying the Drupal batch. + */ +function islandora_regenerate_object_derivatives_batch(AbstractObject $object) { + return array( + 'title' => t('Regenerating all derivatives for @label', array('@label' => $object->label)), + 'operations' => array( + array('islandora_regenerate_object_derivatives_batch_operation', array($object)), + ), + 'init_message' => t('Preparing to regenerate derivatives...'), + 'progress_message' => t('Time elapsed: @elapsed
Estimated time remaning @estimate.'), + 'error_message' => t('An error has occurred.'), + 'file' => drupal_get_path('module', 'islandora') . '/includes/regenerate_derivatives.form.inc', + ); +} + +/** + * Defines the regenerate object derivatives batch operation. + * + * @param AbstractObject $object + * A AbstractObject representing an object within Fedora. + * @param array $context + * An array containing the context of the current batch. + */ +function islandora_regenerate_object_derivatives_batch_operation(AbstractObject $object, &$context) { + module_load_include('inc', 'islandora', 'includes/derivatives'); + $logging_results = islandora_do_derivatives($object, array( + 'force' => TRUE, + )); + islandora_derivative_logging($logging_results); +} + +/** + * Creates a batch to go out and re-create the derivative for a datastream. + * + * @param AbstractDatastream $datastream + * A AbstractDatastream representing a datastream on an object within Fedora. + * + * @return array + * An array specifying the Drupal batch. + */ +function islandora_regenerate_datastream_derivative_batch(AbstractDatastream $datastream) { + return array( + 'title' => t('Regenerating derivatives for the @dsid datastream', array('@dsid' => $datastream->id)), + 'operations' => array( + array('islandora_regenerate_datastream_derivative_batch_operation', array($datastream)), + ), + 'init_message' => t('Preparing to regenerate derivatives...'), + 'progress_message' => t('Time elapsed: @elapsed
Estimated time remaning @estimate.'), + 'error_message' => t('An error has occurred.'), + 'file' => drupal_get_path('module', 'islandora') . '/includes/regenerate_derivatives.form.inc', + ); +} + +/** + * Defines the regenerate datastream derivative batch operation. + * + * @param AbstractDatastream $datastream + * A AbstractDatastream representing a datastream on an object within Fedora. + * @param array $context + * An array containing the context of the current batch. + */ +function islandora_regenerate_datastream_derivative_batch_operation(AbstractDatastream $datastream, &$context) { + module_load_include('inc', 'islandora', 'includes/derivatives'); + $logging_results = islandora_do_derivatives($datastream->parent, array( + 'force' => TRUE, + 'destination_dsid' => $datastream->id, + )); + islandora_derivative_logging($logging_results); +} diff --git a/islandora.module b/islandora.module index 17f13724..fb165f1e 100644 --- a/islandora.module +++ b/islandora.module @@ -36,6 +36,7 @@ define('ISLANDORA_MANAGE_PROPERTIES', 'manage object properties'); define('ISLANDORA_VIEW_DATASTREAM_HISTORY', 'view old datastream versions'); define('ISLANDORA_MANAGE_DELETED_OBJECTS', 'manage deleted objects'); define('ISLANDORA_REVERT_DATASTREAM', 'revert to old datastream'); +define('ISLANDORA_REGENERATE_DERIVATIVES', 'regenerate derivatives for an object'); // Hooks. @@ -209,6 +210,15 @@ function islandora_menu() { 'access callback' => 'islandora_object_access_callback', 'access arguments' => array(ISLANDORA_PURGE, 2), ); + $items['islandora/object/%islandora_object/regenerate'] = array( + 'title' => 'Regenerate all derivatives on an object', + 'file' => 'includes/regenerate_derivatives.form.inc', + 'page callback' => 'drupal_get_form', + 'page arguments' => array('islandora_regenerate_object_derivatives_form', 2), + 'type' => MENU_CALLBACK, + 'access callback' => 'islandora_object_access_callback', + 'access arguments' => array(ISLANDORA_REGENERATE_DERIVATIVES, 2), + ); $items['islandora/object/%islandora_object/manage/datastreams/add'] = array( 'title' => 'Add a datastream', 'file' => 'includes/add_datastream.form.inc', @@ -314,6 +324,16 @@ function islandora_menu() { 'access arguments' => array(ISLANDORA_VIEW_DATASTREAM_HISTORY, 4), 'load arguments' => array(2), ); + $items['islandora/object/%islandora_object/datastream/%islandora_datastream/regenerate'] = array( + 'title' => 'Regenrate datastream derivative', + 'page callback' => 'drupal_get_form', + 'page arguments' => array('islandora_regenerate_datastream_derivative_form', 4), + 'file' => 'includes/regenerate_derivatives.form.inc', + 'type' => MENU_CALLBACK, + 'access callback' => 'islandora_datastream_access', + 'access arguments' => array(ISLANDORA_REGENERATE_DERIVATIVES, 4), + 'load arguments' => array(2), + ); $items['islandora/object/%islandora_object/download_clip'] = array( 'page callback' => 'islandora_download_clip', 'page arguments' => array(2), @@ -460,6 +480,10 @@ function islandora_theme() { 'file' => 'theme/theme.inc', 'variables' => array('datastream' => NULL), ), + 'islandora_datastream_regenerate_link' => array( + 'file' => 'theme/theme.inc', + 'variables' => array('datastream' => NULL), + ), 'islandora_dublin_core_display' => array( 'file' => 'theme/theme.inc', 'template' => 'theme/islandora-dublin-core-display', @@ -534,6 +558,10 @@ function islandora_permission() { 'title' => t('Manage deleted objects'), 'description' => t('Purge or revert deleted objects.'), ), + ISLANDORA_REGENERATE_DERIVATIVES => array( + 'title' => t('Regenerate derivatives'), + 'description' => t('Regenerate derivatives for an object or per datastream.'), + ), ); } @@ -1069,7 +1097,7 @@ function islandora_default_islandora_view_object($object) { * * @param AbstractObject $object * The fedora object to print. - * @param unknown $alter + * @param string $alter * The string representation of the themed viewable object. * * @return array diff --git a/theme/theme.inc b/theme/theme.inc index 6a80fbb6..e057771c 100644 --- a/theme/theme.inc +++ b/theme/theme.inc @@ -25,7 +25,7 @@ function islandora_preprocess_islandora_default_edit(array &$variables) { $header[] = array('data' => t('Versions')); } - $header[] = array('data' => t('Operations'), 'colspan' => '3'); + $header[] = array('data' => t('Operations'), 'colspan' => '4'); $table_attributes = array('class' => array('manage-datastreams')); $rows = array(); @@ -79,6 +79,14 @@ function islandora_preprocess_islandora_default_edit(array &$variables) { 'datastream' => $ds, )), ); + if (user_access(ISLANDORA_REGENERATE_DERIVATIVES)) { + $row[] = array( + 'class' => 'datastream-regenerate', + 'data' => theme('islandora_datastream_regenerate_link', array( + 'datastream' => $ds, + )), + ); + } $rows[] = $row; } $caption = filter_xss($islandora_object->label) . ' - ' . $islandora_object->id; @@ -424,6 +432,27 @@ function theme_islandora_datastream_version_link(array $vars) { } } +/** + * Renders a link that will re-create derivatives for a datastream. + * + * @param array $vars + * An array containing: + * - datastream: An AbstractDatastream to generate the version link from. + * + * @return string + * Markup. + */ +function theme_islandora_datastream_regenerate_link(array $vars) { + $datastream = $vars['datastream']; + $object = $datastream->parent; + $hooks = islandora_invoke_hook_list(ISLANDORA_DERVIATIVE_CREATION_HOOK, $object->models, array($object)); + foreach ($hooks as $hook) { + if ($hook['destination_dsid'] == $datastream->id) { + return l(t('regenerate'), "islandora/object/{$datastream->parent->id}/datastream/{$datastream->id}/regenerate"); + } + } +} + /** * Implements hook_preprocess(). */ From 19bb4b34685fcff71842f27357e3fa9c69bf61d9 Mon Sep 17 00:00:00 2001 From: Jordan Dukart Date: Tue, 7 Jan 2014 19:31:38 +0000 Subject: [PATCH 02/16] Add byte range chunking support to Islandora. --- includes/datastream.inc | 150 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 144 insertions(+), 6 deletions(-) diff --git a/includes/datastream.inc b/includes/datastream.inc index 1d506e3d..72a86c5f 100644 --- a/includes/datastream.inc +++ b/includes/datastream.inc @@ -70,14 +70,30 @@ function islandora_view_datastream(AbstractDatastream $datastream, $download = F islandora_view_datastream_set_cache_headers($datastream); drupal_page_is_cacheable(FALSE); - // Try not to load the file into PHP memory! - // Close and flush ALL the output buffers! - while (@ob_end_flush()) { - }; // New content needed. if ($cache_check === 200) { - $datastream->getContent('php://output'); + // We need to see if the chunking is being requested. This will mainly + // happen with iOS video requests as they do not support any other way + // to receive content for playback. + $chunk_headers = FALSE; + if (isset($_SERVER['HTTP_RANGE'])) { + // Set headers specific to chunking. + $chunk_headers = islandora_view_datastream_set_chunk_headers($datastream); + } + // Try not to load the file into PHP memory! + // Close and flush ALL the output buffers! + while (@ob_end_flush()) { + }; + + if (isset($_SERVER['HTTP_RANGE'])) { + if ($chunk_headers) { + islandora_view_datastream_deliver_chunks($datastream, $chunk_headers); + } + } + else { + $datastream->getContent('php://output'); + } } exit(); } @@ -312,7 +328,7 @@ function islandora_edit_datastream(AbstractDatastream $datastream) { case 0: // No edit implementations. drupal_set_message(t('There are no edit methods specified for this datastream.')); - drupal_goto("islandora/object/{$object->id}/manage/datastreams"); + drupal_goto("islandora/object/{$datastream->parent->id}/manage/datastreams"); break; case 1: @@ -383,3 +399,125 @@ function islandora_datastream_get_view_link(AbstractDatastream $datastream) { 'datastream' => $datastream, )); } + +/** + * Set the headers for the chunking of our content. + * + * @param AbstractDatastream $datastream + * An AbstractDatastream representing a datastream on a Fedora object. + * + * @return bool + * TRUE if there are chunks to be returned, FALSE otherwise. + */ +function islandora_view_datastream_set_chunk_headers(AbstractDatastream $datastream) { + $file_uri = islandora_view_datastream_retrieve_file_uri($datastream); + // The meat of this has been taken from: + // http://mobiforge.com/design-development/content-delivery-mobile-devices. + $size = filesize($file_uri); + $length = $size; + $start = 0; + $end = $size - 1; + + header("Accept-Ranges: 0-$length"); + if (isset($_SERVER['HTTP_RANGE'])) { + $c_start = $start; + $c_end = $end; + // Extract the range string. + list(, $range) = explode('=', $_SERVER['HTTP_RANGE'], 2); + // Make sure the client hasn't sent us a multibyte range. + if (strpos($range, ',') !== FALSE) { + // Not a valid range, notify the client. + header('HTTP/1.1 416 Requested Range Not Satisfiable'); + header("Content-Range: bytes $start-$end/$size"); + exit; + } + // If the range starts with an '-' we start from the beginning. If not, we + // forward the file pointer and make sure to get the end byte if specified. + if (strpos($range, '-') === 0) { + // The n-number of the last bytes is requested. + $c_start = $size - substr($range, 1); + } + else { + $range = explode('-', $range); + $c_start = $range[0]; + $c_end = (isset($range[1]) && is_numeric($range[1])) ? $range[1] : $size; + } + /* Check the range and make sure it's treated according to the specs. + * http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html + */ + // End bytes can not be larger than $end. + $c_end = ($c_end > $end) ? $end : $c_end; + // Validate the requested range and return an error if it's not correct. + if ($c_start > $c_end || $c_start > $size - 1 || $c_end >= $size) { + header('HTTP/1.1 416 Requested Range Not Satisfiable'); + header("Content-Range: bytes $start-$end/$size"); + exit; + } + $start = $c_start; + $end = $c_end; + // Calculate new content length. + $length = $end - $start + 1; + header('HTTP/1.1 206 Partial Content'); + } + // Notify the client the byte range we'll be outputting. + header("Content-Range: bytes $start-$end/$size"); + header("Content-Length: $length"); + return array( + 'start' => $start, + 'end' => $end, + ); +} + +/** + * Deliver back the specified chunks of a file. + * + * @param AbstractDatastream $datastream + * An AbstractDatastream representing a datastream on a Fedora object. + * @param array $params + * An associate array containing the start and ending chunk bytes. + */ +function islandora_view_datastream_deliver_chunks(AbstractDatastream $datastream, $params) { + $file_uri = islandora_view_datastream_retrieve_file_uri($datastream); + // The meat of this has been taken from: + // http://mobiforge.com/design-development/content-delivery-mobile-devices. + $fp = @fopen($file_uri, 'rb'); + fseek($fp, $params['start']); + // Start buffered download. + $buffer = 1024 * 8; + while (!feof($fp) && ($p = ftell($fp)) <= $params['end']) { + if ($p + $buffer > $params['end']) { + // In case we're only outputting a chunk, make sure we don't read past the + // length. + $buffer = $params['end'] - $p + 1; + } + // Reset time limit for big files. + set_time_limit(0); + echo fread($fp, $buffer); + } + fclose($fp); +} + +/** + * Creates/returns the file URI for the content of a datastream for chunking. + * + * @param AbstractDatastream $datastream + * An AbstractDatastream representing a datastream on a Fedora object. + * + * @return string + * The URI of the file. + */ +function islandora_view_datastream_retrieve_file_uri(AbstractDatastream $datastream) { + $mime_detect = new MimeDetect(); + $extension = $mime_detect->getExtension($datastream->mimetype); + $file_uri = 'temporary://chunk_' . $datastream->parent->id . '_' . $datastream->id . '_' . $datastream->createdDate->getTimestamp() . '.' . $extension; + if (!file_exists($file_uri)) { + $file = new stdClass(); + $file->uri = $file_uri; + $file->filename = drupal_basename($file_uri); + $file->filemime = $datastream->mimeType; + $file->status = 0; + $datastream->getContent($file_uri); + file_save($file); + } + return $file_uri; +} From 545cc9d70898fb47254765ab8e4dbc6afa1c84fc Mon Sep 17 00:00:00 2001 From: Alan Stanley Date: Fri, 10 Jan 2014 14:15:41 -0400 Subject: [PATCH 03/16] added object for context in hook --- includes/breadcrumb.inc | 6 ++---- islandora.api.php | 4 +++- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/includes/breadcrumb.inc b/includes/breadcrumb.inc index 05ab1bac..6f975ae3 100644 --- a/includes/breadcrumb.inc +++ b/includes/breadcrumb.inc @@ -28,7 +28,7 @@ function islandora_get_breadcrumbs($object) { $breadcrumbs = islandora_get_breadcrumbs_recursive($object->id, $object->repository); array_pop($breadcrumbs); $context = 'islandora'; - drupal_alter('islandora_breadcrumbs', $breadcrumbs, $context); + drupal_alter('islandora_breadcrumbs', $breadcrumbs, $context, $object); return $breadcrumbs; } @@ -114,9 +114,7 @@ function islandora_get_breadcrumbs_recursive($pid, FedoraRepository $repository, // render the last two links and break (on the next pass). return array_merge( islandora_get_breadcrumbs_recursive($root, $repository, $context), - array( - '...', - ) + array('...') ); } } diff --git a/islandora.api.php b/islandora.api.php index 04d8bd49..1aec1198 100644 --- a/islandora.api.php +++ b/islandora.api.php @@ -691,8 +691,10 @@ function hook_islandora_update_related_objects_properties(AbstractObject $object * Breadcrumbs array to be altered by reference. Each element is markup. * @param string $context * Where the alter is originating from for distinguishing. + * @param AbstractObject $object + * AbstractObject representing object providing breadcrumb path */ -function hook_islandora_breadcrumbs_alter(&$breadcrumbs, $context) { +function hook_islandora_breadcrumbs_alter(&$breadcrumbs, $context, $object) { } From 035ee14fdadf053d4cbffef62c654ac2081322a1 Mon Sep 17 00:00:00 2001 From: Alan Stanley Date: Fri, 10 Jan 2014 14:18:21 -0400 Subject: [PATCH 04/16] added object for context in hook --- islandora.api.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/islandora.api.php b/islandora.api.php index 1aec1198..fb27a543 100644 --- a/islandora.api.php +++ b/islandora.api.php @@ -692,7 +692,7 @@ function hook_islandora_update_related_objects_properties(AbstractObject $object * @param string $context * Where the alter is originating from for distinguishing. * @param AbstractObject $object - * AbstractObject representing object providing breadcrumb path + * (Optional) AbstractObject representing object providing breadcrumb path */ function hook_islandora_breadcrumbs_alter(&$breadcrumbs, $context, $object) { From 9d4aa56e75a15e246bf5f5dfcda03c89eff4fa3d Mon Sep 17 00:00:00 2001 From: Alan Stanley Date: Fri, 10 Jan 2014 14:20:24 -0400 Subject: [PATCH 05/16] added object for context in hook --- islandora.api.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/islandora.api.php b/islandora.api.php index fb27a543..51aeb343 100644 --- a/islandora.api.php +++ b/islandora.api.php @@ -694,7 +694,7 @@ function hook_islandora_update_related_objects_properties(AbstractObject $object * @param AbstractObject $object * (Optional) AbstractObject representing object providing breadcrumb path */ -function hook_islandora_breadcrumbs_alter(&$breadcrumbs, $context, $object) { +function hook_islandora_breadcrumbs_alter(&$breadcrumbs, $context, $object = NULL) { } From 6405a453ce2306ec1744f68e51c03fc575ba8e37 Mon Sep 17 00:00:00 2001 From: Jordan Dukart Date: Fri, 10 Jan 2014 20:24:20 +0000 Subject: [PATCH 06/16] Batch each derivative operation individually. --- includes/derivatives.inc | 57 +++++++++++++++++++++++ includes/regenerate_derivatives.form.inc | 59 +++++++++++++++++++++--- 2 files changed, 110 insertions(+), 6 deletions(-) diff --git a/includes/derivatives.inc b/includes/derivatives.inc index 5ba4bb56..78f00aab 100644 --- a/includes/derivatives.inc +++ b/includes/derivatives.inc @@ -121,3 +121,60 @@ function islandora_derivative_logging(array $logging_results) { } } } + +/** + * Kicks off derivative functions based upon hooks and conditions. + * + * @param AbstractObject $object + * An AbstractObject representing a FedoraObject. + * @param array $options + * An array of parameters containing: + * - force: Bool denoting whether we are forcing the generation of + * derivatives. + * - source_dsid: (Optional) String of the datastream id we are generating + * 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. + * + * @return array + * An array of operations to be called from within a batch. + */ +function islandora_do_batch_derivatives(AbstractObject $object, array $options) { + module_load_include('inc', 'islandora', 'includes/utilities'); + $options += array( + 'force' => FALSE, + ); + $hooks = islandora_invoke_hook_list(ISLANDORA_DERVIATIVE_CREATION_HOOK, $object->models, array($object)); + 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']; + }); + } + + foreach ($hooks as $hook) { + $file = FALSE; + if (isset($hook['file'])) { + $file = $hook['file']; + } + foreach ($hook['function'] as $function) { + $operations[] = array('islandora_derivative_perform_batch_operation', array( + $function, + $file, + $object->id, + $options['force']), + ); + } + } + return $operations; +} diff --git a/includes/regenerate_derivatives.form.inc b/includes/regenerate_derivatives.form.inc index 917154ea..e9c1b986 100644 --- a/includes/regenerate_derivatives.form.inc +++ b/includes/regenerate_derivatives.form.inc @@ -97,13 +97,12 @@ function islandora_regenerate_object_derivatives_form_submit(array $form, array function islandora_regenerate_object_derivatives_batch(AbstractObject $object) { return array( 'title' => t('Regenerating all derivatives for @label', array('@label' => $object->label)), - 'operations' => array( - array('islandora_regenerate_object_derivatives_batch_operation', array($object)), - ), + 'operations' => islandora_do_batch_derivatives($object, array('force' => TRUE)), 'init_message' => t('Preparing to regenerate derivatives...'), 'progress_message' => t('Time elapsed: @elapsed
Estimated time remaning @estimate.'), 'error_message' => t('An error has occurred.'), 'file' => drupal_get_path('module', 'islandora') . '/includes/regenerate_derivatives.form.inc', + 'finished' => 'islandora_regenerate_derivative_batch_finished', ); } @@ -135,13 +134,15 @@ function islandora_regenerate_object_derivatives_batch_operation(AbstractObject function islandora_regenerate_datastream_derivative_batch(AbstractDatastream $datastream) { return array( 'title' => t('Regenerating derivatives for the @dsid datastream', array('@dsid' => $datastream->id)), - 'operations' => array( - array('islandora_regenerate_datastream_derivative_batch_operation', array($datastream)), - ), + 'operations' => islandora_do_batch_derivatives($datastream->parent, array( + 'force' => TRUE, + 'destination_dsid' => $datastream->id, + )), 'init_message' => t('Preparing to regenerate derivatives...'), 'progress_message' => t('Time elapsed: @elapsed
Estimated time remaning @estimate.'), 'error_message' => t('An error has occurred.'), 'file' => drupal_get_path('module', 'islandora') . '/includes/regenerate_derivatives.form.inc', + 'finished' => 'islandora_regenerate_derivative_batch_finished', ); } @@ -161,3 +162,49 @@ function islandora_regenerate_datastream_derivative_batch_operation(AbstractData )); islandora_derivative_logging($logging_results); } + +/** + * Wrapper to call out to batch operations. + * + * @param string $function + * The name of the function we are calling for derivatives. + * @param bool|string $file + * FALSE if there is no file to load, the path to require otherwise + * @param string $pid + * The pid of the object we are performing. + * @param bool $force + * Whether we are forcing derivative regeneration or not. + * @param array $context + * The context of the current batch operation. + */ +function islandora_derivative_perform_batch_operation($function, $file, $pid, $force, &$context) { + if ($file) { + require_once $file; + } + if (function_exists($function)) { + $logging = call_user_func($function, islandora_object_load($pid), $force); + if (!empty($logging)) { + $context['results']['logging'][] = $logging; + } + } + else { + watchdog('islandora', 'Unable to call derivative function @function as it was not found!', array('@function' => $function), WATCHDOG_ERROR); + } +} + +/** + * Finished function for derivative batch regeneration. + * + * @param array $success + * An array of success passed from the batch. + * @param array $results + * An array of results passed from the batch. + * @param array $operations + * An array of operations passed from the batch. + */ +function islandora_regenerate_derivative_batch_finished($success, $results, $operations) { + module_load_include('inc', 'islandora', 'includes/derivatives'); + if (!empty($results['logging'])) { + islandora_derivative_logging($results['logging']); + } +} From 6c340bfbecf1467ba95b33625342e1e1366e4baf Mon Sep 17 00:00:00 2001 From: Jordan Dukart Date: Mon, 13 Jan 2014 12:50:44 +0000 Subject: [PATCH 07/16] Remove superfluous functions. --- includes/regenerate_derivatives.form.inc | 33 ------------------------ 1 file changed, 33 deletions(-) diff --git a/includes/regenerate_derivatives.form.inc b/includes/regenerate_derivatives.form.inc index e9c1b986..5c0dcef7 100644 --- a/includes/regenerate_derivatives.form.inc +++ b/includes/regenerate_derivatives.form.inc @@ -106,22 +106,6 @@ function islandora_regenerate_object_derivatives_batch(AbstractObject $object) { ); } -/** - * Defines the regenerate object derivatives batch operation. - * - * @param AbstractObject $object - * A AbstractObject representing an object within Fedora. - * @param array $context - * An array containing the context of the current batch. - */ -function islandora_regenerate_object_derivatives_batch_operation(AbstractObject $object, &$context) { - module_load_include('inc', 'islandora', 'includes/derivatives'); - $logging_results = islandora_do_derivatives($object, array( - 'force' => TRUE, - )); - islandora_derivative_logging($logging_results); -} - /** * Creates a batch to go out and re-create the derivative for a datastream. * @@ -146,23 +130,6 @@ function islandora_regenerate_datastream_derivative_batch(AbstractDatastream $da ); } -/** - * Defines the regenerate datastream derivative batch operation. - * - * @param AbstractDatastream $datastream - * A AbstractDatastream representing a datastream on an object within Fedora. - * @param array $context - * An array containing the context of the current batch. - */ -function islandora_regenerate_datastream_derivative_batch_operation(AbstractDatastream $datastream, &$context) { - module_load_include('inc', 'islandora', 'includes/derivatives'); - $logging_results = islandora_do_derivatives($datastream->parent, array( - 'force' => TRUE, - 'destination_dsid' => $datastream->id, - )); - islandora_derivative_logging($logging_results); -} - /** * Wrapper to call out to batch operations. * From 63ce9e2b633f02086311e94c33b355bee13e1811 Mon Sep 17 00:00:00 2001 From: Jordan Dukart Date: Mon, 13 Jan 2014 19:03:36 +0000 Subject: [PATCH 08/16] Changes based on code review. --- includes/regenerate_derivatives.form.inc | 7 ++++--- theme/theme.inc | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/includes/regenerate_derivatives.form.inc b/includes/regenerate_derivatives.form.inc index 5c0dcef7..a7ef147c 100644 --- a/includes/regenerate_derivatives.form.inc +++ b/includes/regenerate_derivatives.form.inc @@ -62,7 +62,7 @@ function islandora_regenerate_object_derivatives_form(array $form, array &$form_ $form_state['object'] = $object; return confirm_form($form, t('Are you sure you want to regenerate all the derivatives for %title?', array('%title' => $object->label)), - "islandora/object/{$object}/manage/properties", + "islandora/object/{$object->id}/manage/properties", t('This will create a new version for every datastream on the object. Please wait while this happens.'), t('Regenerate'), t('Cancel') @@ -78,11 +78,10 @@ function islandora_regenerate_object_derivatives_form(array $form, array &$form_ * The Drupal form state. */ function islandora_regenerate_object_derivatives_form_submit(array $form, array &$form_state) { - module_load_include('inc', 'islandora', 'includes/derivatives'); $object = $form_state['object']; $batch = islandora_regenerate_object_derivatives_batch($object); batch_set($batch); - $form_state['redirect'] = "islandora/object/{$object}/manage/properties"; + $form_state['redirect'] = "islandora/object/{$object->id}/manage/properties"; } /** @@ -95,6 +94,7 @@ function islandora_regenerate_object_derivatives_form_submit(array $form, array * An array specifying the Drupal batch. */ function islandora_regenerate_object_derivatives_batch(AbstractObject $object) { + module_load_include('inc', 'islandora', 'includes/derivatives'); return array( 'title' => t('Regenerating all derivatives for @label', array('@label' => $object->label)), 'operations' => islandora_do_batch_derivatives($object, array('force' => TRUE)), @@ -116,6 +116,7 @@ function islandora_regenerate_object_derivatives_batch(AbstractObject $object) { * An array specifying the Drupal batch. */ function islandora_regenerate_datastream_derivative_batch(AbstractDatastream $datastream) { + module_load_include('inc', 'islandora', 'includes/derivatives'); return array( 'title' => t('Regenerating derivatives for the @dsid datastream', array('@dsid' => $datastream->id)), 'operations' => islandora_do_batch_derivatives($datastream->parent, array( diff --git a/theme/theme.inc b/theme/theme.inc index e057771c..0e62f0d7 100644 --- a/theme/theme.inc +++ b/theme/theme.inc @@ -53,7 +53,7 @@ function islandora_preprocess_islandora_default_edit(array &$variables) { 'class' => 'datastream-size', 'data' => islandora_datastream_get_human_readable_size($ds), ); - if (user_access(ISLANDORA_VIEW_DATASTREAM_HISTORY)) { + if (islandora_datastream_access(ISLANDORA_VIEW_DATASTREAM_HISTORY, $ds)) { $row[] = array( 'class' => 'datastream-versions', 'data' => theme('islandora_datastream_version_link', array( @@ -79,7 +79,7 @@ function islandora_preprocess_islandora_default_edit(array &$variables) { 'datastream' => $ds, )), ); - if (user_access(ISLANDORA_REGENERATE_DERIVATIVES)) { + if (islandora_datastream_access(ISLANDORA_REGENERATE_DERIVATIVES, $ds)) { $row[] = array( 'class' => 'datastream-regenerate', 'data' => theme('islandora_datastream_regenerate_link', array( From 91a379067af751e495411dac900e9731dd94704e Mon Sep 17 00:00:00 2001 From: Jordan Dukart Date: Tue, 14 Jan 2014 17:40:17 +0000 Subject: [PATCH 09/16] Only render the regenerate under certain conditions. --- theme/theme.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/theme/theme.inc b/theme/theme.inc index 0e62f0d7..2675ab7a 100644 --- a/theme/theme.inc +++ b/theme/theme.inc @@ -447,7 +447,7 @@ function theme_islandora_datastream_regenerate_link(array $vars) { $object = $datastream->parent; $hooks = islandora_invoke_hook_list(ISLANDORA_DERVIATIVE_CREATION_HOOK, $object->models, array($object)); foreach ($hooks as $hook) { - if ($hook['destination_dsid'] == $datastream->id) { + if (isset($hook['source_dsid']) && isset($object[$hook['source_dsid']]) && $hook['destination_dsid'] == $datastream->id) { return l(t('regenerate'), "islandora/object/{$datastream->parent->id}/datastream/{$datastream->id}/regenerate"); } } From 9a76642364e4977664f05cc4b0948c9b31ebb5bf Mon Sep 17 00:00:00 2001 From: Jordan Dukart Date: Tue, 14 Jan 2014 19:00:06 +0000 Subject: [PATCH 10/16] Make the rendering of the regenerate link respect things. --- islandora.module | 17 +++++++++++++++++ theme/theme.inc | 36 ++++++++++++++---------------------- 2 files changed, 31 insertions(+), 22 deletions(-) diff --git a/islandora.module b/islandora.module index fb165f1e..6537ef21 100644 --- a/islandora.module +++ b/islandora.module @@ -1829,3 +1829,20 @@ function islandora_islandora_metadata_display_info() { ), ); } + +/** + * Implements hook_islandora_datastream_access(). + */ +function islandora_islandora_datastream_access($op, AbstractDatastream $datastream, $user) { + if ($op == ISLANDORA_REGENERATE_DERIVATIVES) { + $object = $datastream->parent; + $hooks = islandora_invoke_hook_list(ISLANDORA_DERVIATIVE_CREATION_HOOK, $object->models, array($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))) { + return TRUE; + } + } + return FALSE; + } + return NULL; +} diff --git a/theme/theme.inc b/theme/theme.inc index 2675ab7a..2d3fe898 100644 --- a/theme/theme.inc +++ b/theme/theme.inc @@ -53,14 +53,12 @@ function islandora_preprocess_islandora_default_edit(array &$variables) { 'class' => 'datastream-size', 'data' => islandora_datastream_get_human_readable_size($ds), ); - if (islandora_datastream_access(ISLANDORA_VIEW_DATASTREAM_HISTORY, $ds)) { - $row[] = array( - 'class' => 'datastream-versions', - 'data' => theme('islandora_datastream_version_link', array( - 'datastream' => $ds, - )), - ); - } + $row[] = array( + 'class' => 'datastream-versions', + 'data' => theme('islandora_datastream_version_link', array( + 'datastream' => $ds, + )), + ); $row[] = array( 'class' => 'datastream-download', 'data' => theme('islandora_datastream_download_link', array( @@ -79,14 +77,12 @@ function islandora_preprocess_islandora_default_edit(array &$variables) { 'datastream' => $ds, )), ); - if (islandora_datastream_access(ISLANDORA_REGENERATE_DERIVATIVES, $ds)) { - $row[] = array( - 'class' => 'datastream-regenerate', - 'data' => theme('islandora_datastream_regenerate_link', array( - 'datastream' => $ds, - )), - ); - } + $row[] = array( + 'class' => 'datastream-regenerate', + 'data' => theme('islandora_datastream_regenerate_link', array( + 'datastream' => $ds, + )), + ); $rows[] = $row; } $caption = filter_xss($islandora_object->label) . ' - ' . $islandora_object->id; @@ -444,12 +440,8 @@ function theme_islandora_datastream_version_link(array $vars) { */ function theme_islandora_datastream_regenerate_link(array $vars) { $datastream = $vars['datastream']; - $object = $datastream->parent; - $hooks = islandora_invoke_hook_list(ISLANDORA_DERVIATIVE_CREATION_HOOK, $object->models, array($object)); - foreach ($hooks as $hook) { - if (isset($hook['source_dsid']) && isset($object[$hook['source_dsid']]) && $hook['destination_dsid'] == $datastream->id) { - return l(t('regenerate'), "islandora/object/{$datastream->parent->id}/datastream/{$datastream->id}/regenerate"); - } + if (islandora_datastream_access(ISLANDORA_REGENERATE_DERIVATIVES, $datastream)) { + return l(t('regenerate'), "islandora/object/{$datastream->parent->id}/datastream/{$datastream->id}/regenerate"); } } From 12620a4bf6bdc98b17532418adf9158225a58c57 Mon Sep 17 00:00:00 2001 From: Jordan Dukart Date: Tue, 14 Jan 2014 19:14:24 +0000 Subject: [PATCH 11/16] Make our datastream_access call work and revert change to themeing of version links. --- islandora.module | 2 +- theme/theme.inc | 14 ++++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/islandora.module b/islandora.module index 6537ef21..9c1ae7d9 100644 --- a/islandora.module +++ b/islandora.module @@ -1839,7 +1839,7 @@ function islandora_islandora_datastream_access($op, AbstractDatastream $datastre $hooks = islandora_invoke_hook_list(ISLANDORA_DERVIATIVE_CREATION_HOOK, $object->models, array($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))) { - return TRUE; + return user_access(ISLANDORA_REGENERATE_DERIVATIVES); } } return FALSE; diff --git a/theme/theme.inc b/theme/theme.inc index 2d3fe898..b783cfa8 100644 --- a/theme/theme.inc +++ b/theme/theme.inc @@ -53,12 +53,14 @@ function islandora_preprocess_islandora_default_edit(array &$variables) { 'class' => 'datastream-size', 'data' => islandora_datastream_get_human_readable_size($ds), ); - $row[] = array( - 'class' => 'datastream-versions', - 'data' => theme('islandora_datastream_version_link', array( - 'datastream' => $ds, - )), - ); + if (islandora_datastream_access(ISLANDORA_VIEW_DATASTREAM_HISTORY, $ds)) { + $row[] = array( + 'class' => 'datastream-versions', + 'data' => theme('islandora_datastream_version_link', array( + 'datastream' => $ds, + )), + ); + } $row[] = array( 'class' => 'datastream-download', 'data' => theme('islandora_datastream_download_link', array( From 587c4e22aee5ac40c496ba159ede812201bf9d33 Mon Sep 17 00:00:00 2001 From: Jordan Dukart Date: Tue, 14 Jan 2014 19:24:17 +0000 Subject: [PATCH 12/16] Fix a bug where user_access/namespaces were not getting checked for datastreams. --- islandora.module | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/islandora.module b/islandora.module index 9c1ae7d9..8173df52 100644 --- a/islandora.module +++ b/islandora.module @@ -1834,15 +1834,22 @@ function islandora_islandora_metadata_display_info() { * Implements hook_islandora_datastream_access(). */ function islandora_islandora_datastream_access($op, AbstractDatastream $datastream, $user) { - if ($op == ISLANDORA_REGENERATE_DERIVATIVES) { + module_load_include('inc', 'islandora', 'includes/utilities'); + $result = islandora_namespace_accessible($datastream->parent->id) && user_access($op, $user); + + if ($result && $op == ISLANDORA_REGENERATE_DERIVATIVES) { + $applicable_hook = FALSE; $object = $datastream->parent; $hooks = islandora_invoke_hook_list(ISLANDORA_DERVIATIVE_CREATION_HOOK, $object->models, array($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))) { - return user_access(ISLANDORA_REGENERATE_DERIVATIVES); + $applicable_hook = TRUE; + break; } } - return FALSE; + if (!$applicable_hook) { + $result = FALSE; + } } - return NULL; + return $result; } From 6e7a659f51af66830226a14437515ebb51a7c37e Mon Sep 17 00:00:00 2001 From: Jordan Dukart Date: Wed, 15 Jan 2014 19:18:27 +0000 Subject: [PATCH 13/16] Update some derivative tests and add a filter function to smarterly filter. --- includes/derivatives.inc | 77 ++++++++++++++++--------- includes/object_properties.form.inc | 5 +- islandora.module | 4 +- tests/derivatives.test | 31 +++++++++- tests/islandora_derivatives_test.module | 9 ++- 5 files changed, 90 insertions(+), 36 deletions(-) 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..dd5dd206 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_some_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_some_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'); + } } /** From c75bc9a0292de051fa8a49acfc8367486d790acd Mon Sep 17 00:00:00 2001 From: Jordan Dukart Date: Wed, 15 Jan 2014 19:20:08 +0000 Subject: [PATCH 14/16] Change the testing function name. --- tests/islandora_derivatives_test.module | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/islandora_derivatives_test.module b/tests/islandora_derivatives_test.module index dd5dd206..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_stanley_datastream', + 'islandora_derivatives_test_create_stanley_datastream', ), ), array( @@ -88,7 +88,7 @@ function islandora_derivatives_test_create_deriv_datastream(AbstractObject $obje * @param bool $force * Whether the derivatives are being forcefully generated or not. */ -function islandora_derivatives_test_create_some_stanley_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'; From d35c4826120752709b33f492f2e451bac0972c8b Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Thu, 16 Jan 2014 20:12:00 +0000 Subject: [PATCH 15/16] Avoid adding on duplicate extensions. As we often use filenames for datastream labels, and then use the labels to generate file names for download, we were ending up with file extensions being doubled up. --- includes/datastream.inc | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/includes/datastream.inc b/includes/datastream.inc index 72a86c5f..26a7a8d5 100644 --- a/includes/datastream.inc +++ b/includes/datastream.inc @@ -53,8 +53,16 @@ function islandora_view_datastream(AbstractDatastream $datastream, $download = F if ($download) { // Browsers will not append all extensions. $mime_detect = new MimeDetect(); - $extension = $mime_detect->getExtension($datastream->mimetype); - $filename = $datastream->label . '.' . $extension; + $extension = '.' . $mime_detect->getExtension($datastream->mimetype); + + // Prevent adding on a duplicate extension. + $extension_length = strlen($extension); + $duplicate_extension_position = strripos($datastream->label, $extension, -$extension_length); + $filename = $datastream->label; + if ($duplicate_extension_position === FALSE) { + $filename .= $extension; + } + header("Content-Disposition: attachment; filename=\"$filename\""); } From 4df5398b711a49e77c1868e5c47ddb9876cf449c Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Mon, 20 Jan 2014 14:01:05 +0000 Subject: [PATCH 16/16] Fix warning when the datastream label was shorter than the extension. --- includes/datastream.inc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/includes/datastream.inc b/includes/datastream.inc index 26a7a8d5..ba2872c2 100644 --- a/includes/datastream.inc +++ b/includes/datastream.inc @@ -56,9 +56,12 @@ function islandora_view_datastream(AbstractDatastream $datastream, $download = F $extension = '.' . $mime_detect->getExtension($datastream->mimetype); // Prevent adding on a duplicate extension. + $label = $datastream->label; $extension_length = strlen($extension); - $duplicate_extension_position = strripos($datastream->label, $extension, -$extension_length); - $filename = $datastream->label; + $duplicate_extension_position = strlen($label) > $extension_length ? + strripos($label, $extension, -$extension_length) : + FALSE; + $filename = $label; if ($duplicate_extension_position === FALSE) { $filename .= $extension; }