diff -u b/core/core.services.yml b/core/core.services.yml --- b/core/core.services.yml +++ b/core/core.services.yml @@ -144,7 +144,7 @@ arguments: ['@request_stack', '@string_translation', '@csrf_token'] form_submitter: class: Drupal\Core\Form\FormSubmitter - arguments: ['@request_stack', '@url_generator', '@current_route_match'] + arguments: ['@request_stack', '@url_generator'] keyvalue: class: Drupal\Core\KeyValueStore\KeyValueFactory arguments: ['@service_container', '@settings'] @@ -179,7 +179,7 @@ arguments: ['@request_stack', '@string_translation', '@csrf_token', '@logger.channel.form'] form_submitter: class: Drupal\Core\Form\FormSubmitter - arguments: ['@request_stack', '@url_generator'] + arguments: ['@request_stack', '@url_generator', '@current_route_match'] form_cache: class: Drupal\Core\Form\FormCache arguments: ['@keyvalue.expirable', '@module_handler', '@current_user', '@csrf_token'] diff -u b/core/lib/Drupal/Core/Form/FormSubmitter.php b/core/lib/Drupal/Core/Form/FormSubmitter.php --- b/core/lib/Drupal/Core/Form/FormSubmitter.php +++ b/core/lib/Drupal/Core/Form/FormSubmitter.php @@ -46,9 +46,9 @@ * @param \Symfony\Component\HttpFoundation\RequestStack $request_stack * The request stack. * @param \Drupal\Core\Routing\UrlGeneratorInterface $url_generator - * The URL generator. + * The URL generator. * @param \Drupal\Core\Routing\RouteMatchInterface $current_route_match - * The route match. + * The route match. */ public function __construct(RequestStack $request_stack, UrlGeneratorInterface $url_generator, RouteMatchInterface $current_route_match) { $this->requestStack = $request_stack; @@ -149,19 +149,14 @@ // If no redirect was specified, redirect to the current path. elseif ($redirect === NULL) { $request = $this->requestStack->getCurrentRequest(); - // @todo Remove dependency on the internal _system_path attribute: - // https://www.drupal.org/node/2293521. - $url = $this->urlGenerator->generateFromPath($request->attributes->get('_system_path'), array( - 'query' => $request->query->all(), - 'absolute' => TRUE, - )); - } - - if ($url) { - // According to RFC 7231, 303 See Other status code must be used to redirect - // user agent (and not default 302 Found). - // @see http://tools.ietf.org/html/rfc7231#section-6.4.4 - return new RedirectResponse($url, Response::HTTP_SEE_OTHER); + if ($route_name = $this->routeMatch->getRouteName()) { + $url = $this->urlGenerator->generateFromRoute($this->routeMatch->getRouteName(), $this->routeMatch->getRawParameters() + ->all(), array( + 'query' => $request->query->all(), + 'absolute' => TRUE, + )); + return new RedirectResponse($url, Response::HTTP_SEE_OTHER); + } } } @@ -220,13 +215,13 @@ } } $request = $this->requestStack->getCurrentRequest(); - if ($route_name = $this->routeMatch->getRouteName()) { - $url = $this->urlGenerator->generateFromRoute($this->routeMatch->getRouteName(), $this->routeMatch->getRawParameters()->all(), array( - 'query' => $request->query->all(), - 'absolute' => TRUE, - )); - return new RedirectResponse($url, Response::HTTP_SEE_OTHER); - } + // @todo Remove dependency on the internal _system_path attribute: + // https://www.drupal.org/node/2293521. + $url = $this->urlGenerator->generateFromPath($request->attributes->get('_system_path'), array( + 'query' => $request->query->all(), + 'absolute' => TRUE, + )); + return new RedirectResponse($url, Response::HTTP_SEE_OTHER); } } interdiff impossible; taking evasive action reverted: --- b/core/tests/Drupal/Tests/Core/Form/FormSubmitterTest.php +++ a/core/tests/Drupal/Tests/Core/Form/FormSubmitterTest.php @@ -10,7 +10,6 @@ use Drupal\Core\DependencyInjection\ContainerBuilder; use Drupal\Core\Url; use Drupal\Tests\UnitTestCase; -use Symfony\Component\HttpFoundation\ParameterBag; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\RequestStack; @@ -33,13 +32,6 @@ protected $urlGenerator; /** - * The route match. - * - * @var \PHPUnit_Framework_MockObject_MockObject|\Drupal\Core\Routing\RouteMatchInterface - */ - protected $routeMatch; - - /** * {@inheritdoc} */ public static function getInfo() { @@ -56,7 +48,6 @@ public function setUp() { parent::setUp(); $this->urlGenerator = $this->getMock('Drupal\Core\Routing\UrlGeneratorInterface'); - $this->routeMatch = $this->getMock('Drupal\Core\Routing\RouteMatchInterface'); } /** @@ -130,6 +121,7 @@ $this->urlGenerator->expects($this->once()) ->method('generateFromPath') ->will($this->returnValueMap(array( + array(NULL, array('query' => array(), 'absolute' => TRUE), ''), array('foo', array('absolute' => TRUE), 'foo'), array('bar', array('query' => array('foo' => 'baz'), 'absolute' => TRUE), 'bar'), array('baz', array('absolute' => TRUE), 'baz'), @@ -143,32 +135,6 @@ } /** - * Tests the redirectForm() method when no redirect was set. - * - * @covers ::redirectForm - */ - public function testRedirectWithResultWithoutSpecifiedRedirect() { - $form_submitter = $this->getFormSubmitter(); - - $this->routeMatch->expects($this->once()) - ->method('getRouteName') - ->will($this->returnValue('test_route')); - $this->routeMatch->expects($this->once()) - ->method('getRawParameters') - ->will($this->returnValue(new ParameterBag(array('key' => 'value')))); - - $this->urlGenerator->expects($this->once()) - ->method('generateFromRoute') - ->with($this->equalTo('test_route'), $this->equalTo(array('key' => 'value'))) - ->will($this->returnValue('test-route')); - - $form_state = $this->getFormStateDefaults(); - $redirect = $form_submitter->redirectForm($form_state); - $this->assertSame('test-route', $redirect->getTargetUrl()); - $this->assertSame(303, $redirect->getStatusCode()); - } - - /** * Tests the redirectForm() with redirect_route when a redirect is expected. * * @covers ::redirectForm @@ -236,6 +202,7 @@ */ public function providerTestRedirectWithResult() { return array( + array(array(), ''), array(array('redirect' => 'foo'), 'foo'), array(array('redirect' => array('foo')), 'foo'), array(array('redirect' => array('foo')), 'foo'), @@ -310,17 +277,13 @@ } /** - * Sets up a form submitter for the test. - * - * @param \Symfony\Component\HttpFoundation\Request $request - * (optional) The request object. * @return \Drupal\Core\Form\FormSubmitterInterface */ + protected function getFormSubmitter() { - protected function getFormSubmitter(Request $request = NULL) { $request_stack = new RequestStack(); + $request_stack->push(new Request()); - $request_stack->push($request ?: new Request()); return $this->getMockBuilder('Drupal\Core\Form\FormSubmitter') + ->setConstructorArgs(array($request_stack, $this->urlGenerator)) - ->setConstructorArgs(array($request_stack, $this->urlGenerator, $this->routeMatch)) ->setMethods(array('batchGet', 'drupalInstallationAttempted')) ->getMock(); } unchanged: --- a/core/tests/Drupal/Tests/Core/Form/FormSubmitterTest.php +++ b/core/tests/Drupal/Tests/Core/Form/FormSubmitterTest.php @@ -11,6 +11,7 @@ use Drupal\Core\Form\FormState; use Drupal\Core\Url; use Drupal\Tests\UnitTestCase; +use Symfony\Component\HttpFoundation\ParameterBag; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\RequestStack; @@ -29,11 +30,19 @@ class FormSubmitterTest extends UnitTestCase { protected $urlGenerator; /** + * The route match. + * + * @var \PHPUnit_Framework_MockObject_MockObject|\Drupal\Core\Routing\RouteMatchInterface + */ + protected $routeMatch; + + /** * {@inheritdoc} */ protected function setUp() { parent::setUp(); $this->urlGenerator = $this->getMock('Drupal\Core\Routing\UrlGeneratorInterface'); + $this->routeMatch = $this->getMock('Drupal\Core\Routing\RouteMatchInterface'); } /** @@ -102,10 +111,6 @@ public function providerTestHandleFormSubmissionWithResponses() { */ public function testRedirectWithNull() { $form_submitter = $this->getFormSubmitter(); - $this->urlGenerator->expects($this->once()) - ->method('generateFromPath') - ->with(NULL, array('query' => array(), 'absolute' => TRUE)) - ->willReturn(''); $form_state = $this->getMock('Drupal\Core\Form\FormStateInterface'); $form_state->expects($this->once()) @@ -146,6 +151,32 @@ public function testRedirectWithUrl(Url $redirect_value, $result, $status = 303) } /** + * Tests the redirectForm() method when no redirect was set. + * + * @covers ::redirectForm + */ + public function testRedirectWithResultWithoutSpecifiedRedirect() { + $form_submitter = $this->getFormSubmitter(); + + $this->routeMatch->expects($this->once()) + ->method('getRouteName') + ->will($this->returnValue('test_route')); + $this->routeMatch->expects($this->once()) + ->method('getRawParameters') + ->will($this->returnValue(new ParameterBag(array('key' => 'value')))); + + $this->urlGenerator->expects($this->once()) + ->method('generateFromRoute') + ->with($this->equalTo('test_route'), $this->equalTo(array('key' => 'value'))) + ->will($this->returnValue('test-route')); + + $form_state = $this->getFormStateDefaults(); + $redirect = $form_submitter->redirectForm($form_state); + $this->assertSame('test-route', $redirect->getTargetUrl()); + $this->assertSame(303, $redirect->getStatusCode()); + } + + /** * Provides test data for testing the redirectForm() method with a route name. * * @return array @@ -230,13 +261,17 @@ public function testExecuteSubmitHandlers() { } /** + * Sets up a form submitter for the test. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * (optional) The request object. * @return \Drupal\Core\Form\FormSubmitterInterface */ - protected function getFormSubmitter() { + protected function getFormSubmitter(Request $request = NULL) { $request_stack = new RequestStack(); - $request_stack->push(new Request()); + $request_stack->push($request ?: new Request()); return $this->getMockBuilder('Drupal\Core\Form\FormSubmitter') - ->setConstructorArgs(array($request_stack, $this->urlGenerator)) + ->setConstructorArgs(array($request_stack, $this->urlGenerator, $this->routeMatch)) ->setMethods(array('batchGet', 'drupalInstallationAttempted')) ->getMock(); }