From f1a47a6d4334317c1d42f22dc569d8c6a8fd0878 Mon Sep 17 00:00:00 2001 From: qadan Date: Mon, 12 Aug 2013 18:18:16 +0000 Subject: [PATCH 1/9] added the fancy image assertion back in --- tests/islandora_web_test_case.inc | 43 ++++++++++++++++++++++++++----- 1 file changed, 36 insertions(+), 7 deletions(-) diff --git a/tests/islandora_web_test_case.inc b/tests/islandora_web_test_case.inc index baf7a8ee..722bdc7b 100644 --- a/tests/islandora_web_test_case.inc +++ b/tests/islandora_web_test_case.inc @@ -168,16 +168,45 @@ class IslandoraWebTestCase extends DrupalWebTestCase { if (!is_object($object)) { $this->fail("Failed. Object passed in is invalid.", 'Islandora'); } - - foreach ($datastreams as $datastream) { - if (isset($object[$datastream])) { - $this->pass("Loaded datastream {$datastream} from PID {$object->id}.", 'Islandora'); - } - else { - $this->fail("Failed to load datastream {$datastream} from PID {$object->id}.", 'Islandora'); + else { + foreach ($datastreams as $datastream) { + if (isset($object[$datastream])) { + $this->pass("Loaded datastream {$datastream} from PID {$object->id}.", 'Islandora'); + } + else { + $this->fail("Failed to load datastream {$datastream} from PID {$object->id}.", 'Islandora'); + } } } + } + /** + * Asserts that an object's given datastreams are common-type image files. + * + * Uses PHPGD to run the assertion check. This means that only certain kinds + * of image files can be checked. Please check the documentation for the PHPGD + * imagecreatefromstring() function to determine what filetypes are valid. + * + * @param AbstractObject $object + * The PID of the object. + * @param array $datastreams + * An array of datastreams to check. + */ + public function assertImageDatastreams($object, array $datastreams) { + if (!is_object($object)) { + $this->fail("Failed. Object passed in is invalid.", 'Islandora'); + } + else { + foreach ($datastreams as $datastream) { + $datastream_string = $object[$datastream]->content; + if (!imagecreatefromstring($datastream_string)) { + $this->fail("Image datastream {$datastream} is either invalid or corrupt.", 'Islandora'); + } + else { + $this->pass("Image datastream {$datastream} is valid.", 'Islandora'); + } + } + } } /** From 926fd23f554d7c3995d432de4ed18d9c8b565618 Mon Sep 17 00:00:00 2001 From: qadan Date: Mon, 12 Aug 2013 18:27:46 +0000 Subject: [PATCH 2/9] peace offering for Travis --- tests/islandora_web_test_case.inc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/islandora_web_test_case.inc b/tests/islandora_web_test_case.inc index 722bdc7b..be3b3b7e 100644 --- a/tests/islandora_web_test_case.inc +++ b/tests/islandora_web_test_case.inc @@ -188,9 +188,9 @@ class IslandoraWebTestCase extends DrupalWebTestCase { * imagecreatefromstring() function to determine what filetypes are valid. * * @param AbstractObject $object - * The PID of the object. + * The PID of the object. * @param array $datastreams - * An array of datastreams to check. + * An array of datastreams to check. */ public function assertImageDatastreams($object, array $datastreams) { if (!is_object($object)) { From 25e53892154d7f7888a61210fb81ceb7fb5de379 Mon Sep 17 00:00:00 2001 From: Jordan Dukart Date: Tue, 20 Aug 2013 14:57:34 +0000 Subject: [PATCH 3/9] Make watchdogs watchdog. --- includes/derivatives.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/derivatives.inc b/includes/derivatives.inc index 4a422535..fbf4b9e7 100644 --- a/includes/derivatives.inc +++ b/includes/derivatives.inc @@ -98,7 +98,7 @@ function islandora_derivative_logging(array $logging_results) { // message and the substitutions needed. We are using // call_user_func until such time as the @ignore changes // are merged into the standard release for Coder. - call_user_func('watchdog', $message['message'], isset($message['message_sub']) ? $message['message_sub'] : array(), isset($message['severity']) ? $message['severity'] : WATCHDOG_NOTICE); + call_user_func('watchdog', 'islandora_derivatives', $message['message'], isset($message['message_sub']) ? $message['message_sub'] : array(), isset($message['severity']) ? $message['severity'] : WATCHDOG_NOTICE); } } } From d6e67c7b34bdabe8c2157f6f9fb6593f59ed2062 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Tue, 20 Aug 2013 17:45:53 +0000 Subject: [PATCH 4/9] Document a feature of hooked access, due to hook invocation. --- islandora.api.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/islandora.api.php b/islandora.api.php index fe12ea21..6d25bae3 100644 --- a/islandora.api.php +++ b/islandora.api.php @@ -477,10 +477,11 @@ function hook_CMODEL_PID_islandora_ingest_steps(array $form_state) { * @param object $user * A loaded user object, as the global $user variable might contain. * - * @return bool|NULL + * @return bool|NULL|array * Either boolean TRUE or FALSE to explicitly allow or deny the operation on * the given object, or NULL to indicate that we are making no assertion - * about the outcome. + * about the outcome. Can also be an array containing multiple + * TRUE/FALSE/NULLs, due to how hooks work. */ function hook_islandora_object_access($op, $object, $user) { switch ($op) { @@ -515,10 +516,11 @@ function hook_CMODEL_PID_islandora_object_access($op, $object, $user) { * @param object $user * A loaded user object, as the global $user variable might contain. * - * @return bool|NULL + * @return bool|NULL|array * Either boolean TRUE or FALSE to explicitly allow or deny the operation on * the given object, or NULL to indicate that we are making no assertion - * about the outcome. + * about the outcome. Can also be an array containing multiple + * TRUE/FALSE/NULLs, due to how hooks work. */ function hook_islandora_datastream_access($op, $object, $user) { switch ($op) { From 798312bedb9a2990fd862fc58f033814ad0c5307 Mon Sep 17 00:00:00 2001 From: Christian Selig Date: Wed, 21 Aug 2013 19:46:02 +0000 Subject: [PATCH 5/9] Added ability to revert to previous versions of datastreams. --- includes/datastream.version.inc | 90 ++++++++++++++++++++++++++++++++- islandora.module | 20 ++++++++ theme/theme.inc | 35 +++++++++++++ 3 files changed, 144 insertions(+), 1 deletion(-) diff --git a/includes/datastream.version.inc b/includes/datastream.version.inc index 1e7c0830..23ac870b 100644 --- a/includes/datastream.version.inc +++ b/includes/datastream.version.inc @@ -18,7 +18,7 @@ function islandora_datastream_version_table($datastream) { $header[] = array('data' => t('Size')); $header[] = array('data' => t('Label')); $header[] = array('data' => t('Mime type')); - $header[] = array('data' => t('Operations')); + $header[] = array('data' => t('Operations'), 'colspan' => '2'); $rows = array(); foreach ($datastream as $version => $datastream_version) { @@ -50,6 +50,13 @@ function islandora_datastream_version_table($datastream) { 'version' => $version, )), ); + $row[] = array( + 'class' => 'datastream-revert', + 'data' => theme('islandora_datastream_revert_link', array( + 'datastream' => $datastream, + 'version' => $version, + )), + ); $rows[] = $row; } @@ -125,3 +132,84 @@ function islandora_delete_datastream_version_form_submit(array $form, array &$fo $form_state['redirect'] = "islandora/object/{$object->id}/datastream/{$datastream->id}/version"; } + +/** + * The admin revert datastream form. + * + * @param array $form + * The Drupal form. + * @param array $form_state + * The Drupal form state. + * @param AbstractDatastream $datastream + * The datastream to be deleted. + * @param string $version + * The version number of the datastream we are trying to delete. + * + * @return array + * The drupal form definition. + */ +function islandora_revert_datastream_version_form(array $form, array &$form_state, AbstractDatastream $datastream, $version) { + if (!isset($datastream[$version]) || count($datastream) < 2) { + return drupal_not_found(); + } + + $form_state['dsid'] = $datastream->id; + $form_state['object_id'] = $datastream->parent->id; + $form_state['version'] = $version; + + return confirm_form($form, + t('Are you sure you want to revert to version @version of the @dsid datastream?', array('@dsid' => $datastream->id, '@version' => $version)), + "islandora/object/{$datastream->parent->id}", + "", + t('Revert'), + t('Cancel') + ); +} + +/** + * Submit handler for the revert datastream form. + * + * Purges/Delete's the given AbstractDatastream if possible. + * + * The ISLANDORA_PRE_PURGE_DATASTREAM_HOOK will query other modules as to + * whether the given FedoraDatastream + * should be: blocked from purging; state set to 'Deleted'; or purged. + * + * @param array $form + * The Drupal form. + * @param array $form_state + * The Drupal form state. + */ +function islandora_revert_datastream_version_form_submit(array $form, array &$form_state) { + $islandora_object = islandora_object_load($form_state['object_id']); + + $datastream_to_revert = $islandora_object[$form_state['dsid']]; + $version = $form_state['version']; + + // Create file holding specified datastream version, and set datastream to it. + $datastream_to_revert_to = $datastream_to_revert[$version]; + if (in_array($datastream_to_revert->controlGroup, array('R', 'E'))) { + $datastream_to_revert->url = $datastream_to_revert_to->url; + } + else { + $filename = file_create_filename('datastream_temp_file', '.'); + $datastream_to_revert_to->getContent($filename); + $datastream_to_revert->setContentFromFile($filename); + file_unmanaged_delete($filename); + } + + if ($datastream_to_revert->mimeType != $datastream_to_revert_to->mimeType) { + $datastream_to_revert->mimeType = $datastream_to_revert_to->mimeType; + } + if ($datastream_to_revert->label != $datastream_to_revert_to->label) { + $datastream_to_revert->label = $datastream_to_revert_to->label; + } + + drupal_set_message(t('%d datastream successfully reverted to version %v for Islandora object %o', array( + '%d' => $datastream_to_revert->id, + '%v' => $version, + '%o' => $islandora_object->label))); + + $form_state['redirect'] = "islandora/object/{$islandora_object->id}/datastream/{$datastream_to_revert->id}/version"; +} + diff --git a/islandora.module b/islandora.module index 96fd5f0d..465719d9 100644 --- a/islandora.module +++ b/islandora.module @@ -34,6 +34,7 @@ define('ISLANDORA_INGEST', 'ingest fedora objects'); define('ISLANDORA_PURGE', 'delete fedora objects and datastreams'); define('ISLANDORA_MANAGE_PROPERTIES', 'manage object properties'); define('ISLANDORA_VIEW_DATASTREAM_HISTORY', 'view old datastream versions'); +define('ISLANDORA_REVERT_DATASTREAM', 'revert to old datastream'); // Hooks. define('ISLANDORA_VIEW_HOOK', 'islandora_view_object'); @@ -276,6 +277,16 @@ function islandora_menu() { 'access arguments' => array(ISLANDORA_PURGE, 4), 'load arguments' => array(2), ); + $items['islandora/object/%islandora_object/datastream/%islandora_datastream/version/%/revert'] = array( + 'title' => 'Revert to datastream version', + 'page arguments' => array('islandora_revert_datastream_version_form', 4, 6), + 'page callback' => 'drupal_get_form', + 'file' => 'includes/datastream.version.inc', + 'type' => MENU_CALLBACK, + 'access callback' => 'islandora_datastream_access', + 'access arguments' => array(ISLANDORA_REVERT_DATASTREAM, 4), + 'load arguments' => array(2), + ); $items['islandora/object/%islandora_object/datastream/%islandora_datastream/version/%/view'] = array( 'title' => 'View datastream version', 'page callback' => 'islandora_view_datastream', @@ -380,6 +391,10 @@ function islandora_theme() { 'file' => 'theme/theme.inc', 'variables' => array('datastream' => NULL, 'version' => NULL), ), + 'islandora_datastream_revert_link' => array( + 'file' => 'theme/theme.inc', + 'variables' => array('datastream' => NULL, 'version' => NULL), + ), 'islandora_datastream_view_link' => array( 'file' => 'theme/theme.inc', 'variables' => array( @@ -432,6 +447,10 @@ function islandora_permission() { 'title' => t('View datastream history'), 'description' => t('View all previous versions of a datastream.'), ), + ISLANDORA_REVERT_DATASTREAM => array( + 'title' => t('Revert datastream history'), + 'description' => t('Revert to a previous version of a datastream.'), + ), ); } @@ -1552,3 +1571,4 @@ function islandora_islandora_datastream_modified(AbstractObject $object, Abstrac )); islandora_derivative_logging($logging_results); } + diff --git a/theme/theme.inc b/theme/theme.inc index 10e1cd29..863ef901 100644 --- a/theme/theme.inc +++ b/theme/theme.inc @@ -339,6 +339,40 @@ function theme_islandora_datastream_delete_link(array $vars) { } } +/** + * Renders a link to allow replacing of a datatream. + * + * @param array $vars + * An array containing: + * - datastream: An AbstractDatastream for which to generate a revert link. + * - version: (optional) the version of the datastream to revert. + * + * @return string + * Markup containing the url to delete a datastream, or empty if inaccessible. + */ +function theme_islandora_datastream_revert_link(array $vars) { + $datastream = $vars['datastream']; + + $can_revert = islandora_datastream_access(ISLANDORA_REVERT_DATASTREAM, $datastream); + + if ($vars['version'] !== NULL) { + if (count($datastream) == 1) { + $can_revert = FALSE; + } + $link = "islandora/object/{$datastream->parent->id}/datastream/{$datastream->id}/version/{$vars['version']}/revert"; + } + else { + $link = "islandora/object/{$datastream->parent->id}/datastream/{$datastream->id}/revert"; + } + + if ($can_revert) { + return l(t('revert'), $link); + } + else { + ''; + } +} + /** * Renders a link to allow editing of a datatream. * @@ -389,3 +423,4 @@ function theme_islandora_datastream_version_link(array $vars) { return ''; } } + From 9b04972ddb6a7740e03b27935fe81c1b9ea74248 Mon Sep 17 00:00:00 2001 From: Christian Selig Date: Thu, 22 Aug 2013 16:40:41 +0000 Subject: [PATCH 6/9] Fixed some small issues including improper comments, not returning properly, unnecessary code and putting the file in temporary:// for safety. --- includes/datastream.version.inc | 10 +++------- theme/theme.inc | 5 +---- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/includes/datastream.version.inc b/includes/datastream.version.inc index 23ac870b..9a72b52d 100644 --- a/includes/datastream.version.inc +++ b/includes/datastream.version.inc @@ -143,7 +143,7 @@ function islandora_delete_datastream_version_form_submit(array $form, array &$fo * @param AbstractDatastream $datastream * The datastream to be deleted. * @param string $version - * The version number of the datastream we are trying to delete. + * The version number of the datastream we are trying to revert to. * * @return array * The drupal form definition. @@ -169,11 +169,7 @@ function islandora_revert_datastream_version_form(array $form, array &$form_stat /** * Submit handler for the revert datastream form. * - * Purges/Delete's the given AbstractDatastream if possible. - * - * The ISLANDORA_PRE_PURGE_DATASTREAM_HOOK will query other modules as to - * whether the given FedoraDatastream - * should be: blocked from purging; state set to 'Deleted'; or purged. + * Reverts the given AbstractDatastream if possible. * * @param array $form * The Drupal form. @@ -192,7 +188,7 @@ function islandora_revert_datastream_version_form_submit(array $form, array &$fo $datastream_to_revert->url = $datastream_to_revert_to->url; } else { - $filename = file_create_filename('datastream_temp_file', '.'); + $filename = file_create_filename('datastream_temp_file', 'temporary://'); $datastream_to_revert_to->getContent($filename); $datastream_to_revert->setContentFromFile($filename); file_unmanaged_delete($filename); diff --git a/theme/theme.inc b/theme/theme.inc index 863ef901..ed4610fb 100644 --- a/theme/theme.inc +++ b/theme/theme.inc @@ -361,15 +361,12 @@ function theme_islandora_datastream_revert_link(array $vars) { } $link = "islandora/object/{$datastream->parent->id}/datastream/{$datastream->id}/version/{$vars['version']}/revert"; } - else { - $link = "islandora/object/{$datastream->parent->id}/datastream/{$datastream->id}/revert"; - } if ($can_revert) { return l(t('revert'), $link); } else { - ''; + return ''; } } From 962f7f46d9d2134c6f51d90a403580b41a768fde Mon Sep 17 00:00:00 2001 From: Christian Selig Date: Thu, 22 Aug 2013 17:05:47 +0000 Subject: [PATCH 7/9] Removed unnecessary newlines. --- includes/datastream.version.inc | 1 - islandora.module | 1 - theme/theme.inc | 1 - 3 files changed, 3 deletions(-) diff --git a/includes/datastream.version.inc b/includes/datastream.version.inc index 9a72b52d..507a182f 100644 --- a/includes/datastream.version.inc +++ b/includes/datastream.version.inc @@ -208,4 +208,3 @@ function islandora_revert_datastream_version_form_submit(array $form, array &$fo $form_state['redirect'] = "islandora/object/{$islandora_object->id}/datastream/{$datastream_to_revert->id}/version"; } - diff --git a/islandora.module b/islandora.module index 465719d9..1a53aebf 100644 --- a/islandora.module +++ b/islandora.module @@ -1571,4 +1571,3 @@ function islandora_islandora_datastream_modified(AbstractObject $object, Abstrac )); islandora_derivative_logging($logging_results); } - diff --git a/theme/theme.inc b/theme/theme.inc index ed4610fb..92878555 100644 --- a/theme/theme.inc +++ b/theme/theme.inc @@ -420,4 +420,3 @@ function theme_islandora_datastream_version_link(array $vars) { return ''; } } - From 2bb19dbac844a8d1bfeaee7767c56afb542001fb Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Thu, 22 Aug 2013 17:46:42 +0000 Subject: [PATCH 8/9] Set global to prevent query logging when outputting datastreams. --- includes/datastream.inc | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/includes/datastream.inc b/includes/datastream.inc index da55dcd5..585fa2f5 100644 --- a/includes/datastream.inc +++ b/includes/datastream.inc @@ -30,6 +30,13 @@ function islandora_download_datastream(AbstractDatastream $datastream) { * The version of the datastream to display */ function islandora_view_datastream(AbstractDatastream $datastream, $download = FALSE, $version = NULL) { + // XXX: Certain features of the Devel module rely on the use of "shutdown + // handlers", such as query logging... The problem is that they might blindly + // add additional output which will break things if what is actually being + // output is anything but a webpage... like an image or video or audio or + // whatever the datastream is here. + $GLOBALS['devel_shutdown'] = FALSE; + if ($version !== NULL) { if (isset($datastream[$version])) { $datastream = $datastream[$version]; From 2ebbf211e7514fe49c20530f1ee171ae08562c9c Mon Sep 17 00:00:00 2001 From: Christian Selig Date: Thu, 22 Aug 2013 19:00:20 +0000 Subject: [PATCH 9/9] Added additional condition to prevent variable from being accessed when it hasn't been set. --- theme/theme.inc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/theme/theme.inc b/theme/theme.inc index 92878555..895ec8fe 100644 --- a/theme/theme.inc +++ b/theme/theme.inc @@ -361,6 +361,9 @@ function theme_islandora_datastream_revert_link(array $vars) { } $link = "islandora/object/{$datastream->parent->id}/datastream/{$datastream->id}/version/{$vars['version']}/revert"; } + else { + $can_revert = FALSE; + } if ($can_revert) { return l(t('revert'), $link);