diff --git a/core/core.api.php b/core/core.api.php index a8b13ca..0ff7f1e 100644 --- a/core/core.api.php +++ b/core/core.api.php @@ -2054,12 +2054,6 @@ function hook_validation_constraint_alter(array &$definitions) { * an array. Here are the details of its elements, all of which are optional: * - callback: The callback to invoke to handle the server side of the * Ajax event. More information on callbacks is below in @ref sub_callback. - * - path: The URL path to use for the request. If omitted, defaults to - * 'system/ajax', which invokes the default Drupal Ajax processing (this will - * call the callback supplied in the 'callback' element). If you supply a - * path, you must set up a routing entry to handle the request yourself and - * return output described in @ref sub_callback below. See the - * @link menu Routing topic @endlink for more information on routing. * - wrapper: The HTML 'id' attribute of the area where the content returned by * the callback should be placed. Note that callbacks have a choice of * returning content or JavaScript commands; 'wrapper' is used for content @@ -2085,6 +2079,13 @@ function hook_validation_constraint_alter(array &$definitions) { * - message: Translated message to display. * - url: For a bar progress indicator, URL path for determining progress. * - interval: For a bar progress indicator, how often to update it. + * - url: A \Drupal\Core\URL to which to submit the Ajax request. If omitted, + * defaults to either the same URL as the form or link destination is for + * someone with JavaScript disabled, or a slightly modified version (e.g., + * with a query parameter added, removed, or changed) of that URL if + * necessary to support Drupal's content negotiation. It is recommended to + * omit this key and use Drupal's content negotiation rather than using + * substantially different URLs between Ajax and non-Ajax. * * @subsection sub_callback Setting up a callback to process Ajax * Once you have set up your form to trigger an Ajax response (see @ref sub_form diff --git a/core/lib/Drupal/Core/Form/FormBuilder.php b/core/lib/Drupal/Core/Form/FormBuilder.php index d966801..75d50c5 100644 --- a/core/lib/Drupal/Core/Form/FormBuilder.php +++ b/core/lib/Drupal/Core/Form/FormBuilder.php @@ -185,9 +185,22 @@ public function buildForm($form_id, FormStateInterface &$form_state) { // Ensure the form ID is prepared. $form_id = $this->getFormId($form_id, $form_state); + $request = $this->requestStack->getCurrentRequest(); + + // Inform $form_state about the request method that's building it, so that + // it can prevent persisting state changes during HTTP methods for which + // that is disallowed by HTTP. + $form_state->setRequestMethod($request->getMethod()); + + // Initialize the form's user input. The user input should include only the + // input meant to be treated as part of what is submitted to the form, so + // we base it on the form's method rather than the request's method. For + // example, when someone does a GET request for + // /node/add/article?destination=foo, which is a form that expects its + // submission method to be POST, the user input during the GET request + // should be initialized to empty rather than to ['destination' => 'foo']. $input = $form_state->getUserInput(); if (!isset($input)) { - $request = $this->requestStack->getCurrentRequest(); $input = $form_state->isMethodType('get') ? $request->query->all() : $request->request->all(); $form_state->setUserInput($input); } @@ -313,8 +326,22 @@ public function buildForm($form_id, FormStateInterface &$form_state) { */ public function rebuildForm($form_id, FormStateInterface &$form_state, $old_form = NULL) { $form = $this->retrieveForm($form_id, $form_state); - // All rebuilt forms will be cached. - $form_state->setCached(); + + // Only GET and POST are valid form methods. If the form receives its input + // via POST, then $form_state must be persisted when it is rebuilt between + // submissions. If the form receives its input via GET, then persisting + // state is forbidden by $form_state->setCached(), and the form must use + // the URL itself to transfer its state across steps. Although $form_state + // throws an exception based on the request method rather than the form's + // method, we base the decision to cache on the form method, because: + // - It's the form method that defines what the form needs to do to manage + // its state. + // - rebuildForm() should only be called after successful input processing, + // which means the request method matches the form method, and if not, + // there's some other error, so it's ok if an exception is thrown. + if ($form_state->isMethodType('POST')) { + $form_state->setCached(); + } // If only parts of the form will be returned to the browser (e.g., Ajax or // RIA clients), or if the form already had a new build ID regenerated when diff --git a/core/lib/Drupal/Core/Form/FormBuilderInterface.php b/core/lib/Drupal/Core/Form/FormBuilderInterface.php index 60ad704..bbfed36 100644 --- a/core/lib/Drupal/Core/Form/FormBuilderInterface.php +++ b/core/lib/Drupal/Core/Form/FormBuilderInterface.php @@ -108,9 +108,7 @@ public function buildForm($form_id, FormStateInterface &$form_state); * form workflow, to be returned for rendering. * * Ajax form submissions are almost always multi-step workflows, so that is - * one common use-case during which form rebuilding occurs. See - * Drupal\system\FormAjaxController::content() for more information about - * creating Ajax-enabled forms. + * one common use-case during which form rebuilding occurs. * * @param string $form_id * The unique string identifying the desired form. If a function with that @@ -130,7 +128,6 @@ public function buildForm($form_id, FormStateInterface &$form_state); * The newly built form. * * @see self::processForm() - * @see \Drupal\system\FormAjaxController::content() */ public function rebuildForm($form_id, FormStateInterface &$form_state, $old_form = NULL); diff --git a/core/lib/Drupal/Core/Form/FormState.php b/core/lib/Drupal/Core/Form/FormState.php index 4a17d6b..885dbc2 100644 --- a/core/lib/Drupal/Core/Form/FormState.php +++ b/core/lib/Drupal/Core/Form/FormState.php @@ -141,16 +141,29 @@ class FormState implements FormStateInterface { /** * The HTTP form method to use for finding the input for this form. * - * May be 'post' or 'get'. Defaults to 'post'. Note that 'get' method forms do + * May be 'POST' or 'GET'. Defaults to 'POST'. Note that 'GET' method forms do * not use form ids so are always considered to be submitted, which can have - * unexpected effects. The 'get' method should only be used on forms that do - * not change data, as that is exclusively the domain of 'post.' + * unexpected effects. The 'GET' method should only be used on forms that do + * not change data, as that is exclusively the domain of 'POST.' * * This property is uncacheable. * * @var string */ - protected $method = 'post'; + protected $method = 'POST'; + + /** + * The HTTP method used by the request building or processing this form. + * + * May be any valid HTTP method. Defaults to 'GET', because even though + * $method is 'POST' for most forms, the form's initial build is usually + * performed as part of a GET request. + * + * This property is uncacheable. + * + * @var string + */ + protected $requestMethod = 'GET'; /** * If set to TRUE the original, unprocessed form structure will be cached, @@ -474,7 +487,13 @@ public function getButtons() { /** * {@inheritdoc} */ - public function setCached($cache = TRUE) { + public function setCached($cache = TRUE, $bypass_get_check = FALSE) { + // Persisting $form_state is a side-effect disallowed during a "safe" HTTP + // method. + if ($cache && !$bypass_get_check && $this->isRequestMethodSafe()) { + throw new \LogicException(sprintf('Form state caching on %s requests is not allowed.', $this->requestMethod)); + } + $this->cache = (bool) $cache; return $this; } @@ -572,6 +591,29 @@ public function isMethodType($method_type) { /** * {@inheritdoc} */ + public function setRequestMethod($method) { + $this->requestMethod = strtoupper($method); + return $this; + } + + /** + * Checks whether the request method is a "safe" HTTP method. + * + * http://www.w3.org/Protocols/rfc2616/rfc2616-sec9.html#sec9.1.1 defines + * GET and HEAD as "safe" methods, meaning they SHOULD NOT have side-effects, + * such as persisting $form_state changes. + * + * @return bool + * + * @see \Symfony\Component\HttpFoundation\Request::isMethodSafe() + */ + protected function isRequestMethodSafe() { + return in_array($this->requestMethod, ['GET', 'HEAD']); + } + + /** + * {@inheritdoc} + */ public function setValidationEnforced($must_validate = TRUE) { $this->must_validate = (bool) $must_validate; return $this; diff --git a/core/lib/Drupal/Core/Form/FormStateInterface.php b/core/lib/Drupal/Core/Form/FormStateInterface.php index e0ca1c6..bd263f6 100644 --- a/core/lib/Drupal/Core/Form/FormStateInterface.php +++ b/core/lib/Drupal/Core/Form/FormStateInterface.php @@ -638,10 +638,17 @@ public function getButtons(); * * @param bool $cache * TRUE if the form should be cached, FALSE otherwise. + * @param bool $bypass_get_check + * (optional) By default GET request cannot be cached, but with that flag + * you can opt into that. * * @return $this + * + * @throws \LogicException + * If the current request is using an HTTP method that must not change + * state (e.g., GET). */ - public function setCached($cache = TRUE); + public function setCached($cache = TRUE, $bypass_get_check = FALSE); /** * Determines if the form should be cached. @@ -731,18 +738,35 @@ public function setLimitValidationErrors($limit_validation_errors); public function getLimitValidationErrors(); /** - * Sets the HTTP form method. + * Sets the HTTP method to use for the form's submission. + * + * This is what the form's "method" attribute should be, not necessarily what + * the current request's HTTP method is. For example, a form can have a + * method attribute of POST, but the request that initially builds it uses + * GET. * * @param string $method - * The HTTP form method. + * Either "GET" or "POST". Other HTTP methods are not valid form submission + * methods. * * @see \Drupal\Core\Form\FormState::$method + * @see self::setRequestMethod(). * * @return $this */ public function setMethod($method); /** + * Sets the HTTP method used by the request that is building the form. + * + * @param string $method + * Can be any valid HTTP method, such as GET, POST, HEAD, etc. + * + * @return $this + */ + public function setRequestMethod($method); + + /** * Returns the HTTP form method. * * @param string diff --git a/core/lib/Drupal/Core/Render/Element/RenderElement.php b/core/lib/Drupal/Core/Render/Element/RenderElement.php index a3b362c..5e7568b 100644 --- a/core/lib/Drupal/Core/Render/Element/RenderElement.php +++ b/core/lib/Drupal/Core/Render/Element/RenderElement.php @@ -128,14 +128,7 @@ public static function preRenderGroup($element) { * @see self::preRenderAjaxForm() */ public static function processAjaxForm(&$element, FormStateInterface $form_state, &$complete_form) { - $element = static::preRenderAjaxForm($element); - - // If the element was processed as an #ajax element, and a custom URL was - // provided, set the form to be cached. - if (!empty($element['#ajax_processed']) && !empty($element['#ajax']['url'])) { - $form_state->setCached(); - } - return $element; + return static::preRenderAjaxForm($element); } /** @@ -238,10 +231,6 @@ public static function preRenderAjaxForm($element) { // content negotiation takes care of formatting the response appropriately. // However, 'url' and 'options' may be set when wanting server processing // to be substantially different for a JavaScript triggered submission. - // One such substantial difference is form elements that use - // #ajax['callback'] for determining which part of the form needs - // re-rendering. For that, we have a special 'system.ajax' route which - // must be manually set. $settings += [ 'url' => NULL, 'options' => ['query' => []], diff --git a/core/lib/Drupal/Core/Theme/AjaxBasePageNegotiator.php b/core/lib/Drupal/Core/Theme/AjaxBasePageNegotiator.php index b15ce93..9d904ee 100644 --- a/core/lib/Drupal/Core/Theme/AjaxBasePageNegotiator.php +++ b/core/lib/Drupal/Core/Theme/AjaxBasePageNegotiator.php @@ -15,12 +15,12 @@ /** * Defines a theme negotiator that deals with the active theme on ajax requests. * - * Many different pages can invoke an Ajax request to system/ajax or another - * generic Ajax path. It is almost always desired for an Ajax response to be - * rendered using the same theme as the base page, because most themes are built - * with the assumption that they control the entire page, so if the CSS for two - * themes are both loaded for a given page, they may conflict with each other. - * For example, Bartik is Drupal's default theme, and Seven is Drupal's default + * Many different pages can invoke an Ajax request to a generic Ajax path. It is + * almost always desired for an Ajax response to be rendered using the same + * theme as the base page, because most themes are built with the assumption + * that they control the entire page, so if the CSS for two themes are both + * loaded for a given page, they may conflict with each other. For example, + * Bartik is Drupal's default theme, and Seven is Drupal's default * administration theme. Depending on whether the "Use the administration theme * when editing or creating content" checkbox is checked, the node edit form may * be displayed in either theme, but the Ajax response to the Field module's diff --git a/core/modules/dblog/src/Tests/DbLogFormInjectionTest.php b/core/modules/dblog/src/Tests/DbLogFormInjectionTest.php index 6957f51..62fa234 100644 --- a/core/modules/dblog/src/Tests/DbLogFormInjectionTest.php +++ b/core/modules/dblog/src/Tests/DbLogFormInjectionTest.php @@ -99,6 +99,9 @@ protected function setUp() { */ public function testLoggerSerialization() { $form_state = new FormState(); + + // Forms are only serialized during POST requests. + $form_state->setRequestMethod('POST'); $form_state->setCached(); $form_builder = $this->container->get('form_builder'); $form_id = $form_builder->getFormId($this, $form_state); diff --git a/core/modules/file/src/Controller/FileWidgetAjaxController.php b/core/modules/file/src/Controller/FileWidgetAjaxController.php index 1c1b7e3..d05a4dd 100644 --- a/core/modules/file/src/Controller/FileWidgetAjaxController.php +++ b/core/modules/file/src/Controller/FileWidgetAjaxController.php @@ -7,13 +7,12 @@ namespace Drupal\file\Controller; -use Drupal\system\Controller\FormAjaxController; use Symfony\Component\HttpFoundation\JsonResponse; /** * Defines a controller to respond to file widget AJAX requests. */ -class FileWidgetAjaxController extends FormAjaxController { +class FileWidgetAjaxController { /** * Returns the progress status for a file upload process. diff --git a/core/modules/simpletest/src/WebTestBase.php b/core/modules/simpletest/src/WebTestBase.php index e17d703..5b7bedc 100644 --- a/core/modules/simpletest/src/WebTestBase.php +++ b/core/modules/simpletest/src/WebTestBase.php @@ -1593,15 +1593,14 @@ protected function drupalGetAjax($path, array $options = array(), array $headers * case, this value needs to be an array with the following keys: * - path: A path to submit the form values to for Ajax-specific processing, * which is likely different than the $path parameter used for retrieving - * the initial form. Defaults to 'system/ajax'. - * - triggering_element: If the value for the 'path' key is 'system/ajax' or - * another generic Ajax processing path, this needs to be set to the name - * of the element. If the name doesn't identify the element uniquely, then - * this should instead be an array with a single key/value pair, - * corresponding to the element name and value. The callback for the - * generic Ajax processing path uses this to find the #ajax information - * for the element, including which specific callback to use for - * processing the request. + * the initial form. + * - triggering_element: If the value for the 'path' key is a generic Ajax + * processing path, this needs to be set to the name of the element. If + * the name doesn't identify the element uniquely, then this should + * instead be an array with a single key/value pair, corresponding to the + * element name and value. The callback for the generic Ajax processing + * path uses this to find the #ajax information for the element, including + * which specific callback to use for processing the request. * * This can also be set to NULL in order to emulate an Internet Explorer * submission of a form with a single text field, and pressing ENTER in that @@ -1649,7 +1648,10 @@ protected function drupalPostForm($path, $edit, $submit, array $options = array( $submit_matches = $this->handleForm($post, $edit, $upload, $ajax ? NULL : $submit, $form); $action = isset($form['action']) ? $this->getAbsoluteUrl((string) $form['action']) : $this->getUrl(); if ($ajax) { - $action = $this->getAbsoluteUrl(!empty($submit['path']) ? $submit['path'] : 'system/ajax'); + if (empty($submit['path'])) { + throw new \Exception('No #ajax path specified.'); + } + $action = $this->getAbsoluteUrl($submit['path']); // Ajax callbacks verify the triggering element if necessary, so while // we may eventually want extra code that verifies it in the // handleForm() function, it's not currently a requirement. @@ -1735,8 +1737,7 @@ protected function drupalPostForm($path, $edit, $submit, array $options = array( * and the value is the button label. i.e.) array('op' => t('Refresh')). * @param $ajax_path * (optional) Override the path set by the Ajax settings of the triggering - * element. In the absence of both the triggering element's Ajax path and - * $ajax_path 'system/ajax' will be used. + * element. * @param $options * (optional) Options to be forwarded to the url generator. * @param $headers @@ -1807,7 +1808,7 @@ protected function drupalPostAjaxForm($path, $edit, $triggering_element, $ajax_p $extra_post = '&' . $this->serializePostValues($extra_post); // Unless a particular path is specified, use the one specified by the - // Ajax settings, or else 'system/ajax'. + // Ajax settings. if (!isset($ajax_path)) { if (isset($ajax_settings['url'])) { // In order to allow to set for example the wrapper envelope query @@ -1824,10 +1825,12 @@ protected function drupalPostAjaxForm($path, $edit, $triggering_element, $ajax_p $parsed_url['path'] ); } - else { - $ajax_path = 'system/ajax'; - } } + + if (empty($ajax_path)) { + throw new \Exception('No #ajax path specified.'); + } + $ajax_path = $this->container->get('unrouted_url_assembler')->assemble('base://' . $ajax_path, $options); // Submit the POST request. diff --git a/core/modules/system/src/Controller/FormAjaxController.php b/core/modules/system/src/Controller/FormAjaxController.php deleted file mode 100644 index 084847a..0000000 --- a/core/modules/system/src/Controller/FormAjaxController.php +++ /dev/null @@ -1,186 +0,0 @@ -logger = $logger; - $this->formBuilder = $form_builder; - $this->renderer = $renderer; - $this->ajaxRenderer = $ajax_renderer; - $this->routeMatch = $route_match; - $this->formAjaxResponseBuilder = $form_ajax_response_builder; - } - - /** - * {@inheritdoc} - */ - public static function create(ContainerInterface $container) { - return new static( - $container->get('logger.factory')->get('ajax'), - $container->get('form_builder'), - $container->get('renderer'), - $container->get('main_content_renderer.ajax'), - $container->get('current_route_match'), - $container->get('form_ajax_response_builder') - ); - } - - /** - * Processes an Ajax form submission. - * - * @param \Symfony\Component\HttpFoundation\Request $request - * The current request object. - * - * @return mixed - * Whatever is returned by the triggering element's #ajax['callback'] - * function. One of: - * - A render array containing the new or updated content to return to the - * browser. This is commonly an element within the rebuilt form. - * - A \Drupal\Core\Ajax\AjaxResponse object containing commands for the - * browser to process. - * - * @throws \Symfony\Component\HttpKernel\Exception\HttpExceptionInterface - */ - public function content(Request $request) { - $ajax_form = $this->getForm($request); - $form = $ajax_form->getForm(); - $form_state = $ajax_form->getFormState(); - $commands = $ajax_form->getCommands(); - - $this->formBuilder->processForm($form['#form_id'], $form, $form_state); - - return $this->formAjaxResponseBuilder->buildResponse($request, $form, $form_state, $commands); - } - - /** - * Gets a form submitted via #ajax during an Ajax callback. - * - * This will load a form from the form cache used during Ajax operations. It - * pulls the form info from the request body. - * - * @param \Symfony\Component\HttpFoundation\Request $request - * The current request object. - * - * @return \Drupal\system\FileAjaxForm - * A wrapper object containing the $form, $form_state, $form_id, - * $form_build_id and an initial list of Ajax $commands. - * - * @throws \Symfony\Component\HttpKernel\Exception\HttpExceptionInterface - */ - protected function getForm(Request $request) { - $form_state = new FormState(); - $form_build_id = $request->request->get('form_build_id'); - - // Get the form from the cache. - $form = $this->formBuilder->getCache($form_build_id, $form_state); - if (!$form) { - // If $form cannot be loaded from the cache, the form_build_id must be - // invalid, which means that someone performed a POST request onto - // system/ajax without actually viewing the concerned form in the browser. - // This is likely a hacking attempt as it never happens under normal - // circumstances. - $this->logger->warning('Invalid form POST data.'); - throw new BadRequestHttpException(); - } - - // Since some of the submit handlers are run, redirects need to be disabled. - $form_state->disableRedirect(); - - // When a form is rebuilt after Ajax processing, its #build_id and #action - // should not change. - // @see \Drupal\Core\Form\FormBuilderInterface::rebuildForm() - $form_state->addRebuildInfo('copy', [ - '#build_id' => TRUE, - '#action' => TRUE, - ]); - - // The form needs to be processed; prepare for that by setting a few - // internal variables. - $form_state->setUserInput($request->request->all()); - $form_id = $form['#form_id']; - - return new FileAjaxForm($form, $form_state, $form_id, $form['#build_id'], []); - } - -} diff --git a/core/modules/system/src/Tests/Ajax/AjaxFormCacheTest.php b/core/modules/system/src/Tests/Ajax/AjaxFormCacheTest.php index f151c33..4ebeed6 100644 --- a/core/modules/system/src/Tests/Ajax/AjaxFormCacheTest.php +++ b/core/modules/system/src/Tests/Ajax/AjaxFormCacheTest.php @@ -37,15 +37,6 @@ public function testFormCacheUsage() { // The number of cache entries should not have changed. $this->assertEqual(0, count($key_value_expirable->getAll())); - - // Visit a form that is explicitly cached, 3 times. - $cached_form_url = Url::fromRoute('ajax_forms_test.cached_form'); - $this->drupalGet($cached_form_url); - $this->drupalGet($cached_form_url); - $this->drupalGet($cached_form_url); - - // The number of cache entries should be exactly 3. - $this->assertEqual(3, count($key_value_expirable->getAll())); } /** diff --git a/core/modules/system/src/Tests/Ajax/AjaxFormPageCacheTest.php b/core/modules/system/src/Tests/Ajax/AjaxFormPageCacheTest.php index 03727b6..136ec69 100644 --- a/core/modules/system/src/Tests/Ajax/AjaxFormPageCacheTest.php +++ b/core/modules/system/src/Tests/Ajax/AjaxFormPageCacheTest.php @@ -35,7 +35,7 @@ protected function getFormBuildId() { } /** - * Create a simple form, then POST to system/ajax to change to it. + * Create a simple form, then POST via AJAX to change to it. */ public function testSimpleAJAXFormValue() { $this->drupalGet('ajax_forms_test_get_form'); diff --git a/core/modules/system/src/Tests/Form/FormStoragePageCacheTest.php b/core/modules/system/src/Tests/Form/FormStoragePageCacheTest.php index 5fa8c34..e0be445 100644 --- a/core/modules/system/src/Tests/Form/FormStoragePageCacheTest.php +++ b/core/modules/system/src/Tests/Form/FormStoragePageCacheTest.php @@ -53,7 +53,7 @@ public function testValidateFormStorageOnCachedPage() { // Trigger validation error by submitting an empty title. $edit = ['title' => '']; $this->drupalPostForm(NULL, $edit, 'Save'); - $this->assertText($build_id_initial, 'Old build id on the page'); + $this->assertText('No old build id', 'No old build id on the page'); $build_id_first_validation = $this->getFormBuildId(); $this->assertNotEqual($build_id_initial, $build_id_first_validation, 'Build id changes when form validation fails'); @@ -74,7 +74,7 @@ public function testValidateFormStorageOnCachedPage() { // Trigger validation error by submitting an empty title. $edit = ['title' => '']; $this->drupalPostForm(NULL, $edit, 'Save'); - $this->assertText($build_id_initial, 'Old build id is initial build id'); + $this->assertText('No old build id', 'No old build id on the page'); $build_id_from_cache_first_validation = $this->getFormBuildId(); $this->assertNotEqual($build_id_initial, $build_id_from_cache_first_validation, 'Build id changes when form validation fails'); $this->assertNotEqual($build_id_first_validation, $build_id_from_cache_first_validation, 'Build id from first user is not reused'); @@ -96,10 +96,15 @@ public function testRebuildFormStorageOnCachedPage() { $this->assertText('No old build id', 'No old build id on the page'); $build_id_initial = $this->getFormBuildId(); - // Trigger rebuild, should regenerate build id. + // Trigger rebuild, should regenerate build id. When a submit handler + // triggers a rebuild, the form is built twice in the same POST request, + // and during the second build, there is an old build ID, but because the + // form is not cached during the initial GET request, it is different from + // that initial build ID. $edit = ['title' => 'something']; $this->drupalPostForm(NULL, $edit, 'Rebuild'); - $this->assertText($build_id_initial, 'Initial build id as old build id on the page'); + $this->assertNoText('No old build id', 'There is an old build id on the page.'); + $this->assertNoText($build_id_initial, 'The old build id is not the initial build id.'); $build_id_first_rebuild = $this->getFormBuildId(); $this->assertNotEqual($build_id_initial, $build_id_first_rebuild, 'Build id changes on first rebuild.'); diff --git a/core/modules/system/src/Tests/Queue/QueueSerializationTest.php b/core/modules/system/src/Tests/Queue/QueueSerializationTest.php index 9108ac2..eb07d51 100644 --- a/core/modules/system/src/Tests/Queue/QueueSerializationTest.php +++ b/core/modules/system/src/Tests/Queue/QueueSerializationTest.php @@ -97,7 +97,7 @@ protected function setUp() { */ public function testQueueSerialization() { $form_state = new FormState(); - $form_state->setCached(); + $form_state->setCached(TRUE, TRUE); $form_builder = $this->container->get('form_builder'); $form_id = $form_builder->getFormId($this, $form_state); $form = $form_builder->retrieveForm($form_id, $form_state); diff --git a/core/modules/system/system.routing.yml b/core/modules/system/system.routing.yml index 12e7d9a..a386196 100644 --- a/core/modules/system/system.routing.yml +++ b/core/modules/system/system.routing.yml @@ -1,12 +1,3 @@ -system.ajax: - path: '/system/ajax' - defaults: - _controller: '\Drupal\system\Controller\FormAjaxController::content' - options: - _theme: ajax_base_page - requirements: - _access: 'TRUE' - system.401: path: '/system/401' defaults: diff --git a/core/modules/system/tests/modules/ajax_forms_test/ajax_forms_test.routing.yml b/core/modules/system/tests/modules/ajax_forms_test/ajax_forms_test.routing.yml index 3caf5ba..ccca279 100644 --- a/core/modules/system/tests/modules/ajax_forms_test/ajax_forms_test.routing.yml +++ b/core/modules/system/tests/modules/ajax_forms_test/ajax_forms_test.routing.yml @@ -30,10 +30,3 @@ ajax_forms_test.lazy_load_form: requirements: _access: 'TRUE' -ajax_forms_test.cached_form: - path: '/ajax_forms_test_cached_form' - defaults: - _title: 'AJAX forms cached form test' - _form: '\Drupal\ajax_forms_test\Form\AjaxFormsTestCachedForm' - requirements: - _access: 'TRUE' diff --git a/core/modules/system/tests/modules/ajax_forms_test/src/Form/AjaxFormsTestCachedForm.php b/core/modules/system/tests/modules/ajax_forms_test/src/Form/AjaxFormsTestCachedForm.php deleted file mode 100644 index 3b22783..0000000 --- a/core/modules/system/tests/modules/ajax_forms_test/src/Form/AjaxFormsTestCachedForm.php +++ /dev/null @@ -1,50 +0,0 @@ - 'select', - '#title' => $this->t('Test 1'), - '#options' => [ - 'option1' => $this->t('Option 1'), - 'option2' => $this->t('Option 2'), - ], - '#ajax' => [ - 'url' => Url::fromRoute('system.ajax'), - ], - ]; - return $form; - } - - /** - * {@inheritdoc} - */ - public function submitForm(array &$form, FormStateInterface $form_state) { - } - -} diff --git a/core/modules/system/tests/modules/ajax_forms_test/src/Form/AjaxFormsTestSimpleForm.php b/core/modules/system/tests/modules/ajax_forms_test/src/Form/AjaxFormsTestSimpleForm.php index 225664d..2fcfd1f 100644 --- a/core/modules/system/tests/modules/ajax_forms_test/src/Form/AjaxFormsTestSimpleForm.php +++ b/core/modules/system/tests/modules/ajax_forms_test/src/Form/AjaxFormsTestSimpleForm.php @@ -57,7 +57,7 @@ public function buildForm(array $form, FormStateInterface $form_state) { ); // This is for testing invalid callbacks that should return a 500 error in - // \Drupal\system\FormAjaxController::content(). + // \Drupal\Core\Form\FormAjaxResponseBuilderInterface::buildResponse(). $invalid_callbacks = array( 'null' => NULL, 'empty' => '', diff --git a/core/modules/system/tests/modules/form_test/form_test.module b/core/modules/system/tests/modules/form_test/form_test.module index d297945..180f89e 100644 --- a/core/modules/system/tests/modules/form_test/form_test.module +++ b/core/modules/system/tests/modules/form_test/form_test.module @@ -84,7 +84,7 @@ function form_test_form_form_test_state_persist_alter(&$form, FormStateInterface // Simulate a form alter implementation inserting form elements that enable // caching of the form, e.g. elements having #ajax. if (\Drupal::request()->get('cache')) { - $form_state->setCached(); + $form_state->setCached(TRUE, TRUE); } } diff --git a/core/modules/system/tests/modules/form_test/src/Form/FormTestStorageForm.php b/core/modules/system/tests/modules/form_test/src/Form/FormTestStorageForm.php index 9510ef4..545b5cf 100644 --- a/core/modules/system/tests/modules/form_test/src/Form/FormTestStorageForm.php +++ b/core/modules/system/tests/modules/form_test/src/Form/FormTestStorageForm.php @@ -85,7 +85,7 @@ public function buildForm(array $form, FormStateInterface $form_state) { if (\Drupal::request()->get('cache')) { // Manually activate caching, so we can test that the storage keeps working // when it's enabled. - $form_state->setCached(); + $form_state->setCached(TRUE, TRUE); } if ($this->getRequest()->get('immutable')) { diff --git a/core/modules/system/tests/modules/form_test/src/Form/FormTestStoragePageCacheForm.php b/core/modules/system/tests/modules/form_test/src/Form/FormTestStoragePageCacheForm.php index e8eef22..9f3ab5a 100644 --- a/core/modules/system/tests/modules/form_test/src/Form/FormTestStoragePageCacheForm.php +++ b/core/modules/system/tests/modules/form_test/src/Form/FormTestStoragePageCacheForm.php @@ -48,7 +48,6 @@ public function buildForm(array $form, FormStateInterface $form_state) { ); $form['#after_build'] = array(array($this, 'form_test_storage_page_cache_old_build_id')); - $form_state->setCached(); return $form; } @@ -73,6 +72,17 @@ function form_test_storage_page_cache_rebuild($form, FormStateInterface $form_st /** * {@inheritdoc} */ + public function validateForm(array &$form, FormStateInterface $form_state) { + // Test using form cache when re-displaying a form due to validation + // errors. + if ($form_state->hasAnyErrors()) { + $form_state->setCached(); + } + } + + /** + * {@inheritdoc} + */ public function submitForm(array &$form, FormStateInterface $form_state) { // Nothing must happen. } diff --git a/core/modules/system/tests/modules/form_test/src/Form/FormTestValidateForm.php b/core/modules/system/tests/modules/form_test/src/Form/FormTestValidateForm.php index e403887..ea9fe06 100644 --- a/core/modules/system/tests/modules/form_test/src/Form/FormTestValidateForm.php +++ b/core/modules/system/tests/modules/form_test/src/Form/FormTestValidateForm.php @@ -51,7 +51,7 @@ public function buildForm(array $form, FormStateInterface $form_state) { // To simplify this test, enable form caching and use form storage to // remember our alteration. - $form_state->setCached(); + $form_state->setCached(TRUE, TRUE); return $form; } diff --git a/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php b/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php index 9e53645..215a497 100644 --- a/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php +++ b/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php @@ -303,6 +303,8 @@ public function testRebuildForm() { $form = $this->formBuilder->buildForm($form_arg, $form_state); $original_build_id = $form['#build_id']; + $this->request->setMethod('POST'); + $form_state->setRequestMethod('POST'); // Rebuild the form, and assert that the build ID has not changed. $form_state->setRebuild(); $input['form_id'] = $form_id; @@ -310,11 +312,51 @@ public function testRebuildForm() { $form_state->addRebuildInfo('copy', ['#build_id' => TRUE]); $this->formBuilder->processForm($form_id, $form, $form_state); $this->assertSame($original_build_id, $form['#build_id']); + $this->assertTrue($form_state->isCached()); // Rebuild the form again, and assert that there is a new build ID. $form_state->setRebuildInfo([]); $form = $this->formBuilder->buildForm($form_arg, $form_state); $this->assertNotSame($original_build_id, $form['#build_id']); + $this->assertTrue($form_state->isCached()); + } + + /** + * Tests the rebuildForm() method for a GET submission. + */ + public function testRebuildFormOnGetRequest() { + $form_id = 'test_form_id'; + $expected_form = $form_id(); + + // The form will be built four times. + $form_arg = $this->getMock('Drupal\Core\Form\FormInterface'); + $form_arg->expects($this->exactly(2)) + ->method('getFormId') + ->will($this->returnValue($form_id)); + $form_arg->expects($this->exactly(4)) + ->method('buildForm') + ->will($this->returnValue($expected_form)); + + // Do an initial build of the form and track the build ID. + $form_state = new FormState(); + $form_state->setMethod('GET'); + $form = $this->formBuilder->buildForm($form_arg, $form_state); + $original_build_id = $form['#build_id']; + + // Rebuild the form, and assert that the build ID has not changed. + $form_state->setRebuild(); + $input['form_id'] = $form_id; + $form_state->setUserInput($input); + $form_state->addRebuildInfo('copy', ['#build_id' => TRUE]); + $this->formBuilder->processForm($form_id, $form, $form_state); + $this->assertSame($original_build_id, $form['#build_id']); + $this->assertFalse($form_state->isCached()); + + // Rebuild the form again, and assert that there is a new build ID. + $form_state->setRebuildInfo([]); + $form = $this->formBuilder->buildForm($form_arg, $form_state); + $this->assertNotSame($original_build_id, $form['#build_id']); + $this->assertFalse($form_state->isCached()); } /** @@ -338,7 +380,7 @@ public function testGetCache() { // Do an initial build of the form and track the build ID. $form_state = (new FormState()) ->addBuildInfo('files', [['module' => 'node', 'type' => 'pages.inc']]) - ->setCached(); + ->setCached(TRUE, TRUE); $form = $this->formBuilder->buildForm($form_arg, $form_state); $cached_form = $form; @@ -407,7 +449,7 @@ public function testFormCacheDeletionCached() { ->with($form_build_id); $form_state = new FormState(); - $form_state->setCached(); + $form_state->setCached(TRUE, TRUE); $this->simulateFormSubmission($form_id, $form_arg, $form_state); } diff --git a/core/tests/Drupal/Tests/Core/Form/FormStateTest.php b/core/tests/Drupal/Tests/Core/Form/FormStateTest.php index b92c5f7..03690bb 100644 --- a/core/tests/Drupal/Tests/Core/Form/FormStateTest.php +++ b/core/tests/Drupal/Tests/Core/Form/FormStateTest.php @@ -422,6 +422,11 @@ public function testIsCached($cache_key, $no_cache_key, $expected) { 'cache' => $cache_key, 'no_cache' => $no_cache_key, ]); + + $form_state->setMethod('POST'); + $this->assertSame($expected, $form_state->isCached()); + + $form_state->setMethod('GET'); $this->assertSame($expected, $form_state->isCached()); } @@ -464,6 +469,28 @@ public function providerTestIsCached() { } /** + * @covers ::setCached + */ + public function testSetCachedPost() { + $form_state = new FormState(); + $form_state->setRequestMethod('POST'); + $form_state->setCached(); + $this->assertTrue($form_state->isCached()); + } + + /** + * @covers ::setCached + * + * @expectedException \LogicException + * @expectedExceptionMessage Form state caching on GET requests is not allowed. + */ + public function testSetCachedGet() { + $form_state = new FormState(); + $form_state->setMethod('GET'); + $form_state->setCached(); + } + + /** * @covers ::isMethodType * @covers ::setMethod * diff --git a/core/tests/Drupal/Tests/Core/Render/Element/RenderElementTest.php b/core/tests/Drupal/Tests/Core/Render/Element/RenderElementTest.php new file mode 100644 index 0000000..a554b8a --- /dev/null +++ b/core/tests/Drupal/Tests/Core/Render/Element/RenderElementTest.php @@ -0,0 +1,116 @@ +requestStack = new RequestStack(); + $this->container = new ContainerBuilder(); + $this->container->set('request_stack', $this->requestStack); + \Drupal::setContainer($this->container); + } + + /** + * @covers ::preRenderAjaxForm + */ + public function testPreRenderAjaxForm() { + $request = Request::create('/test'); + $request->query->set('foo', 'bar'); + $this->requestStack->push($request); + + $prophecy = $this->prophesize('Drupal\Core\Routing\UrlGeneratorInterface'); + $url = '/test?foo=bar&ajax_form=1'; + $prophecy->generateFromRoute('', [], ['query' => ['foo' => 'bar', FormBuilderInterface::AJAX_FORM_REQUEST => TRUE]], FALSE) + ->willReturn($url); + + $url_generator = $prophecy->reveal(); + $this->container->set('url_generator', $url_generator); + + $element = [ + '#type' => 'select', + '#id' => 'test', + '#ajax' => [ + 'wrapper' => 'foo', + 'callback' => 'test-callback', + ], + ]; + + $element = RenderElement::preRenderAjaxForm($element); + + $this->assertTrue($element['#ajax_processed']); + $this->assertEquals($url, $element['#attached']['drupalSettings']['ajax']['test']['url']); + } + + /** + * @covers ::preRenderAjaxForm + */ + public function testPreRenderAjaxFormWithQueryOptions() { + $request = Request::create('/test'); + $request->query->set('foo', 'bar'); + $this->requestStack->push($request); + + $prophecy = $this->prophesize('Drupal\Core\Routing\UrlGeneratorInterface'); + $url = '/test?foo=bar&other=query&ajax_form=1'; + $prophecy->generateFromRoute('', [], ['query' => ['foo' => 'bar', 'other' => 'query', FormBuilderInterface::AJAX_FORM_REQUEST => TRUE]], FALSE) + ->willReturn($url); + + $url_generator = $prophecy->reveal(); + $this->container->set('url_generator', $url_generator); + + $element = [ + '#type' => 'select', + '#id' => 'test', + '#ajax' => [ + 'wrapper' => 'foo', + 'callback' => 'test-callback', + 'options' => [ + 'query' => [ + 'other' => 'query', + ], + ], + ], + ]; + + $element = RenderElement::preRenderAjaxForm($element); + + $this->assertTrue($element['#ajax_processed']); + $this->assertEquals($url, $element['#attached']['drupalSettings']['ajax']['test']['url']); + } + +}