From fc11a4f5c19de1a197a4a25828d9d3e187a161c7 Mon Sep 17 00:00:00 2001
From: Alex Pott <alex.a.pott@googlemail.com>
Date: Sat, 13 Apr 2019 15:36:00 -0700
Subject: [PATCH] Issue #2787185 by botris, mikelutz, kekkis, mikeryan,
lemuelsantos, mvbaalen, quietone, BenStallings, mpotter, heddn: track_changes
does not work when the map is joinable
---
.../src/Plugin/migrate/source/SqlBase.php | 25 ++-
...y.vocabulary.track_changes_import_term.yml | 7 +
.../migrate_track_changes_test.info.yml | 7 +
.../migrations/track_changes_test.yml | 18 ++
.../migrate/source/TrackChangesTest.php | 54 +++++
.../tests/src/Kernel/TrackChangesTest.php | 194 ++++++++++++++++++
6 files changed, 299 insertions(+), 6 deletions(-)
create mode 100644 core/modules/migrate/tests/modules/migrate_track_changes_test/config/install/taxonomy.vocabulary.track_changes_import_term.yml
create mode 100644 core/modules/migrate/tests/modules/migrate_track_changes_test/migrate_track_changes_test.info.yml
create mode 100644 core/modules/migrate/tests/modules/migrate_track_changes_test/migrations/track_changes_test.yml
create mode 100644 core/modules/migrate/tests/modules/migrate_track_changes_test/src/Plugin/migrate/source/TrackChangesTest.php
create mode 100644 core/modules/migrate/tests/src/Kernel/TrackChangesTest.php
diff --git a/core/modules/migrate/src/Plugin/migrate/source/SqlBase.php b/core/modules/migrate/src/Plugin/migrate/source/SqlBase.php
index 4bf846e854eb..ec22c3f65e8a 100644
--- a/core/modules/migrate/src/Plugin/migrate/source/SqlBase.php
+++ b/core/modules/migrate/src/Plugin/migrate/source/SqlBase.php
@@ -108,11 +108,6 @@ abstract class SqlBase extends SourcePluginBase implements ContainerFactoryPlugi
public function __construct(array $configuration, $plugin_id, $plugin_definition, MigrationInterface $migration, StateInterface $state) {
parent::__construct($configuration, $plugin_id, $plugin_definition, $migration);
$this->state = $state;
- // If we are using high water, but haven't yet set a high water mark, skip
- // joining the map table, as we want to get all available records.
- if ($this->getHighWaterProperty() && $this->getHighWater() === NULL) {
- $this->configuration['ignore_map'] = TRUE;
- }
}
/**
@@ -285,7 +280,7 @@ protected function initializeIterator() {
$conditions = $this->query->orConditionGroup();
$condition_added = FALSE;
$added_fields = [];
- if (empty($this->configuration['ignore_map']) && $this->mapJoinable()) {
+ if ($this->mapJoinable()) {
// Build the join to the map table. Because the source key could have
// multiple fields, we need to build things up.
$count = 1;
@@ -404,6 +399,24 @@ public function count($refresh = FALSE) {
* TRUE if we can join against the map table otherwise FALSE.
*/
protected function mapJoinable() {
+
+ // Do not join map if explicitly configured not to.
+ if (isset($this->configuration['ignore_map']) && $this->configuration['ignore_map']) {
+ return FALSE;
+ }
+
+ // If we are using high water, but haven't yet set a high water mark, do not
+ // join the map table, as we want to get all available records.
+ if ($this->getHighWaterProperty() && $this->getHighWater() === NULL) {
+ return FALSE;
+ }
+
+ // If we are tracking changes, we also need to retrieve all rows to compare
+ // hashes
+ if ($this->trackChanges) {
+ return FALSE;
+ }
+
if (!$this->getIds()) {
return FALSE;
}
diff --git a/core/modules/migrate/tests/modules/migrate_track_changes_test/config/install/taxonomy.vocabulary.track_changes_import_term.yml b/core/modules/migrate/tests/modules/migrate_track_changes_test/config/install/taxonomy.vocabulary.track_changes_import_term.yml
new file mode 100644
index 000000000000..8bbc5d7f048f
--- /dev/null
+++ b/core/modules/migrate/tests/modules/migrate_track_changes_test/config/install/taxonomy.vocabulary.track_changes_import_term.yml
@@ -0,0 +1,7 @@
+langcode: en
+status: true
+dependencies: { }
+name: Track changes import term
+vid: track_changes_import_term
+description: ''
+weight: 0
diff --git a/core/modules/migrate/tests/modules/migrate_track_changes_test/migrate_track_changes_test.info.yml b/core/modules/migrate/tests/modules/migrate_track_changes_test/migrate_track_changes_test.info.yml
new file mode 100644
index 000000000000..6f4141069290
--- /dev/null
+++ b/core/modules/migrate/tests/modules/migrate_track_changes_test/migrate_track_changes_test.info.yml
@@ -0,0 +1,7 @@
+type: module
+name: Migration Track Changes Test
+description: 'Provides test fixtures for testing track changes marks.'
+package: Testing
+core: 8.x
+dependencies:
+ - drupal:migrate
diff --git a/core/modules/migrate/tests/modules/migrate_track_changes_test/migrations/track_changes_test.yml b/core/modules/migrate/tests/modules/migrate_track_changes_test/migrations/track_changes_test.yml
new file mode 100644
index 000000000000..31d03476fc66
--- /dev/null
+++ b/core/modules/migrate/tests/modules/migrate_track_changes_test/migrations/track_changes_test.yml
@@ -0,0 +1,18 @@
+id: track_changes_test
+label: Track changes test.
+source:
+ plugin: track_changes_test
+ track_changes: true
+destination:
+ plugin: entity:taxonomy_term
+migration_tags:
+ test: test
+process:
+ name: name
+ vid:
+ plugin: default_value
+ default_value: track_changes_import_term
+ 'description/value': description
+ 'description/format':
+ plugin: default_value
+ default_value: 'basic_html'
diff --git a/core/modules/migrate/tests/modules/migrate_track_changes_test/src/Plugin/migrate/source/TrackChangesTest.php b/core/modules/migrate/tests/modules/migrate_track_changes_test/src/Plugin/migrate/source/TrackChangesTest.php
new file mode 100644
index 000000000000..202a3057923a
--- /dev/null
+++ b/core/modules/migrate/tests/modules/migrate_track_changes_test/src/Plugin/migrate/source/TrackChangesTest.php
@@ -0,0 +1,54 @@
+<?php
+
+namespace Drupal\migrate_track_changes_test\Plugin\migrate\source;
+
+use Drupal\migrate\Plugin\migrate\source\SqlBase;
+
+/**
+ * Source plugin for migration track changes tests.
+ *
+ * @MigrateSource(
+ * id = "track_changes_test"
+ * )
+ */
+class TrackChangesTest extends SqlBase {
+
+ /**
+ * {@inheritdoc}
+ */
+ public function query() {
+ $field_names = array_keys($this->fields());
+ $query = $this
+ ->select('track_changes_term', 't')
+ ->fields('t', $field_names);
+ foreach ($field_names as $field_name) {
+ $query->groupBy($field_name);
+ }
+ return $query;
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function fields() {
+ $fields = [
+ 'tid' => $this->t('Term id'),
+ 'name' => $this->t('Name'),
+ 'description' => $this->t('Description'),
+ ];
+
+ return $fields;
+ }
+
+ /**
+ * {@inheritdoc}
+ */
+ public function getIds() {
+ return [
+ 'tid' => [
+ 'type' => 'integer',
+ ],
+ ];
+ }
+
+}
diff --git a/core/modules/migrate/tests/src/Kernel/TrackChangesTest.php b/core/modules/migrate/tests/src/Kernel/TrackChangesTest.php
new file mode 100644
index 000000000000..6e4bb1a988dd
--- /dev/null
+++ b/core/modules/migrate/tests/src/Kernel/TrackChangesTest.php
@@ -0,0 +1,194 @@
+<?php
+
+namespace Drupal\Tests\migrate\Kernel;
+
+/**
+ * Tests migration track changes property.
+ *
+ * @group migrate
+ */
+class TrackChangesTest extends MigrateTestBase {
+
+ /**
+ * {@inheritdoc}
+ */
+ public static $modules = [
+ 'system',
+ 'user',
+ 'taxonomy',
+ 'migrate',
+ 'migrate_track_changes_test',
+ 'text',
+ ];
+
+ /**
+ * {@inheritdoc}
+ */
+ protected function setUp() {
+ parent::setUp();
+ // Create source test table.
+ $this->sourceDatabase->schema()->createTable('track_changes_term', [
+ 'fields' => [
+ 'tid' => [
+ 'description' => 'Serial',
+ 'type' => 'serial',
+ 'unsigned' => TRUE,
+ 'not null' => TRUE,
+ ],
+ 'name' => [
+ 'description' => 'Name',
+ 'type' => 'varchar',
+ 'length' => 128,
+ 'not null' => TRUE,
+ 'default' => '',
+ ],
+ 'description' => [
+ 'description' => 'Name',
+ 'type' => 'varchar',
+ 'length' => 255,
+ 'not null' => FALSE,
+ 'default' => '',
+ ],
+ ],
+ 'primary key' => [
+ 'tid',
+ ],
+ 'description' => 'Contains taxonomy terms to import',
+ ]);
+
+ // Add 4 items to source table.
+ $this->sourceDatabase->insert('track_changes_term')
+ ->fields([
+ 'name',
+ 'description',
+ ])
+ ->values([
+ 'name' => 'Item 1',
+ 'description' => 'Text item 1',
+ ])
+ ->values([
+ 'name' => 'Item 2',
+ 'description' => 'Text item 2',
+ ])
+ ->values([
+ 'name' => 'Item 3',
+ 'description' => 'Text item 3',
+ ])
+ ->values([
+ 'name' => 'Item 4',
+ 'description' => 'Text item 4',
+ ])
+ ->execute();
+
+ $this->installEntitySchema('taxonomy_term');
+ $this->installEntitySchema('user');
+
+ $this->executeMigration('track_changes_test');
+ }
+
+ /**
+ * Tests track changes property of SqlBase.
+ */
+ public function testTrackChanges() {
+ // Assert all of the terms have been imported.
+ $this->assertTermExists('name', 'Item 1');
+ $this->assertTermExists('name', 'Item 2');
+ $this->assertTermExists('description', 'Text item 3');
+ $this->assertTermExists('description', 'Text item 4');
+
+ // Update Item 1 triggering its track_changes by name.
+ $this->sourceDatabase->update('track_changes_term')
+ ->fields([
+ 'name' => 'Item 1 updated',
+ ])
+ ->condition('name', 'Item 1')
+ ->execute();
+
+ // Update Item 2 keeping it's track_changes name the same.
+ $this->sourceDatabase->update('track_changes_term')
+ ->fields([
+ 'name' => 'Item 2',
+ ])
+ ->condition('name', 'Item 2')
+ ->execute();
+
+ // Update Item 3 triggering its track_changes by field.
+ $this->sourceDatabase->update('track_changes_term')
+ ->fields([
+ 'description' => 'Text item 3 updated',
+ ])
+ ->condition('name', 'Item 3')
+ ->execute();
+
+ // Update Item 2 keeping it's track_changes field the same.
+ $this->sourceDatabase->update('track_changes_term')
+ ->fields([
+ 'description' => 'Text item 4',
+ ])
+ ->condition('name', 'Item 4')
+ ->execute();
+
+ // Execute migration again.
+ $this->executeMigration('track_changes_test');
+
+ // Item with name changes should be updated.
+ $this->assertTermExists('name', 'Item 1 updated');
+ $this->assertTermDoesNotExist('name', 'Item 1');
+
+ // Item without name changes should not be updated.
+ $this->assertTermExists('name', 'Item 2');
+
+ // Item with field changes should be updated.
+ $this->assertTermExists('description', 'Text item 3 updated');
+ $this->assertTermDoesNotExist('description', 'Text item 3');
+
+ // Item without field changes should not be updated.
+ $this->assertTermExists('description', 'Text item 4');
+ }
+
+ /**
+ * Assert that term with given name exists.
+ *
+ * @param string $property
+ * Property to evaluate.
+ * @param string $value
+ * Value to evaluate.
+ */
+ protected function assertTermExists($property, $value) {
+ self::assertTrue($this->termExists($property, $value));
+ }
+
+ /**
+ * Assert that term with given title does not exist.
+ *
+ * @param string $property
+ * Property to evaluate.
+ * @param string $value
+ * Value to evaluate.
+ */
+ protected function assertTermDoesNotExist($property, $value) {
+ self::assertFalse($this->termExists($property, $value));
+ }
+
+ /**
+ * Checks if term with given name exists.
+ *
+ * @param string $property
+ * Property to evaluate.
+ * @param string $value
+ * Value to evaluate.
+ *
+ * @return bool
+ */
+ protected function termExists($property, $value) {
+ $property = $property === 'description' ? 'description__value' : $property;
+ $query = \Drupal::entityQuery('taxonomy_term');
+ $result = $query
+ ->condition($property, $value)
+ ->range(0, 1)
+ ->execute();
+
+ return !empty($result);
+ }
+
+}
--
GitLab