New validation method for annotations

Note that we cannot rely on Doctrine Annotation's built-in property validator, as it cannot currently recognize union types, in particular string|null. We're using annotation constructors and our own validation methods instead. See: doctrine/annotations#129
brick · Mar 22, 2019 · 3056bb4 · 3056bb4
1 parent cb39649
commit 3056bb4
Show file tree

Hide file tree

Showing 6 changed files with 173 additions and 54 deletions.
diff --git a/src/Controller/Annotation/AbstractAnnotation.php b/src/Controller/Annotation/AbstractAnnotation.php
@@ -6,23 +6,142 @@
 
 /**
  * Base class for annotation classes.
+ *
+ * Annotations do not use Doctrine Annotation's built-in property validator, as it cannot currently recognize union
+ * types, in particular string|null. We're using annotation constructors and our own validation methods instead.
  */
 abstract class AbstractAnnotation
 {
     /**
-     * @param array $parameters
+     * @param array  $values
+     * @param string $name
+     * @param bool   $isFirst
      *
-     * @throws \RuntimeException
+     * @return string
+     *
+     * @throws \LogicException
+     */
+    final protected function getRequiredString(array $values, string $name, bool $isFirst = false) : string
+    {
+        $value = $this->getOptionalString($values, $name, $isFirst);
+
+        if ($value === null) {
+            throw new \LogicException(sprintf(
+                'Attribute "%s" of annotation %s is required.',
+                $name,
+                $this->getAnnotationName()
+            ));
+        }
+
+        return $value;
+    }
+
+    /**
+     * @param array  $values
+     * @param string $name
+     * @param bool   $isFirst
+     *
+     * @return string|null
+     *
+     * @throws \LogicException
+     */
+    final protected function getOptionalString(array $values, string $name, bool $isFirst = false) : ?string
+    {
+        if (isset($values[$name])) {
+            $value = $values[$name];
+        } elseif ($isFirst && isset($values['value'])) {
+            $value = $values['value'];
+        } else {
+            return null;
+        }
+
+        if (! is_string($value)) {
+            throw new \LogicException(sprintf(
+                'Attribute "%s" of annotation %s expects a string, %s given.',
+                $name,
+                $this->getAnnotationName(),
+                gettype($value)
+            ));
+        }
+
+        return $value;
+    }
+
+    /**
+     * @param array  $values
+     * @param string $name
+     * @param bool   $isFirst
+     *
+     * @return string[]
+     *
+     * @throws \LogicException
      */
-    public function __construct(array $parameters)
+    final protected function getRequiredStringArray(array $values, string $name, bool $isFirst = false) : array
     {
-        foreach ($parameters as $key => $value) {
-            $method = 'set' . ucfirst($key);
-            if (method_exists($this, $method)) {
-                $this->$method($value);
-            } else {
-                throw new \RuntimeException(sprintf('Unknown key "%s" for annotation "@%s"', $key, get_class($this)));
+        $values = $this->getOptionalStringArray($values, $name, $isFirst);
+
+        if (! $values) {
+            throw new \LogicException(sprintf(
+                'Attribute "%s" of annotation %s must not be empty.',
+                $name,
+                $this->getAnnotationName()
+            ));
+        }
+
+        return $values;
+    }
+
+    /**
+     * @param array  $values
+     * @param string $name
+     * @param bool   $isFirst
+     *
+     * @return string[]
+     *
+     * @throws \LogicException
+     */
+    final protected function getOptionalStringArray(array $values, string $name, bool $isFirst = false) : array
+    {
+        if (isset($values[$name])) {
+            $value = $values[$name];
+        } elseif ($isFirst && isset($values['value'])) {
+            $value = $values['value'];
+        } else {
+            return [];
+        }
+
+        if (is_string($value)) {
+            return [$value];
+        }
+
+        if (is_array($value)) {
+            foreach ($value as $item) {
+                if (! is_string($item)) {
+                    throw new \LogicException(sprintf(
+                        'Attribute "%s" of annotation %s expects an array of strings, %s found in array.',
+                        $name,
+                        $this->getAnnotationName(),
+                        gettype($item)
+                    ));
+                }
             }
+
+            return $value;
         }
+
+        throw new \LogicException(sprintf(
+            'Attribute "%s" of annotation %s expects a string or array of strings, %s given.',
+            $name,
+            $this->getAnnotationName(),
+            gettype($value)
+        ));
+    }
+
+    /**
+     * @return string
+     */
+    final protected function getAnnotationName() : string
+    {
+        return '@' . (new \ReflectionObject($this))->getShortName();
     }
 }
diff --git a/src/Controller/Annotation/Allow.php b/src/Controller/Annotation/Allow.php
@@ -20,16 +20,16 @@ final class Allow extends AbstractAnnotation
     /**
      * The HTTP method(s) the controller action accepts.
      *
-     * @var array
+     * @var string[]
      */
-    private $methods = [];
+    private $methods;
 
     /**
-     * @param string|array $methods
+     * @param array $values
      */
-    public function setValue($methods)
+    public function __construct(array $values)
     {
-        $this->methods = is_array($methods) ? $methods : [$methods];
+        $this->methods = $this->getRequiredStringArray($values, 'methods', true);
     }
 
     /**

diff --git a/src/Controller/Annotation/RequestParam.php b/src/Controller/Annotation/RequestParam.php
@@ -9,40 +9,34 @@
 /**
  * Base class for QueryParam and PostParam.
  */
-abstract class RequestParam
+abstract class RequestParam extends AbstractAnnotation
 {
     /**
+     * The query or post parameter name.
+     *
      * @var string
      */
     private $name;
 
     /**
-     * @var string
+     * The variable to bind to, or null if same as $name.
+     *
+     * @var string|null
      */
     private $bindTo;
 
     /**
-     * Class constructor.
-     *
      * @param array $values
-     *
-     * @throws \RuntimeException
      */
     public function __construct(array $values)
     {
-        if (isset($values['name'])) {
-            $name = $values['name'];
-        } elseif (isset($values['value'])) {
-            $name = $values['value'];
-        } else {
-            throw new \RuntimeException($this->getAnnotationName() . ' requires a parameter name.');
-        }
-
-        $this->name = $name;
-        $this->bindTo = isset($values['bindTo']) ? $values['bindTo'] : $name;
+        $this->name   = $this->getRequiredString($values, 'name', true);
+        $this->bindTo = $this->getOptionalString($values, 'bindTo');
     }
 
     /**
+     * Returns the query or post parameter name.
+     *
      * @return string
      */
     public function getName() : string
@@ -51,19 +45,13 @@ public function getName() : string
     }
 
     /**
+     * Returns the variable to bind to.
+     *
      * @return string
      */
     public function getBindTo() : string
     {
-        return $this->bindTo;
-    }
-
-    /**
-     * @return string
-     */
-    private function getAnnotationName() : string
-    {
-        return '@' . (new \ReflectionObject($this))->getShortName();
+        return $this->bindTo ?? $this->name;
     }
 
     /**

diff --git a/src/Controller/Annotation/Route.php b/src/Controller/Annotation/Route.php
@@ -4,8 +4,6 @@
 
 namespace Brick\App\Controller\Annotation;
 
-use Doctrine\Common\Annotations\Annotation\Required;
-
 /**
  * Defines a route on a controller.
  *
@@ -17,13 +15,11 @@
  * @Annotation
  * @Target({"CLASS", "METHOD"})
  */
-final class Route
+final class Route extends AbstractAnnotation
 {
     /**
      * The path, with optional {named} parameters.
      *
-     * @Required
-     *
      * @var string
      */
     public $path;
@@ -36,7 +32,7 @@ final class Route
      *
      * @var string[]
      */
-    public $patterns = [];
+    public $patterns;
 
     /**
      * The list of HTTP methods (e.g. GET or POST) this route is valid for.
@@ -46,5 +42,15 @@ final class Route
      *
      * @var string[]
      */
-    public $methods = [];
+    public $methods;
+
+    /**
+     * @param array $values
+     */
+    public function __construct(array $values)
+    {
+        $this->path     = $this->getRequiredString($values, 'path', true);
+        $this->patterns = $this->getOptionalStringArray($values, 'patterns');
+        $this->methods  = $this->getOptionalStringArray($values, 'methods');
+    }
 }
diff --git a/src/Controller/Annotation/Secure.php b/src/Controller/Annotation/Secure.php
@@ -22,10 +22,18 @@
  * @Annotation
  * @Target({"CLASS", "METHOD"})
  */
-final class Secure
+final class Secure extends AbstractAnnotation
 {
     /**
      * @var string
      */
     public $hsts;
+
+    /**
+     * @param array $values
+     */
+    public function __construct(array $values)
+    {
+        $this->hsts = $this->getRequiredString($values, 'hsts', true);
+    }
 }
diff --git a/src/Controller/Annotation/Transactional.php b/src/Controller/Annotation/Transactional.php
@@ -42,16 +42,14 @@ final class Transactional extends AbstractAnnotation
     ];
 
     /**
-     * @param string $isolationLevel
-     *
-     * @return void
-     *
-     * @throws \RuntimeException
+     * @param array $values
      */
-    public function setValue(string $isolationLevel) : void
+    public function __construct(array $values)
     {
+        $isolationLevel = $this->getRequiredString($values, 'isolationLevel', true);
+
         if (! isset(self::ISOLATION_LEVELS[$isolationLevel])) {
-            throw new \RuntimeException('Invalid transaction isolation level: ' . $isolationLevel);
+            throw new \LogicException('Invalid transaction isolation level: ' . $isolationLevel);
         }
 
         $this->isolationLevel = self::ISOLATION_LEVELS[$isolationLevel];