From ec428445a142805e734a8f98847f7e70072c5497 Mon Sep 17 00:00:00 2001 From: Manuel Reinhard Date: Thu, 27 Mar 2014 07:27:38 +0100 Subject: [PATCH] Improved ISBN validator --- .../Component/Validator/Constraints/Isbn.php | 36 ++++---- .../Validator/Constraints/IsbnValidator.php | 91 ++++++++++++++----- .../Tests/Constraints/IsbnValidatorTest.php | 14 +-- 3 files changed, 96 insertions(+), 45 deletions(-) diff --git a/src/Symfony/Component/Validator/Constraints/Isbn.php b/src/Symfony/Component/Validator/Constraints/Isbn.php index 4a11e23448..5beb0f05a9 100644 --- a/src/Symfony/Component/Validator/Constraints/Isbn.php +++ b/src/Symfony/Component/Validator/Constraints/Isbn.php @@ -12,34 +12,38 @@ namespace Symfony\Component\Validator\Constraints; use Symfony\Component\Validator\Constraint; -use Symfony\Component\Validator\Exception\MissingOptionsException; /** * @Annotation * * @author The Whole Life To Learn + * @author Manuel Reinhard */ class Isbn extends Constraint { public $isbn10Message = 'This value is not a valid ISBN-10.'; public $isbn13Message = 'This value is not a valid ISBN-13.'; public $bothIsbnMessage = 'This value is neither a valid ISBN-10 nor a valid ISBN-13.'; - public $isbn10; - public $isbn13; + public $type; + public $message; - public function __construct($options = null) + /** + * @deprecated Deprecated since version 2.5, to be removed in 3.0. Use option "type" instead. + * @var Boolean + */ + public $isbn10 = false; + + /** + * @deprecated Deprecated since version 2.5, to be removed in 3.0. Use option "type" instead. + * @var Boolean + */ + public $isbn13 = false; + + /** + * {@inheritDoc} + */ + public function getDefaultOption() { - if (null !== $options && !is_array($options)) { - $options = array( - 'isbn10' => $options, - 'isbn13' => $options, - ); - } - - parent::__construct($options); - - if (null === $this->isbn10 && null === $this->isbn13) { - throw new MissingOptionsException(sprintf('Either option "isbn10" or "isbn13" must be given for constraint "%s".', __CLASS__), array('isbn10', 'isbn13')); - } + return 'type'; } } diff --git a/src/Symfony/Component/Validator/Constraints/IsbnValidator.php b/src/Symfony/Component/Validator/Constraints/IsbnValidator.php index 00e2c5dc1b..7aeb13b1b2 100644 --- a/src/Symfony/Component/Validator/Constraints/IsbnValidator.php +++ b/src/Symfony/Component/Validator/Constraints/IsbnValidator.php @@ -16,10 +16,10 @@ use Symfony\Component\Validator\ConstraintValidator; use Symfony\Component\Validator\Exception\UnexpectedTypeException; /** - * Validates whether the value is a valid ISBN-10 or ISBN-13. + * Validates whether the value is a valid ISBN-10 or ISBN-13 * * @author The Whole Life To Learn - * + * @author Manuel Reinhard * @see https://en.wikipedia.org/wiki/Isbn */ class IsbnValidator extends ConstraintValidator @@ -45,11 +45,43 @@ class IsbnValidator extends ConstraintValidator $value = str_replace('-', '', $value); } - $validation = 0; - $value = strtoupper($value); + if (null == $constraint->type) { + if ($constraint->isbn10 && !$constraint->isbn13) { + $constraint->type = 'isbn10'; + $value = strtoupper($value); + } elseif ($constraint->isbn13 && !$constraint->isbn10) { + $constraint->type = 'isbn13'; + $value = strtoupper($value); + } + } + + if ('isbn10' === $constraint->type) { + if (!$this->validateIsbn10($value)) { + $this->context->addViolation($this->getMessage($constraint, 'isbn10')); + + return; + } + } elseif ('isbn13' === $constraint->type) { + if (!$this->validateIsbn13($value)) { + $this->context->addViolation($this->getMessage($constraint, 'isbn13')); + + return; + } + } else { + if (!$this->validateIsbn10($value) && !$this->validateIsbn13($value)) { + $this->context->addViolation($this->getMessage($constraint)); + + return; + } + } + } + + protected function validateIsbn10($value) + { + $validation = 0; $valueLength = strlen($value); - if (10 === $valueLength && null !== $constraint->isbn10) { + if (10 === $valueLength) { for ($i = 0; $i < 10; $i++) { if ($value[$i] == 'X') { $validation += 10 * intval(10 - $i); @@ -59,13 +91,21 @@ class IsbnValidator extends ConstraintValidator } if ($validation % 11 != 0) { - if (null !== $constraint->isbn13) { - $this->context->addViolation($constraint->bothIsbnMessage); - } else { - $this->context->addViolation($constraint->isbn10Message); - } + return false; + } else { + return true; } - } elseif (13 === $valueLength && null !== $constraint->isbn13) { + } + + return false; + } + + protected function validateIsbn13($value) + { + $validation = 0; + $valueLength = strlen($value); + + if (13 === $valueLength) { for ($i = 0; $i < 13; $i += 2) { $validation += intval($value[$i]); } @@ -74,20 +114,25 @@ class IsbnValidator extends ConstraintValidator } if ($validation % 10 != 0) { - if (null !== $constraint->isbn10) { - $this->context->addViolation($constraint->bothIsbnMessage); - } else { - $this->context->addViolation($constraint->isbn13Message); - } - } - } else { - if (null !== $constraint->isbn10 && null !== $constraint->isbn13) { - $this->context->addViolation($constraint->bothIsbnMessage); - } elseif (null !== $constraint->isbn10) { - $this->context->addViolation($constraint->isbn10Message); + return false; } else { - $this->context->addViolation($constraint->isbn13Message); + return true; } } + + return false; + } + + protected function getMessage($constraint, $type=null) + { + if (null !== $constraint->message) { + return $constraint->message; + } elseif ($type == 'isbn10') { + return $constraint->isbn10Message; + } elseif ($type == 'isbn13') { + return $constraint->isbn13Message; + } else { + return $constraint->bothIsbnMessage; + } } } diff --git a/src/Symfony/Component/Validator/Tests/Constraints/IsbnValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/IsbnValidatorTest.php index 7f0859b999..feb2234fde 100644 --- a/src/Symfony/Component/Validator/Tests/Constraints/IsbnValidatorTest.php +++ b/src/Symfony/Component/Validator/Tests/Constraints/IsbnValidatorTest.php @@ -44,6 +44,7 @@ class IsbnValidatorTest extends \PHPUnit_Framework_TestCase array('0321812700'), array('0-45122-5244'), array('0-4712-92311'), + array('0-9752298-0-X') ); } @@ -58,6 +59,7 @@ class IsbnValidatorTest extends \PHPUnit_Framework_TestCase array('0-4X19-92611'), array('0_45122_5244'), array('2870#971#648'), + array('0-9752298-0-x') ); } @@ -145,7 +147,7 @@ class IsbnValidatorTest extends \PHPUnit_Framework_TestCase */ public function testValidIsbn10($isbn) { - $constraint = new Isbn(array('isbn10' => true)); + $constraint = new Isbn(array('type' => 'isbn10')); $this->context ->expects($this->never()) ->method('addViolation'); @@ -158,7 +160,7 @@ class IsbnValidatorTest extends \PHPUnit_Framework_TestCase */ public function testInvalidIsbn10($isbn) { - $constraint = new Isbn(array('isbn10' => true)); + $constraint = new Isbn(array('type' => 'isbn10')); $this->context ->expects($this->once()) ->method('addViolation') @@ -172,7 +174,7 @@ class IsbnValidatorTest extends \PHPUnit_Framework_TestCase */ public function testValidIsbn13($isbn) { - $constraint = new Isbn(array('isbn13' => true)); + $constraint = new Isbn(array('type' => 'isbn13')); $this->context ->expects($this->never()) ->method('addViolation'); @@ -185,7 +187,7 @@ class IsbnValidatorTest extends \PHPUnit_Framework_TestCase */ public function testInvalidIsbn13($isbn) { - $constraint = new Isbn(array('isbn13' => true)); + $constraint = new Isbn(array('type' => 'isbn13')); $this->context ->expects($this->once()) ->method('addViolation') @@ -199,7 +201,7 @@ class IsbnValidatorTest extends \PHPUnit_Framework_TestCase */ public function testValidIsbn($isbn) { - $constraint = new Isbn(array('isbn10' => true, 'isbn13' => true)); + $constraint = new Isbn(); $this->context ->expects($this->never()) ->method('addViolation'); @@ -212,7 +214,7 @@ class IsbnValidatorTest extends \PHPUnit_Framework_TestCase */ public function testInvalidIsbn($isbn) { - $constraint = new Isbn(array('isbn10' => true, 'isbn13' => true)); + $constraint = new Isbn(); $this->context ->expects($this->once()) ->method('addViolation')