From d59099aab49cc059c57669031530facb518d75d0 Mon Sep 17 00:00:00 2001 From: Adam Vessey Date: Thu, 22 Aug 2013 20:28:34 +0000 Subject: [PATCH 01/18] 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 02/18] 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 03/18] 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 04/18] 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 05/18] 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 06/18] 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 07/18] 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 08/18] 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 09/18] 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 10/18] 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 11/18] 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 12/18] 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 13/18] 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 14/18] 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 15/18] 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 16/18] 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 17/18] 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 18/18] 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) { }