Commit d1c03228 authored by alexpott's avatar alexpott
Browse files

Issue #2252763 by damiankloip, skipyT, martin107, dawehner, dashaforbes: Views...

Issue #2252763 by damiankloip, skipyT, martin107, dawehner, dashaforbes: Views exposed filter form causes enormous form state cache entries
parent b5500b61
......@@ -62,18 +62,15 @@ public function testBulkForm() {
$this->assertTrue($node->isPublished(), 'Node has been published');
// Make sticky action.
$node->setPublished(FALSE);
$node->save();
$this->assertFalse($node->isSticky(), 'Node is not sticky');
$edit = array(
'node_bulk_form[0]' => TRUE,
'action' => 'node_make_sticky_action',
);
$this->drupalPostForm(NULL, $edit, t('Apply'));
// Re-load the node and check the status and sticky flag.
// Re-load the node and check the sticky flag.
$node_storage->resetCache(array($node->id()));
$node = $node_storage->load($node->id());
$this->assertTrue($node->isPublished(), 'Node has been published');
$this->assertTrue($node->isSticky(), 'Node has been made sticky');
// Make unsticky action.
......@@ -88,18 +85,15 @@ public function testBulkForm() {
$this->assertFalse($node->isSticky(), 'Node is not sticky anymore');
// Promote to front page.
$node->setPublished(FALSE);
$node->save();
$this->assertFalse($node->isPromoted(), 'Node is not promoted to the front page');
$edit = array(
'node_bulk_form[0]' => TRUE,
'action' => 'node_promote_action',
);
$this->drupalPostForm(NULL, $edit, t('Apply'));
// Re-load the node and check the status and promoted flag.
// Re-load the node and check the promoted flag.
$node_storage->resetCache(array($node->id()));
$node = $node_storage->load($node->id());
$this->assertTrue($node->isPublished(), 'Node has been published');
$this->assertTrue($node->isPromoted(), 'Node has been promoted to the front page');
// Demote from front page.
......
......@@ -433,4 +433,13 @@ public function isInstallable() {
return (bool) \Drupal::service('views.views_data')->get($this->base_table);
}
/**
* {@inheritdoc}
*/
public function __sleep() {
$keys = parent::__sleep();
unset($keys[array_search('executable', $keys)]);
return $keys;
}
}
......@@ -121,7 +121,7 @@ public function buildForm(array $form, FormStateInterface $form_state) {
$form['#id'] = Html::cleanCssIdentifier('views_exposed_form-' . String::checkPlain($view->storage->id()) . '-' . String::checkPlain($display['id']));
/** @var \Drupal\views\Plugin\views\exposed_form\ExposedFormPluginBase $exposed_form_plugin */
$exposed_form_plugin = $form_state->get('exposed_form_plugin');
$exposed_form_plugin = $view->display_handler->getPlugin('exposed_form');
$exposed_form_plugin->exposedFormAlter($form, $form_state);
// Save the form.
......@@ -134,15 +134,17 @@ public function buildForm(array $form, FormStateInterface $form_state) {
* {@inheritdoc}
*/
public function validateForm(array &$form, FormStateInterface $form_state) {
$view = $form_state->get('view');
foreach (array('field', 'filter') as $type) {
/** @var \Drupal\views\Plugin\views\ViewsHandlerInterface[] $handlers */
$handlers = &$form_state->get('view')->$type;
$handlers = &$view->$type;
foreach ($handlers as $key => $handler) {
$handlers[$key]->validateExposed($form, $form_state);
}
}
/** @var \Drupal\views\Plugin\views\exposed_form\ExposedFormPluginBase $exposed_form_plugin */
$exposed_form_plugin = $form_state->get('exposed_form_plugin');
$exposed_form_plugin = $view->display_handler->getPlugin('exposed_form');
$exposed_form_plugin->exposedFormValidate($form, $form_state);
}
......@@ -157,13 +159,14 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
$handlers[$key]->submitExposed($form, $form_state);
}
}
$view = $form_state->get('view');
$view->exposed_data = $form_state->getValues();
$view->exposed_raw_input = [];
$exclude = array('submit', 'form_build_id', 'form_id', 'form_token', 'exposed_form_plugin', '', 'reset');
$exclude = array('submit', 'form_build_id', 'form_id', 'form_token', 'exposed_form_plugin', 'reset');
/** @var \Drupal\views\Plugin\views\exposed_form\ExposedFormPluginBase $exposed_form_plugin */
$exposed_form_plugin = $form_state->get('exposed_form_plugin');
$exposed_form_plugin = $view->display_handler->getPlugin('exposed_form');
$exposed_form_plugin->exposedFormSubmit($form, $form_state, $exclude);
foreach ($form_state->getValues() as $key => $value) {
......
......@@ -150,7 +150,6 @@ public function renderExposedForm($block = FALSE) {
$form_state->set('ajax', TRUE);
}
$form_state->set('exposed_form_plugin', $this);
$form = \Drupal::formBuilder()->buildForm('\Drupal\views\Form\ViewsExposedForm', $form_state);
if (!$this->view->display_handler->displaysExposed() || (!$block && $this->view->display_handler->getOption('exposed_block'))) {
......
......@@ -54,6 +54,13 @@
*/
abstract class RelationshipPluginBase extends HandlerBase {
/**
* The relationship alias.
*
* @var string
*/
public $alias;
/**
* Overrides \Drupal\views\Plugin\views\HandlerBase::init().
*
......
......@@ -449,6 +449,7 @@ public function testValidateNestedLoops() {
$errors = $executable->validate();
$total_error_count = array_reduce($errors, function ($carry, $item) {
$carry += count($item);
return $carry;
});
// Assert that there were 9 total errors across 3 displays.
......@@ -456,4 +457,29 @@ public function testValidateNestedLoops() {
$this->assertIdentical(3, count($errors));
}
/**
* Tests serialization of the ViewExecutable object.
*/
public function testSerialization() {
$view = Views::getView('test_executable_displays');
$view->setDisplay('page_1');
$view->setArguments(['test']);
$view->setCurrentPage(2);
$serialized = serialize($view);
// Test the view storage object is not present in the actual serialized
// string.
$this->assertIdentical(strpos($serialized, '"Drupal\views\Entity\View"'), FALSE, 'The Drupal\views\Entity\View class was not found in the serialized string.');
/** @var \Drupal\views\ViewExecutable $unserialized */
$unserialized = unserialize($serialized);
$this->assertTrue($unserialized instanceof ViewExecutable);
$this->assertIdentical($view->storage->id(), $unserialized->storage->id(), 'The expected storage entity was loaded on the unserialized view.');
$this->assertIdentical($unserialized->current_display, 'page_1', 'The expected display was set on the unserialized view.');
$this->assertIdentical($unserialized->args, ['test'], 'The expected argument was set on the unserialized view.');
$this->assertIdentical($unserialized->getCurrentPage(), 2, 'The expected current page was set on the unserialized view.');
}
}
......@@ -26,7 +26,7 @@
* An object to contain all of the data to generate a view, plus the member
* functions to build the view query, execute the query and render the output.
*/
class ViewExecutable {
class ViewExecutable implements \Serializable {
use DependencySerializationTrait;
/**
......@@ -2295,4 +2295,49 @@ public function calculateDependencies() {
return $this->storage->calculateDependencies();
}
/**
* {@inheritdoc}
*/
public function serialize() {
return serialize([
// Only serialize the storage entity ID.
$this->storage->id(),
$this->current_display,
$this->args,
$this->current_page,
$this->exposed_input,
$this->exposed_raw_input,
$this->exposed_data,
$this->dom_id,
$this->executed,
]);
}
/**
* {@inheritdoc}
*/
public function unserialize($serialized) {
list($storage, $current_display, $args, $current_page, $exposed_input, $exposed_raw_input, $exposed_data, $dom_id, $executed) = unserialize($serialized);
$this->setRequest(\Drupal::request());
$this->user = \Drupal::currentUser();
$this->storage = \Drupal::entityManager()->getStorage('view')->load($storage);
$this->setDisplay($current_display);
$this->setArguments($args);
$this->setCurrentPage($current_page);
$this->setExposedInput($exposed_input);
$this->exposed_data = $exposed_data;
$this->exposed_raw_input = $exposed_raw_input;
$this->dom_id = $dom_id;
$this->initHandlers();
// If the display was previously executed, execute it now.
if ($executed) {
$this->execute($this->current_display);
}
}
}
......@@ -266,6 +266,7 @@ public function validate(array $form, FormStateInterface $form_state) {
public function save(array $form, FormStateInterface $form_state) {
$view = $this->entity;
$executable = $view->getExecutable();
$executable->initDisplay();
// Go through and remove displayed scheduled for removal.
$displays = $view->get('display');
......
......@@ -115,13 +115,6 @@ class ViewUI implements ViewEntityInterface {
*/
protected $storage;
/**
* The View executable object.
*
* @var \Drupal\views\ViewExecutable
*/
protected $executable;
/**
* Stores a list of database queries run beside the main one from views.
*
......@@ -170,13 +163,9 @@ class ViewUI implements ViewEntityInterface {
* @param \Drupal\views\ViewEntityInterface $storage
* The View storage object to wrap.
*/
public function __construct(ViewEntityInterface $storage, ViewExecutable $executable = NULL) {
public function __construct(ViewEntityInterface $storage) {
$this->entityType = 'view';
$this->storage = $storage;
if (!isset($executable)) {
$executable = Views::executableFactory()->get($this);
}
$this->executable = $executable;
}
/**
......@@ -259,7 +248,7 @@ public function standardSubmit($form, FormStateInterface $form_state) {
$display_id = $form_state->get('display_id');
if ($revert) {
// If it's revert just change the override and return.
$display = &$this->executable->displayHandlers->get($display_id);
$display = &$this->getExecutable()->displayHandlers->get($display_id);
$display->optionsOverride($form, $form_state);
// Don't execute the normal submit handling but still store the changed view into cache.
......@@ -273,7 +262,7 @@ public function standardSubmit($form, FormStateInterface $form_state) {
elseif ($was_defaulted && !$is_defaulted) {
// We were using the default display's values, but we're now overriding
// the default display and saving values specific to this display.
$display = &$this->executable->displayHandlers->get($display_id);
$display = &$this->getExecutable()->displayHandlers->get($display_id);
// optionsOverride toggles the override of this section.
$display->optionsOverride($form, $form_state);
$display->submitOptionsForm($form, $form_state);
......@@ -283,7 +272,7 @@ public function standardSubmit($form, FormStateInterface $form_state) {
// to go back to the default display.
// Overwrite the default display with the current form values, and make
// the current display use the new default values.
$display = &$this->executable->displayHandlers->get($display_id);
$display = &$this->getExecutable()->displayHandlers->get($display_id);
// optionsOverride toggles the override of this section.
$display->optionsOverride($form, $form_state);
$display->submitOptionsForm($form, $form_state);
......@@ -481,7 +470,7 @@ public function submitItemAdd($form, FormStateInterface $form_state) {
if ($was_defaulted && !$is_defaulted) {
// We were using the default display's values, but we're now overriding
// the default display and saving values specific to this display.
$display = &$this->executable->displayHandlers->get($display_id);
$display = &$this->getExecutable()->displayHandlers->get($display_id);
// setOverride toggles the override of this section.
$display->setOverride($section);
}
......@@ -490,7 +479,7 @@ public function submitItemAdd($form, FormStateInterface $form_state) {
// to go back to the default display.
// Overwrite the default display with the current form values, and make
// the current display use the new default values.
$display = &$this->executable->displayHandlers->get($display_id);
$display = &$this->getExecutable()->displayHandlers->get($display_id);
// optionsOverride toggles the override of this section.
$display->setOverride($section);
}
......@@ -503,7 +492,7 @@ public function submitItemAdd($form, FormStateInterface $form_state) {
if ($cut = strpos($field, '$')) {
$field = substr($field, 0, $cut);
}
$id = $this->executable->addHandler($display_id, $type, $table, $field);
$id = $this->getExecutable()->addHandler($display_id, $type, $table, $field);
// check to see if we have group by settings
$key = $type;
......@@ -516,7 +505,7 @@ public function submitItemAdd($form, FormStateInterface $form_state) {
'field' => $field,
);
$handler = Views::handlerManager($key)->getHandler($item);
if ($this->executable->displayHandlers->get('default')->useGroupBy() && $handler->usesGroupBy()) {
if ($this->getExecutable()->displayHandlers->get('default')->useGroupBy() && $handler->usesGroupBy()) {
$this->addFormToStack('handler-group', $display_id, $type, $id);
}
......@@ -564,6 +553,7 @@ public function renderPreview($display_id, $args = array()) {
// Save the current path so it can be restored before returning from this function.
$request_stack = \Drupal::requestStack();
$current_request = $request_stack->getCurrentRequest();
$executable = $this->getExecutable();
// Determine where the query and performance statistics should be output.
$config = \Drupal::config('views.settings');
......@@ -580,11 +570,11 @@ public function renderPreview($display_id, $args = array()) {
$rows = array('query' => array(), 'statistics' => array());
$errors = $this->executable->validate();
$this->executable->destroy();
$errors = $this->getExecutable()->validate();
$executable->destroy();
if (empty($errors)) {
$this->ajax = TRUE;
$this->executable->live_preview = TRUE;
$executable->live_preview = TRUE;
// AJAX happens via HTTP POST but everything expects exposed data to
// be in GET. Copy stuff but remove ajax-framework specific keys.
......@@ -597,19 +587,19 @@ public function renderPreview($display_id, $args = array()) {
unset($exposed_input[$key]);
}
}
$this->executable->setExposedInput($exposed_input);
$executable->setExposedInput($exposed_input);
if (!$this->executable->setDisplay($display_id)) {
if (!$executable->setDisplay($display_id)) {
return [
'#markup' => t('Invalid display id @display', array('@display' => $display_id)),
];
}
$this->executable->setArguments($args);
$executable->setArguments($args);
// Store the current view URL for later use:
if ($this->executable->display_handler->getOption('path')) {
$path = $this->executable->getUrl();
if ($executable->display_handler->getOption('path')) {
$path = $executable->getUrl();
}
// Make view links come back to preview.
......@@ -646,7 +636,7 @@ public function renderPreview($display_id, $args = array()) {
}
// Execute/get the view preview.
$preview = $this->executable->preview($display_id, $args);
$preview = $executable->preview($display_id, $args);
if ($show_additional_queries) {
$this->endQueryCapture();
......@@ -660,13 +650,13 @@ public function renderPreview($display_id, $args = array()) {
// below the view preview.
if ($show_info || $show_query || $show_stats) {
// Get information from the preview for display.
if (!empty($this->executable->build_info['query'])) {
if (!empty($executable->build_info['query'])) {
if ($show_query) {
$query_string = $this->executable->build_info['query'];
$query_string = $executable->build_info['query'];
// Only the sql default class has a method getArguments.
$quoted = array();
if ($this->executable->query instanceof Sql) {
if ($executable->query instanceof Sql) {
$quoted = $query_string->getArguments();
$connection = Database::getConnection();
foreach ($quoted as $key => $val) {
......@@ -722,7 +712,7 @@ public function renderPreview($display_id, $args = array()) {
'#template' => "<strong>{% trans 'Title' %}</strong>",
),
),
Xss::filterAdmin($this->executable->getTitle()),
Xss::filterAdmin($executable->getTitle()),
);
if (isset($path)) {
// @todo Views should expect and store a leading /. See:
......@@ -743,7 +733,7 @@ public function renderPreview($display_id, $args = array()) {
'#template' => "<strong>{% trans 'Query build time' %}</strong>",
),
),
t('@time ms', array('@time' => intval($this->executable->build_time * 100000) / 100)),
t('@time ms', array('@time' => intval($this->getExecutable()->build_time * 100000) / 100)),
);
$rows['statistics'][] = array(
......@@ -753,7 +743,7 @@ public function renderPreview($display_id, $args = array()) {
'#template' => "<strong>{% trans 'Query execute time' %}</strong>",
),
),
t('@time ms', array('@time' => intval($this->executable->execute_time * 100000) / 100)),
t('@time ms', array('@time' => intval($this->getExecutable()->execute_time * 100000) / 100)),
);
$rows['statistics'][] = array(
......@@ -763,10 +753,10 @@ public function renderPreview($display_id, $args = array()) {
'#template' => "<strong>{% trans 'View render time' %}</strong>",
),
),
t('@time ms', array('@time' => intval($this->executable->render_time * 100000) / 100)),
t('@time ms', array('@time' => intval($executable->render_time * 100000) / 100)),
);
}
\Drupal::moduleHandler()->alter('views_preview_info', $rows, $this->executable);
\Drupal::moduleHandler()->alter('views_preview_info', $rows, $executable);
}
else {
// No query was run. Display that information in place of either the
......@@ -873,14 +863,14 @@ public function cacheSet() {
if (isset($executable->current_display)) {
// Add the knowledge of the changed display, too.
$this->changed_display[$executable->current_display] = TRUE;
unset($executable->current_display);
$executable->current_display = NULL;
}
// Unset handlers; we don't want to write these into the cache.
unset($executable->display_handler);
unset($executable->default_display);
// Unset handlers. We don't want to write these into the cache.
$executable->display_handler = NULL;
$executable->default_display = NULL;
$executable->query = NULL;
unset($executable->displayHandlers);
$executable->displayHandlers = NULL;
\Drupal::service('user.shared_tempstore')->get('views')->set($this->id(), $this);
}
......@@ -1239,13 +1229,6 @@ public function addDisplay($plugin_id = 'page', $title = NULL, $id = NULL) {
return $this->storage->addDisplay($plugin_id, $title, $id);
}
/**
* {@inheritdoc}
*/
public function getViewExecutable() {
return $this->storage->getViewExecutable();
}
/**
* {@inheritdoc}
*/
......@@ -1287,4 +1270,5 @@ public function unsetThirdPartySetting($module, $key) {
public function getThirdPartyProviders() {
return $this->storage->getThirdPartyProviders();
}
}
......@@ -9,6 +9,7 @@
use Drupal\Core\Language\LanguageInterface;
use Drupal\Tests\UnitTestCase;
use Drupal\views\Entity\View;
use Drupal\views\ViewExecutable;
use Drupal\views_ui\ViewUI;
use Symfony\Component\DependencyInjection\Container;
......@@ -54,8 +55,9 @@ public function testEntityDecoration() {
->disableOriginalConstructor()
->setConstructorArgs(array($storage))
->getMock();
$storage->set('executable', $executable);
$view_ui = new ViewUI($storage, $executable);
$view_ui = new ViewUI($storage);
foreach ($method_args as $method => $args) {
$method_mock = $storage->expects($this->once())
......@@ -80,6 +82,7 @@ public function testIsLocked() {
->disableOriginalConstructor()
->setConstructorArgs(array($storage))
->getMock();
$storage->set('executable', $executable);
$account = $this->getMock('Drupal\Core\Session\AccountInterface');
$account->expects($this->exactly(2))
->method('id')
......@@ -89,7 +92,7 @@ public function testIsLocked() {
$container->set('current_user', $account);
\Drupal::setContainer($container);
$view_ui = new ViewUI($storage, $executable);
$view_ui = new ViewUI($storage);
// A view_ui without a lock object is not locked.
$this->assertFalse($view_ui->isLocked());
......@@ -113,4 +116,33 @@ public function testIsLocked() {
$this->assertFalse($view_ui->isLocked());
}
/**
* Tests serialization of the ViewUI object.
*/
public function testSerialization() {
// Set a container so the DependencySerializationTrait has it.
$container = new ContainerBuilder();
\Drupal::setContainer($container);
$storage = new View([], 'view');
$executable = $this->getMockBuilder('Drupal\views\ViewExecutable')
->disableOriginalConstructor()
->setConstructorArgs([$storage])
->getMock();
$storage->set('executable', $executable);
$view_ui = new ViewUI($storage);
// Make sure the executable is returned before serializing.
$this->assertInstanceOf('Drupal\views\ViewExecutable', $view_ui->getExecutable());
$serialized = serialize($view_ui);
// Make sure the ViewExecutable class is not found in the serialized string.
$this->assertSame(strpos($serialized, '"Drupal\views\ViewExecutable"'), FALSE);
$unserialized = unserialize($serialized);
$this->assertInstanceOf('Drupal\views_ui\ViewUI', $unserialized);
}
}
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment