From b482b405303d1e2a97f1d3196ee53eb5712a88fa Mon Sep 17 00:00:00 2001 From: Jordan Dukart Date: Mon, 5 Aug 2013 16:55:45 +0000 Subject: [PATCH 01/38] Initial commit for feedback. --- islandora.module | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/islandora.module b/islandora.module index 8a8ee6be..b8109a21 100644 --- a/islandora.module +++ b/islandora.module @@ -52,6 +52,7 @@ define('ISLANDORA_DATASTREAM_INGESTED_HOOK', 'islandora_datastream_ingested'); define('ISLANDORA_DATASTREAM_MODIFIED_HOOK', 'islandora_datastream_modified'); define('ISLANDORA_DATASTREAM_PURGED_HOOK', 'islandora_datastream_purged'); define('ISLANDORA_INGEST_STEP_HOOK', 'islandora_ingest_steps'); +define('ISLANDORA_DERVIATIVE_CREATION_HOOK', 'islandora_derivative'); // Autocomplete paths. define('ISLANDORA_CONTENT_MODELS_AUTOCOMPLETE', 'islandora/autocomplete/content-models'); @@ -1415,3 +1416,36 @@ function islandora_islandora_basic_collection_get_query_filters() { )); } } + +/** + * Implements hook_islandora_datastream_ingested(). + */ +function islandora_islandora_datastream_ingested(AbstractObject $object, AbstractDatastream $datastream) { + $logging_results = islandora_do_derivatives($object); + dd($logging_results, 'logging'); + foreach ($logging_results as $result) { + + } +} + +function islandora_do_derivatives(AbstractObject $object, $force = FALSE, $all = TRUE) { + $hooks = islandora_invoke_hook_list(ISLANDORA_DERVIATIVE_CREATION_HOOK, $object->models, array()); + uasort($hooks, 'drupal_sort_weight'); + $results = array(); + dd($hooks, 'hooks'); + foreach ($hooks as $hook) { + if (!$all && !isset($hook['source_dsid'])) { + continue; + } + if (isset($hook['file'])) { + require_once($hook['file']); + } + foreach ($hook['function'] as $function) { + $logging = call_user_func($function, $object, $force); + if (!empty($logging)) { + $results[] = $logging; + } + } + } + return $results; +} \ No newline at end of file From 5bafbd8151986410929a33e7fe2607a0c8a45694 Mon Sep 17 00:00:00 2001 From: Jordan Dukart Date: Wed, 7 Aug 2013 13:40:14 +0000 Subject: [PATCH 02/38] Updated with tests. --- islandora.info | 1 + islandora.module | 66 +++++++-- tests/derivatives.test | 185 ++++++++++++++++++++++++ tests/islandora_derivatives_test.info | 7 + tests/islandora_derivatives_test.module | 114 +++++++++++++++ 5 files changed, 362 insertions(+), 11 deletions(-) create mode 100644 tests/derivatives.test create mode 100644 tests/islandora_derivatives_test.info create mode 100644 tests/islandora_derivatives_test.module diff --git a/islandora.info b/islandora.info index fea3ead2..4b48bc29 100644 --- a/islandora.info +++ b/islandora.info @@ -18,4 +18,5 @@ files[] = tests/ingest.test files[] = tests/hooked_access.test files[] = tests/islandora_manage_permissions.test files[] = tests/datastream_versions.test +files[] = tests/derivatives.test php = 5.3 diff --git a/islandora.module b/islandora.module index b8109a21..989dd36e 100644 --- a/islandora.module +++ b/islandora.module @@ -1421,31 +1421,75 @@ function islandora_islandora_basic_collection_get_query_filters() { * Implements hook_islandora_datastream_ingested(). */ function islandora_islandora_datastream_ingested(AbstractObject $object, AbstractDatastream $datastream) { - $logging_results = islandora_do_derivatives($object); - dd($logging_results, 'logging'); - foreach ($logging_results as $result) { + $logging_results = islandora_do_derivatives($object, array( + 'source_dsid' => $datastream->id, + )); + islandora_derivative_logging($logging_results); +} - } +/** + * Implements hook_islandora_datastream_modified(). + */ +function islandora_islandora_datastream_modified(AbstractObject $object, AbstractDatastream $datastream) { + $logging_results = islandora_do_derivatives($object, array( + 'source_dsid' => $datastream->id, + 'force' => TRUE, + )); + islandora_derivative_logging($logging_results); } -function islandora_do_derivatives(AbstractObject $object, $force = FALSE, $all = TRUE) { +function islandora_do_derivatives(AbstractObject $object, array $options = array()) { + $options += array( + 'force' => FALSE, + 'source_dsid' => NULL, + 'destination_dsid' => NULL, + ); $hooks = islandora_invoke_hook_list(ISLANDORA_DERVIATIVE_CREATION_HOOK, $object->models, array()); uasort($hooks, 'drupal_sort_weight'); $results = array(); - dd($hooks, 'hooks'); + + if ($options['source_dsid'] !== NULL) { + $hooks = array_filter($hooks, function($filter_hook) use($options) { + return !isset($filter_hook['source_dsid']) || + (isset($filter_hook['source_dsid']) && $filter_hook['source_dsid'] == $options['source_dsid']); + }); + } + if ($options['destination_dsid'] !== NULL) { + $hooks = array_filter($hooks, function($filter_hook) use($options) { + return !isset($filter_hook['destination_dsid']) || + (isset($filter_hook['destination_dsid']) && $filter_hook['destination_dsid'] == $options['destination_dsid']); + }); + } + dd($hooks, 'hooooooks'); foreach ($hooks as $hook) { - if (!$all && !isset($hook['source_dsid'])) { - continue; - } if (isset($hook['file'])) { - require_once($hook['file']); + require_once $hook['file']; } foreach ($hook['function'] as $function) { - $logging = call_user_func($function, $object, $force); + $logging = call_user_func($function, $object, $options['force']); if (!empty($logging)) { $results[] = $logging; } } } return $results; +} + +function islandora_derivative_logging(array $logging_results) { + foreach ($logging_results as $result) { + foreach ($result['messages'] as $message) { + if ($message['type'] >= WATCHDOG_WARNING) { + if ($message['type'] == WATCHDOG_WARNING) { + $status = 'warning'; + } + else { + $status = 'status'; + } + drupal_set_message(format_string($message['message'], isset($message['message_sub']) ? $message['message_sub'] : array()), $status); + } + else { + watchdog('islandora', $message['message'], isset($message['message_sub']) ? $message['message_sub'] : array(), $message['type']); + } + } + } } \ No newline at end of file diff --git a/tests/derivatives.test b/tests/derivatives.test new file mode 100644 index 00000000..a98753f7 --- /dev/null +++ b/tests/derivatives.test @@ -0,0 +1,185 @@ + 'Islandora Derivative Generation', + 'description' => 'Ensure that the derivative generation hooks return appropriate results.', + 'group' => 'Islandora', + ); + } + + /** + * Creates an admin user and a connection to a fedora repository. + * + * @see IslandoraWebTestCase::setUp() + */ + public function setUp() { + parent::setUp( + array( + 'islandora_derivatives_test', + ) + ); + $url = variable_get('islandora_base_url', 'http://localhost:8080/fedora'); + $this->connection = new RepositoryConnection($url, $this->admin->name, $this->admin->pass); + $this->connection->reuseConnection = TRUE; + $this->api = new FedoraApi($this->connection); + $this->cache = new SimpleCache(); + $this->repository = new FedoraRepository($this->api, $this->cache); + $this->pid = $this->randomName() . ":" . $this->randomName(); + } + + /** + * Free any objects/resources created for this test. + * + * @see IslandoraWebTestCase::tearDown() + */ + public function tearDown() { + $tuque = islandora_get_tuque_connection(); + parent::tearDown(); + } + + public function testDerivativeOnIngest() { + global $ingest_method; + $ingest_method = 'modifyDatastream'; + $tuque = islandora_get_tuque_connection(); + $object = $tuque->repository->constructObject($this->pid); + $object->models = array( + 'some:cmodel', + ); + $dsid = 'OBJ'; + $ds = $object->constructDatastream($dsid); + $ds->label = 'Test'; + $ds->content = 'test'; + $object->ingestDatastream($ds); + $tuque->repository->ingestObject($object); + $this->assertDatastreams($object, array( + 'RELS-EXT', + 'DC', + 'OBJ', + 'DERIV', + )); + $this->assertEqual('ingestDatastream', $ingest_method, 'The expected ingest method is "ingestDatastream", got "' . $ingest_method . '".'); + $this->assertEqual('test some string', $object['DERIV']->content, 'The expected content of the DERIV datastream is "test some string", got "' . $object['DERIV']->content . '".'); + } + + public function testDerivativeOnForceExistingDatastream() { + global $ingest_method; + $ingest_method = 'ingestDatastream'; + $object = $this->constructBaseObject(); + $this->constructDERIVDatastream($object); + $islandora_object = islandora_object_load($this->pid); + islandora_do_derivatives($islandora_object, array( + 'force' => TRUE, + )); + $this->assertEqual('modifyDatastream', $ingest_method, 'The expected ingest method is "modifyDatastream", got "' . $ingest_method . '".'); + $this->assertEqual('FORCEFULLY APPENDING CONTENT TO test', $islandora_object['DERIV']->content, 'The expected content of the DERIV datastream is "FORCEFULLY APPENDING CONTENT TO test", got "' . $islandora_object['DERIV']->content . '".'); + } + + public function testDerivativeOnForceNonExistingDatastream() { + global $ingest_method; + $ingest_method = 'modifyDatastream'; + $this->constructBaseObject(); + $object = islandora_object_load($this->pid); + islandora_do_derivatives($object, array( + 'force' => TRUE, + )); + $this->assertEqual('ingestDatastream', $ingest_method, 'The expected ingest method is "ingestDatastream", got "' . $ingest_method . '".'); + $this->assertEqual('FORCEFULLY APPENDING CONTENT TO test', $object['DERIV']->content, 'The expected content of the DERIV datastream is "FORCEFULLY APPENDING CONTENT TO test", got "' . $object['DERIV']->content . '".'); + } + + public function testDerivativeOnModifyExistingDatastream() { + global $ingest_method; + $ingest_method = 'ingestDatastream'; + $object = $this->constructBaseObject(); + $this->constructDERIVDatastream($object); + $connection = islandora_get_tuque_connection(); + $connection->cache->resetCache(); + $islandora_object = islandora_object_load($this->pid); + $changed_content = 'islandora beast'; + $islandora_object['OBJ']->content = $changed_content; + $this->assertEqual('modifyDatastream', $ingest_method, 'The expected ingest method is "modifyDatastream", got "' . $ingest_method . '".'); + $this->assertEqual('FORCEFULLY APPENDING CONTENT TO ' . $changed_content, $islandora_object['DERIV']->content, 'The expected content of the DERIV datastream is "FORCEFULLY APPENDING CONTENT TO islandora beast", got "' . $islandora_object['DERIV']->content . '".'); + } + + public function testDerivativeOnModifyNonExistingDatastream() { + global $ingest_method; + $ingest_method = 'modifyDatastream'; + $this->constructBaseObject(); + // Need to do this as Tuque caches. + $connection = islandora_get_tuque_connection(); + $connection->cache->resetCache(); + $islandora_object = islandora_object_load($this->pid); + $changed_content = 'islandora beast'; + $islandora_object['OBJ']->content = $changed_content; + $this->assertEqual('ingestDatastream', $ingest_method, 'The expected ingest method is "ingestDatastream", got "' . $ingest_method . '".'); + $this->assertEqual('FORCEFULLY APPENDING CONTENT TO ' . $changed_content, $islandora_object['DERIV']->content, 'The expected content of the DERIV datastream is "FORCEFULLY APPENDING CONTENT TO islandora beast", got "' . $islandora_object['DERIV']->content . '".'); + } + + public function testDerivativeFilteringOnSourceDSID() { + global $derivative_functions; + $derivative_functions = array(); + $this->constructBaseObject(); + $object = islandora_object_load($this->pid); + islandora_do_derivatives($object, array( + 'source_dsid' => 'OBJ', + )); + $this->assertEqual(1, count($derivative_functions), 'Expected 1 derivative function for the source_dsid of "OBJ", got ' . count($derivative_functions) . '.'); + $called_function = (string) reset($derivative_functions); + $this->assertEqual('islandora_derivatives_test_create_deriv_datastream', $called_function, 'Expected derivative function is "islandora_derivatives_test_create_deriv_datastream", got "' . $called_function . '".'); + + // Reset the derivative functions array as we are going to use it again. + $derivative_functions = array(); + islandora_do_derivatives($object, array( + 'source_dsid' => 'SOMEWEIRDDATASTREAM', + )); + $this->assertEqual(1, count($derivative_functions), 'Expected 1 derivative function for the source_dsid of "SOMEWEIRDDATASTREAM", got ' . count($derivative_functions) . '.'); + $called_function = (string) reset($derivative_functions); + $this->assertEqual('islandora_derivatives_test_create_some_weird_datastream', $called_function, 'Expected derivative function is "islandora_derivatives_test_create_some_weird_datastream", got "' . $called_function . '".'); + } + + /** + * Helper function that will construct a base object. + */ + public function constructBaseObject() { + $object = $this->repository->constructObject($this->pid); + $object->models = array( + 'some:cmodel', + ); + $dsid = 'OBJ'; + $ds = $object->constructDatastream($dsid); + $ds->label = 'Test'; + $ds->content = 'test'; + $object->ingestDatastream($ds); + $this->repository->ingestObject($object); + return $object; + } + + public function constructDERIVDatastream(AbstractObject $object) { + $dsid = 'DERIV'; + $ds = $object->constructDatastream($dsid); + $ds->label = 'Test'; + $ds->content = 'test'; + $object->ingestDatastream($ds); + return $object; + } + + } + diff --git a/tests/islandora_derivatives_test.info b/tests/islandora_derivatives_test.info new file mode 100644 index 00000000..338b39fe --- /dev/null +++ b/tests/islandora_derivatives_test.info @@ -0,0 +1,7 @@ +name = Islandora Derivatives Generation testing +description = Tests derivative generation hooks. Do not enable. +core = 7.x +package = Testing +hidden = TRUE +files[] = islandora_derivatives_test.module +dependencies[] = islandora diff --git a/tests/islandora_derivatives_test.module b/tests/islandora_derivatives_test.module new file mode 100644 index 00000000..36b6380c --- /dev/null +++ b/tests/islandora_derivatives_test.module @@ -0,0 +1,114 @@ + 'OBJ', + 'destination_dsid' => 'DERIV', + 'weight' => '0', + 'function' => array( + 'islandora_derivatives_test_create_deriv_datastream', + ), + ), + array( + 'source_dsid' => 'SOMEWEIRDDATASTREAM', + 'destination_dsid' => 'STANLEY', + 'weight' => '-1', + 'function' => array( + 'islandora_derivatives_test_create_some_weird_datastream', + ), + ), + ); +} + +function islandora_derivatives_test_create_deriv_datastream(AbstractObject $object, $force = FALSE) { + global $derivative_functions; + $derivative_functions[] = 'islandora_derivatives_test_create_deriv_datastream'; + $return = ''; + if (!isset($object['DERIV']) || (isset($object['DERIV']) && $force === TRUE)) { + if ($force !== TRUE) { + $deriv_string = $object['OBJ']->content . ' some string'; + } + else { + $deriv_string = "FORCEFULLY APPENDING CONTENT TO " . $object['OBJ']->content; + } + $added_successfully = islandora_derivatives_test_add_datastream($object, 'DERIV', $deriv_string); + if ($added_successfully !== TRUE) { + $return = islandora_derivatives_test_failed_adding($added_successfully); + } + else { + $return = array( + 'success' => TRUE, + 'messages' => array( + array( + 'message' => t('The DERIV datastream was added successfully for @pid!'), + 'message_sub' => array('@pid' => $object->id), + 'type' => WATCHDOG_INFO, + ), + ), + ); + } + return $return; + } +} + +/** + * Stub function that should never actually get called because of DSID filtering. + * + * @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) { + global $derivative_functions; + // Add to the global that we got to this function. + $derivative_functions[] = 'islandora_derivatives_test_create_some_weird_datastream'; +} + +function islandora_derivatives_test_add_datastream(AbstractObject $object, $dsid, $deriv_string) { + global $ingest_method; + try { + $ingest = !isset($object[$dsid]); + if ($ingest) { + $ds = $object->constructDatastream($dsid, 'M'); + $ds->label = $dsid; + } + else { + $ds = $object[$dsid]; + } + $ds->content = $deriv_string; + if ($ingest) { + $ingest_method = 'ingestDatastream'; + $object->ingestDatastream($ds); + } + else { + $ingest_method = 'modifyDatastream'; + } + return TRUE; + } + catch (exception $e) { + $message = $e->getMessage(); + return $message; + } +} + +function islandora_derivatives_test_failed_adding($message) { + return array( + 'success' => FALSE, + 'messages' => array( + array( + 'message' => $message, + 'type' => WATCHDOG_ERROR, + ), + ), + ); +} \ No newline at end of file From 30143772f53cafe9193a35865490203fbb8a6dab Mon Sep 17 00:00:00 2001 From: William Panting Date: Wed, 7 Aug 2013 12:49:48 -0300 Subject: [PATCH 03/38] documenting some hooks --- islandora.api.php | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/islandora.api.php b/islandora.api.php index 4d54a00f..d6867c91 100644 --- a/islandora.api.php +++ b/islandora.api.php @@ -541,3 +541,34 @@ function hook_islandora_datastream_access($op, $object, $user) { */ function hook_CMODEL_PID_islandora_datastream_access($op, $object, $user) { } + +/** + * Lets one add to the overview tab in object management. + */ +function hook_islandora_overview_object(AbstractObject $object) { + return drupal_render(drupal_get_form('some_form', $object)); +} + +/** + * Lets one add to the overview tab in object management. + * + * Content model specific. + */ +function hook_CMODEL_PID_islandora_overview_object(AbstractObject $object) { + return drupal_render(drupal_get_form('some_form', $object)); +} +/** + * Lets one alter the overview tab in object management. + */ +function hook_islandora_overview_object_alter(AbstractObject &$object, &$output) { + $output = $output . drupal_render(drupal_get_form('some_form', $object)); +} + +/** + * Lets one alter the overview tab in object management. + * + * Content model specific. + */ +function hook_CMODEL_PID_islandora_overview_object_alter(AbstractObject &$object, &$output) { + $output = $output . drupal_render(drupal_get_form('some_form', $object)); +} From 82da5d74ff3c526f6bd89f4270da130c8fb66e66 Mon Sep 17 00:00:00 2001 From: Jordan Dukart Date: Wed, 7 Aug 2013 18:34:30 +0000 Subject: [PATCH 04/38] Restructuring to the derivate process, documentation, more tests. --- includes/derivatives.inc | 101 ++++++++++++++ islandora.api.php | 72 ++++++++++ islandora.module | 81 ++++-------- tests/derivatives.test | 169 ++++++++++++++++++++---- tests/islandora_derivatives_test.module | 112 ++++++++++++++-- 5 files changed, 438 insertions(+), 97 deletions(-) create mode 100644 includes/derivatives.inc diff --git a/includes/derivatives.inc b/includes/derivatives.inc new file mode 100644 index 00000000..f5058588 --- /dev/null +++ b/includes/derivatives.inc @@ -0,0 +1,101 @@ + FALSE, + ); + $hooks = islandora_invoke_hook_list(ISLANDORA_DERVIATIVE_CREATION_HOOK, $object->models, array()); + 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']; + }); + } + + foreach ($hooks as $hook) { + if (isset($hook['file'])) { + require_once $hook['file']; + } + foreach ($hook['function'] as $function) { + $logging = call_user_func($function, $object, $options['force']); + if (!empty($logging)) { + $results[] = $logging; + } + } + } + return $results; +} + +/** + * Handles the logging of derivative messages. + * + * @param array $logging_results + * An array of messages describing the outcome of the derivative events. + * Each individual message array has the following structure: + * - success: Bool denoting whether the operation was successful. + * - messages: An array structure containing: + * - message: A string passed through t() describing the + * outcome of the operation. + * - message_sub: (Optional) Substitutions to be passed along to t() or + * watchdog. + * - type: A string denoting whether the output is to be + * drupal_set_messaged (dsm) or watchdogged (watchdog). + * - severity: (Optional) A severity level / status to be used when + * logging messages. Uses the defaults of drupal_set_message and + * watchdog if not defined. + */ +function islandora_derivative_logging(array $logging_results) { + foreach ($logging_results as $result) { + foreach ($result['messages'] as $message) { + if ($message['type'] === 'dsm') { + drupal_set_message(filter_xss(format_string($message['message'], isset($message['message_sub']) ? $message['message_sub'] : array()), isset($message['severity']) ? $message['severity'] : 'status')); + } + else { + watchdog('islandora', $message['message'], isset($message['message_sub']) ? $message['message_sub'] : array(), isset($message['severity']) ? $message['severity'] : WATCHDOG_NOTICE); + } + } + } +} diff --git a/islandora.api.php b/islandora.api.php index 129787f4..74bdef81 100644 --- a/islandora.api.php +++ b/islandora.api.php @@ -519,3 +519,75 @@ function hook_islandora_datastream_access($op, $object, $user) { */ function hook_CMODEL_PID_islandora_datastream_access($op, $object, $user) { } + +/** + * Defines derivative functions to be executed based on certain conditions. + * + * This hook fires when an object/datastream is ingested or a datastream is + * modified. + * + * @return array + * An array containing an entry for each derivative to be created. Each entry + * is 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. + * - weight: A string denoting the weight of the function. This value is + * sorted upon to run functions in order. + * - function: An array of function(s) to be ran when constructing + * derivatives. Functions that are defined to be called for derivation + * creation must have the following structure: + * module_name_derivative_creation_function($object, $force = FALSE) + * These functions must return an array in the structure of: + * - success: Bool denoting whether the operation was successful. + * - messages: An array structure containing: + * - message: A string passed through t() describing the + * outcome of the operation. + * - message_sub: (Optional) Substitutions to be passed along to t() or + * watchdog. + * - type: A string denoting whether the output is to be + * drupal_set_messaged (dsm) or watchdogged (watchdog). + * - severity: (Optional) A severity level / status to be used when + * logging messages. Uses the defaults of drupal_set_message and + * watchdog if not defined. + */ +function hook_islandora_derivative() { + return array( + array( + 'source_dsid' => 'OBJ', + 'destination_dsid' => 'DERIV', + 'weight' => '0', + 'function' => array( + 'islandora_derivatives_test_create_deriv_datastream', + ), + ), + array( + 'source_dsid' => 'SOMEWEIRDDATASTREAM', + 'destination_dsid' => 'STANLEY', + 'weight' => '-1', + 'function' => array( + 'islandora_derivatives_test_create_some_weird_datastream', + ), + ), + array( + 'source_dsid' => NULL, + 'destination_dsid' => 'NOSOURCE', + 'weight' => '-3', + 'function' => array( + 'islandora_derivatives_test_create_nosource_datastream', + ), + ), + ); +} + +/** + * Content model specific version of hook_islandora_derivative(). + * + * @see hook_islandora_derivative() + */ +function hook_CMODEL_PID_islandora_derivative() { + +} \ No newline at end of file diff --git a/islandora.module b/islandora.module index 989dd36e..9bec87c7 100644 --- a/islandora.module +++ b/islandora.module @@ -1417,10 +1417,30 @@ function islandora_islandora_basic_collection_get_query_filters() { } } + +/** + * Implements hook_islandora_object_ingested(). + * + * On object ingestion we call the case of source_dsid being NULL only as + * the islandora_islandora_datastream_ingested hook will handle the cases + * where specific values of source_dsid can occur. + */ +function islandora_islandora_object_ingested(AbstractObject $object) { + module_load_include('inc', 'islandora', 'includes/derivatives'); + $logging_results = islandora_do_derivatives($object, array( + 'source_dsid' => NULL, + )); + islandora_derivative_logging($logging_results); +} + /** * Implements hook_islandora_datastream_ingested(). + * + * When a datastream is ingested we filter the derivatives on source_dsid being + * equal to the current ingested datastream's id. */ function islandora_islandora_datastream_ingested(AbstractObject $object, AbstractDatastream $datastream) { + module_load_include('inc', 'islandora', 'includes/derivatives'); $logging_results = islandora_do_derivatives($object, array( 'source_dsid' => $datastream->id, )); @@ -1429,67 +1449,16 @@ function islandora_islandora_datastream_ingested(AbstractObject $object, Abstrac /** * Implements hook_islandora_datastream_modified(). + * + * When a datastream is modified we filter the derivatives on source_dsid being + * 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) { + module_load_include('inc', 'islandora', 'includes/derivatives'); $logging_results = islandora_do_derivatives($object, array( 'source_dsid' => $datastream->id, 'force' => TRUE, )); islandora_derivative_logging($logging_results); } - -function islandora_do_derivatives(AbstractObject $object, array $options = array()) { - $options += array( - 'force' => FALSE, - 'source_dsid' => NULL, - 'destination_dsid' => NULL, - ); - $hooks = islandora_invoke_hook_list(ISLANDORA_DERVIATIVE_CREATION_HOOK, $object->models, array()); - uasort($hooks, 'drupal_sort_weight'); - $results = array(); - - if ($options['source_dsid'] !== NULL) { - $hooks = array_filter($hooks, function($filter_hook) use($options) { - return !isset($filter_hook['source_dsid']) || - (isset($filter_hook['source_dsid']) && $filter_hook['source_dsid'] == $options['source_dsid']); - }); - } - if ($options['destination_dsid'] !== NULL) { - $hooks = array_filter($hooks, function($filter_hook) use($options) { - return !isset($filter_hook['destination_dsid']) || - (isset($filter_hook['destination_dsid']) && $filter_hook['destination_dsid'] == $options['destination_dsid']); - }); - } - dd($hooks, 'hooooooks'); - foreach ($hooks as $hook) { - if (isset($hook['file'])) { - require_once $hook['file']; - } - foreach ($hook['function'] as $function) { - $logging = call_user_func($function, $object, $options['force']); - if (!empty($logging)) { - $results[] = $logging; - } - } - } - return $results; -} - -function islandora_derivative_logging(array $logging_results) { - foreach ($logging_results as $result) { - foreach ($result['messages'] as $message) { - if ($message['type'] >= WATCHDOG_WARNING) { - if ($message['type'] == WATCHDOG_WARNING) { - $status = 'warning'; - } - else { - $status = 'status'; - } - drupal_set_message(format_string($message['message'], isset($message['message_sub']) ? $message['message_sub'] : array()), $status); - } - else { - watchdog('islandora', $message['message'], isset($message['message_sub']) ? $message['message_sub'] : array(), $message['type']); - } - } - } -} \ No newline at end of file diff --git a/tests/derivatives.test b/tests/derivatives.test index a98753f7..13014880 100644 --- a/tests/derivatives.test +++ b/tests/derivatives.test @@ -34,7 +34,7 @@ class IslandoraDerivativesTestCase extends IslandoraWebTestCase { public function setUp() { parent::setUp( array( - 'islandora_derivatives_test', + 'islandora_derivatives_test', ) ); $url = variable_get('islandora_base_url', 'http://localhost:8080/fedora'); @@ -56,9 +56,12 @@ class IslandoraDerivativesTestCase extends IslandoraWebTestCase { parent::tearDown(); } + /** + * Tests that the islandora_islandora_object_ingested hook gets fired. + */ public function testDerivativeOnIngest() { - global $ingest_method; - $ingest_method = 'modifyDatastream'; + global $_islandora_derivative_test_ingest_method; + $_islandora_derivative_test_ingest_method = 'modifyDatastream'; $tuque = islandora_get_tuque_connection(); $object = $tuque->repository->constructObject($this->pid); $object->models = array( @@ -75,53 +78,70 @@ class IslandoraDerivativesTestCase extends IslandoraWebTestCase { 'DC', 'OBJ', 'DERIV', + 'NOSOURCE', )); - $this->assertEqual('ingestDatastream', $ingest_method, 'The expected ingest method is "ingestDatastream", got "' . $ingest_method . '".'); + $this->assertEqual('ingestDatastream', $_islandora_derivative_test_ingest_method, 'The expected ingest method is "ingestDatastream", got "' . $_islandora_derivative_test_ingest_method . '".'); $this->assertEqual('test some string', $object['DERIV']->content, 'The expected content of the DERIV datastream is "test some string", got "' . $object['DERIV']->content . '".'); + $this->assertEqual('NOSOURCE', $object['NOSOURCE']->content, 'The expected content of the NOSOURCE datastream is "NOSOURCE", got "' . $object['NOSOURCE']->content . '".'); + } - public function testDerivativeOnForceExistingDatastream() { - global $ingest_method; - $ingest_method = 'ingestDatastream'; + /** + * Tests the ingest method when when forcing on existing datastreams. + */ + public function testDerivativeOnForceExistingDatastream() { + global $_islandora_derivative_test_ingest_method; + $_islandora_derivative_test_ingest_method = 'ingestDatastream'; $object = $this->constructBaseObject(); - $this->constructDERIVDatastream($object); + $object = $this->constructDERIVDatastream($object); + $this->constructNOSOURCEDatastream($object); $islandora_object = islandora_object_load($this->pid); islandora_do_derivatives($islandora_object, array( 'force' => TRUE, )); - $this->assertEqual('modifyDatastream', $ingest_method, 'The expected ingest method is "modifyDatastream", got "' . $ingest_method . '".'); + $this->assertEqual('modifyDatastream', $_islandora_derivative_test_ingest_method, 'The expected ingest method is "modifyDatastream", got "' . $_islandora_derivative_test_ingest_method . '".'); $this->assertEqual('FORCEFULLY APPENDING CONTENT TO test', $islandora_object['DERIV']->content, 'The expected content of the DERIV datastream is "FORCEFULLY APPENDING CONTENT TO test", got "' . $islandora_object['DERIV']->content . '".'); } + /** + * Tests the ingest method when forcing on non-existing datastreams. + */ public function testDerivativeOnForceNonExistingDatastream() { - global $ingest_method; - $ingest_method = 'modifyDatastream'; + global $_islandora_derivative_test_ingest_method; + $_islandora_derivative_test_ingest_method = 'modifyDatastream'; $this->constructBaseObject(); $object = islandora_object_load($this->pid); islandora_do_derivatives($object, array( 'force' => TRUE, )); - $this->assertEqual('ingestDatastream', $ingest_method, 'The expected ingest method is "ingestDatastream", got "' . $ingest_method . '".'); - $this->assertEqual('FORCEFULLY APPENDING CONTENT TO test', $object['DERIV']->content, 'The expected content of the DERIV datastream is "FORCEFULLY APPENDING CONTENT TO test", got "' . $object['DERIV']->content . '".'); + $this->assertEqual('ingestDatastream', $_islandora_derivative_test_ingest_method, 'The expected ingest method is "ingestDatastream", got "' . $_islandora_derivative_test_ingest_method . '".'); + $this->assertEqual('test some string', $object['DERIV']->content, 'The expected content of the DERIV datastream is "test some string", got "' . $object['DERIV']->content . '".'); } + /** + * Tests the islandora_datastream_modified hook when there are existing DSes. + */ public function testDerivativeOnModifyExistingDatastream() { - global $ingest_method; - $ingest_method = 'ingestDatastream'; + global $_islandora_derivative_test_ingest_method; + $_islandora_derivative_test_ingest_method = 'ingestDatastream'; $object = $this->constructBaseObject(); $this->constructDERIVDatastream($object); + // Need to do this as Tuque caches. $connection = islandora_get_tuque_connection(); $connection->cache->resetCache(); $islandora_object = islandora_object_load($this->pid); $changed_content = 'islandora beast'; $islandora_object['OBJ']->content = $changed_content; - $this->assertEqual('modifyDatastream', $ingest_method, 'The expected ingest method is "modifyDatastream", got "' . $ingest_method . '".'); + $this->assertEqual('modifyDatastream', $_islandora_derivative_test_ingest_method, 'The expected ingest method is "modifyDatastream", got "' . $_islandora_derivative_test_ingest_method . '".'); $this->assertEqual('FORCEFULLY APPENDING CONTENT TO ' . $changed_content, $islandora_object['DERIV']->content, 'The expected content of the DERIV datastream is "FORCEFULLY APPENDING CONTENT TO islandora beast", got "' . $islandora_object['DERIV']->content . '".'); } + /** + * Tests islandora_datastream_modified hook when there are no existing DSes. + */ public function testDerivativeOnModifyNonExistingDatastream() { - global $ingest_method; - $ingest_method = 'modifyDatastream'; + global $_islandora_derivative_test_ingest_method; + $_islandora_derivative_test_ingest_method = 'modifyDatastream'; $this->constructBaseObject(); // Need to do this as Tuque caches. $connection = islandora_get_tuque_connection(); @@ -129,32 +149,98 @@ class IslandoraDerivativesTestCase extends IslandoraWebTestCase { $islandora_object = islandora_object_load($this->pid); $changed_content = 'islandora beast'; $islandora_object['OBJ']->content = $changed_content; - $this->assertEqual('ingestDatastream', $ingest_method, 'The expected ingest method is "ingestDatastream", got "' . $ingest_method . '".'); - $this->assertEqual('FORCEFULLY APPENDING CONTENT TO ' . $changed_content, $islandora_object['DERIV']->content, 'The expected content of the DERIV datastream is "FORCEFULLY APPENDING CONTENT TO islandora beast", got "' . $islandora_object['DERIV']->content . '".'); + $this->assertEqual('ingestDatastream', $_islandora_derivative_test_ingest_method, 'The expected ingest method is "ingestDatastream", got "' . $_islandora_derivative_test_ingest_method . '".'); + $this->assertEqual($changed_content . ' some string', $islandora_object['DERIV']->content, 'The expected content of the DERIV datastream is "islandora beast string", got "' . $islandora_object['DERIV']->content . '".'); } + /** + * Tests derivative hook filtering based upon source_dsid. + */ public function testDerivativeFilteringOnSourceDSID() { - global $derivative_functions; - $derivative_functions = array(); + global $_islandora_derivative_test_derivative_functions; + $_islandora_derivative_test_derivative_functions = array(); $this->constructBaseObject(); $object = islandora_object_load($this->pid); islandora_do_derivatives($object, array( 'source_dsid' => 'OBJ', )); - $this->assertEqual(1, count($derivative_functions), 'Expected 1 derivative function for the source_dsid of "OBJ", got ' . count($derivative_functions) . '.'); - $called_function = (string) reset($derivative_functions); + $this->assertEqual(1, count($_islandora_derivative_test_derivative_functions), 'Expected 1 derivative function for the source_dsid of "OBJ", got ' . count($_islandora_derivative_test_derivative_functions) . '.'); + $called_function = (string) reset($_islandora_derivative_test_derivative_functions); $this->assertEqual('islandora_derivatives_test_create_deriv_datastream', $called_function, 'Expected derivative function is "islandora_derivatives_test_create_deriv_datastream", got "' . $called_function . '".'); // Reset the derivative functions array as we are going to use it again. - $derivative_functions = array(); + $_islandora_derivative_test_derivative_functions = array(); islandora_do_derivatives($object, array( 'source_dsid' => 'SOMEWEIRDDATASTREAM', )); - $this->assertEqual(1, count($derivative_functions), 'Expected 1 derivative function for the source_dsid of "SOMEWEIRDDATASTREAM", got ' . count($derivative_functions) . '.'); - $called_function = (string) reset($derivative_functions); + $this->assertEqual(1, count($_islandora_derivative_test_derivative_functions), 'Expected 1 derivative function for the source_dsid of "SOMEWEIRDDATASTREAM", got ' . count($_islandora_derivative_test_derivative_functions) . '.'); + $called_function = (string) reset($_islandora_derivative_test_derivative_functions); $this->assertEqual('islandora_derivatives_test_create_some_weird_datastream', $called_function, 'Expected derivative function is "islandora_derivatives_test_create_some_weird_datastream", got "' . $called_function . '".'); } + /** + * Tests that only functions were the source_dsid is NULL are fired. + */ + public function testNULLSourceDSID() { + global $_islandora_derivative_test_derivative_functions; + $_islandora_derivative_test_derivative_functions = array(); + $this->constructBaseObject(); + $object = islandora_object_load($this->pid); + islandora_do_derivatives($object, array( + 'source_dsid' => NULL, + )); + $this->assertDatastreams($object, array( + 'DC', + 'RELS-EXT', + 'OBJ', + 'NOSOURCE', + )); + $this->assertEqual(1, count($_islandora_derivative_test_derivative_functions), 'Expected 1 derivative function for the source_dsid of "NULL", got ' . count($_islandora_derivative_test_derivative_functions) . '.'); + $called_function = (string) reset($_islandora_derivative_test_derivative_functions); + $this->assertEqual('islandora_derivatives_test_create_nosource_datastream', $called_function, 'Expected derivative function is "islandora_derivatives_test_create_nosource_datastream", got "' . $called_function . '".'); + $this->assertEqual('NOSOURCE', $object['NOSOURCE']->content, 'The expected content of the NOSOURCE datastream is "NOSOURCE", got "' . $object['NOSOURCE']->content . '".'); + } + + /** + * Tests that when no source_dsid all derivative functions are called. + */ + public function testNoSourceDSIDNoForce() { + global $_islandora_derivative_test_derivative_functions; + $_islandora_derivative_test_derivative_functions = array(); + $this->constructBaseObject(); + $object = islandora_object_load($this->pid); + islandora_do_derivatives($object, array()); + $this->assertDatastreams($object, array( + 'DC', + 'RELS-EXT', + 'OBJ', + 'DERIV', + '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) . '.'); + } + + /** + * Tests that when no source_dsid all derivative functions are called. + */ + public function testNoSourceDSIDForce() { + global $_islandora_derivative_test_derivative_functions; + $_islandora_derivative_test_derivative_functions = array(); + $this->constructBaseObject(); + $object = islandora_object_load($this->pid); + islandora_do_derivatives($object, array( + 'force' => TRUE, + )); + $this->assertDatastreams($object, array( + 'DC', + 'RELS-EXT', + 'OBJ', + 'DERIV', + '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) . '.'); + } + /** * Helper function that will construct a base object. */ @@ -172,14 +258,39 @@ class IslandoraDerivativesTestCase extends IslandoraWebTestCase { return $object; } + /** + * Helper function to construct the DERIV datastream without firing hooks. + * + * @param AbstractObject $object + * An AbstractObject representing a FedoraObject. + * + * @return AbstractObject + * The modified AbstractObject. + */ public function constructDERIVDatastream(AbstractObject $object) { $dsid = 'DERIV'; $ds = $object->constructDatastream($dsid); $ds->label = 'Test'; - $ds->content = 'test'; + $ds->content = 'test some string'; $object->ingestDatastream($ds); return $object; } + /** + * Helper function to construct the NOSOURCE datastream without firing hooks. + * + * @param AbstractObject $object + * An AbstractObject representing a FedoraObject. + * + * @return AbstractObject + * The modified AbstractObject. + */ + public function constructNOSOURCEDatastream(AbstractObject $object) { + $dsid = 'NOSOURCE'; + $ds = $object->constructDatastream($dsid); + $ds->label = 'Test'; + $ds->content = 'NOSOURCE'; + $object->ingestDatastream($ds); + return $object; } - +} diff --git a/tests/islandora_derivatives_test.module b/tests/islandora_derivatives_test.module index 36b6380c..71e043f0 100644 --- a/tests/islandora_derivatives_test.module +++ b/tests/islandora_derivatives_test.module @@ -26,15 +26,35 @@ function islandora_derivatives_test_some_cmodel_islandora_derivative() { 'islandora_derivatives_test_create_some_weird_datastream', ), ), + array( + 'source_dsid' => NULL, + 'destination_dsid' => 'NOSOURCE', + 'weight' => '-3', + 'function' => array( + 'islandora_derivatives_test_create_nosource_datastream', + ), + ), ); } +/** + * Creates the DERIV datastream for use in testing. + * + * @param AbstractObject $object + * An AbstractObject representing a Fedora object. + * @param bool $force + * Whether or not derivative generation is to be forced. + * @return array + * An array detailing the success of the operation. + * + * @see hook_islandora_derivative() + */ function islandora_derivatives_test_create_deriv_datastream(AbstractObject $object, $force = FALSE) { - global $derivative_functions; - $derivative_functions[] = 'islandora_derivatives_test_create_deriv_datastream'; + global $_islandora_derivative_test_derivative_functions; + $_islandora_derivative_test_derivative_functions[] = 'islandora_derivatives_test_create_deriv_datastream'; $return = ''; if (!isset($object['DERIV']) || (isset($object['DERIV']) && $force === TRUE)) { - if ($force !== TRUE) { + if ($force !== TRUE || !isset($object['DERIV'])) { $deriv_string = $object['OBJ']->content . ' some string'; } else { @@ -51,7 +71,7 @@ function islandora_derivatives_test_create_deriv_datastream(AbstractObject $obje array( 'message' => t('The DERIV datastream was added successfully for @pid!'), 'message_sub' => array('@pid' => $object->id), - 'type' => WATCHDOG_INFO, + 'type' => 'dsm', ), ), ); @@ -61,7 +81,7 @@ function islandora_derivatives_test_create_deriv_datastream(AbstractObject $obje } /** - * Stub function that should never actually get called because of DSID filtering. + * Stub function that used only for datastream filtering counts. * * @param AbstractObject $object * An AbstractObject representing a Fedora object. @@ -69,13 +89,69 @@ function islandora_derivatives_test_create_deriv_datastream(AbstractObject $obje * Whether the derivatives are being forcefully generated or not. */ function islandora_derivatives_test_create_some_weird_datastream(AbstractObject $object, $force = FALSE) { - global $derivative_functions; + global $_islandora_derivative_test_derivative_functions; // Add to the global that we got to this function. - $derivative_functions[] = 'islandora_derivatives_test_create_some_weird_datastream'; + $_islandora_derivative_test_derivative_functions[] = 'islandora_derivatives_test_create_some_weird_datastream'; } +/** + * Creates the NOSOURCE datastream for use in testing. + * + * @param AbstractObject $object + * An AbstractObject representing a Fedora object. + * @param bool $force + * Whether or not derivative generation is to be forced. + * @return array + * An array detailing the success of the operation. + * + * @see hook_islandora_derivative() + */ +function islandora_derivatives_test_create_nosource_datastream(AbstractObject $object, $force = FALSE) { + global $_islandora_derivative_test_derivative_functions; + $_islandora_derivative_test_derivative_functions[] = 'islandora_derivatives_test_create_nosource_datastream'; + $return = ''; + if (!isset($object['NOSOURCE']) || (isset($object['NOSOURCE']) && $force === TRUE)) { + if ($force !== TRUE || !isset($object['NOSOURCE'])) { + $deriv_string = 'NOSOURCE'; + } + else { + $deriv_string = "FORCEFULLY APPENDING CONTENT TO " . $object['NOSOURCE']->content; + } + $added_successfully = islandora_derivatives_test_add_datastream($object, 'NOSOURCE', $deriv_string); + if ($added_successfully !== TRUE) { + $return = islandora_derivatives_test_failed_adding($added_successfully); + } + else { + $return = array( + 'success' => TRUE, + 'messages' => array( + array( + 'message' => t('The DERIV datastream was added successfully for @pid!'), + 'message_sub' => array('@pid' => $object->id), + 'type' => 'dsm', + ), + ), + ); + } + return $return; + } +} + +/** + * Helper function that adds/modifies the datastream to the object in testing. + * + * @param AbstractObject $object + * An AbstractObject representing a Fedora object. + * @param string $dsid + * The datastream id for which we are adding/modifying. + * @param string $deriv_string + * The content of the datastream we are adding. + * + * @return bool|string + * A bool if the operation was successfully, the error message otherwise. + */ function islandora_derivatives_test_add_datastream(AbstractObject $object, $dsid, $deriv_string) { - global $ingest_method; + global $_islandora_derivative_test_ingest_method; try { $ingest = !isset($object[$dsid]); if ($ingest) { @@ -87,11 +163,11 @@ function islandora_derivatives_test_add_datastream(AbstractObject $object, $dsid } $ds->content = $deriv_string; if ($ingest) { - $ingest_method = 'ingestDatastream'; + $_islandora_derivative_test_ingest_method = 'ingestDatastream'; $object->ingestDatastream($ds); } else { - $ingest_method = 'modifyDatastream'; + $_islandora_derivative_test_ingest_method = 'modifyDatastream'; } return TRUE; } @@ -101,14 +177,26 @@ function islandora_derivatives_test_add_datastream(AbstractObject $object, $dsid } } +/** + * Returns a message if we failed to add a derivative. + * + * @see hook_islandora_derivative() + * + * @param string $message + * The error message to be returned back. + * + * @return array + * An array describing the outcome of our failure. + */ function islandora_derivatives_test_failed_adding($message) { return array( 'success' => FALSE, 'messages' => array( array( 'message' => $message, - 'type' => WATCHDOG_ERROR, + 'type' => 'watchdog', + 'severity' => WATCHDOG_ERROR, ), ), ); -} \ No newline at end of file +} From 69010df8ea04d8c895b27363cbdab3970a7246c1 Mon Sep 17 00:00:00 2001 From: Jordan Dukart Date: Wed, 7 Aug 2013 19:06:09 +0000 Subject: [PATCH 05/38] Filter xssing too much, minor doc fixes. --- includes/derivatives.inc | 2 +- islandora.api.php | 4 +++- tests/islandora_derivatives_test.module | 4 ++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/includes/derivatives.inc b/includes/derivatives.inc index f5058588..f707f88a 100644 --- a/includes/derivatives.inc +++ b/includes/derivatives.inc @@ -91,7 +91,7 @@ function islandora_derivative_logging(array $logging_results) { foreach ($logging_results as $result) { foreach ($result['messages'] as $message) { if ($message['type'] === 'dsm') { - drupal_set_message(filter_xss(format_string($message['message'], isset($message['message_sub']) ? $message['message_sub'] : array()), isset($message['severity']) ? $message['severity'] : 'status')); + drupal_set_message(filter_xss(format_string($message['message'], isset($message['message_sub']) ? $message['message_sub'] : array())), isset($message['severity']) ? $message['severity'] : 'status'); } else { watchdog('islandora', $message['message'], isset($message['message_sub']) ? $message['message_sub'] : array(), isset($message['severity']) ? $message['severity'] : WATCHDOG_NOTICE); diff --git a/islandora.api.php b/islandora.api.php index 74bdef81..da6dae77 100644 --- a/islandora.api.php +++ b/islandora.api.php @@ -553,6 +553,8 @@ function hook_CMODEL_PID_islandora_datastream_access($op, $object, $user) { * - severity: (Optional) A severity level / status to be used when * logging messages. Uses the defaults of drupal_set_message and * watchdog if not defined. + * - file: A string denoting the path to the file where the function + * is being called from. */ function hook_islandora_derivative() { return array( @@ -590,4 +592,4 @@ function hook_islandora_derivative() { */ function hook_CMODEL_PID_islandora_derivative() { -} \ No newline at end of file +} diff --git a/tests/islandora_derivatives_test.module b/tests/islandora_derivatives_test.module index 71e043f0..80530296 100644 --- a/tests/islandora_derivatives_test.module +++ b/tests/islandora_derivatives_test.module @@ -6,7 +6,7 @@ */ /** - * Implements hook_islandora_CMODEL_derivative(). + * Implements hook_islandora_CMODEL_PID_derivative(). */ function islandora_derivatives_test_some_cmodel_islandora_derivative() { return array( @@ -146,7 +146,7 @@ function islandora_derivatives_test_create_nosource_datastream(AbstractObject $o * The datastream id for which we are adding/modifying. * @param string $deriv_string * The content of the datastream we are adding. - * + * * @return bool|string * A bool if the operation was successfully, the error message otherwise. */ From 0b53c36cea67c814a641851e8dc81b9ae0d1a22c Mon Sep 17 00:00:00 2001 From: Jordan Dukart Date: Thu, 8 Aug 2013 18:06:55 +0000 Subject: [PATCH 06/38] Coding standards ignore. --- includes/derivatives.inc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/includes/derivatives.inc b/includes/derivatives.inc index f707f88a..67979d4a 100644 --- a/includes/derivatives.inc +++ b/includes/derivatives.inc @@ -94,7 +94,11 @@ function islandora_derivative_logging(array $logging_results) { drupal_set_message(filter_xss(format_string($message['message'], isset($message['message_sub']) ? $message['message_sub'] : array())), isset($message['severity']) ? $message['severity'] : 'status'); } else { + // @codingStandardsIgnoreStart + // We know what we are doing here. Passing through the translated + // message and the substitutions needed. watchdog('islandora', $message['message'], isset($message['message_sub']) ? $message['message_sub'] : array(), isset($message['severity']) ? $message['severity'] : WATCHDOG_NOTICE); + // @codingStandardsIgnoreEnd } } } From a0fc2179da3cb212d88e78dc66bdd3a043631e3d Mon Sep 17 00:00:00 2001 From: Jordan Dukart Date: Thu, 8 Aug 2013 18:45:36 +0000 Subject: [PATCH 07/38] Coder hackery. --- includes/derivatives.inc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/includes/derivatives.inc b/includes/derivatives.inc index 67979d4a..4a422535 100644 --- a/includes/derivatives.inc +++ b/includes/derivatives.inc @@ -94,11 +94,11 @@ function islandora_derivative_logging(array $logging_results) { drupal_set_message(filter_xss(format_string($message['message'], isset($message['message_sub']) ? $message['message_sub'] : array())), isset($message['severity']) ? $message['severity'] : 'status'); } else { - // @codingStandardsIgnoreStart // We know what we are doing here. Passing through the translated - // message and the substitutions needed. - watchdog('islandora', $message['message'], isset($message['message_sub']) ? $message['message_sub'] : array(), isset($message['severity']) ? $message['severity'] : WATCHDOG_NOTICE); - // @codingStandardsIgnoreEnd + // 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); } } } From aaa0a8ddc63408e2d7e237cb15185119a25ac753 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Tue, 13 Aug 2013 14:33:12 +0000 Subject: [PATCH 08/38] Fix TOKEN_TIMEOUT constant. --- includes/authtokens.inc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/includes/authtokens.inc b/includes/authtokens.inc index 9d5783a9..36e8689f 100644 --- a/includes/authtokens.inc +++ b/includes/authtokens.inc @@ -10,7 +10,7 @@ // Token lifespan(seconds): after this duration the token expires. // 5 minutes. -define('TOKEN_TIMEOUT', 300); +define('ISLANDORA_AUTHTOKEN_TOKEN_TIMEOUT', 300); /** * Request Islandora to construct an object/datastream authentication token. @@ -92,7 +92,7 @@ function islandora_validate_object_token($pid, $dsid, $token) { ->condition('pid', $pid, '=') ->condition('dsid', $dsid, '=') ->condition('time', $time, '<=') - ->condition('time', $time - TOKEN_TIMEOUT, '>') + ->condition('time', $time - ISLANDORA_AUTHTOKEN_TOKEN_TIMEOUT, '>') ->execute() ->fetchAll(); if ($result) { @@ -131,6 +131,6 @@ function islandora_validate_object_token($pid, $dsid, $token) { function islandora_remove_expired_tokens() { $time = time(); db_delete("islandora_authtokens") - ->condition('time', $time - TOKEN_TIMEOUT, '<') + ->condition('time', $time - ISLANDORA_AUTHTOKEN_TOKEN_TIMEOUT, '<') ->execute(); } From febd3a3feef1117ff5afe115995b61c8f050982f Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Tue, 13 Aug 2013 15:09:34 +0000 Subject: [PATCH 09/38] Carry through changes. --- includes/object_properties.form.inc | 2 +- includes/utilities.inc | 4 +- islandora.module | 85 +++++++++++++---------- tests/authtokens.test | 2 +- tests/hooked_access.test | 6 +- tests/islandora_hooked_access_test.module | 2 +- tests/islandora_manage_permissions.test | 50 ++++++------- theme/theme.inc | 18 ++--- 8 files changed, 91 insertions(+), 78 deletions(-) diff --git a/includes/object_properties.form.inc b/includes/object_properties.form.inc index 44d016b7..51e48a5f 100644 --- a/includes/object_properties.form.inc +++ b/includes/object_properties.form.inc @@ -60,7 +60,7 @@ function islandora_object_properties_form(array $form, array &$form_state, Abstr ), 'delete' => array( '#type' => 'submit', - '#access' => islandora_object_access(FEDORA_PURGE, $object), + '#access' => islandora_object_access(ISLANDORA_PURGE, $object), '#value' => t('Delete'), '#submit' => array('islandora_object_properties_form_delete'), '#limit_validation_errors' => array(array('pid')), diff --git a/includes/utilities.inc b/includes/utilities.inc index c8d1ae5e..91988918 100644 --- a/includes/utilities.inc +++ b/includes/utilities.inc @@ -391,10 +391,10 @@ function islandora_get_datastreams_requirements_from_models(array $models) { * - "optional": A boolean indicating if the given stream is optional. */ function islandora_get_datastreams_requirements_from_content_model(AbstractObject $object) { - if (empty($object[DS_COMP_STREAM]) || !islandora_datastream_access(FEDORA_VIEW_OBJECTS, $object[DS_COMP_STREAM])) { + if (empty($object[ISLANDORA_DS_COMP_STREAM]) || !islandora_datastream_access(ISLANDORA_VIEW_OBJECTS, $object[ISLANDORA_DS_COMP_STREAM])) { return array(); } - $xml = new SimpleXMLElement($object[DS_COMP_STREAM]->content); + $xml = new SimpleXMLElement($object[ISLANDORA_DS_COMP_STREAM]->content); foreach ($xml->dsTypeModel as $ds) { $dsid = (string) $ds['ID']; $optional = strtolower((string) $ds['optional']); diff --git a/islandora.module b/islandora.module index 7d58c865..6ec9d03a 100644 --- a/islandora.module +++ b/islandora.module @@ -24,15 +24,15 @@ */ // Common datastreams. -define('DS_COMP_STREAM', 'DS-COMPOSITE-MODEL'); +define('ISLANDORA_DS_COMP_STREAM', 'DS-COMPOSITE-MODEL'); // Permissions. -define('FEDORA_VIEW_OBJECTS', 'view fedora repository objects'); -define('FEDORA_METADATA_EDIT', 'edit fedora metadata'); -define('FEDORA_ADD_DS', 'add fedora datastreams'); -define('FEDORA_INGEST', 'ingest fedora objects'); -define('FEDORA_PURGE', 'delete fedora objects and datastreams'); -define('FEDORA_MANAGE_PROPERTIES', 'manage object properties'); +define('ISLANDORA_VIEW_OBJECTS', 'view fedora repository objects'); +define('ISLANDORA_METADATA_EDIT', 'edit fedora metadata'); +define('ISLANDORA_ADD_DS', 'add fedora datastreams'); +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'); // Hooks. @@ -57,6 +57,19 @@ define('ISLANDORA_INGEST_STEP_HOOK', 'islandora_ingest_steps'); // Autocomplete paths. define('ISLANDORA_CONTENT_MODELS_AUTOCOMPLETE', 'islandora/autocomplete/content-models'); +/** + * @deprecated Constants. + */ +// @codingStandardsIgnoreStart +define('DS_COMP_STREAM', ISLANDORA_DS_COMP_STREAM); +define('FEDORA_VIEW_OBJECTS', ISLANDORA_VIEW_OBJECTS); +define('FEDORA_METADATA_EDIT', ISLANDORA_METADATA_EDIT); +define('FEDORA_ADD_DS', ISLANDORA_ADD_DS); +define('FEDORA_INGEST', ISLANDORA_INGEST); +define('FEDORA_PURGE', ISLANDORA_PURGE); +define('FEDORA_MANAGE_PROPERTIES', ISLANDORA_MANAGE_PROPERTIES); +// @codingStandardsIgnoreEnd + /** * Implements hook_menu(). * @@ -85,7 +98,7 @@ function islandora_menu() { 'title' => 'Solution packs', 'description' => 'Install content models and collections required by installed solution packs.', 'page callback' => 'islandora_solution_packs_admin', - 'access arguments' => array(FEDORA_ADD_DS), + 'access arguments' => array(ISLANDORA_ADD_DS), 'file' => 'includes/solution_packs.inc', 'type' => MENU_NORMAL_ITEM, ); @@ -93,7 +106,7 @@ function islandora_menu() { 'title' => 'Islandora Repository', 'page callback' => 'islandora_view_default_object', 'type' => MENU_NORMAL_ITEM, - 'access arguments' => array(FEDORA_VIEW_OBJECTS), + 'access arguments' => array(ISLANDORA_VIEW_OBJECTS), ); $items['islandora/object/%islandora_object'] = array( 'title callback' => 'islandora_drupal_title', @@ -102,14 +115,14 @@ function islandora_menu() { 'page arguments' => array(2), 'type' => MENU_NORMAL_ITEM, 'access callback' => 'islandora_object_access_callback', - 'access arguments' => array(FEDORA_VIEW_OBJECTS, 2), + 'access arguments' => array(ISLANDORA_VIEW_OBJECTS, 2), ); $items['islandora/object/%islandora_object/print'] = array( 'page callback' => 'islandora_printer_object', 'page arguments' => array(2), 'type' => MENU_NORMAL_ITEM, 'access callback' => 'islandora_object_access_callback', - 'access arguments' => array(FEDORA_VIEW_OBJECTS, 2), + 'access arguments' => array(ISLANDORA_VIEW_OBJECTS, 2), ); $items['islandora/object/%islandora_object/view'] = array( 'title' => 'View', @@ -129,11 +142,11 @@ function islandora_menu() { 'access callback' => 'islandora_object_manage_access_callback', 'access arguments' => array( array( - FEDORA_MANAGE_PROPERTIES, - FEDORA_METADATA_EDIT, - FEDORA_ADD_DS, - FEDORA_PURGE, - FEDORA_INGEST, + ISLANDORA_MANAGE_PROPERTIES, + ISLANDORA_METADATA_EDIT, + ISLANDORA_ADD_DS, + ISLANDORA_PURGE, + ISLANDORA_INGEST, ), 2), ); @@ -151,9 +164,9 @@ function islandora_menu() { 'access callback' => 'islandora_object_manage_access_callback', 'access arguments' => array( array( - FEDORA_METADATA_EDIT, - FEDORA_ADD_DS, - FEDORA_PURGE, + ISLANDORA_METADATA_EDIT, + ISLANDORA_ADD_DS, + ISLANDORA_PURGE, ), 2), 'weight' => -10, ); @@ -165,7 +178,7 @@ function islandora_menu() { 'page arguments' => array('islandora_object_properties_form', 2), 'type' => MENU_LOCAL_TASK, 'access callback' => 'islandora_object_access_callback', - 'access arguments' => array(FEDORA_MANAGE_PROPERTIES, 2), + 'access arguments' => array(ISLANDORA_MANAGE_PROPERTIES, 2), 'weight' => -5, ); $items['islandora/object/%islandora_object/delete'] = array( @@ -175,7 +188,7 @@ function islandora_menu() { 'page arguments' => array('islandora_delete_object_form', 2), 'type' => MENU_CALLBACK, 'access callback' => 'islandora_object_access_callback', - 'access arguments' => array(FEDORA_PURGE, 2), + 'access arguments' => array(ISLANDORA_PURGE, 2), ); $items['islandora/object/%islandora_object/manage/datastreams/add'] = array( 'title' => 'Add a datastream', @@ -184,7 +197,7 @@ function islandora_menu() { 'page arguments' => array('islandora_add_datastream_form', 2), 'type' => MENU_LOCAL_ACTION, 'access callback' => 'islandora_object_access_callback', - 'access arguments' => array(FEDORA_ADD_DS, 2), + 'access arguments' => array(ISLANDORA_ADD_DS, 2), ); $items['islandora/object/%islandora_object/manage/datastreams/add/autocomplete'] = array( 'file' => 'includes/add_datastream.form.inc', @@ -192,7 +205,7 @@ function islandora_menu() { 'page arguments' => array(2), 'type' => MENU_CALLBACK, 'access callback' => 'islandora_object_access_callback', - 'access arguments' => array(FEDORA_ADD_DS, 2), + 'access arguments' => array(ISLANDORA_ADD_DS, 2), ); $items['islandora/object/%islandora_object/datastream/%islandora_datastream'] = array( 'title' => 'View datastream', @@ -201,7 +214,7 @@ function islandora_menu() { 'type' => MENU_CALLBACK, 'file' => 'includes/datastream.inc', 'access callback' => 'islandora_datastream_access', - 'access arguments' => array(FEDORA_VIEW_OBJECTS, 4), + 'access arguments' => array(ISLANDORA_VIEW_OBJECTS, 4), 'load arguments' => array(2), ); // This menu item uses token authentication in islandora_tokened_object. @@ -209,7 +222,7 @@ function islandora_menu() { 'title' => 'View datastream', 'load arguments' => array('%map'), 'access callback' => 'islandora_object_datastream_tokened_access_callback', - 'access arguments' => array(FEDORA_VIEW_OBJECTS, 2, 4), + 'access arguments' => array(ISLANDORA_VIEW_OBJECTS, 2, 4), 'type' => MENU_DEFAULT_LOCAL_TASK, ); $items['islandora/object/%islandora_object/datastream/%islandora_datastream/download'] = array( @@ -219,7 +232,7 @@ function islandora_menu() { 'type' => MENU_CALLBACK, 'file' => 'includes/datastream.inc', 'access callback' => 'islandora_datastream_access', - 'access arguments' => array(FEDORA_VIEW_OBJECTS, 4), + 'access arguments' => array(ISLANDORA_VIEW_OBJECTS, 4), 'load arguments' => array(2), ); $items['islandora/object/%islandora_object/datastream/%islandora_datastream/edit'] = array( @@ -229,7 +242,7 @@ function islandora_menu() { 'type' => MENU_CALLBACK, 'file' => 'includes/datastream.inc', 'access callback' => 'islandora_datastream_access', - 'access arguments' => array(FEDORA_METADATA_EDIT, 4), + 'access arguments' => array(ISLANDORA_METADATA_EDIT, 4), 'load arguments' => array(2), ); $items['islandora/object/%islandora_object/datastream/%islandora_datastream/delete'] = array( @@ -239,7 +252,7 @@ function islandora_menu() { 'file' => 'includes/delete_datastream.form.inc', 'type' => MENU_CALLBACK, 'access callback' => 'islandora_datastream_access', - 'access arguments' => array(FEDORA_PURGE, 4), + 'access arguments' => array(ISLANDORA_PURGE, 4), 'load arguments' => array(2), ); $items['islandora/object/%islandora_object/datastream/%islandora_datastream/version'] = array( @@ -259,7 +272,7 @@ function islandora_menu() { 'file' => 'includes/datastream.version.inc', 'type' => MENU_CALLBACK, 'access callback' => 'islandora_datastream_access', - 'access arguments' => array(FEDORA_PURGE, 4), + 'access arguments' => array(ISLANDORA_PURGE, 4), 'load arguments' => array(2), ); $items['islandora/object/%islandora_object/datastream/%islandora_datastream/version/%/view'] = array( @@ -277,7 +290,7 @@ function islandora_menu() { 'page arguments' => array(2), 'type' => MENU_CALLBACK, 'access callback' => 'islandora_object_access', - 'access arguments' => array(FEDORA_VIEW_OBJECTS, 2), + 'access arguments' => array(ISLANDORA_VIEW_OBJECTS, 2), 'load arguments' => array(2), ); $items[ISLANDORA_CONTENT_MODELS_AUTOCOMPLETE] = array( @@ -390,27 +403,27 @@ function islandora_theme() { */ function islandora_permission() { return array( - FEDORA_VIEW_OBJECTS => array( + ISLANDORA_VIEW_OBJECTS => array( 'title' => t('View repository objects'), 'description' => t('View objects in the repository. Note: Fedora XACML security policies may override this permission.'), ), - FEDORA_ADD_DS => array( + ISLANDORA_ADD_DS => array( 'title' => t('Add datastreams to repository objects'), 'description' => t('Add datastreams to objects in the repository. Note: Fedora XACML security policies may override this position.'), ), - FEDORA_METADATA_EDIT => array( + ISLANDORA_METADATA_EDIT => array( 'title' => t('Edit metadata'), 'description' => t('Edit metadata for objects in the repository.'), ), - FEDORA_INGEST => array( + ISLANDORA_INGEST => array( 'title' => t('Create new repository objects'), 'description' => t('Create new objects in the repository.'), ), - FEDORA_PURGE => array( + ISLANDORA_PURGE => array( 'title' => t('Permanently remove objects from the repository'), 'description' => t('Permanently remove objects from the repository.'), ), - FEDORA_MANAGE_PROPERTIES => array( + ISLANDORA_MANAGE_PROPERTIES => array( 'title' => t('Manage object properties'), 'description' => t('Modify object labels, owner IDs, and states.'), ), diff --git a/tests/authtokens.test b/tests/authtokens.test index ccd3088a..352689e2 100644 --- a/tests/authtokens.test +++ b/tests/authtokens.test @@ -74,7 +74,7 @@ class IslandoraAuthtokensTestCase extends IslandoraWebTestCase { $this->drupalGet("islandora/object/{$newpid}/datastream/JP2/view"); $this->assertResponse(403, 'Page not found as anonymous'); - $account = $this->drupalCreateUser(array(FEDORA_VIEW_OBJECTS)); + $account = $this->drupalCreateUser(array(ISLANDORA_VIEW_OBJECTS)); $this->drupalLogin($account); $this->drupalGet("islandora/object/{$newpid}/datastream/JP2/view"); diff --git a/tests/hooked_access.test b/tests/hooked_access.test index 0039460e..894eee80 100644 --- a/tests/hooked_access.test +++ b/tests/hooked_access.test @@ -37,9 +37,9 @@ class IslandoraHookedAccessTestCase extends IslandoraWebTestCase { $this->objects = array( 'test:testAccessHook', ); - $this->op = FEDORA_VIEW_OBJECTS; - $this->other_op = FEDORA_INGEST; - $this->denied_op = FEDORA_PURGE; + $this->op = ISLANDORA_VIEW_OBJECTS; + $this->other_op = ISLANDORA_INGEST; + $this->denied_op = ISLANDORA_PURGE; $this->purgeTestObjects(); $this->dsid = 'asdf'; $this->createTestObjects(); diff --git a/tests/islandora_hooked_access_test.module b/tests/islandora_hooked_access_test.module index de417738..ea033589 100644 --- a/tests/islandora_hooked_access_test.module +++ b/tests/islandora_hooked_access_test.module @@ -9,7 +9,7 @@ * Implements hook_islandora_object_access(). */ function islandora_hooked_access_test_islandora_object_access($op, $object, $user) { - if ($op == FEDORA_PURGE) { + if ($op == ISLANDORA_PURGE) { return FALSE; } if (isset($_SESSION['islandora_hooked_access_test']) && $_SESSION['islandora_hooked_access_test'] === func_get_args()) { diff --git a/tests/islandora_manage_permissions.test b/tests/islandora_manage_permissions.test index 8fc1549f..e17d1501 100644 --- a/tests/islandora_manage_permissions.test +++ b/tests/islandora_manage_permissions.test @@ -33,16 +33,16 @@ class IslandoraPermissionsTestCase extends IslandoraWebTestCase { * Test manage permissions. */ public function testManagePermissions() { - // Test permission FEDORA_VIEW_OBJECTS. + // Test permission ISLANDORA_VIEW_OBJECTS. // Create a user with permission. - $user = $this->drupalCreateUser(array(FEDORA_VIEW_OBJECTS)); + $user = $this->drupalCreateUser(array(ISLANDORA_VIEW_OBJECTS)); // Log the user in. $this->drupalLogin($user); $this->clickLink(t('Islandora Repository')); $this->assertNoLink('Manage', 'Manage tab is not on current page.'); - // Test permission FEDORA_VIEW_OBJECTS, FEDORA_MANAGE_PROPERTIES. - $user = $this->drupalCreateUser(array(FEDORA_VIEW_OBJECTS, FEDORA_MANAGE_PROPERTIES)); + // Test permission ISLANDORA_VIEW_OBJECTS, ISLANDORA_MANAGE_PROPERTIES. + $user = $this->drupalCreateUser(array(ISLANDORA_VIEW_OBJECTS, ISLANDORA_MANAGE_PROPERTIES)); $this->drupalLogin($user); $this->clickLink(t('Islandora Repository')); $this->assertLink('Manage', 0, 'Manage tab is on current page.'); @@ -51,8 +51,8 @@ class IslandoraPermissionsTestCase extends IslandoraWebTestCase { $this->assertNoLink('Datastreams', 'Datastreams tab is not on current page.'); $this->assertNoLink('Collection', 'Collection tab is not on current page.'); - // Test permission FEDORA_VIEW_OBJECTS, FEDORA_ADD_DS. - $user = $this->drupalCreateUser(array(FEDORA_VIEW_OBJECTS, FEDORA_ADD_DS)); + // Test permission ISLANDORA_VIEW_OBJECTS, ISLANDORA_ADD_DS. + $user = $this->drupalCreateUser(array(ISLANDORA_VIEW_OBJECTS, ISLANDORA_ADD_DS)); $this->drupalLogin($user); $this->clickLink(t('Islandora Repository')); $this->assertLink('Manage', 0, 'Manage tab is on current page.'); @@ -61,8 +61,8 @@ class IslandoraPermissionsTestCase extends IslandoraWebTestCase { $this->assertNoLink('Properties', 'Properties tab is not on current page.'); $this->assertNoLink('Collection', 'Collection tab is not on current page.'); - // Test permission FEDORA_VIEW_OBJECTS, FEDORA_METADATA_EDIT. - $user = $this->drupalCreateUser(array(FEDORA_VIEW_OBJECTS, FEDORA_METADATA_EDIT)); + // Test permission ISLANDORA_VIEW_OBJECTS, ISLANDORA_METADATA_EDIT. + $user = $this->drupalCreateUser(array(ISLANDORA_VIEW_OBJECTS, ISLANDORA_METADATA_EDIT)); $this->drupalLogin($user); $this->clickLink(t('Islandora Repository')); $this->assertLink('Manage', 0, 'Manage tab is on current page.'); @@ -71,8 +71,8 @@ class IslandoraPermissionsTestCase extends IslandoraWebTestCase { $this->assertNoLink('Properties', 'Properties tab is not on current page.'); $this->assertNoLink('Collection', 'Collection tab is not on current page.'); - // Test permission FEDORA_VIEW_OBJECTS, FEDORA_PURGE. - $user = $this->drupalCreateUser(array(FEDORA_VIEW_OBJECTS, FEDORA_PURGE)); + // Test permission ISLANDORA_VIEW_OBJECTS, ISLANDORA_PURGE. + $user = $this->drupalCreateUser(array(ISLANDORA_VIEW_OBJECTS, ISLANDORA_PURGE)); $this->drupalLogin($user); $this->clickLink(t('Islandora Repository')); $this->assertLink('Manage', 0, 'Manage tab is on current page.'); @@ -98,39 +98,39 @@ class IslandoraPermissionsTestCase extends IslandoraWebTestCase { $ret = islandora_user_access($object, array()); $this->assertFalse($ret, 'User access denied when no permissions are provided.'); // Test access with matching permission. - $user = $this->drupalCreateUser(array(FEDORA_VIEW_OBJECTS)); - $ret = islandora_user_access($object, array(FEDORA_VIEW_OBJECTS), array(), TRUE, $user); + $user = $this->drupalCreateUser(array(ISLANDORA_VIEW_OBJECTS)); + $ret = islandora_user_access($object, array(ISLANDORA_VIEW_OBJECTS), array(), TRUE, $user); $this->assertTrue($ret, 'User access granted when permissions match.'); // Test access with matching permission but access any is FALSE. - $user = $this->drupalCreateUser(array(FEDORA_VIEW_OBJECTS)); - $ret = islandora_user_access($object, array(FEDORA_VIEW_OBJECTS, FEDORA_PURGE), array(), FALSE, $user); + $user = $this->drupalCreateUser(array(ISLANDORA_VIEW_OBJECTS)); + $ret = islandora_user_access($object, array(ISLANDORA_VIEW_OBJECTS, ISLANDORA_PURGE), array(), FALSE, $user); $this->assertFalse($ret, 'User access denied for matching permission but with access any set to FALSE.'); // Test access with non-matching permission. - $user = $this->drupalCreateUser(array(FEDORA_PURGE)); - $ret = islandora_user_access($object, array(FEDORA_VIEW_OBJECTS), array(), TRUE, $user); + $user = $this->drupalCreateUser(array(ISLANDORA_PURGE)); + $ret = islandora_user_access($object, array(ISLANDORA_VIEW_OBJECTS), array(), TRUE, $user); $this->assertFalse($ret, 'User access denied when permissions did not match.'); // Test access with both permissions and content model. - $user = $this->drupalCreateUser(array(FEDORA_VIEW_OBJECTS)); + $user = $this->drupalCreateUser(array(ISLANDORA_VIEW_OBJECTS)); $model = $object->models; $model = reset($model); - $ret = islandora_user_access($object, array(FEDORA_VIEW_OBJECTS), array($model), TRUE, $user); + $ret = islandora_user_access($object, array(ISLANDORA_VIEW_OBJECTS), array($model), TRUE, $user); $this->assertTrue($ret, 'User access granted for matching permission and model.'); // Test access with matching permissions and non-matching content model. - $user = $this->drupalCreateUser(array(FEDORA_VIEW_OBJECTS)); - $ret = islandora_user_access($object, array(FEDORA_VIEW_OBJECTS), array('islandora:obviouslyNotACModel'), TRUE, $user); + $user = $this->drupalCreateUser(array(ISLANDORA_VIEW_OBJECTS)); + $ret = islandora_user_access($object, array(ISLANDORA_VIEW_OBJECTS), array('islandora:obviouslyNotACModel'), TRUE, $user); $this->assertFalse($ret, 'User access denied for matching permission and non-matching model.'); // Test access with all matching permissions and one matching model but // access any is FALSE. - $user = $this->drupalCreateUser(array(FEDORA_VIEW_OBJECTS, FEDORA_PURGE)); + $user = $this->drupalCreateUser(array(ISLANDORA_VIEW_OBJECTS, ISLANDORA_PURGE)); $model = $object->models; $model = reset($model); - $ret = islandora_user_access($object, array(FEDORA_VIEW_OBJECTS, FEDORA_PURGE), array($model, 'islandora:obviouslyNotACModel'), FALSE, $user); + $ret = islandora_user_access($object, array(ISLANDORA_VIEW_OBJECTS, ISLANDORA_PURGE), array($model, 'islandora:obviouslyNotACModel'), FALSE, $user); $this->assertFalse($ret, 'User access denied for all matching permissions and one matching model but with access any set to FALSE.'); - $ret = islandora_user_access($object, array(FEDORA_VIEW_OBJECTS, FEDORA_PURGE), array($model), FALSE, $user); + $ret = islandora_user_access($object, array(ISLANDORA_VIEW_OBJECTS, ISLANDORA_PURGE), array($model), FALSE, $user); $this->assertTrue($ret, 'User access granted for all matching permissions and matching models with access any set to FALSE.'); // Test passing in a Datastream. - $user = $this->drupalCreateUser(array(FEDORA_VIEW_OBJECTS, FEDORA_PURGE)); - $ret = islandora_user_access($object['DC'], array(FEDORA_VIEW_OBJECTS), array(), TRUE, $user); + $user = $this->drupalCreateUser(array(ISLANDORA_VIEW_OBJECTS, ISLANDORA_PURGE)); + $ret = islandora_user_access($object['DC'], array(ISLANDORA_VIEW_OBJECTS), array(), TRUE, $user); $this->assertTrue($ret, 'User access granted for matching permissions, with a datastream given instead of an object.'); } } diff --git a/theme/theme.inc b/theme/theme.inc index d988ee77..10e1cd29 100644 --- a/theme/theme.inc +++ b/theme/theme.inc @@ -115,7 +115,7 @@ function islandora_preprocess_islandora_default(&$variables) { $download_path = islandora_datastream_get_url($ds, 'download'); $datastreams[$id]['id'] = $id; $datastreams[$id]['label'] = $label; - $datastreams[$id]['label_link'] = islandora_datastream_access(FEDORA_VIEW_OBJECTS, $ds) ? + $datastreams[$id]['label_link'] = islandora_datastream_access(ISLANDORA_VIEW_OBJECTS, $ds) ? l($label, $download_path) : $label; $datastreams[$id]['download_url'] = $download_path; @@ -130,14 +130,14 @@ function islandora_preprocess_islandora_default(&$variables) { } $variables['datastreams'] = $datastreams; // Objects in fcrepo4 don't always contain a DC datastream. - if (isset($islandora_object['DC']) && islandora_datastream_access(FEDORA_VIEW_OBJECTS, $islandora_object['DC'])) { + if (isset($islandora_object['DC']) && islandora_datastream_access(ISLANDORA_VIEW_OBJECTS, $islandora_object['DC'])) { $dc_object = DublinCore::importFromXMLString($islandora_object['DC']->content); $dc_array = $dc_object->asArray(); } $variables['dc_array'] = isset($dc_array) ? $dc_array : array(); $variables['islandora_dublin_core'] = isset($dc_object) ? $dc_object : NULL; $variables['islandora_object_label'] = $islandora_object->label; - if (isset($islandora_object['TN']) && islandora_datastream_access(FEDORA_VIEW_OBJECTS, $islandora_object['TN'])) { + if (isset($islandora_object['TN']) && islandora_datastream_access(ISLANDORA_VIEW_OBJECTS, $islandora_object['TN'])) { $variables['islandora_thumbnail_url'] = url("islandora/object/{$islandora_object->id}/datastream/TN/view"); } } @@ -219,11 +219,11 @@ function islandora_preprocess_islandora_objects(array &$variables) { $o = islandora_object_load($o); $url = "islandora/object/{$o->id}"; $link_options = array('html' => TRUE, 'attributes' => array('title' => $o->label)); - $img = islandora_datastream_access(FEDORA_VIEW_OBJECTS, $o['TN']) ? + $img = islandora_datastream_access(ISLANDORA_VIEW_OBJECTS, $o['TN']) ? theme('image', array('path' => url("$url/datastream/TN/view"), 'attributes' => array())) : ''; $description = NULL; - if (isset($o['DC']) && islandora_datastream_access(FEDORA_VIEW_OBJECTS, $o['DC'])) { + if (isset($o['DC']) && islandora_datastream_access(ISLANDORA_VIEW_OBJECTS, $o['DC'])) { $dc = DublinCore::importFromXMLString($o['DC']->content); if ($dc) { $dc = $dc->asArray(); @@ -260,7 +260,7 @@ function theme_islandora_datastream_download_link(array $vars) { module_load_include('inc', 'islandora', 'includes/utilities'); $label = t('download'); - return islandora_datastream_access(FEDORA_VIEW_OBJECTS, $datastream) ? + return islandora_datastream_access(ISLANDORA_VIEW_OBJECTS, $datastream) ? l($label, islandora_datastream_get_url($datastream, 'download')) : ''; } @@ -289,7 +289,7 @@ function theme_islandora_datastream_view_link(array $vars) { } if ($vars['version'] === NULL) { - $perm = FEDORA_VIEW_OBJECTS; + $perm = ISLANDORA_VIEW_OBJECTS; } else { $perm = ISLANDORA_VIEW_DATASTREAM_HISTORY; @@ -319,7 +319,7 @@ function theme_islandora_datastream_delete_link(array $vars) { $datastreams = module_invoke_all('islandora_undeletable_datastreams', $datastream->parent->models); - $can_delete = !in_array($datastream->id, $datastreams) && islandora_datastream_access(FEDORA_PURGE, $datastream); + $can_delete = !in_array($datastream->id, $datastreams) && islandora_datastream_access(ISLANDORA_PURGE, $datastream); if ($vars['version'] !== NULL) { if (count($datastream) == 1) { @@ -354,7 +354,7 @@ function theme_islandora_datastream_edit_link(array $vars) { $edit_registry = module_invoke_all('islandora_edit_datastream_registry', $datastream->parent, $datastream); - $can_edit = count($edit_registry) > 0 && islandora_datastream_access(FEDORA_METADATA_EDIT, $datastream); + $can_edit = count($edit_registry) > 0 && islandora_datastream_access(ISLANDORA_METADATA_EDIT, $datastream); return $can_edit ? l(t('edit'), "islandora/object/{$datastream->parent->id}/datastream/{$datastream->id}/edit") : From 248d7611d7152a3b23cc2e23be61d0780745852f Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Wed, 14 Aug 2013 14:53:50 +0000 Subject: [PATCH 10/38] Add some examples. --- islandora.drush.inc | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/islandora.drush.inc b/islandora.drush.inc index b2e75c47..6ba73cb5 100644 --- a/islandora.drush.inc +++ b/islandora.drush.inc @@ -26,6 +26,10 @@ function islandora_drush_command() { 'drupal dependencies' => array( 'islandora', ), + 'examples' => array( + 'drush -u 1 ispiro --module=islandora' => dt('Install missing solution pack objects for the "islandora" module.'), + 'drush -u 1 ispiro --module=islandora --force' => dt('Install all solution pack objects for the "islandora" module, purging any which currently exist.'), + ), 'bootstrap' => DRUSH_BOOTSTRAP_DRUPAL_LOGIN, ); $commands['islandora-solution-pack-uninstall-required-objects'] = array( @@ -36,13 +40,17 @@ function islandora_drush_command() { 'required' => TRUE, ), 'force' => array( - 'description' => dt('Force reinstallation of the objects.'), + 'description' => dt('Force uninstallation of the objects.'), ), ), 'aliases' => array('ispuro'), 'drupal dependencies' => array( 'islandora', ), + 'examples' => array( + 'drush -u 1 ispuro --module=islandora' => dt('Uninstall solution pack objects for the "islandora" module.'), + 'drush -u 1 ispuro --module=islandora --force' => dt('Force uninstallation of all solution pack objects for the "islandora" module.'), + ), 'bootstrap' => DRUSH_BOOTSTRAP_DRUPAL_LOGIN, ); $commands['islandora-solution-pack-required-objects-status'] = array( @@ -56,6 +64,10 @@ function islandora_drush_command() { 'drupal dependencies' => array( 'islandora', ), + 'examples' => array( + 'drush -u 1 ispros' => dt('Get the status of all solution pack objects.'), + 'drush -u 1 ispros --module=islandora' => dt('Get the status of solution pack objects for the "islandora" module.'), + ), 'bootstrap' => DRUSH_BOOTSTRAP_DRUPAL_LOGIN, ); From 430febedb8889e495350d7af999bc735246a12b1 Mon Sep 17 00:00:00 2001 From: root Date: Wed, 14 Aug 2013 18:19:25 +0000 Subject: [PATCH 11/38] Fixed links not working if Drupal is in a subfolder (e.g.: multi-site). --- js/add_print.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/js/add_print.js b/js/add_print.js index 7af5c3ca..0059ed88 100644 --- a/js/add_print.js +++ b/js/add_print.js @@ -8,11 +8,11 @@ */ (function ($) { $(document).ready(function() { - $('.tabs .primary').append(''); + $('.tabs .primary').append(''); $('#print_btn').css("cursor","pointer"); $('#print_btn').click(function() { - window.location=Drupal.settings.islandora.print_link; + window.location=Drupal.settings.basePath + Drupal.settings.islandora.print_link; }); }); -})(jQuery); - +})(jQuery); + From 9a985d1d0208272bc8d36f137a26404d36096441 Mon Sep 17 00:00:00 2001 From: Christian Selig Date: Wed, 14 Aug 2013 20:11:41 +0000 Subject: [PATCH 12/38] Removed prefixed slashes that caused links to break. --- islandora.module | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/islandora.module b/islandora.module index 7d2c0acb..11595b67 100644 --- a/islandora.module +++ b/islandora.module @@ -796,13 +796,13 @@ function islandora_view_object(AbstractObject $object) { $path = drupal_get_path('module', 'islandora'); drupal_add_js(array( 'islandora' => array( - 'print_img' => $path . '/images/print-icon.png'), + 'print_img' => $path . 'images/print-icon.png'), ), array( 'type' => 'setting')); drupal_add_js(array( 'islandora' => array( - 'print_link' => '/islandora/object/' . $object->id . '/print')), + 'print_link' => 'islandora/object/' . $object->id . '/print')), array('type' => 'setting')); drupal_add_js($path . '/js/add_print.js'); From 2dac96d31211274dda3832289aad2f4d3bf93f83 Mon Sep 17 00:00:00 2001 From: Christian Selig Date: Thu, 15 Aug 2013 13:47:11 +0000 Subject: [PATCH 13/38] Fixed link to print image not being formed correctly. --- islandora.module | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/islandora.module b/islandora.module index 11595b67..36c43313 100644 --- a/islandora.module +++ b/islandora.module @@ -796,7 +796,7 @@ function islandora_view_object(AbstractObject $object) { $path = drupal_get_path('module', 'islandora'); drupal_add_js(array( 'islandora' => array( - 'print_img' => $path . 'images/print-icon.png'), + 'print_img' => $path . '/images/print-icon.png'), ), array( 'type' => 'setting')); From 25e53892154d7f7888a61210fb81ceb7fb5de379 Mon Sep 17 00:00:00 2001 From: Jordan Dukart Date: Tue, 20 Aug 2013 14:57:34 +0000 Subject: [PATCH 14/38] 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 15/38] 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 16/38] 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 17/38] 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 18/38] 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 19/38] 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 20/38] 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); From d59099aab49cc059c57669031530facb518d75d0 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Thu, 22 Aug 2013 20:28:34 +0000 Subject: [PATCH 21/38] Add response headers. ... Still need to react to requests. --- includes/datastream.inc | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/includes/datastream.inc b/includes/datastream.inc index 585fa2f5..74831fd3 100644 --- a/includes/datastream.inc +++ b/includes/datastream.inc @@ -46,9 +46,20 @@ function islandora_view_datastream(AbstractDatastream $datastream, $download = F } } - header_remove('Cache-Control'); header_remove('Expires'); header('Content-type: ' . $datastream->mimetype); + $cache_control = array(); + if ($datastream->parent->repository->api->connection->username == 'anonymous') { + $cache_control[] = 'public'; + } + else { + $cache_control[] = 'private'; + } + $cache_control[] = 'must-revalidate'; + header('Cache-Control: ' . implode(', ', $cache_control)); + header('Last-Modified: '. $datastream->createdDate->format('D, d M Y H:i:s \G\M\T')); + header('Etag: "' . $datastream->createdDate->format('Uu') . '"'); + if ($datastream->controlGroup == 'M' || $datastream->controlGroup == 'X') { header('Content-length: ' . $datastream->size); } From 630ab0e5ae6965add81a90f1ec866d9a3e870998 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Fri, 23 Aug 2013 13:36:47 +0000 Subject: [PATCH 22/38] Implement the HTTP cache validation shenanigans. --- includes/datastream.inc | 143 +++++++++++++++++++++++++++++++++++----- 1 file changed, 128 insertions(+), 15 deletions(-) diff --git a/includes/datastream.inc b/includes/datastream.inc index 74831fd3..f52ed3a5 100644 --- a/includes/datastream.inc +++ b/includes/datastream.inc @@ -46,20 +46,7 @@ function islandora_view_datastream(AbstractDatastream $datastream, $download = F } } - header_remove('Expires'); header('Content-type: ' . $datastream->mimetype); - $cache_control = array(); - if ($datastream->parent->repository->api->connection->username == 'anonymous') { - $cache_control[] = 'public'; - } - else { - $cache_control[] = 'private'; - } - $cache_control[] = 'must-revalidate'; - header('Cache-Control: ' . implode(', ', $cache_control)); - header('Last-Modified: '. $datastream->createdDate->format('D, d M Y H:i:s \G\M\T')); - header('Etag: "' . $datastream->createdDate->format('Uu') . '"'); - if ($datastream->controlGroup == 'M' || $datastream->controlGroup == 'X') { header('Content-length: ' . $datastream->size); } @@ -70,13 +57,139 @@ function islandora_view_datastream(AbstractDatastream $datastream, $download = F $filename = $datastream->label . '.' . $extension; header("Content-Disposition: attachment; filename=\"$filename\""); } + + $cache_check = islandora_view_datastream_cache_check($datastream); + if ($cache_check !== 200) { + if ($cache_check === 304) { + header('HTTP/1.1 304 Not Modified'); + } + elseif ($cache_check === 412) { + header('HTTP/1.0 412 Precondition Failed'); + } + } + islandora_view_datastream_set_cache_headers($datastream); + drupal_page_is_cacheable(FALSE); // Try not to load the file into PHP memory! - ob_end_flush(); - $datastream->getContent('php://output'); + // Close and flush ALL the output buffers! + while(@ob_end_flush()) {}; + + // New content needed. + if ($cache_check === 200) { + $datastream->getContent('php://output'); + } exit(); } +/** + * Parse "etags" from HTTP If-Match or If-None-Match headers. + * + * @param string $header_value + * The value from the headers. + * + * @return array + * An array containing all the etags present. + */ +function islandora_parse_http_match_headers($header_value) { + $matches = array(); + $count = preg_match_all('/(((W\/)?("?)(\*|.+?)\4)(, +)?)/', $header_value, $matches); + return $matches[5]; +} + +/** + * Validate cache headers. + * + * @param AbstractDatastream $datastrea + * + * @return int + * An integer representing the HTTP response code. One of: + * - 200: Proceed as normal. + * - 304: Resource hasn't changed; pass cache validation. + * - 412: Resource has channged; fail cache validation. + * + * @see http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html + */ +function islandora_view_datastream_cache_check(AbstractDatastream $datastream) { + // Let's assume that if we get here, we'll be able to complete the request. + $return = 200; + + if (isset($_SERVER['HTTP_IF_MODIFIED_SINCE'])) { + $modified_since = DateTime::createFromFormat('D, d M Y H:i:s e', $_SERVER['HTTP_IF_MODIFIED_SINCE']); + if ($datastream->createdDate->getTimestamp() - $modified_since->getTimestamp() > 0) { + // Changed! + return $return; + } + else { + $return = 304; + } + } + if ($return === 200 && isset($_SERVER['HTTP_IF_UNMODIFIED_SINCE'])) { + $unmodified_since = DateTime::createFromFormat('D, d M Y H:i:s e', $_SERVER['HTTP_IF_UNMODIFIED_SINCE']); + if ($datastream->createdDate->getTimestamp() !== $unmodified_since->getTimestamp()) { + // Changed! + $return = 412; + } + else { + return $return; + } + } + + // Only consider Etags we have provided. + if (isset($datastream->checksum)) { + $tags = array(); + foreach ($datastream as $offset => $version) { + if (isset($version->checksum)) { + $tags[$offset] = $version->checksum; + } + } + + if ($return === 200 && isset($_SERVER['HTTP_IF_MATCH'])) { + $request_tags = islandora_parse_http_match_headers($_SERVER['HTTP_IF_MATCH']); + if (in_array('*', $request_tags) || count(array_intersect($tags, $request_tags)) > 0) { + // There's a match... Let things go ahead. + return $return; + } + else { + $return = 412; + } + } + if (in_array($return, array(200, 304), TRUE) && isset($_SERVER['HTTP_IF_NONE_MATCH'])) { + $request_tags = islandora_parse_http_match_headers($_SERVER['HTTP_IF_NONE_MATCH']); + if (in_array('*', $request_tags) || count(array_intersect($tags, $request_tags)) > 0) { + $return = 304; + } + else { + $return = 200; + } + } + } + + return $return; +} + +/** + * Set various HTTP headers for caching. + * + * @param AbstractDatastream $datastream + * The datastream being viewed/downloaded. + */ +function islandora_view_datastream_set_cache_headers(AbstractDatastream $datastream) { + header_remove('Expires'); + $cache_control = array(); + if ($datastream->parent->repository->api->connection->username == 'anonymous') { + $cache_control[] = 'public'; + } + else { + $cache_control[] = 'private'; + } + $cache_control[] = 'must-revalidate'; + header('Cache-Control: ' . implode(', ', $cache_control)); + header('Last-Modified: '. $datastream->createdDate->format('D, d M Y H:i:s \G\M\T')); + if (isset($datastream->checksum)) { + header("Etag: \"{$datastream->checksum}\""); + } +} + /** * Get the human readable size of the given datastream. * From 55c3fc0b9055d90d4128d548019ee1bedfd94604 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Fri, 23 Aug 2013 14:13:18 +0000 Subject: [PATCH 23/38] Force cache revalidation. --- includes/datastream.inc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/includes/datastream.inc b/includes/datastream.inc index f52ed3a5..43f05478 100644 --- a/includes/datastream.inc +++ b/includes/datastream.inc @@ -174,7 +174,7 @@ function islandora_view_datastream_cache_check(AbstractDatastream $datastream) { * The datastream being viewed/downloaded. */ function islandora_view_datastream_set_cache_headers(AbstractDatastream $datastream) { - header_remove('Expires'); + header('Expires: Sun, 19 Nov 1978 05:00:00 GMT'); $cache_control = array(); if ($datastream->parent->repository->api->connection->username == 'anonymous') { $cache_control[] = 'public'; @@ -183,6 +183,7 @@ function islandora_view_datastream_set_cache_headers(AbstractDatastream $datastr $cache_control[] = 'private'; } $cache_control[] = 'must-revalidate'; + $cache_control[] = 'max-age=0'; header('Cache-Control: ' . implode(', ', $cache_control)); header('Last-Modified: '. $datastream->createdDate->format('D, d M Y H:i:s \G\M\T')); if (isset($datastream->checksum)) { From 64c90936d9f1f67a87d1d9d8d1403e6790e181fd Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Fri, 23 Aug 2013 16:01:01 +0000 Subject: [PATCH 24/38] Coding standards. --- includes/datastream.inc | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/includes/datastream.inc b/includes/datastream.inc index 43f05478..27a25f6e 100644 --- a/includes/datastream.inc +++ b/includes/datastream.inc @@ -64,7 +64,7 @@ function islandora_view_datastream(AbstractDatastream $datastream, $download = F header('HTTP/1.1 304 Not Modified'); } elseif ($cache_check === 412) { - header('HTTP/1.0 412 Precondition Failed'); + header('HTTP/1.0 412 Precondition Failed'); } } islandora_view_datastream_set_cache_headers($datastream); @@ -72,7 +72,8 @@ function islandora_view_datastream(AbstractDatastream $datastream, $download = F 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()) {}; + while (@ob_end_flush()) { + }; // New content needed. if ($cache_check === 200) { @@ -93,13 +94,13 @@ function islandora_view_datastream(AbstractDatastream $datastream, $download = F function islandora_parse_http_match_headers($header_value) { $matches = array(); $count = preg_match_all('/(((W\/)?("?)(\*|.+?)\4)(, +)?)/', $header_value, $matches); - return $matches[5]; + return $matches[5]; } /** * Validate cache headers. * - * @param AbstractDatastream $datastrea + * @param AbstractDatastream $datastream * * @return int * An integer representing the HTTP response code. One of: @@ -174,6 +175,7 @@ function islandora_view_datastream_cache_check(AbstractDatastream $datastream) { * The datastream being viewed/downloaded. */ function islandora_view_datastream_set_cache_headers(AbstractDatastream $datastream) { + // Force cache revalidation. header('Expires: Sun, 19 Nov 1978 05:00:00 GMT'); $cache_control = array(); if ($datastream->parent->repository->api->connection->username == 'anonymous') { @@ -185,7 +187,7 @@ function islandora_view_datastream_set_cache_headers(AbstractDatastream $datastr $cache_control[] = 'must-revalidate'; $cache_control[] = 'max-age=0'; header('Cache-Control: ' . implode(', ', $cache_control)); - header('Last-Modified: '. $datastream->createdDate->format('D, d M Y H:i:s \G\M\T')); + header('Last-Modified: ' . $datastream->createdDate->format('D, d M Y H:i:s \G\M\T')); if (isset($datastream->checksum)) { header("Etag: \"{$datastream->checksum}\""); } From f0c2e2dd88ddf78bea23a603aab6d00d169f1ac6 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Fri, 23 Aug 2013 17:22:33 +0000 Subject: [PATCH 25/38] Code standards, again. --- includes/datastream.inc | 1 + 1 file changed, 1 insertion(+) diff --git a/includes/datastream.inc b/includes/datastream.inc index 27a25f6e..0eda5d4d 100644 --- a/includes/datastream.inc +++ b/includes/datastream.inc @@ -101,6 +101,7 @@ function islandora_parse_http_match_headers($header_value) { * Validate cache headers. * * @param AbstractDatastream $datastream + * The datastream for which to check the request headers against. * * @return int * An integer representing the HTTP response code. One of: From b002191085827a8482bd8c6cddeae921598e85e4 Mon Sep 17 00:00:00 2001 From: MorganDawe Date: Tue, 27 Aug 2013 13:46:26 -0300 Subject: [PATCH 26/38] Added the previously removed print callback, as it was required by another module and was breaking clipping. --- islandora.module | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/islandora.module b/islandora.module index 1a53aebf..ed2268db 100644 --- a/islandora.module +++ b/islandora.module @@ -297,6 +297,15 @@ function islandora_menu() { 'access arguments' => array(ISLANDORA_VIEW_DATASTREAM_HISTORY, 4), 'load arguments' => array(2), ); + $items['islandora/object/%islandora_object/printer'] = array( + 'title' => 'Print Object', + 'page callback' => 'islandora_print_object', + 'page arguments' => array(2), + 'type' => MENU_CALLBACK, + 'access callback' => 'islandora_object_access', + 'access arguments' => array(FEDORA_VIEW_OBJECTS, 2), + 'load arguments' => array(2), + ); $items['islandora/object/%islandora_object/download_clip'] = array( 'page callback' => 'islandora_download_clip', 'page arguments' => array(2), @@ -341,6 +350,12 @@ function islandora_theme() { 'variables' => array('islandora_object' => NULL), ), // Default edit page. + 'islandora_default_print' => array( + 'file' => 'theme/theme.inc', + 'template' => 'theme/islandora-object-print', + 'variables' => array('islandora_object' => NULL), + ), + // Default edit page. 'islandora_default_edit' => array( 'file' => 'theme/theme.inc', 'template' => 'theme/islandora-object-edit', @@ -454,6 +469,22 @@ function islandora_permission() { ); } +/** + * Renders the print page for the given object. + * + * Modules can either implement preprocess functions to append content onto the + * 'content' variable, or override the display by providing a theme suggestion. + * + * @param AbstractObject $object + * The object. + * + * @return array + * A renderable array. + */ +function islandora_print_object(AbstractObject $object) { + drupal_set_title($object->label); + return theme('islandora_object_print', array('object' => $object)); +} /** * Implements hook_forms(). */ From fcb2bf280573ef275c8004eb2a91490c9ff9d095 Mon Sep 17 00:00:00 2001 From: Nigel Banks Date: Tue, 27 Aug 2013 23:41:57 +0200 Subject: [PATCH 27/38] Prevents a white screen when a viewer is set but not enabled. If a user selects a viewer and then disables that module, but does not change any solution packs viewers settings this can occur. --- includes/solution_packs.inc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/includes/solution_packs.inc b/includes/solution_packs.inc index a80c41a5..7cf0351a 100644 --- a/includes/solution_packs.inc +++ b/includes/solution_packs.inc @@ -745,7 +745,9 @@ function islandora_get_viewer($params = NULL, $variable_id = NULL, $fedora_objec $viewer_id = islandora_get_viewer_id($variable_id); if ($viewer_id AND $params !== NULL) { $callback = islandora_get_viewer_callback($viewer_id); - return $callback($params, $fedora_object); + if (function_exists($callback)) { + return $callback($params, $fedora_object); + } } } return FALSE; From 76e09b2471f3bd0259769be5ed74d093cc8807db Mon Sep 17 00:00:00 2001 From: MorganDawe Date: Wed, 28 Aug 2013 09:50:56 -0300 Subject: [PATCH 28/38] Fixed Menu path conflicts with printing dc_data and clipping large image and newspaper. --- islandora.module | 42 +++++++++++++++++++----------------------- js/add_print.js | 1 + 2 files changed, 20 insertions(+), 23 deletions(-) diff --git a/islandora.module b/islandora.module index ed2268db..a8250fcc 100644 --- a/islandora.module +++ b/islandora.module @@ -119,13 +119,22 @@ function islandora_menu() { 'access callback' => 'islandora_object_access_callback', 'access arguments' => array(ISLANDORA_VIEW_OBJECTS, 2), ); - $items['islandora/object/%islandora_object/print'] = array( + $items['islandora/object/%islandora_object/print_object'] = array( 'page callback' => 'islandora_printer_object', 'page arguments' => array(2), 'type' => MENU_NORMAL_ITEM, - 'access callback' => 'islandora_object_access_callback', + 'access callback' => 'islandora_object_access', 'access arguments' => array(ISLANDORA_VIEW_OBJECTS, 2), ); + $items['islandora/object/%islandora_object/print'] = array( + 'title' => 'Print Object', + 'page callback' => 'islandora_print_object', + 'page arguments' => array(2), + 'type' => MENU_CALLBACK, + 'access callback' => 'islandora_object_access', + 'access arguments' => array(FEDORA_VIEW_OBJECTS, 2), + 'load arguments' => array(2), + ); $items['islandora/object/%islandora_object/view'] = array( 'title' => 'View', 'type' => MENU_DEFAULT_LOCAL_TASK, @@ -151,13 +160,11 @@ function islandora_menu() { ISLANDORA_INGEST, ), 2), ); - $items['islandora/object/%islandora_object/manage/overview'] = array( 'title' => 'Overview', 'type' => MENU_DEFAULT_LOCAL_TASK, 'weight' => -20, ); - $items['islandora/object/%islandora_object/manage/datastreams'] = array( 'title' => 'Datastreams', 'type' => MENU_LOCAL_TASK, @@ -172,7 +179,6 @@ function islandora_menu() { ), 2), 'weight' => -10, ); - $items['islandora/object/%islandora_object/manage/properties'] = array( 'title' => 'Properties', 'page callback' => 'drupal_get_form', @@ -297,15 +303,6 @@ function islandora_menu() { 'access arguments' => array(ISLANDORA_VIEW_DATASTREAM_HISTORY, 4), 'load arguments' => array(2), ); - $items['islandora/object/%islandora_object/printer'] = array( - 'title' => 'Print Object', - 'page callback' => 'islandora_print_object', - 'page arguments' => array(2), - 'type' => MENU_CALLBACK, - 'access callback' => 'islandora_object_access', - 'access arguments' => array(FEDORA_VIEW_OBJECTS, 2), - 'load arguments' => array(2), - ); $items['islandora/object/%islandora_object/download_clip'] = array( 'page callback' => 'islandora_download_clip', 'page arguments' => array(2), @@ -350,12 +347,6 @@ function islandora_theme() { 'variables' => array('islandora_object' => NULL), ), // Default edit page. - 'islandora_default_print' => array( - 'file' => 'theme/theme.inc', - 'template' => 'theme/islandora-object-print', - 'variables' => array('islandora_object' => NULL), - ), - // Default edit page. 'islandora_default_edit' => array( 'file' => 'theme/theme.inc', 'template' => 'theme/islandora-object-edit', @@ -366,8 +357,13 @@ function islandora_theme() { 'file' => 'includes/solution_packs.inc', 'render element' => 'form', ), - // Print object view. + // Print used by the clipper. 'islandora_object_print' => array( + 'file' => 'theme/theme.inc', + 'variables' => array('object' => NULL, 'content' => array()), + ), + // Print object view, prints islandora objects. + 'islandora_object_print_object' => array( 'file' => 'theme/theme.inc', 'template' => 'theme/islandora-object-print', 'variables' => array( @@ -866,7 +862,7 @@ function islandora_view_object(AbstractObject $object) { drupal_add_js(array( 'islandora' => array( - 'print_link' => 'islandora/object/' . $object->id . '/print')), + 'print_link' => 'islandora/object/' . $object->id . '/print_object')), array('type' => 'setting')); drupal_add_js($path . '/js/add_print.js'); @@ -999,7 +995,7 @@ function islandora_default_islandora_printer_object($object, $alter) { } $variables = isset($dc_object) ? $dc_object->asArray() : array(); - $output = theme('islandora_object_print', array( + $output = theme('islandora_object_print_object', array( 'object' => $object, 'dc_array' => $variables, 'islandora_content' => $alter)); diff --git a/js/add_print.js b/js/add_print.js index 0059ed88..014ec2e3 100644 --- a/js/add_print.js +++ b/js/add_print.js @@ -10,6 +10,7 @@ $(document).ready(function() { $('.tabs .primary').append(''); $('#print_btn').css("cursor","pointer"); + console.log(Drupal.settings.basePath + Drupal.settings.islandora.print_link); $('#print_btn').click(function() { window.location=Drupal.settings.basePath + Drupal.settings.islandora.print_link; }); From 81cc6be97748b359e8cf4543cca10b18c83e6ce0 Mon Sep 17 00:00:00 2001 From: MorganDawe Date: Wed, 28 Aug 2013 09:58:21 -0300 Subject: [PATCH 29/38] Removed console log function and reference to old view object permission. --- islandora.module | 2 +- js/add_print.js | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/islandora.module b/islandora.module index a8250fcc..8fa20898 100644 --- a/islandora.module +++ b/islandora.module @@ -132,7 +132,7 @@ function islandora_menu() { 'page arguments' => array(2), 'type' => MENU_CALLBACK, 'access callback' => 'islandora_object_access', - 'access arguments' => array(FEDORA_VIEW_OBJECTS, 2), + 'access arguments' => array(ISLANDORA_VIEW_OBJECTS, 2), 'load arguments' => array(2), ); $items['islandora/object/%islandora_object/view'] = array( diff --git a/js/add_print.js b/js/add_print.js index 014ec2e3..0059ed88 100644 --- a/js/add_print.js +++ b/js/add_print.js @@ -10,7 +10,6 @@ $(document).ready(function() { $('.tabs .primary').append(''); $('#print_btn').css("cursor","pointer"); - console.log(Drupal.settings.basePath + Drupal.settings.islandora.print_link); $('#print_btn').click(function() { window.location=Drupal.settings.basePath + Drupal.settings.islandora.print_link; }); From 9335fc3b3a96a16eb0d71aff7eb04214e2ec7d2e Mon Sep 17 00:00:00 2001 From: William Panting Date: Wed, 28 Aug 2013 10:33:00 -0300 Subject: [PATCH 30/38] api doc clarification --- islandora.api.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/islandora.api.php b/islandora.api.php index 6d25bae3..7fe54828 100644 --- a/islandora.api.php +++ b/islandora.api.php @@ -86,7 +86,7 @@ function hook_CMODEL_PID_islandora_view_object_alter(&$object, &$rendered) { } /** - * Generate an object's management display. + * Generate an object's datastreams management display. * * @param AbstractObject $object * A Tuque FedoraObject @@ -98,7 +98,7 @@ function hook_islandora_edit_object($object) { } /** - * Generate an object's management display for the given content model. + * Generate an object's datastreams management display based on content model. * * Content models PIDs have colons and hyphens changed to underscores, to * create the hook name. @@ -113,7 +113,7 @@ function hook_CMODEL_PID_islandora_edit_object($object) { } /** - * Allow management display output to be altered. + * Allow datastreams management display output to be altered. * * @param AbstractObject $object * A Tuque FedoraObject From 67999de1e788aa37d138222f6c26b1da1b8798f0 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Wed, 28 Aug 2013 17:04:34 +0000 Subject: [PATCH 31/38] Add checkbox to disable caching functionality. --- includes/admin.form.inc | 6 ++++++ includes/datastream.inc | 17 ++++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/includes/admin.form.inc b/includes/admin.form.inc index d34bd5d5..b0bb0642 100644 --- a/includes/admin.form.inc +++ b/includes/admin.form.inc @@ -55,6 +55,12 @@ function islandora_repository_admin(array $form, array &$form_state) { '#description' => t('The PID of the Root Collection Object'), '#required' => TRUE, ), + 'islandora_use_datastream_cache_headers' => array( + '#type' => 'checkbox', + '#title' => t('Generate/parse datastream HTTP cache headers'), + '#description' => t('HTTP caching can reduce network traffic, by allowing clients to used cached copies.'), + '#default_value' => variable_get('islandora_use_datastream_cache_headers', TRUE), + ), ), 'islandora_namespace' => array( '#type' => 'fieldset', diff --git a/includes/datastream.inc b/includes/datastream.inc index 0eda5d4d..6a537c2b 100644 --- a/includes/datastream.inc +++ b/includes/datastream.inc @@ -85,6 +85,15 @@ function islandora_view_datastream(AbstractDatastream $datastream, $download = F /** * Parse "etags" from HTTP If-Match or If-None-Match headers. * + * Parses from the CSV-like struture supported by HTTP headers into an array, + * so `"asdf", "fdsa", W/"2132"` should become an array containing the strings: + * - asdf + * - fdsa + * - 2132 + * + * @see http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.24 + * @see http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.26 + * * @param string $header_value * The value from the headers. * @@ -93,7 +102,9 @@ function islandora_view_datastream(AbstractDatastream $datastream, $download = F */ function islandora_parse_http_match_headers($header_value) { $matches = array(); + // Match the CSV-like structure supported by the HTTP headers. $count = preg_match_all('/(((W\/)?("?)(\*|.+?)\4)(, +)?)/', $header_value, $matches); + // The fifth sub-expression/group is which will contain the etags. return $matches[5]; } @@ -105,13 +116,17 @@ function islandora_parse_http_match_headers($header_value) { * * @return int * An integer representing the HTTP response code. One of: - * - 200: Proceed as normal. + * - 200: Proceed as normal. (Full download). * - 304: Resource hasn't changed; pass cache validation. * - 412: Resource has channged; fail cache validation. * * @see http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html */ function islandora_view_datastream_cache_check(AbstractDatastream $datastream) { + if (!variable_get('islandora_use_datastream_cache_headers', TRUE)) { + return 200; + } + // Let's assume that if we get here, we'll be able to complete the request. $return = 200; From cc5c9825408c2ae09e51c6b178ac2278fb9b2269 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Wed, 28 Aug 2013 19:03:55 +0000 Subject: [PATCH 32/38] Add test to test datastream cache responses. --- islandora.info | 1 + tests/datastream_cache.test | 163 ++++++++++++++++++++++++++++++++++++ 2 files changed, 164 insertions(+) create mode 100644 tests/datastream_cache.test diff --git a/islandora.info b/islandora.info index 4b48bc29..36ed0dff 100644 --- a/islandora.info +++ b/islandora.info @@ -18,5 +18,6 @@ files[] = tests/ingest.test files[] = tests/hooked_access.test files[] = tests/islandora_manage_permissions.test files[] = tests/datastream_versions.test +files[] = tests/datastream_cache.test files[] = tests/derivatives.test php = 5.3 diff --git a/tests/datastream_cache.test b/tests/datastream_cache.test new file mode 100644 index 00000000..ffd752c5 --- /dev/null +++ b/tests/datastream_cache.test @@ -0,0 +1,163 @@ + 'Datastream Cache Headers', + 'description' => 'Check our headers work as we expect them to.', + 'group' => 'Islandora', + ); + } + + /** + * Creates an admin user and a connection to a fedora repository. + * + * @see IslandoraWebTestCase::setUp() + */ + public function setUp() { + parent::setUp(); + $this->repository = $this->admin->repository; + $this->purgeTestObjects(); + } + + /** + * Free any objects/resources created for this test. + * + * @see IslandoraWebTestCase::tearDown() + */ + public function tearDown() { + $this->purgeTestObjects(); + parent::tearDown(); + } + + /** + * Purge any objects created by the test's in this class. + */ + public function purgeTestObjects() { + $objects = array( + 'test:test', + ); + foreach ($objects as $object) { + try { + $object = $this->repository->getObject($object); + $this->repository->purgeObject($object->id); + } + catch (Exception $e) { + // Meh... Either it didn't exist or the purge failed. + } + } + } + + public function createTestObject() { + $object = $this->repository->constructObject('test:test'); + $object->label = 'Test object'; + $object->models = 'test:model'; + $datastream = $object->constructDatastream('asdf', 'M'); + $datastream->label = 'datastream of doom'; + $datastream->mimetype = 'text/plain'; + $datastream->content = 'And then things happened.'; + $datastream->checksumType = 'SHA-1'; + $object->ingestDatastream($datastream); + $this->repository->ingestObject($object); + return $object; + } + + /** + * Test HTTP cache headers. + */ + public function testCacheHeaders() { + $object = $this->createTestObject(); + $datastream = $object['asdf']; + + $user = $this->drupalCreateUser(array(ISLANDORA_VIEW_OBJECTS)); + $this->drupalLogin($user); + + // Test If-Modified-Since. + $result = $this->drupalGet("islandora/object/{$object->id}/datastream/{$datastream->id}/view", array(), array( + 'If-Modified-Since: ' . $datastream->createdDate->format('D, d M Y H:i:s \G\M\T'), + )); + $this->assertResponse(304); + $result = $this->drupalGet("islandora/object/{$object->id}/datastream/{$datastream->id}/view", array(), array( + 'If-Modified-Since: ' . $datastream->createdDate->sub(new DateInterval('P1M'))->format('D, d M Y H:i:s \G\M\T'), + )); + $this->assertResponse(200); + + // Test If-Unmodified-Since. + $result = $this->drupalGet("islandora/object/{$object->id}/datastream/{$datastream->id}/view", array(), array( + 'If-Unmodified-Since: ' . $datastream->createdDate->format('D, d M Y H:i:s \G\M\T'), + )); + $this->assertResponse(200); + $result = $this->drupalGet("islandora/object/{$object->id}/datastream/{$datastream->id}/view", array(), array( + 'If-Unmodified-Since: ' . $datastream->createdDate->sub(new DateInterval('P1M'))->format('D, d M Y H:i:s \G\M\T'), + )); + $this->assertResponse(412); + + // Test If-Match. + $result = $this->drupalGet("islandora/object/{$object->id}/datastream/{$datastream->id}/view", array(), array( + format_string('If-Match: "!checksum"', array( + '!checksum' => $datastream->checksum, + )), + )); + $this->assertResponse(200); + $result = $this->drupalGet("islandora/object/{$object->id}/datastream/{$datastream->id}/view", array(), array( + format_string('If-Match: "!checksum"', array( + '!checksum' => 'dont-match' . $datastream->checksum, + )), + )); + $this->assertResponse(412); + + // Test If-None-Match. + $result = $this->drupalGet("islandora/object/{$object->id}/datastream/{$datastream->id}/view", array(), array( + format_string('If-None-Match: "!checksum"', array( + '!checksum' => $datastream->checksum, + )), + )); + $this->assertResponse(304); + $result = $this->drupalGet("islandora/object/{$object->id}/datastream/{$datastream->id}/view", array(), array( + format_string('If-None-Match: "!checksum"', array( + '!checksum' => 'dont-match' . $datastream->checksum, + )), + )); + $this->assertResponse(200); + + // Test combination of If-None-Match and If-Modified-Since + $result = $this->drupalGet("islandora/object/{$object->id}/datastream/{$datastream->id}/view", array(), array( + 'If-Modified-Since: ' . $datastream->createdDate->format('D, d M Y H:i:s \G\M\T'), + format_string('If-None-Match: "!checksum"', array( + '!checksum' => $datastream->checksum, + )), + )); + $this->assertResponse(304); + $result = $this->drupalGet("islandora/object/{$object->id}/datastream/{$datastream->id}/view", array(), array( + 'If-Modified-Since: ' . $datastream->createdDate->format('D, d M Y H:i:s \G\M\T'), + format_string('If-None-Match: "!checksum"', array( + '!checksum' => 'dont-match' . $datastream->checksum, + )), + )); + $this->assertResponse(200); + $result = $this->drupalGet("islandora/object/{$object->id}/datastream/{$datastream->id}/view", array(), array( + 'If-Modified-Since: ' . $datastream->createdDate->sub(new DateInterval('P1M'))->format('D, d M Y H:i:s \G\M\T'), + format_string('If-None-Match: "!checksum"', array( + '!checksum' => $datastream->checksum, + )), + )); + $this->assertResponse(200); + } +} From 6b4768d9f69834da35d132132c46ae18853db009 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Wed, 28 Aug 2013 19:29:46 +0000 Subject: [PATCH 33/38] Add missing function comment. --- tests/datastream_cache.test | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/datastream_cache.test b/tests/datastream_cache.test index ffd752c5..0be7dd54 100644 --- a/tests/datastream_cache.test +++ b/tests/datastream_cache.test @@ -65,7 +65,10 @@ class IslandoraDatastreamCacheTestCase extends IslandoraWebTestCase { } } - public function createTestObject() { + /** + * Create our test object. + */ + protected function createTestObject() { $object = $this->repository->constructObject('test:test'); $object->label = 'Test object'; $object->models = 'test:model'; From 44d83c7ed90468d31f1badb49f750648aa513789 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Thu, 29 Aug 2013 12:49:46 +0000 Subject: [PATCH 34/38] Fix some comments for Jordan. --- includes/datastream.inc | 4 ++-- tests/datastream_cache.test | 8 +------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/includes/datastream.inc b/includes/datastream.inc index 6a537c2b..f7e20b27 100644 --- a/includes/datastream.inc +++ b/includes/datastream.inc @@ -118,8 +118,8 @@ function islandora_parse_http_match_headers($header_value) { * An integer representing the HTTP response code. One of: * - 200: Proceed as normal. (Full download). * - 304: Resource hasn't changed; pass cache validation. - * - 412: Resource has channged; fail cache validation. - * + * - 412: Resource has changed; fail cache validation. + * * @see http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html */ function islandora_view_datastream_cache_check(AbstractDatastream $datastream) { diff --git a/tests/datastream_cache.test b/tests/datastream_cache.test index 0be7dd54..8118c08a 100644 --- a/tests/datastream_cache.test +++ b/tests/datastream_cache.test @@ -2,13 +2,7 @@ /** * @file - * Tests to see if the hooks get called when appropriate. - * - * In the test module 'islandora_hooks_test' there are implementations - * of hooks being tested. These implementations modifies the session, and - * that's how we test if the hook gets called. - * - * To make sense of these tests reference islandora_hooks_test.module. + * Tests to verify the cache headers we provide. */ class IslandoraDatastreamCacheTestCase extends IslandoraWebTestCase { From dca15b1ccdd2df044298b3a7126000a41e2c0a05 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Thu, 29 Aug 2013 19:52:28 +0000 Subject: [PATCH 35/38] Respond with the same headers as we did before, if caching is disabled. --- includes/datastream.inc | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/includes/datastream.inc b/includes/datastream.inc index f7e20b27..1de45581 100644 --- a/includes/datastream.inc +++ b/includes/datastream.inc @@ -191,21 +191,27 @@ function islandora_view_datastream_cache_check(AbstractDatastream $datastream) { * The datastream being viewed/downloaded. */ function islandora_view_datastream_set_cache_headers(AbstractDatastream $datastream) { - // Force cache revalidation. - header('Expires: Sun, 19 Nov 1978 05:00:00 GMT'); - $cache_control = array(); - if ($datastream->parent->repository->api->connection->username == 'anonymous') { - $cache_control[] = 'public'; + if (variable_get('islandora_use_datastream_cache_headers', TRUE)) { + // Force cache revalidation. + header('Expires: Sun, 19 Nov 1978 05:00:00 GMT'); + $cache_control = array(); + if ($datastream->parent->repository->api->connection->username == 'anonymous') { + $cache_control[] = 'public'; + } + else { + $cache_control[] = 'private'; + } + $cache_control[] = 'must-revalidate'; + $cache_control[] = 'max-age=0'; + header('Cache-Control: ' . implode(', ', $cache_control)); + header('Last-Modified: ' . $datastream->createdDate->format('D, d M Y H:i:s \G\M\T')); + if (isset($datastream->checksum)) { + header("Etag: \"{$datastream->checksum}\""); + } } else { - $cache_control[] = 'private'; - } - $cache_control[] = 'must-revalidate'; - $cache_control[] = 'max-age=0'; - header('Cache-Control: ' . implode(', ', $cache_control)); - header('Last-Modified: ' . $datastream->createdDate->format('D, d M Y H:i:s \G\M\T')); - if (isset($datastream->checksum)) { - header("Etag: \"{$datastream->checksum}\""); + header_remove('Cache-Control'); + header_remove('Expires'); } } From a653a25c9c81b7e9a795db6ffba21d1c4b0cd671 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Thu, 29 Aug 2013 20:04:44 +0000 Subject: [PATCH 36/38] Coding standards... Trailing space. --- includes/datastream.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/datastream.inc b/includes/datastream.inc index 1de45581..1d506e3d 100644 --- a/includes/datastream.inc +++ b/includes/datastream.inc @@ -211,7 +211,7 @@ function islandora_view_datastream_set_cache_headers(AbstractDatastream $datastr } else { header_remove('Cache-Control'); - header_remove('Expires'); + header_remove('Expires'); } } From 7fef116c00f55abbaa9dbbe6dd638e3bbb0c2927 Mon Sep 17 00:00:00 2001 From: Jordan Dukart Date: Tue, 3 Sep 2013 19:39:00 +0000 Subject: [PATCH 37/38] Add alterable breadcrumbs to Islandora. --- includes/breadcrumb.inc | 1 + islandora.api.php | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/includes/breadcrumb.inc b/includes/breadcrumb.inc index 872eb70a..24b9fbbe 100644 --- a/includes/breadcrumb.inc +++ b/includes/breadcrumb.inc @@ -27,6 +27,7 @@ function islandora_get_breadcrumbs($object) { $breadcrumbs = islandora_get_breadcrumbs_recursive($object->id, $object->repository); array_pop($breadcrumbs); + drupal_alter('islandora_breadcrumbs', $breadcrumbs); return $breadcrumbs; } diff --git a/islandora.api.php b/islandora.api.php index 6d25bae3..1d6754c9 100644 --- a/islandora.api.php +++ b/islandora.api.php @@ -648,3 +648,10 @@ function hook_islandora_derivative() { function hook_CMODEL_PID_islandora_derivative() { } + +/** + * Alters breadcrumbs used on Solr search results and collection views. + */ +function hook_islandora_breadcrumbs_alter(&$breadcrumbs) { + +} From 0c50962a63e91276f21a543f098ff1f9a37d8815 Mon Sep 17 00:00:00 2001 From: Jordan Dukart Date: Tue, 3 Sep 2013 19:55:07 +0000 Subject: [PATCH 38/38] Add context for ease of use. --- includes/breadcrumb.inc | 3 ++- islandora.api.php | 9 +++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/includes/breadcrumb.inc b/includes/breadcrumb.inc index 24b9fbbe..05ab1bac 100644 --- a/includes/breadcrumb.inc +++ b/includes/breadcrumb.inc @@ -27,7 +27,8 @@ function islandora_get_breadcrumbs($object) { $breadcrumbs = islandora_get_breadcrumbs_recursive($object->id, $object->repository); array_pop($breadcrumbs); - drupal_alter('islandora_breadcrumbs', $breadcrumbs); + $context = 'islandora'; + drupal_alter('islandora_breadcrumbs', $breadcrumbs, $context); return $breadcrumbs; } diff --git a/islandora.api.php b/islandora.api.php index 1d6754c9..3a773e2b 100644 --- a/islandora.api.php +++ b/islandora.api.php @@ -650,8 +650,13 @@ function hook_CMODEL_PID_islandora_derivative() { } /** - * Alters breadcrumbs used on Solr search results and collection views. + * Alters breadcrumbs used on Solr search results and within Islandora views. + * + * @param array $breadcrumbs + * Breadcrumbs array to be altered by reference. Each element is markup. + * @param string $context + * Where the alter is originating from for distinguishing. */ -function hook_islandora_breadcrumbs_alter(&$breadcrumbs) { +function hook_islandora_breadcrumbs_alter(&$breadcrumbs, $context) { }