From 189be7243d40765ad1cfd8075eb0a9b5580f651e Mon Sep 17 00:00:00 2001 From: Alex Pott Date: Thu, 23 Apr 2015 10:41:03 +0100 Subject: [PATCH] Issue #2473759 by tim.plunkett, Berdir, effulgentsia: Form caches should be deleted after submission --- core/lib/Drupal/Core/Form/FormBuilder.php | 16 ++++++- core/lib/Drupal/Core/Form/FormCache.php | 8 ++++ .../Drupal/Core/Form/FormCacheInterface.php | 8 ++++ .../comment/src/Tests/CommentPreviewTest.php | 46 +++++++++++++++++++ .../src/Tests/Element/PathElementFormTest.php | 2 +- .../EntityAutocompleteElementFormTest.php | 2 +- .../Tests/Form/FormDefaultHandlersTest.php | 8 ++++ .../Tests/Core/Form/FormBuilderTest.php | 44 ++++++++++++++++++ .../Drupal/Tests/Core/Form/FormCacheTest.php | 16 +++++++ .../Drupal/Tests/Core/Form/FormTestBase.php | 5 ++ 10 files changed, 152 insertions(+), 3 deletions(-) diff --git a/core/lib/Drupal/Core/Form/FormBuilder.php b/core/lib/Drupal/Core/Form/FormBuilder.php index 4bc6272420d..4b995c132bb 100644 --- a/core/lib/Drupal/Core/Form/FormBuilder.php +++ b/core/lib/Drupal/Core/Form/FormBuilder.php @@ -351,6 +351,13 @@ class FormBuilder implements FormBuilderInterface, FormValidatorInterface, FormS $this->formCache->setCache($form_build_id, $form, $form_state); } + /** + * {@inheritdoc} + */ + public function deleteCache($form_build_id) { + $this->formCache->deleteCache($form_build_id); + } + /** * {@inheritdoc} */ @@ -488,8 +495,15 @@ class FormBuilder implements FormBuilderInterface, FormValidatorInterface, FormS Html::resetSeenIds(); } + // If there are no errors and the form is not rebuilding, submit the form. if (!$form_state->isRebuilding() && !FormState::hasAnyErrors()) { - if ($submit_response = $this->formSubmitter->doSubmitForm($form, $form_state)) { + $submit_response = $this->formSubmitter->doSubmitForm($form, $form_state); + // If this form was cached, delete it from the cache after submission. + if ($form_state->isCached()) { + $this->deleteCache($form['#build_id']); + } + // If the form submission directly returned a response, return it now. + if ($submit_response) { return $submit_response; } } diff --git a/core/lib/Drupal/Core/Form/FormCache.php b/core/lib/Drupal/Core/Form/FormCache.php index a35000da4ea..33719449550 100644 --- a/core/lib/Drupal/Core/Form/FormCache.php +++ b/core/lib/Drupal/Core/Form/FormCache.php @@ -216,4 +216,12 @@ class FormCache implements FormCacheInterface { } } + /** + * {@inheritdoc} + */ + public function deleteCache($form_build_id) { + $this->keyValueExpirableFactory->get('form')->delete($form_build_id); + $this->keyValueExpirableFactory->get('form_state')->delete($form_build_id); + } + } diff --git a/core/lib/Drupal/Core/Form/FormCacheInterface.php b/core/lib/Drupal/Core/Form/FormCacheInterface.php index 3a18b6c8c0e..310b892ca95 100644 --- a/core/lib/Drupal/Core/Form/FormCacheInterface.php +++ b/core/lib/Drupal/Core/Form/FormCacheInterface.php @@ -34,4 +34,12 @@ interface FormCacheInterface { */ public function setCache($form_build_id, $form, FormStateInterface $form_state); + /** + * Deletes a form in the cache. + * + * @param string $form_build_id + * The unique form build ID. + */ + public function deleteCache($form_build_id); + } diff --git a/core/modules/comment/src/Tests/CommentPreviewTest.php b/core/modules/comment/src/Tests/CommentPreviewTest.php index d038514732e..75aca4d0655 100644 --- a/core/modules/comment/src/Tests/CommentPreviewTest.php +++ b/core/modules/comment/src/Tests/CommentPreviewTest.php @@ -64,6 +64,52 @@ class CommentPreviewTest extends CommentTestBase { $this->assertFieldByXPath("//article[contains(@class, 'preview')]//div[contains(@class, 'user-picture')]//img", NULL, 'User picture displayed.'); } + /** + * Tests comment preview. + */ + public function testCommentPreviewDuplicateSubmission() { + // As admin user, configure comment settings. + $this->drupalLogin($this->adminUser); + $this->setCommentPreview(DRUPAL_OPTIONAL); + $this->setCommentForm(TRUE); + $this->setCommentSubject(TRUE); + $this->setCommentSettings('default_mode', CommentManagerInterface::COMMENT_MODE_THREADED, 'Comment paging changed.'); + $this->drupalLogout(); + + // Login as web user. + $this->drupalLogin($this->webUser); + + // As the web user, fill in the comment form and preview the comment. + $edit = array(); + $edit['subject[0][value]'] = $this->randomMachineName(8); + $edit['comment_body[0][value]'] = $this->randomMachineName(16); + $this->drupalPostForm('node/' . $this->node->id(), $edit, t('Preview')); + + // Check that the preview is displaying the title and body. + $this->assertTitle(t('Preview comment | Drupal'), 'Page title is "Preview comment".'); + $this->assertText($edit['subject[0][value]'], 'Subject displayed.'); + $this->assertText($edit['comment_body[0][value]'], 'Comment displayed.'); + + // Check that the title and body fields are displayed with the correct values. + $this->assertFieldByName('subject[0][value]', $edit['subject[0][value]'], 'Subject field displayed.'); + $this->assertFieldByName('comment_body[0][value]', $edit['comment_body[0][value]'], 'Comment field displayed.'); + + // Store the content of this page. + $content = $this->getRawContent(); + $this->drupalPostForm(NULL, [], 'Save'); + $this->assertText('Your comment has been posted.'); + $elements = $this->xpath('//section[contains(@class, "comment-wrapper")]/article'); + $this->assertEqual(1, count($elements)); + + // Reset the content of the page to simulate the browser's back button, and + // re-submit the form. + $this->setRawContent($content); + $this->drupalPostForm(NULL, [], 'Save'); + $this->assertText('Your comment has been posted.'); + $elements = $this->xpath('//section[contains(@class, "comment-wrapper")]/article'); + $this->assertEqual(2, count($elements)); + } + /** * Tests comment edit, preview, and save. */ diff --git a/core/modules/system/src/Tests/Element/PathElementFormTest.php b/core/modules/system/src/Tests/Element/PathElementFormTest.php index dce7df6362f..5698951c62b 100644 --- a/core/modules/system/src/Tests/Element/PathElementFormTest.php +++ b/core/modules/system/src/Tests/Element/PathElementFormTest.php @@ -42,7 +42,7 @@ class PathElementFormTest extends KernelTestBase implements FormInterface { */ protected function setUp() { parent::setUp(); - $this->installSchema('system', array('router', 'sequences')); + $this->installSchema('system', ['router', 'sequences', 'key_value_expire']); $this->installEntitySchema('user'); \Drupal::service('router.builder')->rebuild(); /** @var \Drupal\user\RoleInterface $role */ diff --git a/core/modules/system/src/Tests/Entity/Element/EntityAutocompleteElementFormTest.php b/core/modules/system/src/Tests/Entity/Element/EntityAutocompleteElementFormTest.php index 85ae6065ab0..0fdd9af97a0 100644 --- a/core/modules/system/src/Tests/Entity/Element/EntityAutocompleteElementFormTest.php +++ b/core/modules/system/src/Tests/Entity/Element/EntityAutocompleteElementFormTest.php @@ -50,7 +50,7 @@ class EntityAutocompleteElementFormTest extends EntityUnitTestBase implements Fo protected function setUp() { parent::setUp(); - $this->installSchema('system', array('router')); + $this->installSchema('system', ['router', 'key_value_expire']); \Drupal::service('router.builder')->rebuild(); $this->testUser = User::create(array( diff --git a/core/modules/system/src/Tests/Form/FormDefaultHandlersTest.php b/core/modules/system/src/Tests/Form/FormDefaultHandlersTest.php index 3717c6f1aca..94187af9a4f 100644 --- a/core/modules/system/src/Tests/Form/FormDefaultHandlersTest.php +++ b/core/modules/system/src/Tests/Form/FormDefaultHandlersTest.php @@ -26,6 +26,14 @@ class FormDefaultHandlersTest extends KernelTestBase implements FormInterface { */ public static $modules = array('system'); + /** + * {@inheritdoc} + */ + protected function setUp() { + parent::setUp(); + $this->installSchema('system', ['key_value_expire']); + } + /** * {@inheritdoc} */ diff --git a/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php b/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php index ce7bfee674d..825bd8a8b76 100644 --- a/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php +++ b/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php @@ -383,6 +383,50 @@ class FormBuilderTest extends FormTestBase { $this->assertSame('test-form-id--2', $form['#id']); } + /** + * Tests that a cached form is deleted after submit. + */ + public function testFormCacheDeletionCached() { + $form_id = 'test_form_id'; + $form_build_id = $this->randomMachineName(); + + $expected_form = $form_id(); + $expected_form['#build_id'] = $form_build_id; + $form_arg = $this->getMockForm($form_id, $expected_form); + $form_arg->expects($this->once()) + ->method('submitForm') + ->willReturnCallback(function (array &$form, FormStateInterface $form_state) { + // Mimic EntityForm by cleaning the $form_state upon submit. + $form_state->cleanValues(); + }); + + $this->formCache->expects($this->once()) + ->method('deleteCache') + ->with($form_build_id); + + $form_state = new FormState(); + $form_state->setCached(); + $this->simulateFormSubmission($form_id, $form_arg, $form_state); + } + + /** + * Tests that an uncached form does not trigger cache set or delete. + */ + public function testFormCacheDeletionUncached() { + $form_id = 'test_form_id'; + $form_build_id = $this->randomMachineName(); + + $expected_form = $form_id(); + $expected_form['#build_id'] = $form_build_id; + $form_arg = $this->getMockForm($form_id, $expected_form); + + $this->formCache->expects($this->never()) + ->method('deleteCache'); + + $form_state = new FormState(); + $this->simulateFormSubmission($form_id, $form_arg, $form_state); + } + } class TestForm implements FormInterface { diff --git a/core/tests/Drupal/Tests/Core/Form/FormCacheTest.php b/core/tests/Drupal/Tests/Core/Form/FormCacheTest.php index 4f145dc5ba9..1d9a67a1eea 100644 --- a/core/tests/Drupal/Tests/Core/Form/FormCacheTest.php +++ b/core/tests/Drupal/Tests/Core/Form/FormCacheTest.php @@ -478,6 +478,22 @@ class FormCacheTest extends UnitTestCase { $this->formCache->setCache($form_build_id, $form, $form_state); } + + /** + * @covers ::deleteCache + */ + public function testDeleteCache() { + $form_build_id = 'the_form_build_id'; + + $this->formCacheStore->expects($this->once()) + ->method('delete') + ->with($form_build_id); + $this->formStateCacheStore->expects($this->once()) + ->method('delete') + ->with($form_build_id); + $this->formCache->deleteCache($form_build_id); + } + /** * Ensures SafeMarkup does not bleed from one test to another. */ diff --git a/core/tests/Drupal/Tests/Core/Form/FormTestBase.php b/core/tests/Drupal/Tests/Core/Form/FormTestBase.php index 98fe0001a99..12b32609d39 100644 --- a/core/tests/Drupal/Tests/Core/Form/FormTestBase.php +++ b/core/tests/Drupal/Tests/Core/Form/FormTestBase.php @@ -10,6 +10,7 @@ namespace Drupal\Tests\Core\Form { use Drupal\Component\Utility\Html; use Drupal\Core\Form\FormBuilder; use Drupal\Core\Form\FormInterface; +use Drupal\Core\Form\FormState; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Session\AccountInterface; use Drupal\Tests\UnitTestCase; @@ -195,6 +196,7 @@ abstract class FormTestBase extends UnitTestCase { */ protected function tearDown() { Html::resetSeenIds(); + (new FormState())->clearErrors(); } /** @@ -285,6 +287,9 @@ abstract class FormTestBase extends UnitTestCase { * array. */ public function getInfo($type) { + $types['hidden'] = [ + '#input' => TRUE, + ]; $types['token'] = array( '#input' => TRUE, );