From 7727de73e5d40fe92e07b2b74e10162c55dfeacf Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Mon, 9 Jul 2012 20:34:45 +0200 Subject: [PATCH] [Form] Deprecated Form::bindRequest() and replaced it by a PRE_BIND listener --- UPGRADE-2.1.md | 15 + src/Symfony/Component/Form/CHANGELOG.md | 1 + .../EventListener/BindRequestListener.php | 85 ++++++ .../Form/Extension/Core/Type/FormType.php | 2 + src/Symfony/Component/Form/Form.php | 39 +-- .../Component/Form/Tests/AbstractFormTest.php | 5 +- .../Component/Form/Tests/CompoundFormTest.php | 13 +- .../EventListener/BindRequestListenerTest.php | 285 ++++++++++++++++++ 8 files changed, 407 insertions(+), 38 deletions(-) create mode 100644 src/Symfony/Component/Form/Extension/Core/EventListener/BindRequestListener.php create mode 100644 src/Symfony/Component/Form/Tests/Extension/Core/EventListener/BindRequestListenerTest.php diff --git a/UPGRADE-2.1.md b/UPGRADE-2.1.md index 773ff2cffe..5e7b600947 100644 --- a/UPGRADE-2.1.md +++ b/UPGRADE-2.1.md @@ -875,6 +875,7 @@ * `getClientData` * `getChildren` * `hasChildren` + * `bindRequest` Before: @@ -906,6 +907,20 @@ if (count($form) > 0) { ``` + Instead of `bindRequest`, you should now simply call `bind`: + + Before: + + ``` + $form->bindRequest($request); + ``` + + After: + + ``` + $form->bind($request); + ``` + * The option "validation_constraint" was deprecated and will be removed in Symfony 2.3. You should use the option "constraints" instead, where you can pass one or more constraints for a form. diff --git a/src/Symfony/Component/Form/CHANGELOG.md b/src/Symfony/Component/Form/CHANGELOG.md index 00ee6c9763..857ca7cbe3 100644 --- a/src/Symfony/Component/Form/CHANGELOG.md +++ b/src/Symfony/Component/Form/CHANGELOG.md @@ -145,3 +145,4 @@ CHANGELOG * DateType, TimeType and DateTimeType now show empty values again if not required * [BC BREAK] fixed rendering of errors for DateType, BirthdayType and similar ones * [BC BREAK] fixed: form constraints are only validated if they belong to the validated group + * deprecated `bindRequest` in `Form` and replaced it by a listener to FormEvents::PRE_BIND diff --git a/src/Symfony/Component/Form/Extension/Core/EventListener/BindRequestListener.php b/src/Symfony/Component/Form/Extension/Core/EventListener/BindRequestListener.php new file mode 100644 index 0000000000..868a5ae42f --- /dev/null +++ b/src/Symfony/Component/Form/Extension/Core/EventListener/BindRequestListener.php @@ -0,0 +1,85 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Form\Extension\Core\EventListener; + +use Symfony\Component\Form\FormEvents; +use Symfony\Component\Form\FormEvent; +use Symfony\Component\Form\Exception\FormException; +use Symfony\Component\EventDispatcher\EventSubscriberInterface; +use Symfony\Component\HttpFoundation\Request; + +/** + * @author Bernhard Schussek + */ +class BindRequestListener implements EventSubscriberInterface +{ + public static function getSubscribedEvents() + { + // High priority in order to supersede other listeners + return array(FormEvents::PRE_BIND => array('preBind', 128)); + } + + public function preBind(FormEvent $event) + { + $form = $event->getForm(); + + /* @var Request $request */ + $request = $event->getData(); + + // Only proceed if we actually deal with a Request + if (!$request instanceof Request) { + return; + } + + $name = $form->getConfig()->getName(); + $default = $form->getConfig()->getCompound() ? array() : null; + + // Store the bound data in case of a post request + switch ($request->getMethod()) { + case 'POST': + case 'PUT': + case 'DELETE': + case 'PATCH': + if ('' === $name) { + // Form bound without name + $params = $request->request->all(); + $files = $request->files->all(); + } else { + $params = $request->request->get($name, $default); + $files = $request->files->get($name, $default); + } + + if (is_array($params) && is_array($files)) { + $data = array_replace_recursive($params, $files); + } else { + $data = $params ?: $files; + } + + break; + + case 'GET': + $data = '' === $name + ? $request->query->all() + : $request->query->get($name, $default); + + break; + + default: + throw new FormException(sprintf( + 'The request method "%s" is not supported', + $request->getMethod() + )); + } + + $event->setData($data); + } +} diff --git a/src/Symfony/Component/Form/Extension/Core/Type/FormType.php b/src/Symfony/Component/Form/Extension/Core/Type/FormType.php index 706b92c9a0..a07b4b877d 100644 --- a/src/Symfony/Component/Form/Extension/Core/Type/FormType.php +++ b/src/Symfony/Component/Form/Extension/Core/Type/FormType.php @@ -17,6 +17,7 @@ use Symfony\Component\Form\FormBuilderInterface; use Symfony\Component\Form\FormInterface; use Symfony\Component\Form\FormFactoryInterface; use Symfony\Component\Form\FormViewInterface; +use Symfony\Component\Form\Extension\Core\EventListener\BindRequestListener; use Symfony\Component\Form\Extension\Core\EventListener\TrimListener; use Symfony\Component\Form\Extension\Core\DataMapper\PropertyPathMapper; use Symfony\Component\EventDispatcher\EventDispatcher; @@ -44,6 +45,7 @@ class FormType extends AbstractType ->setCompound($options['compound']) ->setData($options['data']) ->setDataMapper($options['compound'] ? new PropertyPathMapper() : null) + ->addEventSubscriber(new BindRequestListener()) ; if ($options['trim']) { diff --git a/src/Symfony/Component/Form/Form.php b/src/Symfony/Component/Form/Form.php index 729625fe51..f2a2e9959d 100644 --- a/src/Symfony/Component/Form/Form.php +++ b/src/Symfony/Component/Form/Form.php @@ -585,44 +585,13 @@ class Form implements \IteratorAggregate, FormInterface * @return Form This form * * @throws FormException if the method of the request is not one of GET, POST or PUT + * + * @deprecated Deprecated since version 2.1, to be removed in 2.3. Use + * {@link FormConfigInterface::bind()} instead. */ public function bindRequest(Request $request) { - $name = $this->config->getName(); - - // Store the bound data in case of a post request - switch ($request->getMethod()) { - case 'POST': - case 'PUT': - case 'DELETE': - case 'PATCH': - if ('' === $name) { - // Form bound without name - $params = $request->request->all(); - $files = $request->files->all(); - } elseif ($this->config->getCompound()) { - // Form bound with name and children - $params = $request->request->get($name, array()); - $files = $request->files->get($name, array()); - } else { - // Form bound with name, but without children - $params = $request->request->get($name, null); - $files = $request->files->get($name, null); - } - if (is_array($params) && is_array($files)) { - $data = array_replace_recursive($params, $files); - } else { - $data = $params ?: $files; - } - break; - case 'GET': - $data = '' === $name ? $request->query->all() : $request->query->get($name, array()); - break; - default: - throw new FormException(sprintf('The request method "%s" is not supported', $request->getMethod())); - } - - return $this->bind($data); + return $this->bind($request); } /** diff --git a/src/Symfony/Component/Form/Tests/AbstractFormTest.php b/src/Symfony/Component/Form/Tests/AbstractFormTest.php index b4a2477368..8393d61d0b 100644 --- a/src/Symfony/Component/Form/Tests/AbstractFormTest.php +++ b/src/Symfony/Component/Form/Tests/AbstractFormTest.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Form\Tests; use Symfony\Component\Form\FormBuilder; +use Symfony\Component\EventDispatcher\EventDispatcher; use Symfony\Component\EventDispatcher\EventDispatcherInterface; abstract class AbstractFormTest extends \PHPUnit_Framework_TestCase @@ -37,7 +38,9 @@ abstract class AbstractFormTest extends \PHPUnit_Framework_TestCase $this->markTestSkipped('The "EventDispatcher" component is not available'); } - $this->dispatcher = $this->getMock('Symfony\Component\EventDispatcher\EventDispatcherInterface'); + // We need an actual dispatcher to bind the deprecated + // bindRequest() method + $this->dispatcher = new EventDispatcher();; $this->factory = $this->getMock('Symfony\Component\Form\FormFactoryInterface'); $this->form = $this->createForm(); } diff --git a/src/Symfony/Component/Form/Tests/CompoundFormTest.php b/src/Symfony/Component/Form/Tests/CompoundFormTest.php index be36f88895..8c9652d30b 100644 --- a/src/Symfony/Component/Form/Tests/CompoundFormTest.php +++ b/src/Symfony/Component/Form/Tests/CompoundFormTest.php @@ -14,6 +14,7 @@ namespace Symfony\Component\Form\Tests; use Symfony\Component\Form\Form; use Symfony\Component\Form\FormView; use Symfony\Component\Form\FormError; +use Symfony\Component\Form\Extension\Core\EventListener\BindRequestListener; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\File\UploadedFile; use Symfony\Component\EventDispatcher\EventDispatcher; @@ -360,6 +361,7 @@ class FormTest extends AbstractFormTest $form = $this->getBuilder('author') ->setCompound(true) ->setDataMapper($this->getDataMapper()) + ->addEventSubscriber(new BindRequestListener()) ->getForm(); $form->add($this->getBuilder('name')->getForm()); $form->add($this->getBuilder('image')->getForm()); @@ -408,6 +410,7 @@ class FormTest extends AbstractFormTest $form = $this->getBuilder('') ->setCompound(true) ->setDataMapper($this->getDataMapper()) + ->addEventSubscriber(new BindRequestListener()) ->getForm(); $form->add($this->getBuilder('name')->getForm()); $form->add($this->getBuilder('image')->getForm()); @@ -449,7 +452,9 @@ class FormTest extends AbstractFormTest 'REQUEST_METHOD' => $method, )); - $form = $this->getBuilder('image')->getForm(); + $form = $this->getBuilder('image') + ->addEventSubscriber(new BindRequestListener()) + ->getForm(); $form->bindRequest($request); @@ -480,7 +485,9 @@ class FormTest extends AbstractFormTest 'REQUEST_METHOD' => $method, )); - $form = $this->getBuilder('name')->getForm(); + $form = $this->getBuilder('name') + ->addEventSubscriber(new BindRequestListener()) + ->getForm(); $form->bindRequest($request); @@ -509,6 +516,7 @@ class FormTest extends AbstractFormTest $form = $this->getBuilder('author') ->setCompound(true) ->setDataMapper($this->getDataMapper()) + ->addEventSubscriber(new BindRequestListener()) ->getForm(); $form->add($this->getBuilder('firstName')->getForm()); $form->add($this->getBuilder('lastName')->getForm()); @@ -538,6 +546,7 @@ class FormTest extends AbstractFormTest $form = $this->getBuilder('') ->setCompound(true) ->setDataMapper($this->getDataMapper()) + ->addEventSubscriber(new BindRequestListener()) ->getForm(); $form->add($this->getBuilder('firstName')->getForm()); $form->add($this->getBuilder('lastName')->getForm()); diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/EventListener/BindRequestListenerTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/EventListener/BindRequestListenerTest.php new file mode 100644 index 0000000000..a3d235f226 --- /dev/null +++ b/src/Symfony/Component/Form/Tests/Extension/Core/EventListener/BindRequestListenerTest.php @@ -0,0 +1,285 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Form\Tests\Extension\Core\EventListener; + +use Symfony\Component\Form\Extension\Core\EventListener\BindRequestListener; +use Symfony\Component\Form\Form; +use Symfony\Component\Form\FormConfig; +use Symfony\Component\Form\FormEvent; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\File\UploadedFile; + +/** + * @author Bernhard Schussek + */ +class BindRequestListenerTest extends \PHPUnit_Framework_TestCase +{ + private $values; + + private $filesPlain; + + private $filesNested; + + /** + * @var UploadedFile + */ + private $uploadedFile; + + protected function setUp() + { + $path = tempnam(sys_get_temp_dir(), 'sf2'); + touch($path); + + $this->values = array( + 'name' => 'Bernhard', + 'image' => array('filename' => 'foobar.png'), + ); + + $this->filesPlain = array( + 'image' => array( + 'error' => UPLOAD_ERR_OK, + 'name' => 'upload.png', + 'size' => 123, + 'tmp_name' => $path, + 'type' => 'image/png' + ), + ); + + $this->filesNested = array( + 'error' => array('image' => UPLOAD_ERR_OK), + 'name' => array('image' => 'upload.png'), + 'size' => array('image' => 123), + 'tmp_name' => array('image' => $path), + 'type' => array('image' => 'image/png'), + ); + + $this->uploadedFile = new UploadedFile($path, 'upload.png', 'image/png', 123, UPLOAD_ERR_OK); + } + + protected function tearDown() + { + unlink($this->uploadedFile->getRealPath()); + } + + public function requestMethodProvider() + { + return array( + array('POST'), + array('PUT'), + array('DELETE'), + array('PATCH'), + ); + } + + /** + * @dataProvider requestMethodProvider + */ + public function testBindRequest($method) + { + if (!class_exists('Symfony\Component\HttpFoundation\Request')) { + $this->markTestSkipped('The "HttpFoundation" component is not available'); + } + + $values = array('author' => $this->values); + $files = array('author' => $this->filesNested); + $request = new Request(array(), $values, array(), array(), $files, array( + 'REQUEST_METHOD' => $method, + )); + + $dispatcher = $this->getMock('Symfony\Component\EventDispatcher\EventDispatcherInterface'); + $config = new FormConfig('author', null, $dispatcher); + $form = new Form($config); + $event = new FormEvent($form, $request); + + $listener = new BindRequestListener(); + $listener->preBind($event); + + $this->assertEquals(array( + 'name' => 'Bernhard', + 'image' => $this->uploadedFile, + ), $event->getData()); + } + + /** + * @dataProvider requestMethodProvider + */ + public function testBindRequestWithEmptyName($method) + { + if (!class_exists('Symfony\Component\HttpFoundation\Request')) { + $this->markTestSkipped('The "HttpFoundation" component is not available'); + } + + $request = new Request(array(), $this->values, array(), array(), $this->filesPlain, array( + 'REQUEST_METHOD' => $method, + )); + + $dispatcher = $this->getMock('Symfony\Component\EventDispatcher\EventDispatcherInterface'); + $config = new FormConfig('', null, $dispatcher); + $form = new Form($config); + $event = new FormEvent($form, $request); + + $listener = new BindRequestListener(); + $listener->preBind($event); + + $this->assertEquals(array( + 'name' => 'Bernhard', + 'image' => $this->uploadedFile, + ), $event->getData()); + } + + /** + * @dataProvider requestMethodProvider + */ + public function testBindEmptyRequestToCompoundForm($method) + { + if (!class_exists('Symfony\Component\HttpFoundation\Request')) { + $this->markTestSkipped('The "HttpFoundation" component is not available'); + } + + $request = new Request(array(), array(), array(), array(), array(), array( + 'REQUEST_METHOD' => $method, + )); + + $dispatcher = $this->getMock('Symfony\Component\EventDispatcher\EventDispatcherInterface'); + $config = new FormConfig('author', null, $dispatcher); + $config->setCompound(true); + $config->setDataMapper($this->getMock('Symfony\Component\Form\DataMapperInterface')); + $form = new Form($config); + $event = new FormEvent($form, $request); + + $listener = new BindRequestListener(); + $listener->preBind($event); + + // Default to empty array + $this->assertEquals(array(), $event->getData()); + } + + /** + * @dataProvider requestMethodProvider + */ + public function testBindEmptyRequestToSimpleForm($method) + { + if (!class_exists('Symfony\Component\HttpFoundation\Request')) { + $this->markTestSkipped('The "HttpFoundation" component is not available'); + } + + $request = new Request(array(), array(), array(), array(), array(), array( + 'REQUEST_METHOD' => $method, + )); + + $dispatcher = $this->getMock('Symfony\Component\EventDispatcher\EventDispatcherInterface'); + $config = new FormConfig('author', null, $dispatcher); + $config->setCompound(false); + $form = new Form($config); + $event = new FormEvent($form, $request); + + $listener = new BindRequestListener(); + $listener->preBind($event); + + // Default to null + $this->assertNull($event->getData()); + } + + public function testBindGetRequest() + { + if (!class_exists('Symfony\Component\HttpFoundation\Request')) { + $this->markTestSkipped('The "HttpFoundation" component is not available'); + } + + $values = array('author' => $this->values); + $request = new Request($values, array(), array(), array(), array(), array( + 'REQUEST_METHOD' => 'GET', + )); + + $dispatcher = $this->getMock('Symfony\Component\EventDispatcher\EventDispatcherInterface'); + $config = new FormConfig('author', null, $dispatcher); + $form = new Form($config); + $event = new FormEvent($form, $request); + + $listener = new BindRequestListener(); + $listener->preBind($event); + + $this->assertEquals(array( + 'name' => 'Bernhard', + 'image' => array('filename' => 'foobar.png'), + ), $event->getData()); + } + + public function testBindGetRequestWithEmptyName() + { + if (!class_exists('Symfony\Component\HttpFoundation\Request')) { + $this->markTestSkipped('The "HttpFoundation" component is not available'); + } + + $request = new Request($this->values, array(), array(), array(), array(), array( + 'REQUEST_METHOD' => 'GET', + )); + + $dispatcher = $this->getMock('Symfony\Component\EventDispatcher\EventDispatcherInterface'); + $config = new FormConfig('', null, $dispatcher); + $form = new Form($config); + $event = new FormEvent($form, $request); + + $listener = new BindRequestListener(); + $listener->preBind($event); + + $this->assertEquals(array( + 'name' => 'Bernhard', + 'image' => array('filename' => 'foobar.png'), + ), $event->getData()); + } + + public function testBindEmptyGetRequestToCompoundForm() + { + if (!class_exists('Symfony\Component\HttpFoundation\Request')) { + $this->markTestSkipped('The "HttpFoundation" component is not available'); + } + + $request = new Request(array(), array(), array(), array(), array(), array( + 'REQUEST_METHOD' => 'GET', + )); + + $dispatcher = $this->getMock('Symfony\Component\EventDispatcher\EventDispatcherInterface'); + $config = new FormConfig('author', null, $dispatcher); + $config->setCompound(true); + $config->setDataMapper($this->getMock('Symfony\Component\Form\DataMapperInterface')); + $form = new Form($config); + $event = new FormEvent($form, $request); + + $listener = new BindRequestListener(); + $listener->preBind($event); + + $this->assertEquals(array(), $event->getData()); + } + + public function testBindEmptyGetRequestToSimpleForm() + { + if (!class_exists('Symfony\Component\HttpFoundation\Request')) { + $this->markTestSkipped('The "HttpFoundation" component is not available'); + } + + $request = new Request(array(), array(), array(), array(), array(), array( + 'REQUEST_METHOD' => 'GET', + )); + + $dispatcher = $this->getMock('Symfony\Component\EventDispatcher\EventDispatcherInterface'); + $config = new FormConfig('author', null, $dispatcher); + $config->setCompound(false); + $form = new Form($config); + $event = new FormEvent($form, $request); + + $listener = new BindRequestListener(); + $listener->preBind($event); + + $this->assertNull($event->getData()); + } +}