Drupal
/
drupal
mirror of https://git.drupalcode.org/project/drupal.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Issue #3052492 by amateescu, plach, alexpott: ViewsEntitySchemaSubscriber should not make an entity update fail if a view cannot be resaved

merge-requests/1119/head
Alex Pott 2019-05-06 18:14:25 +01:00
parent 041895feda
commit d9e8e19c29
No known key found for this signature in database
GPG Key ID: 31905460D4A69276
5 changed files with 187 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
core/modules/system/tests/modules/entity_test_update/entity_test_update.module
View File

@ -8,6 +8,7 @@
use Drupal\Core\Field\BaseFieldDefinition;
use Drupal\Core\StringTranslation\TranslatableMarkup;
use Drupal\entity_test_update\Entity\EntityTestUpdate;
use Drupal\Core\Entity\EntityInterface;
use Drupal\Core\Entity\EntityTypeInterface;
/**
@ -45,6 +46,15 @@ function entity_test_update_entity_type_alter(array &$entity_types) {
$entity_types['entity_test_update'] = \Drupal::state()->get('entity_test_update.entity_type', $entity_types['entity_test_update']);
}
/**
* Implements hook_ENTITY_TYPE_presave() for the 'view' entity type.
*/
function entity_test_update_view_presave(EntityInterface $entity) {
if (\Drupal::state()->get('entity_test_update.throw_view_exception') === $entity->id()) {
throw new \LogicException('The view could not be saved.');
}
}
/**
* Creates a given number of 'entity_test_update' entities.
*

63
core/modules/views/src/EventSubscriber/ViewsEntitySchemaSubscriber.php
View File

@ -8,7 +8,9 @@ use Drupal\Core\Entity\EntityTypeInterface;
use Drupal\Core\Entity\EntityTypeListenerInterface;
use Drupal\Core\Entity\EntityTypeManagerInterface;
use Drupal\Core\Entity\Sql\SqlContentEntityStorage;
use Drupal\views\ViewEntityInterface;
use Drupal\views\Views;
use Psr\Log\LoggerInterface;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
/**
@ -81,14 +83,28 @@ class ViewsEntitySchemaSubscriber implements EntityTypeListenerInterface, EventS
*/
protected $entityTypeManager;
/**
* The default logger service.
*
* @var \Psr\Log\LoggerInterface
*/
protected $logger;
/**
* Constructs a ViewsEntitySchemaSubscriber.
*
* @param \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager
* The entity type manager.
* @param \Psr\Log\LoggerInterface $logger
* A logger instance.
*/
public function __construct(EntityTypeManagerInterface $entity_type_manager) {
public function __construct(EntityTypeManagerInterface $entity_type_manager, LoggerInterface $logger = NULL) {
$this->entityTypeManager = $entity_type_manager;
if (!$logger) {
@trigger_error('Calling ViewsEntitySchemaSubscriber::__construct() with the $logger argument is supported in drupal:8.7.1 and will be required before drupal:9.0.0. See https://www.drupal.org/project/drupal/issues/3052492.', E_USER_DEPRECATED);
$logger = \Drupal::service('logger.channel.default');
}
$this->logger = $logger;
}
/**
@ -194,12 +210,28 @@ class ViewsEntitySchemaSubscriber implements EntityTypeListenerInterface, EventS
}
}
foreach ($all_views as $view) {
// All changes done to the views here can be trusted and this might be
// called during updates, when it is not safe to rely on configuration
// containing valid schema. Trust the data and disable schema validation
// and casting.
$view->trustData()->save();
// Filter the list of views that needs to be updated.
$views_to_update = array_filter($all_views, function (ViewEntityInterface $view) {
return $view->get('_updated') === TRUE;
});
foreach ($views_to_update as $view) {
try {
// All changes done to the views here can be trusted and this might be
// called during updates, when it is not safe to rely on configuration
// containing valid schema. Trust the data and disable schema validation
// and casting.
$view->set('_updated', NULL);
$view->trustData()->save();
}
catch (\Exception $e) {
// In case the view could not be saved, log an error message that the
// view needs to be updated manually instead of failing the entire
// entity update process.
$this->logger->critical("The %view_id view could not be updated automatically while processing an entity schema update for the %entity_type_id entity type.", [
'%view_id' => $view->id(),
'%entity_type_id' => $entity_type->id(),
]);
}
}
}
@ -244,7 +276,7 @@ class ViewsEntitySchemaSubscriber implements EntityTypeListenerInterface, EventS
continue;
}
foreach ($display['display_options'][$handler_type] as $id => &$handler_config) {
$process($handler_config);
$process($handler_config, $view);
if ($handler_config === NULL) {
unset($display['display_options'][$handler_type][$id]);
}
@ -270,12 +302,14 @@ class ViewsEntitySchemaSubscriber implements EntityTypeListenerInterface, EventS
foreach ($all_views as $view) {
if ($view->get('base_table') == $old_base_table) {
$view->set('base_table', $new_base_table);
$view->set('_updated', TRUE);
}
}
$this->processHandlers($all_views, function (array &$handler_config) use ($entity_type_id, $old_base_table, $new_base_table) {
$this->processHandlers($all_views, function (array &$handler_config, ViewEntityInterface $view) use ($entity_type_id, $old_base_table, $new_base_table) {
if (isset($handler_config['entity_type']) && $handler_config['entity_type'] == $entity_type_id && $handler_config['table'] == $old_base_table) {
$handler_config['table'] = $new_base_table;
$view->set('_updated', TRUE);
}
});
}
@ -296,12 +330,14 @@ class ViewsEntitySchemaSubscriber implements EntityTypeListenerInterface, EventS
foreach ($all_views as $view) {
if ($view->get('base_table') == $old_data_table) {
$view->set('base_table', $new_data_table);
$view->set('_updated', TRUE);
}
}
$this->processHandlers($all_views, function (array &$handler_config) use ($entity_type_id, $old_data_table, $new_data_table) {
$this->processHandlers($all_views, function (array &$handler_config, ViewEntityInterface $view) use ($entity_type_id, $old_data_table, $new_data_table) {
if (isset($handler_config['entity_type']) && $handler_config['entity_type'] == $entity_type_id && $handler_config['table'] == $old_data_table) {
$handler_config['table'] = $new_data_table;
$view->set('_updated', TRUE);
}
});
}
@ -329,11 +365,12 @@ class ViewsEntitySchemaSubscriber implements EntityTypeListenerInterface, EventS
$data_table = $new_data_table;
$this->processHandlers($all_views, function (array &$handler_config) use ($entity_type_id, $base_table, $data_table, $base_table_fields, $data_table_fields) {
$this->processHandlers($all_views, function (array &$handler_config, ViewEntityInterface $view) use ($entity_type_id, $base_table, $data_table, $base_table_fields, $data_table_fields) {
if (isset($handler_config['entity_type']) && isset($handler_config['entity_field']) && $handler_config['entity_type'] == $entity_type_id) {
// Move all fields which just exists on the data table.
if ($handler_config['table'] == $base_table && in_array($handler_config['entity_field'], $data_table_fields) && !in_array($handler_config['entity_field'], $base_table_fields)) {
$handler_config['table'] = $data_table;
$view->set('_updated', TRUE);
}
}
});
@ -353,10 +390,11 @@ class ViewsEntitySchemaSubscriber implements EntityTypeListenerInterface, EventS
*/
protected function dataTableRemoval($all_views, $entity_type_id, $old_data_table, $base_table) {
// We move back the data table back to the base table.
$this->processHandlers($all_views, function (array &$handler_config) use ($entity_type_id, $old_data_table, $base_table) {
$this->processHandlers($all_views, function (array &$handler_config, ViewEntityInterface $view) use ($entity_type_id, $old_data_table, $base_table) {
if (isset($handler_config['entity_type']) && $handler_config['entity_type'] == $entity_type_id) {
if ($handler_config['table'] == $old_data_table) {
$handler_config['table'] = $base_table;
$view->set('_updated', TRUE);
}
}
});
@ -378,6 +416,7 @@ class ViewsEntitySchemaSubscriber implements EntityTypeListenerInterface, EventS
if (in_array($view->get('base_table'), [$revision_base_table, $revision_data_table])) {
// Let's disable the views as we no longer support revisions.
$view->setStatus(FALSE);
$view->set('_updated', TRUE);
}
// For any kind of field, let's rely on the broken handler functionality.

11
core/modules/views/tests/modules/views_test_data/views_test_data.module
View File

@ -6,6 +6,7 @@
*/
use Drupal\Core\Form\FormStateInterface;
use Drupal\views\ViewEntityInterface;
/**
* Access callback for the generic handler test.
@ -131,3 +132,13 @@ function views_test_data_test_pre_render_function($element) {
function views_test_data_form_views_form_test_form_multiple_default_alter(&$form, FormStateInterface $form_state, $form_id) {
\Drupal::messenger()->addStatus(t('Test base form ID with Views forms and arguments.'));
}
/**
* Implements hook_ENTITY_TYPE_update() for the 'view' entity type.
*/
function views_test_data_view_update(ViewEntityInterface $view) {
// Use state to keep track of how many times a file is saved.
$view_save_count = \Drupal::state()->get('views_test_data.view_save_count', []);
$view_save_count[$view->id()] = isset($view_save_count[$view->id()]) ? $view_save_count[$view->id()] + 1 : 1;
\Drupal::state()->set('views_test_data.view_save_count', $view_save_count);
}

114
core/modules/views/tests/src/Kernel/EventSubscriber/ViewsEntitySchemaSubscriberIntegrationTest.php
View File

@ -138,6 +138,13 @@ class ViewsEntitySchemaSubscriberIntegrationTest extends ViewsKernelTestBase {
$display = $view->getDisplay('default');
$this->assertEqual('entity_test_update_new', $display['display_options']['fields']['id']['table']);
$this->assertEqual('entity_test_update_new', $display['display_options']['fields']['name']['table']);
// Check that only the impacted views have been updated.
$this->assertUpdatedViews([
'test_view_entity_test',
'test_view_entity_test_data',
'test_view_entity_test_additional_base_field',
]);
}
/**
@ -166,6 +173,12 @@ class ViewsEntitySchemaSubscriberIntegrationTest extends ViewsKernelTestBase {
$display = $view->getDisplay('default');
$this->assertEqual('entity_test_update', $display['display_options']['fields']['id']['table']);
$this->assertEqual('entity_test_update_data_new', $display['display_options']['fields']['name']['table']);
// Check that only the impacted views have been updated.
$this->assertUpdatedViews([
'test_view_entity_test',
'test_view_entity_test_data',
]);
}
/**
@ -194,6 +207,11 @@ class ViewsEntitySchemaSubscriberIntegrationTest extends ViewsKernelTestBase {
$display = $view->getDisplay('default');
$this->assertEqual('entity_test_update_revision_new', $display['display_options']['fields']['id']['table']);
$this->assertEqual('entity_test_update_revision_new', $display['display_options']['fields']['name']['table']);
// Check that only the impacted views have been updated.
$this->assertUpdatedViews([
'test_view_entity_test_revision',
]);
}
/**
@ -222,6 +240,12 @@ class ViewsEntitySchemaSubscriberIntegrationTest extends ViewsKernelTestBase {
$display = $view->getDisplay('default');
$this->assertEqual('entity_test_update_revision', $display['display_options']['fields']['id']['table']);
$this->assertEqual('entity_test_update_revision_data_new', $display['display_options']['fields']['name']['table']);
// Check that only the impacted views have been updated.
$this->assertUpdatedViews([
'test_view_entity_test',
'test_view_entity_test_revision',
]);
}
/**
@ -239,6 +263,11 @@ class ViewsEntitySchemaSubscriberIntegrationTest extends ViewsKernelTestBase {
$display = $view->getDisplay('default');
$this->assertEqual('entity_test_update', $display['display_options']['fields']['id']['table']);
$this->assertEqual('entity_test_update_data', $display['display_options']['fields']['name']['table']);
// Check that only the impacted views have been updated.
$this->assertUpdatedViews([
'test_view_entity_test',
]);
}
/**
@ -256,6 +285,9 @@ class ViewsEntitySchemaSubscriberIntegrationTest extends ViewsKernelTestBase {
$display = $view->getDisplay('default');
$this->assertEqual('entity_test_update', $display['display_options']['fields']['id']['table']);
$this->assertEqual('entity_test_update', $display['display_options']['fields']['name']['table']);
// Check that only the impacted views have been updated.
$this->assertUpdatedViews([]);
}
/**
@ -270,6 +302,11 @@ class ViewsEntitySchemaSubscriberIntegrationTest extends ViewsKernelTestBase {
$view = $entity_storage->load('test_view_entity_test_revision');
$this->assertFalse($view->status());
// Check that only the impacted views have been updated.
$this->assertUpdatedViews([
'test_view_entity_test_revision',
]);
}
/**
@ -381,6 +418,13 @@ class ViewsEntitySchemaSubscriberIntegrationTest extends ViewsKernelTestBase {
$this->assertEqual('entity_test_update', $view->get('base_table'));
$this->assertEqual('entity_test_update', $display['display_options']['fields']['id']['table']);
$this->assertEqual('entity_test_update', $display['display_options']['fields']['name']['table']);
// Check that only the impacted views have been updated.
$this->assertUpdatedViews([
'test_view_entity_test',
'test_view_entity_test_data',
'test_view_entity_test_revision',
]);
}
/**
@ -409,6 +453,52 @@ class ViewsEntitySchemaSubscriberIntegrationTest extends ViewsKernelTestBase {
$this->assertEqual('entity_test_update_revision', $view->get('base_table'));
$this->assertEqual('entity_test_update_revision', $display['display_options']['fields']['id']['table']);
$this->assertEqual('entity_test_update_revision', $display['display_options']['fields']['name']['table']);
// Check that only the impacted views have been updated.
$this->assertUpdatedViews([
'test_view_entity_test',
'test_view_entity_test_data',
'test_view_entity_test_revision',
]);
}
/**
* Tests the case when a view could not be updated automatically.
*/
public function testViewSaveException() {
$this->renameBaseTable();
\Drupal::state()->set('entity_test_update.throw_view_exception', 'test_view_entity_test');
$this->applyEntityUpdates('entity_test_update');
/** @var \Drupal\views\Entity\View $view */
$entity_storage = $this->entityTypeManager->getStorage('view');
$view = $entity_storage->load('test_view_entity_test');
// Check that the table names were not updated automatically for the
// 'test_view_entity_test' view.
$this->assertEquals('entity_test_update', $view->get('base_table'));
$display = $view->getDisplay('default');
$this->assertEquals('entity_test_update', $display['display_options']['fields']['id']['table']);
$this->assertEquals('entity_test_update', $display['display_options']['fields']['name']['table']);
// Check that the other two views impacted by the entity update were updated
// automatically.
$view = $entity_storage->load('test_view_entity_test_data');
$this->assertEquals('entity_test_update_new', $view->get('base_table'));
$display = $view->getDisplay('default');
$this->assertEquals('entity_test_update_new', $display['display_options']['fields']['id']['table']);
$this->assertEquals('entity_test_update_data', $display['display_options']['fields']['name']['table']);
$view = $entity_storage->load('test_view_entity_test_additional_base_field');
$this->assertEquals('entity_test_update_new', $view->get('base_table'));
$display = $view->getDisplay('default');
$this->assertEquals('entity_test_update_new', $display['display_options']['fields']['id']['table']);
$this->assertEquals('entity_test_update_new', $display['display_options']['fields']['new_base_field']['table']);
$this->assertUpdatedViews([
'test_view_entity_test_data',
'test_view_entity_test_additional_base_field',
]);
}
/**
@ -429,4 +519,28 @@ class ViewsEntitySchemaSubscriberIntegrationTest extends ViewsKernelTestBase {
return [$view, $display];
}
/**
* Checks that the passed-in view IDs were the only ones updated.
*
* @param string[] $updated_view_ids
* An array of view IDs.
*/
protected function assertUpdatedViews($updated_view_ids) {
$all_view_ids = array_keys($this->entityTypeManager->getStorage('view')->loadMultiple());
$view_save_count = \Drupal::state()->get('views_test_data.view_save_count', []);
foreach ($all_view_ids as $view_id) {
if (in_array($view_id, $updated_view_ids, TRUE)) {
$this->assertTrue(isset($view_save_count[$view_id]), "The $view_id view has been updated.");
}
else {
$this->assertFalse(isset($view_save_count[$view_id]), "The $view_id view has not been updated.");
}
}
// Check that all test cases are updating only a subset of all the available
// views.
$this->assertGreaterThan(count($updated_view_ids), count($all_view_ids));
}
}

2
core/modules/views/views.services.yml
View File

@ -77,7 +77,7 @@ services:
class: Drupal\views\ExposedFormCache
views.entity_schema_subscriber:
class: Drupal\views\EventSubscriber\ViewsEntitySchemaSubscriber
arguments: ['@entity_type.manager']
arguments: ['@entity_type.manager', '@logger.channel.default']
tags:
- { name: 'event_subscriber' }
views.date_sql: