diff --git a/filesystem/File.php b/filesystem/File.php
index 2830832f3..bb99e8fb6 100644
--- a/filesystem/File.php
+++ b/filesystem/File.php
@@ -238,7 +238,7 @@ class File extends DataObject {
*/
public static function find($filename) {
// Get the base file if $filename points to a resampled file
- $filename = preg_replace('/_resampled\/[^-]+-/', '', $filename);
+ $filename = Image::strip_resampled_prefix($filename);
// Split to folders and the actual filename, and traverse the structure.
$parts = explode("/", $filename);
diff --git a/filesystem/Filesystem.php b/filesystem/Filesystem.php
index 8060d2c4c..0d8051603 100644
--- a/filesystem/Filesystem.php
+++ b/filesystem/Filesystem.php
@@ -78,6 +78,29 @@ class Filesystem extends Object {
}
}
+ /**
+ * Remove a directory, but only if it is empty.
+ *
+ * @param string $folder Absolute folder path
+ * @param boolean $recursive Remove contained empty folders before attempting to remove this one
+ * @return boolean True on success, false on failure.
+ */
+ public static function remove_folder_if_empty($folder, $recursive = true) {
+ if (!is_readable($folder)) return false;
+ $handle = opendir($folder);
+ while (false !== ($entry = readdir($handle))) {
+ if ($entry != "." && $entry != "..") {
+ // if an empty folder is detected, remove that one first and move on
+ if($recursive && is_dir($entry) && self::remove_folder_if_empty($entry)) continue;
+ // if a file was encountered, or a subdirectory was not empty, return false.
+ return false;
+ }
+ }
+ // if we are still here, the folder is empty.
+ rmdir($folder);
+ return true;
+ }
+
/**
* Cleanup function to reset all the Filename fields. Visit File/fixfiles to call.
*/
diff --git a/forms/HtmlEditorField.php b/forms/HtmlEditorField.php
index add08a0ff..d51139c4c 100644
--- a/forms/HtmlEditorField.php
+++ b/forms/HtmlEditorField.php
@@ -458,14 +458,13 @@ class HtmlEditorField_Toolbar extends RequestHandler {
// but GridField doesn't allow for this kind of metadata customization at the moment.
if($url = $request->getVar('FileURL')) {
if(Director::is_absolute_url($url) && !Director::is_site_url($url)) {
- $url = $url;
$file = new File(array(
'Title' => basename($url),
'Filename' => $url
));
} else {
$url = Director::makeRelative($request->getVar('FileURL'));
- $url = preg_replace('/_resampled\/[^-]+-/', '', $url);
+ $url = Image::strip_resampled_prefix($url);
$file = File::get()->filter('Filename', $url)->first();
if(!$file) $file = new File(array(
'Title' => basename($url),
diff --git a/model/Image.php b/model/Image.php
index c72a29c8e..992b61cb4 100644
--- a/model/Image.php
+++ b/model/Image.php
@@ -93,6 +93,17 @@ class Image extends File implements Flushable {
return self::config()->backend;
}
+ /**
+ * Retrieve the original filename from the path of a transformed image.
+ * Any other filenames pass through unchanged.
+ *
+ * @param string $path
+ * @return string
+ */
+ public static function strip_resampled_prefix($path) {
+ return preg_replace('/_resampled\/(.+\/|[^-]+-)/', '', $path);
+ }
+
/**
* Set up template methods to access the transformations generated by 'generate' methods.
*/
@@ -114,6 +125,7 @@ class Image extends File implements Flushable {
$urlLink .= "";
$urlLink .= "{$this->RelativeLink()}";
$urlLink .= "";
+ // todo: check why the above code is here, since $urlLink is not used?
//attach the addition file information for an image to the existing FieldGroup create in the parent class
$fileAttributes = $fields->fieldByName('Root.Main.FilePreview')->fieldByName('FilePreviewData');
@@ -697,12 +709,25 @@ class Image extends File implements Flushable {
public function cacheFilename($format) {
$args = func_get_args();
array_shift($args);
+
+ // Note: $folder holds the *original* file, while the Image we're working with
+ // may be a formatted image in a child directory (this happens when we're chaining formats)
$folder = $this->ParentID ? $this->Parent()->Filename : ASSETS_DIR . "/";
$format = $format . Convert::base64url_encode($args);
- $filename = $format . "-" . $this->Name;
- $patterns = $this->getFilenamePatterns($this->Name);
- if (!preg_match($patterns['FullPattern'], $filename)) {
+ $filename = $format . "/" . $this->Name;
+
+ $pattern = $this->getFilenamePatterns($this->Name);
+
+ // Any previous formats need to be derived from this Image's directory, and prepended to the new filename
+ $prepend = array();
+ preg_match_all($pattern['GeneratorPattern'], $this->Filename, $matches, PREG_SET_ORDER);
+ foreach($matches as $formatdir) {
+ $prepend[] = $formatdir[0];
+ }
+ $filename = implode($prepend) . $filename;
+
+ if (!preg_match($pattern['FullPattern'], $filename)) {
throw new InvalidArgumentException('Filename ' . $filename
. ' that should be used to cache a resized image is invalid');
}
@@ -826,9 +851,9 @@ class Image extends File implements Flushable {
$generateFuncs = implode('|', $generateFuncs);
$base64url_match = "[a-zA-Z0-9_~]*={0,2}";
return array(
- 'FullPattern' => "/^((?P{$generateFuncs})(?P" . $base64url_match . ")\-)+"
+ 'FullPattern' => "/^((?P{$generateFuncs})(?P" . $base64url_match . ")\/)+"
. preg_quote($filename) . "$/i",
- 'GeneratorPattern' => "/(?P{$generateFuncs})(?P" . $base64url_match . ")\-/i"
+ 'GeneratorPattern' => "/(?P{$generateFuncs})(?P" . $base64url_match . ")\//i"
);
}
@@ -842,40 +867,35 @@ class Image extends File implements Flushable {
$folder = $this->ParentID ? $this->Parent()->Filename : ASSETS_DIR . '/';
$cacheDir = Director::getAbsFile($folder . '_resampled/');
+ // Find all paths with the same filename as this Image (the path contains the transformation info)
if(is_dir($cacheDir)) {
- if($handle = opendir($cacheDir)) {
- while(($file = readdir($handle)) !== false) {
- // ignore all entries starting with a dot
- if(substr($file, 0, 1) != '.' && is_file($cacheDir . $file)) {
- $cachedFiles[] = $file;
- }
+ $files = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($cacheDir));
+ foreach($files as $path => $file){
+ if ($file->getFilename() == $this->Name) {
+ $cachedFiles[] = $path;
}
- closedir($handle);
}
}
$pattern = $this->getFilenamePatterns($this->Name);
- foreach($cachedFiles as $cfile) {
- if(preg_match($pattern['FullPattern'], $cfile, $matches)) {
- if(Director::fileExists($cacheDir . $cfile)) {
- $subFilename = substr($cfile, 0, -1 * strlen($this->Name));
- preg_match_all($pattern['GeneratorPattern'], $subFilename, $subMatches, PREG_SET_ORDER);
-
- $generatorArray = array();
- foreach ($subMatches as $singleMatch) {
- $generatorArray[] = array('Generator' => $singleMatch['Generator'],
- 'Args' => Convert::base64url_decode($singleMatch['Args']));
- }
-
- // Using array_reverse is important, as a cached image will
- // have the generators settings in the filename in reversed
- // order: the last generator given in the filename is the
- // first that was used. Later resizements are prepended
- $generatedImages[] = array ( 'FileName' => $cacheDir . $cfile,
- 'Generators' => array_reverse($generatorArray) );
- }
+ // Reconstruct the image transformation(s) from the format-folder(s) in the path
+ // (if chained, they contain the transformations in the correct order)
+ foreach($cachedFiles as $cf_path) {
+ preg_match_all($pattern['GeneratorPattern'], $cf_path, $matches, PREG_SET_ORDER);
+
+ $generatorArray = array();
+ foreach ($matches as $singleMatch) {
+ $generatorArray[] = array(
+ 'Generator' => $singleMatch['Generator'],
+ 'Args' => Convert::base64url_decode($singleMatch['Args'])
+ );
}
+
+ $generatedImages[] = array(
+ 'FileName' => $cf_path,
+ 'Generators' => $generatorArray
+ );
}
return $generatedImages;
@@ -922,8 +942,14 @@ class Image extends File implements Flushable {
$numDeleted = 0;
$generatedImages = $this->getGeneratedImages();
foreach($generatedImages as $singleImage) {
- unlink($singleImage['FileName']);
+ $path = $singleImage['FileName'];
+ unlink($path);
$numDeleted++;
+ do {
+ $path = dirname($path);
+ }
+ // remove the folder if it's empty (and it's not the assets folder)
+ while(!preg_match('/assets$/', $path) && Filesystem::remove_folder_if_empty($path));
}
return $numDeleted;
diff --git a/tests/forms/HtmlEditorFieldTest.php b/tests/forms/HtmlEditorFieldTest.php
index 52bb3aa79..a3586a690 100644
--- a/tests/forms/HtmlEditorFieldTest.php
+++ b/tests/forms/HtmlEditorFieldTest.php
@@ -89,7 +89,7 @@ class HtmlEditorFieldTest extends FunctionalTest {
$this->assertEquals(20, (int)$xml[0]['height'], 'Height tag of resized image is set.');
$neededFilename = 'assets/_resampled/ResizedImage' . Convert::base64url_encode(array(10,20)) .
- '-HTMLEditorFieldTest_example.jpg';
+ '/HTMLEditorFieldTest_example.jpg';
$this->assertEquals($neededFilename, (string)$xml[0]['src'], 'Correct URL of resized image is set.');
$this->assertTrue(file_exists(BASE_PATH.DIRECTORY_SEPARATOR.$neededFilename), 'File for resized image exists');
diff --git a/tests/model/ImageTest.php b/tests/model/ImageTest.php
index 5ba108662..d9af5d84d 100644
--- a/tests/model/ImageTest.php
+++ b/tests/model/ImageTest.php
@@ -119,7 +119,6 @@ class ImageTest extends SapphireTest {
* of the output image do not resample the file.
*/
public function testReluctanceToResampling() {
-
$image = $this->objFromFixture('Image', 'imageWithoutTitle');
$this->assertTrue($image->isSize(300, 300));
@@ -170,7 +169,6 @@ class ImageTest extends SapphireTest {
* of the output image resample the file when force_resample is set to true.
*/
public function testForceResample() {
-
$image = $this->objFromFixture('Image', 'imageWithoutTitle');
$this->assertTrue($image->isSize(300, 300));
@@ -315,23 +313,24 @@ class ImageTest extends SapphireTest {
$this->assertContains($argumentString, $imageThird->getFullPath(),
'Image contains background color for padded resizement');
- $imageThirdPath = $imageThird->getFullPath();
- $filesInFolder = $folder->find(dirname($imageThirdPath));
+ $resampledFolder = dirname($image->getFullPath()) . "/_resampled";
+ $filesInFolder = $folder->find($resampledFolder);
$this->assertEquals(3, count($filesInFolder),
'Image folder contains only the expected number of images before regeneration');
+ $imageThirdPath = $imageThird->getFullPath();
$hash = md5_file($imageThirdPath);
$this->assertEquals(3, $image->regenerateFormattedImages(),
'Cached images were regenerated in the right number');
$this->assertEquals($hash, md5_file($imageThirdPath), 'Regeneration of third image is correct');
/* Check that no other images exist, to ensure that the regeneration did not create other images */
- $this->assertEquals($filesInFolder, $folder->find(dirname($imageThirdPath)),
+ $this->assertEquals($filesInFolder, $folder->find($resampledFolder),
'Image folder contains only the expected image files after regeneration');
}
public function testRegenerateImages() {
- $image = $this->objFromFixture('Image', 'imageWithMetacharacters');
+ $image = $this->objFromFixture('Image', 'imageWithoutTitle');
$image_generated = $image->ScaleWidth(200);
$p = $image_generated->getFullPath();
$this->assertTrue(file_exists($p), 'Resized image exists after creation call');
@@ -346,7 +345,7 @@ class ImageTest extends SapphireTest {
* ToDo: This doesn't seem like something that is worth testing - what is the point of this?
*/
public function testRegenerateImagesWithRenaming() {
- $image = $this->objFromFixture('Image', 'imageWithMetacharacters');
+ $image = $this->objFromFixture('Image', 'imageWithoutTitle');
$image_generated = $image->ScaleWidth(200);
$p = $image_generated->getFullPath();
$this->assertTrue(file_exists($p), 'Resized image exists after creation call');
@@ -356,6 +355,7 @@ class ImageTest extends SapphireTest {
$newArgumentString = Convert::base64url_encode(array(300));
$newPath = str_replace($oldArgumentString, $newArgumentString, $p);
+ if(!file_exists(dirname($newPath))) mkdir(dirname($newPath));
$newRelative = str_replace($oldArgumentString, $newArgumentString, $image_generated->getFileName());
rename($p, $newPath);
$this->assertFalse(file_exists($p), 'Resized image does not exist at old path after renaming');
@@ -368,7 +368,7 @@ class ImageTest extends SapphireTest {
}
public function testGeneratedImageDeletion() {
- $image = $this->objFromFixture('Image', 'imageWithMetacharacters');
+ $image = $this->objFromFixture('Image', 'imageWithoutTitle');
$image_generated = $image->ScaleWidth(200);
$p = $image_generated->getFullPath();
$this->assertTrue(file_exists($p), 'Resized image exists after creation call');
@@ -381,7 +381,7 @@ class ImageTest extends SapphireTest {
* Tests that generated images with multiple image manipulations are all deleted
*/
public function testMultipleGenerateManipulationCallsImageDeletion() {
- $image = $this->objFromFixture('Image', 'imageWithMetacharacters');
+ $image = $this->objFromFixture('Image', 'imageWithoutTitle');
$firstImage = $image->ScaleWidth(200);
$firstImagePath = $firstImage->getFullPath();
@@ -400,7 +400,7 @@ class ImageTest extends SapphireTest {
* Tests path properties of cached images with multiple image manipulations
*/
public function testPathPropertiesCachedImage() {
- $image = $this->objFromFixture('Image', 'imageWithMetacharacters');
+ $image = $this->objFromFixture('Image', 'imageWithoutTitle');
$firstImage = $image->ScaleWidth(200);
$firstImagePath = $firstImage->getRelativePath();
$this->assertEquals($firstImagePath, $firstImage->Filename);
@@ -410,6 +410,33 @@ class ImageTest extends SapphireTest {
$this->assertEquals($secondImagePath, $secondImage->Filename);
}
+ /**
+ * Tests the static function Image::strip_resampled_prefix, to ensure that
+ * the original filename can be extracted from the path of transformed images,
+ * both in current and previous formats
+ */
+ public function testStripResampledPrefix() {
+ $orig_image = $this->objFromFixture('Image', 'imageWithoutTitleContainingDots');
+
+ // current format (3.3+). Example:
+ // assets/ImageTest/_resampled/ScaleHeightWzIwMF0=/ScaleWidthWzQwMF0=/test.image.with.dots.png;
+ $firstImage = $orig_image->ScaleWidth(200);
+ $secondImage = $firstImage->ScaleHeight(200);
+ $paths_1 = $firstImage->Filename;
+ $paths_2 = $secondImage->Filename;
+
+ // 3.2 format (did not work for multiple transformations)
+ $paths_3 = 'assets/ImageTest/_resampled/ScaleHeightWzIwMF0=-test.image.with.dots.png';
+
+ // 3.1 (and earlier) format (did not work for multiple transformations)
+ $paths_4 = 'assets/ImageTest/_resampled/ScaleHeight200-test.image.with.dots.png';
+
+ $this->assertEquals($orig_image->Filename, Image::strip_resampled_prefix($paths_1));
+ $this->assertEquals($orig_image->Filename, Image::strip_resampled_prefix($paths_2));
+ $this->assertEquals($orig_image->Filename, Image::strip_resampled_prefix($paths_3));
+ $this->assertEquals($orig_image->Filename, Image::strip_resampled_prefix($paths_4));
+ }
+
/**
* Test all generate methods
*/