Issue #2473759 by tim.plunkett, Berdir, effulgentsia: Form caches should be...

Issue #2473759 by tim.plunkett, Berdir, effulgentsia: Form caches should be deleted after submission

core/lib/Drupal/Core/Form/FormBuilder.php

@@ -351,6 +351,13 @@ public function setCache($form_build_id, $form, FormStateInterface $form_state)
     $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 @@ public function processForm($form_id, &$form, FormStateInterface &$form_state) {
         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;
         }
       }

core/lib/Drupal/Core/Form/FormCache.php

@@ -216,4 +216,12 @@ public function setCache($form_build_id, $form, FormStateInterface $form_state)
     }
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function deleteCache($form_build_id) {
+    $this->keyValueExpirableFactory->get('form')->delete($form_build_id);
+    $this->keyValueExpirableFactory->get('form_state')->delete($form_build_id);
+  }
+
 }

core/lib/Drupal/Core/Form/FormCacheInterface.php

@@ -34,4 +34,12 @@ public function getCache($form_build_id, FormStateInterface $form_state);
    */
   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);
+
 }

core/modules/comment/src/Tests/CommentPreviewTest.php

@@ -64,6 +64,52 @@ function testCommentPreview() {
     $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.
    */

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 */

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(

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}
    */

core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php

@@ -383,6 +383,50 @@ public function testUniqueHtmlId() {
     $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 {

core/tests/Drupal/Tests/Core/Form/FormCacheTest.php

@@ -478,6 +478,22 @@ public function testSetCacheBuildIdMismatch() {
     $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.
    */

core/tests/Drupal/Tests/Core/Form/FormTestBase.php

@@ -10,6 +10,7 @@
 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 @@ protected function setUp() {
    */
   protected function tearDown() {
     Html::resetSeenIds();
+    (new FormState())->clearErrors();
   }
 
   /**
@@ -285,6 +287,9 @@ protected function assertFormElement(array $expected_form, array $actual_form, $
    *   array.
    */
   public function getInfo($type) {
+    $types['hidden'] = [
+      '#input' => TRUE,
+    ];
     $types['token'] = array(
       '#input' => TRUE,
     );