From 8065ee3ee6fe5f8627859e9af32da19e363370a4 Mon Sep 17 00:00:00 2001 From: catch <catch@35733.no-reply.drupal.org> Date: Fri, 22 Oct 2021 10:18:35 +0100 Subject: [PATCH] Issue #3134470 by heddn, gabesullice, sudiptadas19, ridhimaabrol24, quietone, mikelutz, Kristen Pol, alexpott, Wim Leers, larowlan, longwave: Switch to entity owner in EntityContentBase during validation --- .../migrate/destination/EntityComment.php | 10 +- .../migrate/destination/EntityContentBase.php | 44 ++++++- .../migrate/destination/EntityRevision.php | 5 +- .../Kernel/MigrateEntityContentBaseTest.php | 23 +++- .../MigrateEntityContentValidationTest.php | 111 +++++++++++++++++- .../destination/EntityContentBaseTest.php | 12 +- .../destination/EntityRevisionTest.php | 7 +- .../Unit/destination/EntityRevisionTest.php | 12 +- .../Plugin/migrate/destination/EntityUser.php | 10 +- 9 files changed, 214 insertions(+), 20 deletions(-) diff --git a/core/modules/comment/src/Plugin/migrate/destination/EntityComment.php b/core/modules/comment/src/Plugin/migrate/destination/EntityComment.php index d096a3b53226..2c8fd56b2f59 100644 --- a/core/modules/comment/src/Plugin/migrate/destination/EntityComment.php +++ b/core/modules/comment/src/Plugin/migrate/destination/EntityComment.php @@ -5,6 +5,7 @@ use Drupal\Core\Entity\EntityFieldManagerInterface; use Drupal\Core\Entity\EntityStorageInterface; use Drupal\Core\Field\FieldTypePluginManagerInterface; +use Drupal\Core\Session\AccountSwitcherInterface; use Drupal\Core\State\StateInterface; use Drupal\migrate\Plugin\MigrationInterface; use Drupal\migrate\Plugin\migrate\destination\EntityContentBase; @@ -53,9 +54,11 @@ class EntityComment extends EntityContentBase { * The field type plugin manager service. * @param \Drupal\Core\State\StateInterface $state * The state storage object. + * @param \Drupal\Core\Session\AccountSwitcherInterface|null $account_switcher + * The account switcher service. */ - public function __construct(array $configuration, $plugin_id, $plugin_definition, MigrationInterface $migration, EntityStorageInterface $storage, array $bundles, EntityFieldManagerInterface $entity_field_manager, FieldTypePluginManagerInterface $field_type_manager, StateInterface $state) { - parent::__construct($configuration, $plugin_id, $plugin_definition, $migration, $storage, $bundles, $entity_field_manager, $field_type_manager); + public function __construct(array $configuration, $plugin_id, $plugin_definition, MigrationInterface $migration, EntityStorageInterface $storage, array $bundles, EntityFieldManagerInterface $entity_field_manager, FieldTypePluginManagerInterface $field_type_manager, StateInterface $state, AccountSwitcherInterface $account_switcher = NULL) { + parent::__construct($configuration, $plugin_id, $plugin_definition, $migration, $storage, $bundles, $entity_field_manager, $field_type_manager, $account_switcher); $this->state = $state; } @@ -73,7 +76,8 @@ public static function create(ContainerInterface $container, array $configuratio array_keys($container->get('entity_type.bundle.info')->getBundleInfo($entity_type)), $container->get('entity_field.manager'), $container->get('plugin.manager.field.field_type'), - $container->get('state') + $container->get('state'), + $container->get('account_switcher') ); } diff --git a/core/modules/migrate/src/Plugin/migrate/destination/EntityContentBase.php b/core/modules/migrate/src/Plugin/migrate/destination/EntityContentBase.php index ce94893bcd12..bcee94a83d43 100644 --- a/core/modules/migrate/src/Plugin/migrate/destination/EntityContentBase.php +++ b/core/modules/migrate/src/Plugin/migrate/destination/EntityContentBase.php @@ -8,6 +8,7 @@ use Drupal\Core\Entity\EntityStorageInterface; use Drupal\Core\Entity\FieldableEntityInterface; use Drupal\Core\Field\FieldTypePluginManagerInterface; +use Drupal\Core\Session\AccountSwitcherInterface; use Drupal\Core\TypedData\TranslatableInterface; use Drupal\Core\TypedData\TypedDataInterface; use Drupal\migrate\Audit\HighestIdInterface; @@ -17,6 +18,7 @@ use Drupal\migrate\MigrateException; use Drupal\migrate\Plugin\MigrateIdMapInterface; use Drupal\migrate\Row; +use Drupal\user\EntityOwnerInterface; use Symfony\Component\DependencyInjection\ContainerInterface; // cspell:ignore validatable @@ -105,6 +107,13 @@ class EntityContentBase extends Entity implements HighestIdInterface, MigrateVal */ protected $fieldTypeManager; + /** + * The account switcher service. + * + * @var \Drupal\Core\Session\AccountSwitcherInterface + */ + protected $accountSwitcher; + /** * Constructs a content entity. * @@ -124,11 +133,18 @@ class EntityContentBase extends Entity implements HighestIdInterface, MigrateVal * The entity field manager. * @param \Drupal\Core\Field\FieldTypePluginManagerInterface $field_type_manager * The field type plugin manager service. + * @param \Drupal\Core\Session\AccountSwitcherInterface $account_switcher + * The account switcher service. */ - public function __construct(array $configuration, $plugin_id, $plugin_definition, MigrationInterface $migration, EntityStorageInterface $storage, array $bundles, EntityFieldManagerInterface $entity_field_manager, FieldTypePluginManagerInterface $field_type_manager) { + public function __construct(array $configuration, $plugin_id, $plugin_definition, MigrationInterface $migration, EntityStorageInterface $storage, array $bundles, EntityFieldManagerInterface $entity_field_manager, FieldTypePluginManagerInterface $field_type_manager, AccountSwitcherInterface $account_switcher = NULL) { parent::__construct($configuration, $plugin_id, $plugin_definition, $migration, $storage, $bundles); $this->entityFieldManager = $entity_field_manager; $this->fieldTypeManager = $field_type_manager; + if ($account_switcher === NULL) { + @trigger_error('Calling ' . __NAMESPACE__ . '\EntityContentBase::__construct() without the $account_switcher argument is deprecated in drupal:9.3.0 and will be required in drupal:10.0.0. See https://www.drupal.org/node/3142975', E_USER_DEPRECATED); + $account_switcher = \Drupal::service('account_switcher'); + } + $this->accountSwitcher = $account_switcher; } /** @@ -144,7 +160,8 @@ public static function create(ContainerInterface $container, array $configuratio $container->get('entity_type.manager')->getStorage($entity_type), array_keys($container->get('entity_type.bundle.info')->getBundleInfo($entity_type)), $container->get('entity_field.manager'), - $container->get('plugin.manager.field.field_type') + $container->get('plugin.manager.field.field_type'), + $container->get('account_switcher') ); } @@ -187,7 +204,28 @@ public function isEntityValidationRequired(FieldableEntityInterface $entity) { * {@inheritdoc} */ public function validateEntity(FieldableEntityInterface $entity) { - $violations = $entity->validate(); + // Entity validation can require the user that owns the entity. Switch to + // use that user during validation. + // As an example: + // @see \Drupal\Core\Entity\Plugin\Validation\Constraint\ValidReferenceConstraint + $account = $entity instanceof EntityOwnerInterface ? $entity->getOwner() : NULL; + // Validate account exists as the owner reference could be invalid for any + // number of reasons. + if ($account) { + $this->accountSwitcher->switchTo($account); + } + // This finally block ensures that the account is always switched back, even + // if an exception was thrown. Any validation exceptions are intentionally + // left unhandled. They should be caught and logged by a catch block + // surrounding the row import and then added to the migration messages table + // for the current row. + try { + $violations = $entity->validate(); + } finally { + if ($account) { + $this->accountSwitcher->switchBack(); + } + } if (count($violations) > 0) { throw new EntityValidationException($violations); diff --git a/core/modules/migrate/src/Plugin/migrate/destination/EntityRevision.php b/core/modules/migrate/src/Plugin/migrate/destination/EntityRevision.php index 55ac460b5d3f..b387815724b3 100644 --- a/core/modules/migrate/src/Plugin/migrate/destination/EntityRevision.php +++ b/core/modules/migrate/src/Plugin/migrate/destination/EntityRevision.php @@ -6,6 +6,7 @@ use Drupal\Core\Entity\EntityFieldManagerInterface; use Drupal\Core\Entity\EntityStorageInterface; use Drupal\Core\Field\FieldTypePluginManagerInterface; +use Drupal\Core\Session\AccountSwitcherInterface; use Drupal\Core\StringTranslation\TranslatableMarkup; use Drupal\migrate\MigrateException; use Drupal\migrate\Plugin\MigrationInterface; @@ -114,11 +115,11 @@ class EntityRevision extends EntityContentBase { /** * {@inheritdoc} */ - public function __construct(array $configuration, $plugin_id, $plugin_definition, MigrationInterface $migration, EntityStorageInterface $storage, array $bundles, EntityFieldManagerInterface $entity_field_manager, FieldTypePluginManagerInterface $field_type_manager) { + public function __construct(array $configuration, $plugin_id, $plugin_definition, MigrationInterface $migration, EntityStorageInterface $storage, array $bundles, EntityFieldManagerInterface $entity_field_manager, FieldTypePluginManagerInterface $field_type_manager, AccountSwitcherInterface $account_switcher) { $plugin_definition += [ 'label' => new TranslatableMarkup('@entity_type revisions', ['@entity_type' => $storage->getEntityType()->getSingularLabel()]), ]; - parent::__construct($configuration, $plugin_id, $plugin_definition, $migration, $storage, $bundles, $entity_field_manager, $field_type_manager); + parent::__construct($configuration, $plugin_id, $plugin_definition, $migration, $storage, $bundles, $entity_field_manager, $field_type_manager, $account_switcher); } /** diff --git a/core/modules/migrate/tests/src/Kernel/MigrateEntityContentBaseTest.php b/core/modules/migrate/tests/src/Kernel/MigrateEntityContentBaseTest.php index b27022157537..1123737bcfa5 100644 --- a/core/modules/migrate/tests/src/Kernel/MigrateEntityContentBaseTest.php +++ b/core/modules/migrate/tests/src/Kernel/MigrateEntityContentBaseTest.php @@ -93,7 +93,8 @@ protected function createDestination(array $configuration) { $this->storage, [], $this->container->get('entity_field.manager'), - $this->container->get('plugin.manager.field.field_type') + $this->container->get('plugin.manager.field.field_type'), + $this->container->get('account_switcher') ); } @@ -301,4 +302,24 @@ public function testStubRows() { } } + /** + * Test BC injection of account switcher service. + * + * @group legacy + */ + public function testAccountSwitcherBackwardsCompatibility() { + $this->expectDeprecation('Calling Drupal\migrate\Plugin\migrate\destination\EntityContentBase::__construct() without the $account_switcher argument is deprecated in drupal:9.3.0 and will be required in drupal:10.0.0. See https://www.drupal.org/node/3142975'); + $destination = new EntityContentBase( + [], + 'fake_plugin_id', + [], + $this->createMock(MigrationInterface::class), + $this->storage, + [], + $this->container->get('entity_field.manager'), + $this->container->get('plugin.manager.field.field_type') + ); + $this->assertInstanceOf(EntityContentBase::class, $destination); + } + } diff --git a/core/modules/migrate/tests/src/Kernel/MigrateEntityContentValidationTest.php b/core/modules/migrate/tests/src/Kernel/MigrateEntityContentValidationTest.php index c0cf029412f3..43d9ebda0b13 100644 --- a/core/modules/migrate/tests/src/Kernel/MigrateEntityContentValidationTest.php +++ b/core/modules/migrate/tests/src/Kernel/MigrateEntityContentValidationTest.php @@ -2,11 +2,18 @@ namespace Drupal\Tests\migrate\Kernel; +use Drupal\field\Entity\FieldConfig; +use Drupal\field\Entity\FieldStorageConfig; +use Drupal\filter\Entity\FilterFormat; +use Drupal\filter\FilterFormatInterface; use Drupal\KernelTests\KernelTestBase; use Drupal\migrate\Event\MigrateEvents; use Drupal\migrate\Event\MigrateIdMapMessageEvent; use Drupal\migrate\MigrateExecutable; +use Drupal\user\Entity\Role; +use Drupal\user\Entity\User; use Drupal\user\Plugin\Validation\Constraint\UserNameConstraint; +use Drupal\user\RoleInterface; /** * Tests validation of an entity during migration. @@ -18,7 +25,16 @@ class MigrateEntityContentValidationTest extends KernelTestBase { /** * {@inheritdoc} */ - protected static $modules = ['migrate', 'system', 'user', 'entity_test']; + protected static $modules = [ + 'entity_test', + 'field', + 'filter', + 'filter_test', + 'migrate', + 'system', + 'text', + 'user', + ]; /** * Messages accumulated during the migration run. @@ -33,9 +49,11 @@ class MigrateEntityContentValidationTest extends KernelTestBase { protected function setUp(): void { parent::setUp(); - $this->installConfig(['system', 'user']); $this->installEntitySchema('user'); + $this->installEntitySchema('user_role'); $this->installEntitySchema('entity_test'); + $this->installSchema('system', ['sequences']); + $this->installConfig(['field', 'filter_test', 'system', 'user']); $this->container ->get('event_dispatcher') @@ -140,6 +158,95 @@ public function test2() { $this->assertArrayNotHasKey(3, $this->messages, 'Fourth message should not exist.'); } + /** + * Tests validation for entities that are instances of EntityOwnerInterface. + */ + public function testEntityOwnerValidation() { + // Text format access is impacted by user permissions. + $filter_test_format = FilterFormat::load('filter_test'); + assert($filter_test_format instanceof FilterFormatInterface); + + // Create 2 users, an admin user who has permission to use this text format + // and another who does not have said access. + $role = Role::create([ + 'id' => 'admin', + 'label' => 'admin', + 'is_admin' => TRUE, + ]); + assert($role instanceof RoleInterface); + $role->grantPermission($filter_test_format->getPermissionName()); + $role->save(); + $admin_user = User::create([ + 'name' => 'foobar', + 'mail' => 'foobar@example.com', + ]); + $admin_user->addRole($role->id()); + $admin_user->save(); + $normal_user = User::create([ + 'name' => 'normal user', + 'mail' => 'normal@example.com', + ]); + $normal_user->save(); + + // Add a "body" field with the text format. + $field_name = mb_strtolower($this->randomMachineName()); + $field_storage = FieldStorageConfig::create([ + 'field_name' => $field_name, + 'entity_type' => 'entity_test', + 'type' => 'text', + ]); + $field_storage->save(); + FieldConfig::create([ + 'field_storage' => $field_storage, + 'bundle' => 'entity_test', + ])->save(); + + // Attempt to migrate entities. The first record is owned by an admin user. + $definition = [ + 'source' => [ + 'plugin' => 'embedded_data', + 'data_rows' => [ + [ + 'id' => 1, + 'uid' => $admin_user->id(), + 'body' => [ + 'value' => 'foo', + 'format' => 'filter_test', + ], + ], + [ + 'id' => 2, + 'uid' => $normal_user->id(), + 'body' => [ + 'value' => 'bar', + 'format' => 'filter_test', + ], + ], + ], + 'ids' => [ + 'id' => ['type' => 'integer'], + ], + ], + 'process' => [ + 'id' => 'id', + 'user_id' => 'uid', + "$field_name/value" => 'body/value', + "$field_name/format" => 'body/format', + ], + 'destination' => [ + 'plugin' => 'entity:entity_test', + 'validate' => TRUE, + ], + ]; + $this->container->get('current_user')->setAccount($normal_user); + $this->runImport($definition); + + // The second user import should fail validation because they do not have + // access to use "filter_test" filter. + $this->assertSame(sprintf('2: [entity_test: 2]: user_id.0.target_id=This entity (<em class="placeholder">user</em>: <em class="placeholder">%s</em>) cannot be referenced.||%s.0.format=The value you selected is not a valid choice.', $normal_user->id(), $field_name), $this->messages[0]); + $this->assertArrayNotHasKey(1, $this->messages); + } + /** * Reacts to map message event. * diff --git a/core/modules/migrate/tests/src/Unit/Plugin/migrate/destination/EntityContentBaseTest.php b/core/modules/migrate/tests/src/Unit/Plugin/migrate/destination/EntityContentBaseTest.php index 7003db87880f..3319b71a19d2 100644 --- a/core/modules/migrate/tests/src/Unit/Plugin/migrate/destination/EntityContentBaseTest.php +++ b/core/modules/migrate/tests/src/Unit/Plugin/migrate/destination/EntityContentBaseTest.php @@ -9,6 +9,7 @@ use Drupal\Core\Entity\ContentEntityInterface; use Drupal\Core\Field\FieldTypePluginManagerInterface; +use Drupal\Core\Session\AccountSwitcherInterface; use Drupal\migrate\MigrateException; use Drupal\migrate\Plugin\migrate\destination\EntityContentBase; use Drupal\migrate\Plugin\MigrateIdMapInterface; @@ -34,7 +35,9 @@ public function testImport() { $this->storage->reveal(), $bundles, $this->entityFieldManager->reveal(), - $this->prophesize(FieldTypePluginManagerInterface::class)->reveal()); + $this->prophesize(FieldTypePluginManagerInterface::class)->reveal(), + $this->prophesize(AccountSwitcherInterface::class)->reveal() + ); $entity = $this->prophesize(ContentEntityInterface::class); $entity->isValidationRequired() ->shouldBeCalledTimes(1); @@ -63,7 +66,9 @@ public function testImportEntityLoadFailure() { $this->storage->reveal(), $bundles, $this->entityFieldManager->reveal(), - $this->prophesize(FieldTypePluginManagerInterface::class)->reveal()); + $this->prophesize(FieldTypePluginManagerInterface::class)->reveal(), + $this->prophesize(AccountSwitcherInterface::class)->reveal() + ); $destination->setEntity(FALSE); $this->expectException(MigrateException::class); $this->expectExceptionMessage('Unable to get entity'); @@ -88,7 +93,8 @@ public function testUntranslatable() { $this->storage->reveal(), [], $this->entityFieldManager->reveal(), - $this->prophesize(FieldTypePluginManagerInterface::class)->reveal() + $this->prophesize(FieldTypePluginManagerInterface::class)->reveal(), + $this->prophesize(AccountSwitcherInterface::class)->reveal() ); $this->expectException(MigrateException::class); $this->expectExceptionMessage('The "foo" entity type does not support translations.'); diff --git a/core/modules/migrate/tests/src/Unit/Plugin/migrate/destination/EntityRevisionTest.php b/core/modules/migrate/tests/src/Unit/Plugin/migrate/destination/EntityRevisionTest.php index c061143f22a8..1ed293cfdb38 100644 --- a/core/modules/migrate/tests/src/Unit/Plugin/migrate/destination/EntityRevisionTest.php +++ b/core/modules/migrate/tests/src/Unit/Plugin/migrate/destination/EntityRevisionTest.php @@ -5,6 +5,7 @@ use Drupal\Core\Entity\EntityStorageInterface; use Drupal\Core\Entity\EntityTypeInterface; use Drupal\Core\Field\FieldTypePluginManagerInterface; +use Drupal\Core\Session\AccountSwitcherInterface; use Drupal\migrate\MigrateException; use Drupal\migrate\Plugin\MigrationInterface; use Drupal\migrate\Plugin\migrate\destination\EntityRevision; @@ -53,7 +54,8 @@ public function testUnrevisionable() { $this->storage->reveal(), [], $this->entityFieldManager->reveal(), - $this->prophesize(FieldTypePluginManagerInterface::class)->reveal() + $this->prophesize(FieldTypePluginManagerInterface::class)->reveal(), + $this->prophesize(AccountSwitcherInterface::class)->reveal() ); $this->expectException(MigrateException::class); $this->expectExceptionMessage('The "foo" entity type does not support revisions.'); @@ -81,7 +83,8 @@ public function testUntranslatable() { $this->storage->reveal(), [], $this->entityFieldManager->reveal(), - $this->prophesize(FieldTypePluginManagerInterface::class)->reveal() + $this->prophesize(FieldTypePluginManagerInterface::class)->reveal(), + $this->prophesize(AccountSwitcherInterface::class)->reveal() ); $this->expectException(MigrateException::class); $this->expectExceptionMessage('The "foo" entity type does not support translations.'); diff --git a/core/modules/migrate/tests/src/Unit/destination/EntityRevisionTest.php b/core/modules/migrate/tests/src/Unit/destination/EntityRevisionTest.php index 28ec1251936d..94a96a9c0839 100644 --- a/core/modules/migrate/tests/src/Unit/destination/EntityRevisionTest.php +++ b/core/modules/migrate/tests/src/Unit/destination/EntityRevisionTest.php @@ -10,6 +10,7 @@ use Drupal\Core\Entity\ContentEntityInterface; use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Entity\EntityTypeInterface; +use Drupal\Core\Session\AccountSwitcherInterface; use Drupal\migrate\Plugin\MigrationInterface; use Drupal\migrate\Plugin\migrate\destination\EntityRevision as RealEntityRevision; use Drupal\migrate\Row; @@ -43,6 +44,13 @@ class EntityRevisionTest extends UnitTestCase { */ protected $fieldTypeManager; + /** + * A mock account switcher. + * + * @var \Prophecy\Prophecy\ObjectProphecy|\Drupal\Core\Session\AccountSwitcherInterface + */ + protected $accountSwitcher; + protected function setUp(): void { parent::setUp(); @@ -57,6 +65,7 @@ protected function setUp(): void { $this->entityFieldManager = $this->prophesize('\Drupal\Core\Entity\EntityFieldManagerInterface'); $this->fieldTypeManager = $this->prophesize('\Drupal\Core\Field\FieldTypePluginManagerInterface'); + $this->accountSwitcher = $this->prophesize(AccountSwitcherInterface::class); } /** @@ -194,7 +203,8 @@ protected function getEntityRevisionDestination(array $configuration = [], $plug $this->storage->reveal(), [], $this->entityFieldManager->reveal(), - $this->fieldTypeManager->reveal() + $this->fieldTypeManager->reveal(), + $this->accountSwitcher->reveal() ); } diff --git a/core/modules/user/src/Plugin/migrate/destination/EntityUser.php b/core/modules/user/src/Plugin/migrate/destination/EntityUser.php index c4a0291c2a95..3f8c20b65f2f 100644 --- a/core/modules/user/src/Plugin/migrate/destination/EntityUser.php +++ b/core/modules/user/src/Plugin/migrate/destination/EntityUser.php @@ -8,6 +8,7 @@ use Drupal\Core\Field\FieldTypePluginManagerInterface; use Drupal\Core\Field\Plugin\Field\FieldType\EmailItem; use Drupal\Core\Password\PasswordInterface; +use Drupal\Core\Session\AccountSwitcherInterface; use Drupal\migrate\Plugin\MigrationInterface; use Drupal\migrate\Plugin\migrate\destination\EntityContentBase; use Drupal\migrate\Row; @@ -95,9 +96,11 @@ class EntityUser extends EntityContentBase { * The field type plugin manager service. * @param \Drupal\Core\Password\PasswordInterface $password * The password service. + * @param \Drupal\Core\Session\AccountSwitcherInterface|null $account_switcher + * The account switcher service. */ - public function __construct(array $configuration, $plugin_id, $plugin_definition, MigrationInterface $migration, EntityStorageInterface $storage, array $bundles, EntityFieldManagerInterface $entity_field_manager, FieldTypePluginManagerInterface $field_type_manager, PasswordInterface $password) { - parent::__construct($configuration, $plugin_id, $plugin_definition, $migration, $storage, $bundles, $entity_field_manager, $field_type_manager); + public function __construct(array $configuration, $plugin_id, $plugin_definition, MigrationInterface $migration, EntityStorageInterface $storage, array $bundles, EntityFieldManagerInterface $entity_field_manager, FieldTypePluginManagerInterface $field_type_manager, PasswordInterface $password, AccountSwitcherInterface $account_switcher = NULL) { + parent::__construct($configuration, $plugin_id, $plugin_definition, $migration, $storage, $bundles, $entity_field_manager, $field_type_manager, $account_switcher); $this->password = $password; } @@ -115,7 +118,8 @@ public static function create(ContainerInterface $container, array $configuratio array_keys($container->get('entity_type.bundle.info')->getBundleInfo($entity_type)), $container->get('entity_field.manager'), $container->get('plugin.manager.field.field_type'), - $container->get('password') + $container->get('password'), + $container->get('account_switcher') ); } -- GitLab