diff --git a/core/core.services.yml b/core/core.services.yml index f00c450..51a8a11 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -45,6 +45,13 @@ services: factory_method: get factory_service: cache_factory arguments: [field] + cache.form: + class: Drupal\Core\Cache\CacheBackendInterface + tags: + - { name: cache.bin } + factory_method: get + factory_service: cache_factory + arguments: [form] cache.menu: class: Drupal\Core\Cache\CacheBackendInterface tags: @@ -108,7 +115,7 @@ services: arguments: [default] form_builder: class: Drupal\Core\Form\FormBuilder - arguments: ['@module_handler', '@keyvalue.expirable', '@event_dispatcher', '@url_generator', '@string_translation', '@?csrf_token', '@?http_kernel'] + arguments: ['@module_handler', '@keyvalue.expirable', '@cache.form', '@event_dispatcher', '@url_generator', '@string_translation', '@?csrf_token', '@?http_kernel'] calls: - [setRequest, ['@?request']] keyvalue: diff --git a/core/includes/install.core.inc b/core/includes/install.core.inc index 24d3269..7e9d928 100644 --- a/core/includes/install.core.inc +++ b/core/includes/install.core.inc @@ -397,7 +397,7 @@ function install_begin_request(&$install_state) { // Register the translation services. install_register_translation_service($container); - foreach (array('bootstrap', 'config', 'cache', 'menu', 'page', 'path') as $bin) { + foreach (array('bootstrap', 'config', 'cache', 'form', 'menu', 'page', 'path') as $bin) { $container ->register("cache.$bin", 'Drupal\Core\Cache\MemoryBackend') ->addArgument($bin); @@ -453,6 +453,7 @@ function install_begin_request(&$install_state) { $container->register('form_builder', 'Drupal\Core\Form\FormBuilder') ->addArgument(new Reference('module_handler')) ->addArgument(new Reference('keyvalue.expirable')) + ->addArgument(new Reference('cache.form')) ->addArgument(new Reference('event_dispatcher')) ->addArgument(new Reference('url_generator')) ->addArgument(new Reference('string_translation')) diff --git a/core/lib/Drupal/Core/Form/FormBuilder.php b/core/lib/Drupal/Core/Form/FormBuilder.php index 43d15a6..4af2f9e 100644 --- a/core/lib/Drupal/Core/Form/FormBuilder.php +++ b/core/lib/Drupal/Core/Form/FormBuilder.php @@ -12,6 +12,7 @@ use Drupal\Component\Utility\Unicode; use Drupal\Component\Utility\Url; use Drupal\Core\Access\CsrfTokenGenerator; +use Drupal\Core\Cache\CacheBackendInterface; use Drupal\Core\Extension\ModuleHandlerInterface; use Drupal\Core\HttpKernel; use Drupal\Core\KeyValueStore\KeyValueExpirableFactoryInterface; @@ -46,6 +47,13 @@ class FormBuilder implements FormBuilderInterface { protected $keyValueExpirableFactory; /** + * The cache backend for the form cache. + * + * @var \Drupal\Core\Cache\CacheBackendInterface + */ + protected $formCache; + + /** * The event dispatcher. * * @var \Symfony\Component\EventDispatcher\EventDispatcherInterface @@ -125,6 +133,8 @@ class FormBuilder implements FormBuilderInterface { * The module handler. * @param \Drupal\Core\KeyValueStore\KeyValueExpirableFactoryInterface $key_value_expirable_factory * The keyvalue expirable factory. + * @param \Drupal\Core\Cache\CacheBackendInterface $form_cache + * The cache backend for the form cache. * @param \Symfony\Component\EventDispatcher\EventDispatcherInterface $event_dispatcher * The event dispatcher. * @param \Drupal\Core\Routing\UrlGeneratorInterface $url_generator @@ -136,9 +146,10 @@ class FormBuilder implements FormBuilderInterface { * @param \Drupal\Core\HttpKernel $http_kernel * The HTTP kernel. */ - public function __construct(ModuleHandlerInterface $module_handler, KeyValueExpirableFactoryInterface $key_value_expirable_factory, EventDispatcherInterface $event_dispatcher, UrlGeneratorInterface $url_generator, TranslationInterface $translation_manager, CsrfTokenGenerator $csrf_token = NULL, HttpKernel $http_kernel = NULL) { + public function __construct(ModuleHandlerInterface $module_handler, KeyValueExpirableFactoryInterface $key_value_expirable_factory, CacheBackendInterface $form_cache, EventDispatcherInterface $event_dispatcher, UrlGeneratorInterface $url_generator, TranslationInterface $translation_manager, CsrfTokenGenerator $csrf_token = NULL, HttpKernel $http_kernel = NULL) { $this->moduleHandler = $module_handler; $this->keyValueExpirableFactory = $key_value_expirable_factory; + $this->formCache = $form_cache; $this->eventDispatcher = $event_dispatcher; $this->urlGenerator = $url_generator; $this->translationManager = $translation_manager; @@ -361,27 +372,28 @@ public function rebuildForm($form_id, &$form_state, $old_form = NULL) { * {@inheritdoc} */ public function getCache($form_build_id, &$form_state) { - if ($form = $this->keyValueExpirableFactory->get('form')->get($form_build_id)) { + $stored_form_state = $this->keyValueExpirableFactory->get('form_state')->get($form_build_id); + if (isset($stored_form_state)) { $user = $this->currentUser(); - if ((isset($form['#cache_token']) && $this->csrfToken->validate($form['#cache_token'])) || (!isset($form['#cache_token']) && $user->isAnonymous())) { - if ($stored_form_state = $this->keyValueExpirableFactory->get('form_state')->get($form_build_id)) { - // Re-populate $form_state for subsequent rebuilds. - $form_state = $stored_form_state + $form_state; - - // If the original form is contained in include files, load the files. - // @see form_load_include() - $form_state['build_info'] += array('files' => array()); - foreach ($form_state['build_info']['files'] as $file) { - if (is_array($file)) { - $file += array('type' => 'inc', 'name' => $file['module']); - $this->moduleHandler->loadInclude($file['module'], $file['type'], $file['name']); - } - elseif (file_exists($file)) { - require_once DRUPAL_ROOT . '/' . $file; - } + if ((isset($stored_form_state['csrf_token']) && $this->csrfToken->validate($stored_form_state['csrf_token'])) || (!isset($stored_form_state['csrf_token']) && $user->isAnonymous())) { + $form_state = $stored_form_state + $form_state; + + // If the original form is contained in include files, load the files. + // @see form_load_include() + $form_state['build_info'] += array('files' => array()); + foreach ($form_state['build_info']['files'] as $file) { + if (is_array($file)) { + $file += array('type' => 'inc', 'name' => $file['module']); + $this->moduleHandler->loadInclude($file['module'], $file['type'], $file['name']); } + elseif (file_exists($file)) { + require_once DRUPAL_ROOT . '/' . $file; + } + } + + if ($form = $this->formCache->get($form_build_id)) { + return $form->data; } - return $form; } } } @@ -394,17 +406,14 @@ public function setCache($form_build_id, $form, $form_state) { $expire = 21600; // Cache form structure. - if (isset($form)) { - if ($this->currentUser()->isAuthenticated()) { - $form['#cache_token'] = $this->csrfToken->get(); - } - $this->keyValueExpirableFactory->get('form')->setWithExpire($form_build_id, $form, $expire); - } + $this->formCache->set($form_build_id, $form, REQUEST_TIME + $expire); // Cache form state. - if ($data = array_diff_key($form_state, array_flip($this->getUncacheableKeys()))) { - $this->keyValueExpirableFactory->get('form_state')->setWithExpire($form_build_id, $data, $expire); + $data = array_diff_key($form_state, array_flip($this->getUncacheableKeys())); + if ($this->currentUser()->isAuthenticated()) { + $data['csrf_token'] = $this->csrfToken->get(); } + $this->keyValueExpirableFactory->get('form_state')->setWithExpire($form_build_id, $data, $expire); } /** diff --git a/core/modules/system/lib/Drupal/system/Tests/Form/FormCacheTest.php b/core/modules/system/lib/Drupal/system/Tests/Form/FormCacheTest.php index 0e6d629..66e093b 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Form/FormCacheTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Form/FormCacheTest.php @@ -49,26 +49,26 @@ function testCacheToken() { $this->container->set('current_user', new UserSession(array('uid' => 1))); form_set_cache($this->form_build_id, $this->form, $this->form_state); - $cached_form_state = form_state_defaults(); - $cached_form = form_get_cache($this->form_build_id, $cached_form_state); - $this->assertEqual($this->form['#property'], $cached_form['#property']); - $this->assertTrue(!empty($cached_form['#cache_token']), 'Form has a cache token'); - $this->assertEqual($this->form_state['example'], $cached_form_state['example']); + $form_state = form_state_defaults(); + $form = form_get_cache($this->form_build_id, $form_state); + $this->assertEqual($this->form['#property'], $form['#property']); + $this->assertTrue(!empty($form_state['csrf_token']), 'Form state has a CSRF token for authenticated users'); + $this->assertEqual($this->form_state['example'], $form_state['example']); // Test that the form cache isn't loaded when the session/token has changed. // Change the private key. (We cannot change the session ID because this // will break the parent site test runner batch.) \Drupal::state()->set('system.private_key', 'invalid'); - $cached_form_state = form_state_defaults(); - $cached_form = form_get_cache($this->form_build_id, $cached_form_state); - $this->assertFalse($cached_form, 'No form returned from cache'); - $this->assertTrue(empty($cached_form_state['example'])); + $form_state = form_state_defaults(); + $form = form_get_cache($this->form_build_id, $form_state); + $this->assertFalse($form, 'No form returned from cache'); + $this->assertTrue(empty($form_state['example'])); // Test that loading the cache with a different form_id fails. $wrong_form_build_id = $this->randomName(9); - $cached_form_state = form_state_defaults(); - $this->assertFalse(form_get_cache($wrong_form_build_id, $cached_form_state), 'No form returned from cache'); - $this->assertTrue(empty($cached_form_state['example']), 'Cached form state was not loaded'); + $form_state = form_state_defaults(); + $this->assertFalse(form_get_cache($wrong_form_build_id, $form_state), 'No form returned from cache'); + $this->assertTrue(empty($form_state['example']), 'Cached form state was not loaded'); } /** @@ -80,11 +80,11 @@ function testNoCacheToken() { $this->form_state['example'] = $this->randomName(); form_set_cache($this->form_build_id, $this->form, $this->form_state); - $cached_form_state = form_state_defaults(); - $cached_form = form_get_cache($this->form_build_id, $cached_form_state); - $this->assertEqual($this->form['#property'], $cached_form['#property']); - $this->assertTrue(empty($cached_form['#cache_token']), 'Form has no cache token'); - $this->assertEqual($this->form_state['example'], $cached_form_state['example']); + $form_state = form_state_defaults(); + $form = form_get_cache($this->form_build_id, $form_state); + $this->assertEqual($this->form['#property'], $form['#property']); + $this->assertTrue(empty($form_state['csrf_token']), 'Form state has no CSRF token for anonymous users'); + $this->assertEqual($this->form_state['example'], $form_state['example']); } } diff --git a/core/modules/system/lib/Drupal/system/Tests/Form/StorageTest.php b/core/modules/system/lib/Drupal/system/Tests/Form/StorageTest.php index 766878e..1bec49e 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Form/StorageTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Form/StorageTest.php @@ -93,6 +93,37 @@ function testFormCached() { } /** + * Tests using the form with clearing the cache during the multistep sequence. + * + * $form_state storage in the keyvalue store should continue to work + * regardless of the $form cache. + */ + function testFormCacheCleared() { + $this->drupalGet('form_test/form-storage', array('query' => array('cache' => 1))); + $this->assertText('Form constructions: 1'); + $this->container->get('cache.form')->deleteAll(); + + // Use form rebuilding triggered by a submit button. + $edit = array('title' => 'new', 'value' => 'value_is_set'); + $this->drupalPostForm(NULL, $edit, 'Continue submit'); + $this->assertText('Form constructions: 2'); + $this->assertText('Form constructions: 3'); + $this->container->get('cache.form')->deleteAll(); + + // Reset the form to the values of the storage, using a form rebuild + // triggered by button of type button. + $this->drupalPostForm(NULL, array('title' => 'changed'), 'Reset'); + $this->assertFieldByName('title', 'new', 'Values have been resetted.'); + $this->assertText('Form constructions: 4'); + $this->assertText('Form constructions: 5'); + $this->container->get('cache.form')->deleteAll(); + + $this->drupalPostForm(NULL, $edit, 'Save'); + $this->assertText('Form constructions: 6'); + $this->assertText('Title: new', 'The form storage has stored the values.'); + } + + /** * Tests validation when form storage is used. */ function testValidation() { diff --git a/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php b/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php index 0b52a75..73ed945 100644 --- a/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php +++ b/core/tests/Drupal/Tests/Core/Form/FormBuilderTest.php @@ -603,17 +603,18 @@ public function testGetCache() { $form = $this->formBuilder->buildForm($form_arg, $form_state); $cached_form = $form; - $cached_form['#cache_token'] = 'csrf_token'; - // The form cache, form_state cache, and CSRF token validation will only be - // called on the cached form. + $stored_form_state = $form_state; + $stored_form_state['csrf_token'] = 'csrf_token'; + // The form cache, form_state keyvalue, and CSRF token validation will only + // be called on the cached form. $this->formCache->expects($this->once()) - ->method('setWithExpire'); + ->method('set'); $this->formCache->expects($this->once()) ->method('get') - ->will($this->returnValue($cached_form)); - $this->formStateCache->expects($this->once()) + ->will($this->returnValue((object) array('data' => $cached_form))); + $this->formStateKeyValue->expects($this->once()) ->method('get') - ->will($this->returnValue($form_state)); + ->will($this->returnValue($stored_form_state)); // The final form build will not trigger any actual form building, but will // use the form cache. diff --git a/core/tests/Drupal/Tests/Core/Form/FormTestBase.php b/core/tests/Drupal/Tests/Core/Form/FormTestBase.php index 3b7a97f..cb36f2c 100644 --- a/core/tests/Drupal/Tests/Core/Form/FormTestBase.php +++ b/core/tests/Drupal/Tests/Core/Form/FormTestBase.php @@ -43,18 +43,18 @@ protected $moduleHandler; /** - * The expirable key value store used by form cache. + * The cache backend for the form cache. * - * @var \PHPUnit_Framework_MockObject_MockObject|\Drupal\Core\KeyValueStore\KeyValueStoreExpirableInterface + * @var \PHPUnit_Framework_MockObject_MockObject|\Drupal\Core\Cache\CacheBackendInterface */ protected $formCache; /** - * The expirable key value store used by form state cache. + * The expirable key value store used for persisting form state. * * @var \PHPUnit_Framework_MockObject_MockObject|\Drupal\Core\KeyValueStore\KeyValueStoreExpirableInterface */ - protected $formStateCache; + protected $formStateKeyValue; /** * The current user. @@ -104,16 +104,15 @@ public function setUp() { $this->moduleHandler = $this->getMock('Drupal\Core\Extension\ModuleHandlerInterface'); - $this->formCache = $this->getMock('Drupal\Core\KeyValueStore\KeyValueStoreExpirableInterface'); - $this->formStateCache = $this->getMock('Drupal\Core\KeyValueStore\KeyValueStoreExpirableInterface'); + $this->formCache = $this->getMock('Drupal\Core\Cache\CacheBackendInterface'); + $this->formStateKeyValue = $this->getMock('Drupal\Core\KeyValueStore\KeyValueStoreExpirableInterface'); $this->keyValueExpirableFactory = $this->getMockBuilder('Drupal\Core\KeyValueStore\KeyValueExpirableFactory') ->disableOriginalConstructor() ->getMock(); $this->keyValueExpirableFactory->expects($this->any()) ->method('get') ->will($this->returnValueMap(array( - array('form', $this->formCache), - array('form_state', $this->formStateCache), + array('form_state', $this->formStateKeyValue), ))); $this->eventDispatcher = $this->getMock('Symfony\Component\EventDispatcher\EventDispatcherInterface'); @@ -142,7 +141,7 @@ protected function tearDown() { * Sets up a new form builder object to test. */ protected function setupFormBuilder() { - $this->formBuilder = new TestFormBuilder($this->moduleHandler, $this->keyValueExpirableFactory, $this->eventDispatcher, $this->urlGenerator, $this->translationManager, $this->csrfToken, $this->httpKernel); + $this->formBuilder = new TestFormBuilder($this->moduleHandler, $this->keyValueExpirableFactory, $this->formCache, $this->eventDispatcher, $this->urlGenerator, $this->translationManager, $this->csrfToken, $this->httpKernel); $this->formBuilder->setRequest($this->request); $this->formBuilder->setCurrentUser($this->account); }