From aab19fa5b390732d552821037d3669c1e9d748f0 Mon Sep 17 00:00:00 2001 From: Ismo Vuorinen Date: Thu, 14 Sep 2017 14:22:36 +0300 Subject: [PATCH] Fixed throwing errors, added tests to test them --- src/ivuorinen/Palette/Palette.php | 97 ++++++++++++------------------- tests/PaletteTest.php | 17 ++++++ 2 files changed, 55 insertions(+), 59 deletions(-) diff --git a/src/ivuorinen/Palette/Palette.php b/src/ivuorinen/Palette/Palette.php index 555fa4e..25775d5 100644 --- a/src/ivuorinen/Palette/Palette.php +++ b/src/ivuorinen/Palette/Palette.php @@ -124,22 +124,14 @@ class Palette */ public function getPalette() { - try { + // We check for input + if (empty($this->filename)) { + throw new \ErrorException('Image was not provided'); + } - // We check for input - if (empty($this->filename)) { - throw new \ErrorException('Image was not provided'); - } - - // We check for readability - if (!is_readable($this->filename)) { - throw new \ErrorException("Image {$this->filename} is not readable"); - } - - } catch (\ErrorException $e) { - user_error($e->getMessage(), E_USER_ERROR); - - return false; + // We check for readability + if (!is_readable($this->filename)) { + throw new \ErrorException("Image {$this->filename} is not readable"); } $this->colorsArray = $this->countColors(); @@ -155,6 +147,7 @@ class Palette * countColors returns an array of colors in the image * * @return array|boolean Array of colors sorted by times used + * @throws \ErrorException */ private function countColors() { @@ -165,18 +158,14 @@ class Palette $img = $this->imageTypeToResource(); if (!$img && $img !== null) { - user_error("Unable to open: {$this->filename}", E_USER_ERROR); - - return false; + throw new \ErrorException("Unable to open: {$this->filename}"); } // Get image size and check if it's really an image $size = @getimagesize($this->filename); if ($size === false) { - user_error("Unable to get image size data: {$this->filename}", E_USER_ERROR); - - return false; + throw new \ErrorException("Unable to get image size data: {$this->filename}"); } // This is pretty naive approach, @@ -211,24 +200,17 @@ class Palette */ public function save() { - try { - // Check for destination - if (empty($this->destination)) { - throw new \InvalidArgumentException('No destination given for save'); - } + // Check for destination + if (empty($this->destination)) { + throw new \InvalidArgumentException('No destination given for save'); + } - // Check destination writability - $this->checkDestination(); + // Check destination writability + $this->checkDestination(); - // Check for data we should write - if (empty($this->colorsArray)) { - throw new \ErrorException("Couldn't detect colors from image: {$this->filename}"); - } - - } catch (\ErrorException $e) { - user_error($e->getMessage(), E_USER_ERROR); - - return false; + // Check for data we should write + if (empty($this->colorsArray)) { + throw new \ErrorException("Couldn't detect colors from image: {$this->filename}"); } // Encode for saving @@ -247,10 +229,19 @@ class Palette * imagecreatefrom{gif|jpeg|png} for further processing * * @return resource Image resource based on content + * @throws \ErrorException */ private function imageTypeToResource() { - $type = exif_imagetype($this->filename); + try { + if (filesize($this->filename) < 12) { + throw new \ErrorException('File size smaller than 12'); + } + $type = exif_imagetype($this->filename); + } catch (\Exception $e) { + throw new \ErrorException($e->getMessage()); + } + switch ($type) { case '1': // IMAGETYPE_GIF return @imagecreatefromgif($this->filename); @@ -260,9 +251,7 @@ class Palette return @imagecreatefrompng($this->filename); default: $image_type_code = image_type_to_mime_type($type); - user_error("Unknown image type: {$image_type_code} ({$type}): {$this->filename}"); - - return false; + throw new \ErrorException("Unknown image type: {$image_type_code} ({$type}): {$this->filename}"); } } @@ -282,28 +271,18 @@ class Palette $destination_dir = dirname($this->destination); // Test if we have destination directory - try { - if (!@mkdir($destination_dir, 0755) && !is_dir($destination_dir)) { - throw new \ErrorException("Couldn't create missing destination dir: {$destination_dir}"); - } - } catch (\ErrorException $e) { - user_error($e->getMessage()); - - return false; + if (!@mkdir($destination_dir, 0755) && !is_dir($destination_dir)) { + throw new \ErrorException("Couldn't create missing destination dir: {$destination_dir}"); } // Test if we can write to it - try { - if (is_writable($destination_dir)) { - return true; - } - chmod($destination_dir, 0755); + if (is_writable($destination_dir)) { + return true; + } + chmod($destination_dir, 0755); - if (!is_writable($destination_dir)) { - throw new \ErrorException("Destination directory not writable: {$destination_dir}"); - } - } catch (\ErrorException $e) { - user_error($e->getMessage()); + if (!is_writable($destination_dir)) { + throw new \ErrorException("Destination directory not writable: {$destination_dir}"); } return true; diff --git a/tests/PaletteTest.php b/tests/PaletteTest.php index c8ff1fc..b36b82b 100644 --- a/tests/PaletteTest.php +++ b/tests/PaletteTest.php @@ -29,4 +29,21 @@ class PaletteTest extends \PHPUnit_Framework_TestCase $this->assertEquals($image, $palette->filename); } } + + public function test_failure_no_image() + { + $palette = new \ivuorinen\Palette\Palette(''); + $this->expectException(ErrorException::class); + $this->expectExceptionMessage('Image was not provided'); + $palette->getPalette(); + } + + public function test_failure_not_an_image() + { + $palette = new \ivuorinen\Palette\Palette(); + $palette->filename = 'NOT_HERE'; + $this->expectException(ErrorException::class); + $this->expectExceptionMessage('Image ' . $palette->filename . ' is not readable'); + $palette->getPalette(); + } }