Issue #2787185 by botris, mikelutz, kekkis, mikeryan, lemuelsantos, mvbaalen,...

Issue #2787185 by botris, mikelutz, kekkis, mikeryan, lemuelsantos, mvbaalen, quietone, BenStallings, mpotter, heddn: track_changes does not work when the map is joinable

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;
     }

core/modules/migrate/tests/modules/migrate_track_changes_test/config/install/taxonomy.vocabulary.track_changes_import_term.yml

+langcode: en
+status: true
+dependencies: {  }
+name: Track changes import term
+vid: track_changes_import_term
+description: ''
+weight: 0

core/modules/migrate/tests/modules/migrate_track_changes_test/migrate_track_changes_test.info.yml

+type: module
+name: Migration Track Changes Test
+description: 'Provides test fixtures for testing track changes marks.'
+package: Testing
+core: 8.x
+dependencies:
+  - drupal:migrate

core/modules/migrate/tests/modules/migrate_track_changes_test/migrations/track_changes_test.yml

+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'

core/modules/migrate/tests/modules/migrate_track_changes_test/src/Plugin/migrate/source/TrackChangesTest.php

+<?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',
+      ],
+    ];
+  }
+
+}

core/modules/migrate/tests/src/Kernel/TrackChangesTest.php

+<?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);
+  }
+
+}