Improved ISBN validator

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 <thewholelifetolearn@gmail.com>
+ * @author Manuel Reinhard <manu@sprain.ch>
  */
 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 $type;
-    public $isbn13;
+    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)) {
+        return 'type';
-            $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'));
-        }
     }
 }

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 <thewholelifetolearn@gmail.com>
- *
+ * @author Manuel Reinhard <manu@sprain.ch>
  * @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;
+        if (null == $constraint->type) {
-        $value = strtoupper($value);
+            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) {
+                return false;
-                    $this->context->addViolation($constraint->bothIsbnMessage);
+            } else {
-                } else {
+                return true;
-                    $this->context->addViolation($constraint->isbn10Message);
-                }
             }
-        } 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) {
+                return false;
-                    $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);
             } 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;
+        }
     }
 }

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')