Issue #2504815 by neclimdul, phenaproxima, mikeryan: d6 to d8 migration throws...

Issue #2504815 by neclimdul, phenaproxima, mikeryan: d6 to d8 migration throws integrity contraint warning with duplicate file paths

core/modules/file/src/Plugin/migrate/destination/EntityFile.php

@@ -73,6 +73,20 @@ public static function create(ContainerInterface $container, array $configuratio
     );
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  protected function getEntity(Row $row, array $old_destination_id_values) {
+    $destination = $row->getDestinationProperty($this->configuration['destination_path_property']);
+    $entity = $this->storage->loadByProperties(['uri' => $destination]);
+    if ($entity) {
+      return reset($entity);
+    }
+    else {
+      return parent::getEntity($row, $old_destination_id_values);
+    }
+  }
+
   /**
    * {@inheritdoc}
    */

core/modules/file/src/Plugin/migrate/process/d6/FileUri.php

@@ -24,20 +24,19 @@ class FileUri extends ProcessPluginBase {
    * {@inheritdoc}
    */
   public function transform($value, MigrateExecutableInterface $migrate_executable, Row $row, $destination_property) {
-
     list($filepath, $file_directory_path, $temp_directory_path, $is_public) = $value;
 
     // Specific handling using $temp_directory_path for temporary files.
     if (substr($filepath, 0, strlen($temp_directory_path)) === $temp_directory_path) {
       $uri = preg_replace('/^' . preg_quote($temp_directory_path, '/') . '/', '', $filepath);
-      return "temporary://$uri";
+      return 'temporary://' . ltrim($uri, '/');
     }
 
     // Strip the files path from the uri instead of using basename
     // so any additional folders in the path are preserved.
     $uri = preg_replace('/^' . preg_quote($file_directory_path, '/') . '/', '', $filepath);
 
-    return $is_public ? "public://$uri" : "private://$uri";
+    return ($is_public ? 'public' : 'private') . '://' . ltrim($uri, '/');
   }
 
 }

core/modules/file/src/Plugin/migrate/source/d6/File.php

@@ -44,18 +44,13 @@ class File extends DrupalSqlBase {
    * {@inheritdoc}
    */
   public function query() {
-    $query = $this->select('files', 'f')->fields('f', array(
-      'fid',
-      'uid',
-      'filename',
-      'filepath',
-      'filemime',
-      'filesize',
-      'status',
-      'timestamp',
-    ));
-    $query->orderBy('timestamp');
-    return $query;
+    return $this->select('files', 'f')
+      ->fields('f')
+      ->orderBy('timestamp')
+      // If two or more files have the same timestamp, they'll end up in a
+      // non-deterministic order. Ordering by fid (or any other unique field)
+      // will prevent this.
+      ->orderBy('fid');
   }
 
   /**

core/modules/file/src/Tests/Migrate/d6/MigrateFileTest.php

@@ -8,12 +8,13 @@
 namespace Drupal\file\Tests\Migrate\d6;
 
 use Drupal\Component\Utility\Random;
+use Drupal\file\Entity\File;
+use Drupal\file\FileInterface;
 use Drupal\migrate\Entity\Migration;
 use Drupal\migrate\Tests\MigrateDumpAlterInterface;
 use Drupal\Core\Database\Database;
 use Drupal\migrate_drupal\Tests\d6\MigrateDrupal6TestBase;
 use Drupal\simpletest\TestBase;
-use Drupal\file\Entity\File;
 
 /**
  * file migration.
@@ -46,17 +47,41 @@ protected function setUp() {
     $this->executeMigration($migration);
   }
 
+  /**
+   * Asserts a file entity.
+   *
+   * @param integer $fid
+   *  The file ID.
+   * @param string $name
+   *  The expected file name.
+   * @param integer $size
+   *  The expected file size.
+   * @param string $uri
+   *  The expected file URI.
+   * @param string $type
+   *  The expected MIME type.
+   * @param integer $uid
+   *  The expected file owner ID.
+   */
+  protected function assertEntity($fid, $name, $size, $uri, $type, $uid) {
+    /** @var \Drupal\file\FileInterface $file */
+    $file = File::load($fid);
+    $this->assertTrue($file instanceof FileInterface);
+    $this->assertIdentical($name, $file->getFilename());
+    $this->assertIdentical($size, $file->getSize());
+    $this->assertIdentical($uri, $file->getFileUri());
+    $this->assertIdentical($type, $file->getMimeType());
+    $this->assertIdentical($uid, $file->getOwnerId());
+  }
+
   /**
    * Tests the Drupal 6 files to Drupal 8 migration.
    */
   public function testFiles() {
-    /** @var \Drupal\file\FileInterface $file */
-    $file = File::load(1);
-    $this->assertIdentical('Image1.png', $file->getFilename());
-    $this->assertIdentical('39325', $file->getSize());
-    $this->assertIdentical('public://image-1.png', $file->getFileUri());
-    $this->assertIdentical('image/png', $file->getMimeType());
-    $this->assertIdentical("1", $file->getOwnerId());
+    $this->assertEntity(1, 'Image1.png', '39325', 'public://image-1.png', 'image/png', '1');
+    $this->assertEntity(2, 'Image2.jpg', '1831', 'public://image-2.jpg', 'image/jpeg', '1');
+    $this->assertEntity(3, 'Image-test.gif', '183', 'public://image-test.gif', 'image/jpeg', '1');
+    $this->assertEntity(5, 'html-1.txt', '24', 'public://html-1.txt', 'text/plain', '1');
 
     // Test that we can re-import and also test with file_directory_path set.
     \Drupal::database()
@@ -84,6 +109,10 @@ public function testFiles() {
     // Ensure that a temporary file has been migrated.
     $file = File::load(6);
     $this->assertIdentical('temporary://' . static::getUniqueFilename(), $file->getFileUri());
+
+    // File 7, created in static::migrateDumpAlter(), shares a path with
+    // file 5, which means it should be skipped entirely.
+    $this->assertNull(File::load(7));
   }
 
   /**
@@ -105,8 +134,10 @@ public static function migrateDumpAlter(TestBase $test) {
     static::$tempFilename = $test->getDatabasePrefix() . $random->name() . '.jpg';
     $file_path = $temp_directory . '/' . static::$tempFilename;
     file_put_contents($file_path, '');
-    Database::getConnection('default', 'migrate')
-      ->update('files')
+
+    $db = Database::getConnection('default', 'migrate');
+
+    $db->update('files')
       ->condition('fid', 6)
       ->fields(array(
         'filename' => static::$tempFilename,
@@ -114,6 +145,14 @@ public static function migrateDumpAlter(TestBase $test) {
       ))
       ->execute();
 
+    $file = (array) $db->select('files')
+      ->fields('files')
+      ->condition('fid', 5)
+      ->execute()
+      ->fetchObject();
+    unset($file['fid']);
+    $db->insert('files')->fields($file)->execute();
+
     return static::$tempFilename;
   }
 

core/modules/file/tests/src/Unit/Plugin/migrate/process/d6/FileUriTest.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Tests\file\Unit\Plugin\migrate\process\d6\FileUriTest.
+ */
+
+namespace Drupal\Tests\file\Unit\Plugin\migrate\process\d6;
+
+use Drupal\file\Plugin\migrate\process\d6\FileUri;
+use Drupal\migrate\MigrateExecutable;
+use Drupal\migrate\MigrateMessage;
+use Drupal\migrate\Row;
+use Drupal\Tests\migrate\Unit\MigrateTestCase;
+
+/**
+ * @coversDefaultClass \Drupal\file\Plugin\migrate\process\d6\FileUri
+ * @group file
+ */
+class FileUriTest extends MigrateTestCase {
+
+  protected $migrationConfiguration = [
+    'id' => 'test',
+  ];
+
+  public function testPublic() {
+    $value = [
+      'sites/default/files/foo.jpg',
+      'sites/default/files',
+      '/tmp',
+      TRUE,
+    ];
+    $this->assertEquals('public://foo.jpg', $this->doTransform($value));
+  }
+
+  public function testPublicUnknownBasePath() {
+    $value = [
+      '/path/to/public/files/foo.jpg',
+      'sites/default/files',
+      '/tmp',
+      TRUE,
+    ];
+    $this->assertEquals('public://path/to/public/files/foo.jpg', $this->doTransform($value));
+  }
+
+  public function testPrivate() {
+    $value = [
+      'sites/default/files/baz.gif',
+      'sites/default/files',
+      '/tmp',
+      FALSE,
+    ];
+    $this->assertEquals('private://baz.gif', $this->doTransform($value));
+  }
+
+  public function testPrivateUnknownBasePath() {
+    $value = [
+      '/path/to/private/files/baz.gif',
+      'sites/default/files',
+      '/tmp',
+      FALSE,
+    ];
+    $this->assertEquals('private://path/to/private/files/baz.gif', $this->doTransform($value));
+  }
+
+  public function testTemporary() {
+    $value = [
+      '/tmp/bar.png',
+      'sites/default/files',
+      '/tmp',
+      TRUE,
+    ];
+    $this->assertEquals('temporary://bar.png', $this->doTransform($value));
+  }
+
+  protected function doTransform(array $value) {
+    $executable = new MigrateExecutable($this->getMigration(), new MigrateMessage());
+    $row = new Row([], []);
+
+    return (new FileUri([], 'file_uri', []))
+      ->transform($value, $executable, $row, 'foobaz');
+  }
+
+}