Unverified Commit d9e8e19c authored by Alex Pott's avatar Alex Pott
Browse files

Issue #3052492 by amateescu, plach, alexpott: ViewsEntitySchemaSubscriber... 

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

core/modules/system/tests/modules/entity_test_update/entity_test_update.module

+10 −0
Changes for core/modules/system/tests/modules/entity_test_update/entity_test_update.module: 10 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -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.

 *

core/modules/views/src/EventSubscriber/ViewsEntitySchemaSubscriber.php

+46 −7
Changes for core/modules/views/src/EventSubscriber/ViewsEntitySchemaSubscriber.php: 46 added lines, 7 removed lines.
Original line number Diff line number Diff line
@@ -8,7 +8,9 @@ 
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,13 +210,29 @@ public function onEntityTypeUpdate(EntityTypeInterface $entity_type, EntityTypeI 
      }

    }



    foreach ($all_views as $view) {

    // 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 @@ protected function processHandlers(array $all_views, callable $process) { 
            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 @@ protected function baseTableRename($all_views, $entity_type_id, $old_base_table, 
    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 @@ protected function dataTableRename($all_views, $entity_type_id, $old_data_table, 
    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 @@ protected function dataTableAddition($all_views, EntityTypeInterface $entity_typ 


    $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 @@ protected function dataTableAddition($all_views, EntityTypeInterface $entity_typ 
   */

  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 @@ protected function revisionRemoval($all_views, EntityTypeInterface $original) { 
      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.

core/modules/views/tests/modules/views_test_data/views_test_data.module

+11 −0
Changes for core/modules/views/tests/modules/views_test_data/views_test_data.module: 11 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -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);

}

core/modules/views/tests/src/Kernel/EventSubscriber/ViewsEntitySchemaSubscriberIntegrationTest.php

+114 −0
Changes for core/modules/views/tests/src/Kernel/EventSubscriber/ViewsEntitySchemaSubscriberIntegrationTest.php: 114 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -138,6 +138,13 @@ public function testBaseTableRename() { 
    $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 @@ public function testDataTableRename() { 
    $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 @@ public function testRevisionBaseTableRename() { 
    $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 @@ public function testRevisionDataTableRename() { 
    $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 @@ public function testDataTableAddition() { 
    $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 @@ public function testRevisionEnabling() { 
    $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 @@ public function testRevisionDisabling() { 
    $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 @@ public function testVariousTableUpdates() { 
    $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 @@ public function testVariousTableUpdatesForRevisionView() { 
    $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 @@ protected function getUpdatedViewAndDisplay($revision = FALSE) { 
    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));

  }



}

core/modules/views/views.services.yml

+1 −1
Changes for core/modules/views/views.services.yml: 1 added line, 1 removed line.
Original line number Diff line number Diff line
@@ -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: