[zfcampus#249] Proper fix for content-negotiation

POST is the only one that should allow `content_name` in the payload; otherwise, we're using PATCH or DELETE with the `content_name` in the URI. As such, this means two input filters. This patch creates CreateContentNegotiationInputFilter, which extends ContentNegotiationInputFilterTest and ensures that `content_name` is present.
guliano · Jan 26, 2015 · 674f66c · 674f66c
1 parent 170f010
commit 674f66c
Show file tree

Hide file tree

Showing 5 changed files with 168 additions and 16 deletions.
diff --git a/config/module.config.php b/config/module.config.php
@@ -1282,6 +1282,7 @@
             'ZF\Apigility\Admin\InputFilter\Authorization' => 'ZF\Apigility\Admin\InputFilter\AuthorizationInputFilter',
             'ZF\Apigility\Admin\InputFilter\DbAdapter' => 'ZF\Apigility\Admin\InputFilter\DbAdapterInputFilter',
             'ZF\Apigility\Admin\InputFilter\ContentNegotiation' => 'ZF\Apigility\Admin\InputFilter\ContentNegotiationInputFilter',
+            'ZF\Apigility\Admin\InputFilter\CreateContentNegotiation' => 'ZF\Apigility\Admin\InputFilter\CreateContentNegotiationInputFilter',
 
             'ZF\Apigility\Admin\InputFilter\Module' => 'ZF\Apigility\Admin\InputFilter\ModuleInputFilter',
             'ZF\Apigility\Admin\InputFilter\Version' => 'ZF\Apigility\Admin\InputFilter\VersionInputFilter',
@@ -1314,6 +1315,7 @@
 
         'ZF\Apigility\Admin\Controller\ContentNegotiation' => array(
             'input_filter' => 'ZF\Apigility\Admin\InputFilter\ContentNegotiation',
+            'POST' => 'ZF\Apigility\Admin\InputFilter\CreateContentNegotiation',
         ),
 
         'ZF\Apigility\Admin\Controller\Module' => array(

diff --git a/src/InputFilter/ContentNegotiationInputFilter.php b/src/InputFilter/ContentNegotiationInputFilter.php
@@ -23,16 +23,6 @@ public function isValid()
         $isValid = true;
 
         foreach ($this->data as $className => $mediaTypes) {
-            if ($className === 'content_name' && is_string($mediaTypes)) {
-                continue;
-            }
-
-            if ($className === 'content_name' && ! is_string($mediaTypes)) {
-                $this->messages[$className][] = 'Content name (' . $className . ') is invalid; must be a string';
-                $isValid = false;
-                continue;
-            }
-
             if (! class_exists($className)) {
                 $this->messages[$className][] = 'Class name (' . $className . ') does not exist';
                 $isValid = false;

diff --git a/src/InputFilter/CreateContentNegotiationInputFilter.php b/src/InputFilter/CreateContentNegotiationInputFilter.php
@@ -0,0 +1,52 @@
+<?php
+/**
+ * @license   http://opensource.org/licenses/BSD-3-Clause BSD-3-Clause
+ * @copyright Copyright (c) 2014 Zend Technologies USA Inc. (http://www.zend.com)
+ */
+
+namespace ZF\Apigility\Admin\InputFilter;
+
+class CreateContentNegotiationInputFilter extends ContentNegotiationInputFilter
+{
+    /**
+     * Is the data set valid?
+     *
+     * @return bool
+     */
+    public function isValid()
+    {
+        $this->messages = array();
+        $isValid = true;
+
+        if (! array_key_exists('content_name', $this->data)) {
+            $this->messages['content_name'][] = 'No content_name was provided; must be present for new negotiators.';
+            $isValid = false;
+        }
+
+        if (array_key_exists('content_name', $this->data) && ! is_string($this->data['content_name'])) {
+            $this->messages['content_name'][] = 'Content name provided is invalid; must be a string';
+            $isValid = false;
+        }
+
+        if (! $isValid) {
+            return false;
+        }
+
+        $contentName = $this->data['content_name'];
+        unset($this->data['content_name']);
+
+        $isValid = parent::isValid();
+
+        $this->data['content_name'] = $contentName;
+
+        return $isValid;
+    }
+
+    /**
+     * @return array
+     */
+    public function getMessages()
+    {
+        return $this->messages;
+    }
+}
diff --git a/test/InputFilter/ContentNegotiationInputFilterTest.php b/test/InputFilter/ContentNegotiationInputFilterTest.php
@@ -19,12 +19,6 @@ public function dataProviderIsValid()
                     'Zend\View\Model\ViewModel' => array('text/html', 'application/xhtml+xml'),
                 ),
             ),
-            'with-content-name' => array(
-                array(
-                    'content_name' => 'test',
-                    'Zend\View\Model\ViewModel' => array('text/html', 'application/xhtml+xml'),
-                )
-            ),
         );
     }
 

diff --git a/test/InputFilter/CreateContentNegotiationInputFilterTest.php b/test/InputFilter/CreateContentNegotiationInputFilterTest.php
@@ -0,0 +1,114 @@
+<?php
+/**
+ * @license   http://opensource.org/licenses/BSD-3-Clause BSD-3-Clause
+ * @copyright Copyright (c) 2014 Zend Technologies USA Inc. (http://www.zend.com)
+ */
+
+namespace ZFTest\Apigility\Admin\InputFilter;
+
+use PHPUnit_Framework_TestCase as TestCase;
+use ZF\Apigility\Admin\InputFilter\CreateContentNegotiationInputFilter;
+
+class CreateContentNegotiationInputFilterTest extends TestCase
+{
+    public function dataProviderIsValid()
+    {
+        return array(
+            'with-content-name' => array(
+                array(
+                    'content_name' => 'test',
+                    'Zend\View\Model\ViewModel' => array('text/html', 'application/xhtml+xml'),
+                )
+            ),
+        );
+    }
+
+    public function dataProviderIsInvalid()
+    {
+        return array(
+            'missing-content-name' => array(
+                array(
+                    'Zend\View\Model\ViewModel' => array('text/html', 'application/xhtml+xml'),
+                ),
+                array(
+                    'content_name' => array('No content_name was provided; must be present for new negotiators.'),
+                ),
+            ),
+            'null-content-name' => array(
+                array(
+                    'content_name' => null,
+                    'Zend\View\Model\ViewModel' => array('text/html', 'application/xhtml+xml'),
+                ),
+                array(
+                    'content_name' => array('Content name provided is invalid; must be a string'),
+                ),
+            ),
+            'bool-content-name' => array(
+                array(
+                    'content_name' => true,
+                    'Zend\View\Model\ViewModel' => array('text/html', 'application/xhtml+xml'),
+                ),
+                array(
+                    'content_name' => array('Content name provided is invalid; must be a string'),
+                ),
+            ),
+            'int-content-name' => array(
+                array(
+                    'content_name' => 1,
+                    'Zend\View\Model\ViewModel' => array('text/html', 'application/xhtml+xml'),
+                ),
+                array(
+                    'content_name' => array('Content name provided is invalid; must be a string'),
+                ),
+            ),
+            'float-content-name' => array(
+                array(
+                    'content_name' => 1.1,
+                    'Zend\View\Model\ViewModel' => array('text/html', 'application/xhtml+xml'),
+                ),
+                array(
+                    'content_name' => array('Content name provided is invalid; must be a string'),
+                ),
+            ),
+            'array-content-name' => array(
+                array(
+                    'content_name' => array('content_name'),
+                    'Zend\View\Model\ViewModel' => array('text/html', 'application/xhtml+xml'),
+                ),
+                array(
+                    'content_name' => array('Content name provided is invalid; must be a string'),
+                ),
+            ),
+            'object-content-name' => array(
+                array(
+                    'content_name' => (object) array('content_name'),
+                    'Zend\View\Model\ViewModel' => array('text/html', 'application/xhtml+xml'),
+                ),
+                array(
+                    'content_name' => array('Content name provided is invalid; must be a string'),
+                ),
+            ),
+        );
+    }
+
+    /**
+     * @dataProvider dataProviderIsValid
+     */
+    public function testIsValid($data)
+    {
+        $filter = new CreateContentNegotiationInputFilter;
+        $filter->setData($data);
+        $this->assertTrue($filter->isValid(), var_export($filter->getMessages(), 1));
+    }
+
+    /**
+     * @dataProvider dataProviderIsInvalid
+     */
+    public function testIsInvalid($data, $messages)
+    {
+        $filter = new CreateContentNegotiationInputFilter;
+        $filter->setData($data);
+        $this->assertFalse($filter->isValid());
+        $this->assertEquals($messages, $filter->getMessages());
+    }
+}