diff --git a/model/Image.php b/model/Image.php index 55c2f632d..31561ade2 100644 --- a/model/Image.php +++ b/model/Image.php @@ -224,13 +224,12 @@ class Image extends File implements Flushable { * @return Image */ public function SetRatioSize($width, $height) { - // Prevent divide by zero on missing/blank file - if(empty($this->width) || empty($this->height)) return null; + if(!$this->getWidth() || !$this->getHeight()) return null; // Check if image is already sized to the correct dimension - $widthRatio = $width / $this->width; - $heightRatio = $height / $this->height; + $widthRatio = $width / $this->getWidth(); + $heightRatio = $height / $this->getHeight(); if( $widthRatio < $heightRatio ) { // Target is higher aspect ratio than image, so check width if($this->isWidth($width) && !Config::inst()->get('Image', 'force_resample')) return $this; @@ -510,7 +509,8 @@ class Image extends File implements Flushable { /** * Generate a resized copy of this image with the given width & height. - * Use in templates with $ResizedImage. + * This can be used in templates with $ResizedImage but should be avoided, + * as it's the only image manipulation function which can skew an image. * * @param integer $width Width to resize to * @param integer $height Height to resize to @@ -648,9 +648,9 @@ class Image extends File implements Flushable { public function regenerateFormattedImages() { if(!$this->Filename) return 0; - // Without this, not a single file would be written - // caused by a check in getFormattedImage() - $_GET['flush'] = 1; + // Without this, not a single file would be written + // caused by a check in getFormattedImage() + $this->flush(); $numGenerated = 0; $generatedImages = $this->getGeneratedImages(); diff --git a/tests/model/DataDifferencerTest.php b/tests/model/DataDifferencerTest.php index b0c9d7490..45ebd87c0 100644 --- a/tests/model/DataDifferencerTest.php +++ b/tests/model/DataDifferencerTest.php @@ -55,8 +55,8 @@ class DataDifferencerTest extends SapphireTest { $differ = new DataDifferencer($obj1v1, $obj1v2); $obj1Diff = $differ->diffedData(); - $this->assertContains($image1->Filename, $obj1Diff->getField('Image')); - $this->assertContains($image2->Filename, $obj1Diff->getField('Image')); + $this->assertContains($image1->Name, $obj1Diff->getField('Image')); + $this->assertContains($image2->Name, $obj1Diff->getField('Image')); $this->assertContains('obj2obj1', str_replace(' ','',$obj1Diff->getField('HasOneRelationID'))); } diff --git a/tests/model/GDImageTest.php b/tests/model/GDImageTest.php index 92e00546c..f950ff977 100644 --- a/tests/model/GDImageTest.php +++ b/tests/model/GDImageTest.php @@ -10,22 +10,9 @@ class GDImageTest extends ImageTest { return; } - parent::setUp(); - Image::set_backend("GDBackend"); - // Create a test files for each of the fixture references - $fileIDs = $this->allFixtureIDs('Image'); - foreach($fileIDs as $fileID) { - $file = DataObject::get_by_id('Image', $fileID); - - $image = imagecreatetruecolor(300,300); - - imagepng($image, BASE_PATH."/{$file->Filename}"); - imagedestroy($image); - - $file->write(); - } + parent::setUp(); } public function tearDown() { diff --git a/tests/model/ImageTest.php b/tests/model/ImageTest.php index a8b5b2124..fe1e467e1 100644 --- a/tests/model/ImageTest.php +++ b/tests/model/ImageTest.php @@ -30,19 +30,30 @@ class ImageTest extends SapphireTest { if(!file_exists(BASE_PATH."/$folder->Filename")) mkdir(BASE_PATH."/$folder->Filename"); } + + // Copy test images for each of the fixture references + $imageIDs = $this->allFixtureIDs('Image'); + foreach($imageIDs as $imageID) { + $image = DataObject::get_by_id('Image', $imageID); + $filePath = BASE_PATH."/$image->Filename"; + $sourcePath = str_replace('assets/ImageTest/', 'framework/tests/model/testimages/', $filePath); + if(!file_exists($filePath)) { + if (!copy($sourcePath, $filePath)) user_error('Failed to copy test images', E_USER_ERROR); + } + } } public function tearDown() { if($this->origBackend) Image::set_backend($this->origBackend); - /* Remove the test files that we've created */ + // Remove the test files that we've created $fileIDs = $this->allFixtureIDs('Image'); foreach($fileIDs as $fileID) { $file = DataObject::get_by_id('Image', $fileID); if($file && file_exists(BASE_PATH."/$file->Filename")) unlink(BASE_PATH."/$file->Filename"); } - /* Remove the test folders that we've crated */ + // Remove the test folders that we've created $folderIDs = $this->allFixtureIDs('Folder'); foreach($folderIDs as $folderID) { $folder = DataObject::get_by_id('Folder', $folderID); @@ -298,6 +309,10 @@ class ImageTest extends SapphireTest { $this->assertTrue(file_exists($p), 'Resized image exists after regeneration call'); } + /** + * Tests that cached images are regenerated properly after a cached file is renamed with new arguments + * 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_generated = $image->SetWidth(200); @@ -311,8 +326,8 @@ class ImageTest extends SapphireTest { $newPath = str_replace($oldArgumentString, $newArgumentString, $p); $newRelative = str_replace($oldArgumentString, $newArgumentString, $image_generated->getFileName()); rename($p, $newPath); - $this->assertFalse(file_exists($p), 'Resized image does not exist after movement call under old name'); - $this->assertTrue(file_exists($newPath), 'Resized image exists after movement call under new name'); + $this->assertFalse(file_exists($p), 'Resized image does not exist at old path after renaming'); + $this->assertTrue(file_exists($newPath), 'Resized image exists at new path after renaming'); $this->assertEquals(1, $image->regenerateFormattedImages(), 'Cached images were regenerated in the right number'); diff --git a/tests/model/ImageTest.yml b/tests/model/ImageTest.yml index ac1dfcaf4..b677f2db8 100644 --- a/tests/model/ImageTest.yml +++ b/tests/model/ImageTest.yml @@ -1,6 +1,6 @@ Folder: folder1: - Filename: assets/ImageTest + Filename: assets/ImageTest/ Image: imageWithTitle: Title: This is a image Title @@ -16,3 +16,11 @@ Image: Title: This is a/an image Title Filename: assets/ImageTest/test_image.png Parent: =>Folder.folder1 + lowQualityJPEG: + Title: This is a low quality JPEG + Filename: assets/ImageTest/test_image_low-quality.jpg + Parent: =>Folder.folder1 + highQualityJPEG: + Title: This is a high quality JPEG + Filename: assets/ImageTest/test_image_high-quality.jpg + Parent: =>Folder.folder1 \ No newline at end of file diff --git a/tests/model/ImagickImageTest.php b/tests/model/ImagickImageTest.php index f63827d0b..7e44de3bd 100644 --- a/tests/model/ImagickImageTest.php +++ b/tests/model/ImagickImageTest.php @@ -8,22 +8,8 @@ class ImagickImageTest extends ImageTest { return; } - parent::setUp(); - Image::set_backend("ImagickBackend"); - // Create a test files for each of the fixture references - $fileIDs = $this->allFixtureIDs('Image'); - foreach($fileIDs as $fileID) { - $file = DataObject::get_by_id('Image', $fileID); - - $image = new Imagick(); - - $image->newImage(300,300, new ImagickPixel("white")); - $image->setImageFormat("png"); - $image->writeImage(BASE_PATH."/{$file->Filename}"); - - $file->write(); - } + parent::setUp(); } } diff --git a/tests/model/testimages/test.image.with.dots.png b/tests/model/testimages/test.image.with.dots.png index a95602a86..21df58520 100644 Binary files a/tests/model/testimages/test.image.with.dots.png and b/tests/model/testimages/test.image.with.dots.png differ diff --git a/tests/model/testimages/test_image.png b/tests/model/testimages/test_image.png index a95602a86..abe33e5f5 100644 Binary files a/tests/model/testimages/test_image.png and b/tests/model/testimages/test_image.png differ diff --git a/tests/model/testimages/test_image_high-quality.jpg b/tests/model/testimages/test_image_high-quality.jpg new file mode 100644 index 000000000..f90315784 Binary files /dev/null and b/tests/model/testimages/test_image_high-quality.jpg differ diff --git a/tests/model/testimages/test_image_low-quality.jpg b/tests/model/testimages/test_image_low-quality.jpg new file mode 100644 index 000000000..ccc8346ab Binary files /dev/null and b/tests/model/testimages/test_image_low-quality.jpg differ