Support CDN rewriting of ImageCache paths
| Project: | CDN integration |
| Version: | 6.x-1.x-dev |
| Component: | Module |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | active |
Jump to:
Requested feature - Support the rewriting of paths to images that have been created by the ImageCache module. Presumably, this would require two things:
1) Additions/changes to the custom_file_url_rewrite() function in the cdn.module to support paths that were created by ImageCache in addition to the Drupal core paths.
2) Additions/changes to the imagecache_create_url() function in the imagecache.module file. I am assuming these changes would look very similar to the Drupal Core modifications inside of file.inc. The only difference would be to either call a new function custom_file_imagecache_url_rewrite or a modification of the custom_file_url_rewrite as mentioned above, with a notice that it is dealing with imagecache paths and not Drupal core paths.
#1
I'm actually thinking it may be better to do it a bit differently. Inside of the ImageCache module and imagecache_create_url() function, sort out all of the path issues first and then pass the full path to the ImageCache-created images to custom_file_url_rewrite. That way the CDN module can keep on doing its thing and it just happens to get the right path from the get-go rather than having to deal with passing presets or other variables.
Thoughts?
#2
I ran into this same problem recently. I have attached the code I used to side-step the issue at the theme layer although I'm not sure this is optimal.
<?php
/**
* Implementation of theme_imagecache()
* NOTE: change theme_ to your theme's name and put this in template.php
*/
function theme_imagecache($presetname, $path, $alt = '', $title = '', $attributes = NULL, $getsize = TRUE) {
// Check is_null() so people can intentionally pass an empty array of
// to override the defaults completely.
if (is_null($attributes)) {
$attributes = array('class' => 'imagecache imagecache-'. $presetname);
}
if ($getsize && ($image = image_get_info(imagecache_create_path($presetname, $path)))) {
$attributes['width'] = $image['width'];
$attributes['height'] = $image['height'];
}
$attributes = drupal_attributes($attributes);
$path = _imagecache_strip_file_directory($path);
if (module_exists('transliteration')) {
$path = transliteration_get($path);
}
// check to see if the file exists locally. If not get it so imagecache will
// build the derivative image
if (!file_exists(file_directory_path() .'/imagecache/'. $presetname .'/'. $path)) {
drupal_http_request(url(file_directory_path() .'/imagecache/'. $presetname .'/'. $path, array('absolute' => TRUE)));
}
$args = array('absolute' => TRUE, 'query' => empty($bypass_browser_cache) ? NULL : time());
if (variable_get('file_downloads', FILE_DOWNLOADS_PUBLIC) == FILE_DOWNLOADS_PUBLIC) {
$imagecache_url = file_create_url(file_directory_path() .'/imagecache/'. $presetname .'/'. $path);
}
else {
$imagecache_url = url('system/files/imagecache/'. $presetname .'/'. $path, $args);
}
return '<img src="'. $imagecache_url .'" alt="'. check_plain($alt) .'" title="'. check_plain($title) .'" '. $attributes .' />';
}
?>
Basically I put the imagecache_create_url() code in theme_imagecache and added a line to check to see if the derivative image had been built yet. If not I requested it without the CDN so the derivative would be built. I'm not sure if others had this problem as well but in my case the CDN wasn't causing the derivative images to be built.
#3
I took a bit of time and wrote a patch to imagecache.module's imagecache_create_url function today. This is very similar to the Drupal core CDN patch because it is based on that function. I'm just starting to test this, but it works like my other patch where it will not rewrite the URL if the page is HTTPS (because I'm using Cloudfront and they do not support serving static content via HTTPS.)
Here it is. I have done very limited testing but so far it works for me. All of the images are already in the CDN because file conveyor is pushing them there for me. I believe this will also work properly to create the initial cached images since custom_file_url_rewrite will fail and serve the content locally which should trigger the image cache creation process.
function imagecache_create_url($presetname, $filepath, $bypass_browser_cache = FALSE) {
// Check if the CDN function is available
if (function_exists('custom_file_url_rewrite')) {
// If we have CDN, don't rewrite if the page is secure
if (function_exists('securepages_is_secure') && securepages_is_secure()) {
// We fail this test with a securepage and go to serving from locally below
$rewritten_path = FALSE;
} else {
$rewritten_path = custom_file_url_rewrite($filepath);
if ($rewritten_path != FALSE) {
// If that function worked, we stop right here and return the new path.
return $rewritten_path;
}
}
}
// Otherwise serve the file from Drupal's web server. This point will only
// be reached when either no custom_file_url_rewrite() function has been
// defined, or when that function returned FALSE, thereby indicating that it
// cannot (or doesn't wish to) rewrite the URL. This is typically because
// the file doesn't match some conditions to be served from a CDN or static
// file server, or because the file has not yet been synced to the CDN or
// static file server.
$path = _imagecache_strip_file_directory($filepath);
if (module_exists('transliteration')) {
$path = transliteration_get($path);
}
$args = array('absolute' => TRUE, 'query' => empty($bypass_browser_cache) ? NULL : time());
switch (variable_get('file_downloads', FILE_DOWNLOADS_PUBLIC)) {
case FILE_DOWNLOADS_PUBLIC:
return url($GLOBALS['base_url'] . '/' . file_directory_path() .'/imagecache/'. $presetname .'/'. $path, $args);
case FILE_DOWNLOADS_PRIVATE:
return url('system/files/imagecache/'. $presetname .'/'. $path, $args);
}
}
#4
Maybe we need to add some kind of admin UI checkbox and if statement inside cdn.module's custom_file_url_rewrite function to disable rewriting of CDN paths for SSL. I'm still not convinced I am doing it in the right place.
#5
#3 still not working right - it's returning the path of the original large image, not the imagecached small image.
#6
OK, here's one that works to pull out the correct imagecache paths from the CDN.
I would submit this for review in the imagecache module page, but I'd like to hear if anyone else has feedback on the SSL/SecurePages stuff first and the best place to integrate it - either in core patches and imagecache patches or in the function inside of cdn.module.
function imagecache_create_url($presetname, $filepath, $bypass_browser_cache = FALSE) {
// Check if the CDN function is available
if (function_exists('custom_file_url_rewrite')) {
// If we have CDN, don't rewrite if the page is secure
if (function_exists('securepages_is_secure') && securepages_is_secure()) {
// We fail this test with a securepage and go to serving from locally below
$rewritten_path = FALSE;
} else {
// First create the correct imagecache preset path
$path = _imagecache_strip_file_directory($filepath);
if (module_exists('transliteration')) {
$path = transliteration_get($path);
}
$icpath = ('/' . file_directory_path() .'/imagecache/'. $presetname .'/'. $path);
$rewritten_path = custom_file_url_rewrite($icpath);
if ($rewritten_path != FALSE) {
// If that function worked, we stop right here and return the new path.
return $rewritten_path;
}
}
}
// Otherwise serve the file from Drupal's web server. This point will only
// be reached when either no custom_file_url_rewrite() function has been
// defined, or when that function returned FALSE, thereby indicating that it
// cannot (or doesn't wish to) rewrite the URL. This is typically because
// the file doesn't match some conditions to be served from a CDN or static
// file server, or because the file has not yet been synced to the CDN or
// static file server.
$path = _imagecache_strip_file_directory($filepath);
if (module_exists('transliteration')) {
$path = transliteration_get($path);
}
$args = array('absolute' => TRUE, 'query' => empty($bypass_browser_cache) ? NULL : time());
switch (variable_get('file_downloads', FILE_DOWNLOADS_PUBLIC)) {
case FILE_DOWNLOADS_PUBLIC:
return url($GLOBALS['base_url'] . '/' . file_directory_path() .'/imagecache/'. $presetname .'/'. $path, $args);
case FILE_DOWNLOADS_PRIVATE:
return url('system/files/imagecache/'. $presetname .'/'. $path, $args);
}
}
#7
Please wait with posting that to the imagecache issue queue.
The CDN integration module uses the
custom_file_url_rewritefunction. But the Drupal core patch uses a hook:hook_file_url_alter(). Initially, they agreed with the special function approach, as the same approach is used for rewriting URLs. But now they're shifting all of the URL rewriting functions (including file URL rewriting) to using hooks, to prevent confusing developers with this special case, and because then you can have a module implementing it, instead of having to add some special code.So, I will first have to update the CDN integration module to use the same approach as D7 core. Issue: #610322: Upgrade the Drupal 6 core patch to match the committed Drupal 7 core patch.
#8
It looks like the preceding slash is unnecessary in $icpath. This:
$icpath = ('/' . file_directory_path() .'/imagecache/'. $presetname .'/'. $path);works better as this:
$icpath = (file_directory_path() .'/imagecache/'. $presetname .'/'. $path);Otherwise I get double slashes like: http://example.com//files/imagecache/thumb_small/somefile.jpg
Other than that it appears to solve the problem for me. Thank you very much.
#9
Subscribing.