diff --git a/core/modules/file/migration_templates/d6_file.yml b/core/modules/file/migration_templates/d6_file.yml index a4693cf..ba951a8 100644 --- a/core/modules/file/migration_templates/d6_file.yml +++ b/core/modules/file/migration_templates/d6_file.yml @@ -1,21 +1,40 @@ -# Every migration that saves into {file_managed} must have the d6_file -# migration as an optional dependency to ensure d6_file runs first. +# Every migration that references a file by fid should specify this migration +# as an optional dependency. id: d6_file label: Files migration_tags: - Drupal 6 source: plugin: d6_file + constants: + # source_base_path must be set by the tool configuring this migration. It + # represents the fully qualified path relative to which uris in the files + # table are specified, and must end with a /. + source_base_path: '' process: fid: fid filename: filename - uri: + source_full_path: + - + plugin: concat + delimiter: / + source: + - constants/source_base_path + - filepath + - + plugin: urlencode + destination_full_path: plugin: file_uri source: - filepath - file_directory_path - temp_directory_path - is_public + uri: + plugin: file_copy + source: + - '@source_full_path' + - '@destination_full_path' filemime: filemime filesize: filesize status: status @@ -23,4 +42,3 @@ process: uid: uid destination: plugin: entity:file - urlencode: true diff --git a/core/modules/file/migration_templates/d7_file.yml b/core/modules/file/migration_templates/d7_file.yml index b10cca6..abda991 100644 --- a/core/modules/file/migration_templates/d7_file.yml +++ b/core/modules/file/migration_templates/d7_file.yml @@ -6,10 +6,28 @@ migration_tags: - Drupal 7 source: plugin: d7_file + constants: + # source_base_path must be set by the tool configuring this migration. It + # represents the fully qualified path relative to which uris in the files + # table are specified, and must end with a /. + source_base_path: '' process: fid: fid filename: filename - uri: uri + source_full_path: + - + plugin: concat + delimiter: / + source: + - constants/source_base_path + - filepath + - + plugin: urlencode + uri: + plugin: file_copy + source: + - '@source_full_path' + - uri filemime: filemime # filesize is dynamically computed when file entities are saved, so there is # no point in migrating it. @@ -22,5 +40,3 @@ process: uid: uid destination: plugin: entity:file - source_path_property: filepath - urlencode: true diff --git a/core/modules/file/src/Plugin/migrate/destination/EntityFile.php b/core/modules/file/src/Plugin/migrate/destination/EntityFile.php index 900db6a..d234be2 100644 --- a/core/modules/file/src/Plugin/migrate/destination/EntityFile.php +++ b/core/modules/file/src/Plugin/migrate/destination/EntityFile.php @@ -3,23 +3,12 @@ namespace Drupal\file\Plugin\migrate\destination; use Drupal\Component\Utility\Unicode; -use Drupal\Core\Entity\EntityManagerInterface; -use Drupal\Core\Entity\EntityStorageInterface; -use Drupal\Core\Field\FieldTypePluginManagerInterface; use Drupal\Core\Field\Plugin\Field\FieldType\UriItem; -use Drupal\Core\File\FileSystemInterface; -use Drupal\Core\StreamWrapper\LocalStream; -use Drupal\Core\StreamWrapper\StreamWrapperManagerInterface; -use Drupal\migrate\Plugin\MigrationInterface; use Drupal\migrate\Row; use Drupal\migrate\MigrateException; use Drupal\migrate\Plugin\migrate\destination\EntityContentBase; -use Symfony\Component\DependencyInjection\ContainerInterface; /** - * Every migration that uses this destination must have an optional - * dependency on the d6_file migration to ensure it runs first. - * * @MigrateDestination( * id = "entity:file" * ) @@ -27,50 +16,9 @@ class EntityFile extends EntityContentBase { /** - * @var \Drupal\Core\StreamWrapper\StreamWrapperManagerInterface + * Column name of the file URI. */ - protected $streamWrapperManager; - - /** - * @var \Drupal\Core\File\FileSystemInterface - */ - protected $fileSystem; - - /** - * {@inheritdoc} - */ - public function __construct(array $configuration, $plugin_id, $plugin_definition, MigrationInterface $migration, EntityStorageInterface $storage, array $bundles, EntityManagerInterface $entity_manager, FieldTypePluginManagerInterface $field_type_manager, StreamWrapperManagerInterface $stream_wrappers, FileSystemInterface $file_system) { - $configuration += array( - 'source_base_path' => '', - 'source_path_property' => 'filepath', - 'destination_path_property' => 'uri', - 'move' => FALSE, - 'urlencode' => FALSE, - ); - parent::__construct($configuration, $plugin_id, $plugin_definition, $migration, $storage, $bundles, $entity_manager, $field_type_manager); - - $this->streamWrapperManager = $stream_wrappers; - $this->fileSystem = $file_system; - } - - /** - * {@inheritdoc} - */ - public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition, MigrationInterface $migration = NULL) { - $entity_type = static::getEntityTypeId($plugin_id); - return new static( - $configuration, - $plugin_id, - $plugin_definition, - $migration, - $container->get('entity.manager')->getStorage($entity_type), - array_keys($container->get('entity.manager')->getBundleInfo($entity_type)), - $container->get('entity.manager'), - $container->get('plugin.manager.field.field_type'), - $container->get('stream_wrapper_manager'), - $container->get('file_system') - ); - } + const URI_COLUMN = 'uri'; /** * {@inheritdoc} @@ -82,8 +30,10 @@ protected function getEntity(Row $row, array $old_destination_id_values) { return parent::getEntity($row, $old_destination_id_values); } - $destination = $row->getDestinationProperty($this->configuration['destination_path_property']); - $entity = $this->storage->loadByProperties(['uri' => $destination]); + // By default the entity key (fid) would be used, but we want to make sure + // we're loading the matching uri. + $destination = $row->getDestinationProperty(static::URI_COLUMN); + $entity = $this->storage->loadByProperties([static::URI_COLUMN => $destination]); if ($entity) { return reset($entity); } @@ -95,188 +45,13 @@ protected function getEntity(Row $row, array $old_destination_id_values) { /** * {@inheritdoc} */ - public function import(Row $row, array $old_destination_id_values = array()) { - // For stub rows, there is no real file to deal with, let the stubbing - // process create the stub entity. - if ($row->isStub()) { - return parent::import($row, $old_destination_id_values); - } - - $file = $row->getSourceProperty($this->configuration['source_path_property']); - $destination = $row->getDestinationProperty($this->configuration['destination_path_property']); - $source = $this->configuration['source_base_path'] . $file; - - // Ensure the source file exists, if it's a local URI or path. - if ($this->isLocalUri($source) && !file_exists($source)) { - throw new MigrateException("File '$source' does not exist."); - } - - // If the start and end file is exactly the same, there is nothing to do. - if ($this->isLocationUnchanged($source, $destination)) { - return parent::import($row, $old_destination_id_values); - } - - $replace = $this->getOverwriteMode($row); - $success = $this->writeFile($source, $destination, $replace); - if (!$success) { - $dir = $this->getDirectory($destination); - if (file_prepare_directory($dir, FILE_CREATE_DIRECTORY)) { - $success = $this->writeFile($source, $destination, $replace); - } - else { - throw new MigrateException("Could not create directory '$dir'"); - } - } - - if ($success) { - return parent::import($row, $old_destination_id_values); - } - else { - throw new MigrateException("File $source could not be copied to $destination."); - } - } - - /** - * Tries to move or copy a file. - * - * @param string $source - * The source path or URI. - * @param string $destination - * The destination path or URI. - * @param int $replace - * (optional) FILE_EXISTS_REPLACE (default) or FILE_EXISTS_RENAME. - * - * @return bool - * TRUE on success, FALSE on failure. - */ - protected function writeFile($source, $destination, $replace = FILE_EXISTS_REPLACE) { - if ($this->configuration['move']) { - return (boolean) file_unmanaged_move($source, $destination, $replace); - } - else { - $destination = file_destination($destination, $replace); - $source = $this->urlencode($source); - return @copy($source, $destination); - } - } - - /** - * Determines how to handle file conflicts. - * - * @param \Drupal\migrate\Row $row - * - * @return int - * Either FILE_EXISTS_REPLACE (default) or FILE_EXISTS_RENAME, depending - * on the current configuration. - */ - protected function getOverwriteMode(Row $row) { - if (!empty($this->configuration['rename'])) { - $entity_id = $row->getDestinationProperty($this->getKey('id')); - if ($entity_id && ($entity = $this->storage->load($entity_id))) { - return FILE_EXISTS_RENAME; - } - } - return FILE_EXISTS_REPLACE; - } - - /** - * Returns the directory component of a URI or path. - * - * For URIs like public://foo.txt, the full physical path of public:// - * will be returned, since a scheme by itself will trip up certain file - * API functions (such as file_prepare_directory()). - * - * @param string $uri - * The URI or path. - * - * @return string|false - * The directory component of the path or URI, or FALSE if it could not - * be determined. - */ - protected function getDirectory($uri) { - $dir = $this->fileSystem->dirname($uri); - if (substr($dir, -3) == '://') { - return $this->fileSystem->realpath($dir); - } - else { - return $dir; - } - } - - /** - * Returns if the source and destination URIs represent identical paths. - * If either URI is a remote stream, will return FALSE. - * - * @param string $source - * The source URI. - * @param string $destination - * The destination URI. - * - * @return bool - * TRUE if the source and destination URIs refer to the same physical path, - * otherwise FALSE. - */ - protected function isLocationUnchanged($source, $destination) { - if ($this->isLocalUri($source) && $this->isLocalUri($destination)) { - return $this->fileSystem->realpath($source) === $this->fileSystem->realpath($destination); - } - else { - return FALSE; - } - } - - /** - * Returns if the given URI or path is considered local. - * - * A URI or path is considered local if it either has no scheme component, - * or the scheme is implemented by a stream wrapper which extends - * \Drupal\Core\StreamWrapper\LocalStream. - * - * @param string $uri - * The URI or path to test. - * - * @return bool - */ - protected function isLocalUri($uri) { - $scheme = $this->fileSystem->uriScheme($uri); - return $scheme === FALSE || $this->streamWrapperManager->getViaScheme($scheme) instanceof LocalStream; - } - - /** - * Urlencode all the components of a remote filename. - * - * @param string $filename - * The filename of the file to be urlencoded. - * - * @return string - * The urlencoded filename. - */ - protected function urlencode($filename) { - // Only apply to a full URL - if ($this->configuration['urlencode'] && strpos($filename, '://')) { - $components = explode('/', $filename); - foreach ($components as $key => $component) { - $components[$key] = rawurlencode($component); - } - $filename = implode('/', $components); - // Actually, we don't want certain characters encoded - $filename = str_replace('%3A', ':', $filename); - $filename = str_replace('%3F', '?', $filename); - $filename = str_replace('%26', '&', $filename); - } - return $filename; - } - - /** - * {@inheritdoc} - */ protected function processStubRow(Row $row) { // We stub the uri value ourselves so we can create a real stub file for it. - if (!$row->getDestinationProperty('uri')) { + if (!$row->getDestinationProperty(static::URI_COLUMN)) { $field_definitions = $this->entityManager ->getFieldDefinitions($this->storage->getEntityTypeId(), $this->getKey('bundle')); - $value = UriItem::generateSampleValue($field_definitions['uri']); + $value = UriItem::generateSampleValue($field_definitions[static::URI_COLUMN]); if (empty($value)) { throw new MigrateException('Stubbing failed, unable to generate value for field uri'); } @@ -285,10 +60,10 @@ protected function processStubRow(Row $row) { // Make it into a proper public file uri, stripping off the existing // scheme if present. $value = 'public://' . preg_replace('|^[a-z]+://|i', '', $value); - $value = Unicode::substr($value, 0, $field_definitions['uri']->getSetting('max_length')); + $value = Unicode::substr($value, 0, $field_definitions[static::URI_COLUMN]->getSetting('max_length')); // Create a real file, so File::preSave() can do filesize() on it. touch($value); - $row->setDestinationProperty('uri', $value); + $row->setDestinationProperty(static::URI_COLUMN, $value); } parent::processStubRow($row); } diff --git a/core/modules/file/src/Plugin/migrate/process/FileCopy.php b/core/modules/file/src/Plugin/migrate/process/FileCopy.php new file mode 100644 index 0000000..9cee5ae --- /dev/null +++ b/core/modules/file/src/Plugin/migrate/process/FileCopy.php @@ -0,0 +1,209 @@ + FALSE, + 'rename' => FALSE, + ); + parent::__construct($configuration, $plugin_id, $plugin_definition); + $this->streamWrapperManager = $stream_wrappers; + $this->fileSystem = $file_system; + } + + /** + * {@inheritdoc} + */ + public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) { + return new static( + $configuration, + $plugin_id, + $plugin_definition, + $container->get('stream_wrapper_manager'), + $container->get('file_system') + ); + } + + /** + * {@inheritdoc} + */ + public function transform($value, MigrateExecutableInterface $migrate_executable, Row $row, $destination_property) { + // If we're stubbing a file entity, return a uri of NULL so it will get + // stubbed by the general process. + if ($row->isStub()) { + return NULL; + } + list($source, $destination) = $value; + + // Ensure the source file exists, if it's a local URI or path. + if ($this->isLocalUri($source) && !file_exists($source)) { + throw new MigrateException("File '$source' does not exist."); + } + + // If the start and end file is exactly the same, there is nothing to do. + if ($this->isLocationUnchanged($source, $destination)) { + return $destination; + } + + $replace = $this->getOverwriteMode(); + // We attempt the copy first to avoid calling file_prepare_directory() any + // more than absolutely necessary. + if ($this->writeFile($source, $destination, $replace)) { + return $destination; + } + $dir = $this->getDirectory($destination); + if (!file_prepare_directory($dir, FILE_CREATE_DIRECTORY)) { + throw new MigrateException("Could not create directory '$dir'"); + } + if ($this->writeFile($source, $destination, $replace)) { + return $destination; + } + throw new MigrateException("File $source could not be copied to $destination."); + } + + /** + * Tries to move or copy a file. + * + * @param string $source + * The source path or URI. + * @param string $destination + * The destination path or URI. + * @param int $replace + * (optional) FILE_EXISTS_REPLACE (default) or FILE_EXISTS_RENAME. + * + * @return bool + * TRUE on success, FALSE on failure. + */ + protected function writeFile($source, $destination, $replace = FILE_EXISTS_REPLACE) { + if ($this->configuration['move']) { + return (boolean) file_unmanaged_move($source, $destination, $replace); + } + $destination = file_destination($destination, $replace); + return @copy($source, $destination); + } + + /** + * Determines how to handle file conflicts. + * + * @return int + * Either FILE_EXISTS_REPLACE (default) or FILE_EXISTS_RENAME, depending + * on the current configuration. + */ + protected function getOverwriteMode() { + if (!empty($this->configuration['rename'])) { + return FILE_EXISTS_RENAME; + } + return FILE_EXISTS_REPLACE; + } + + /** + * Returns the directory component of a URI or path. + * + * For URIs like public://foo.txt, the full physical path of public:// + * will be returned, since a scheme by itself will trip up certain file + * API functions (such as file_prepare_directory()). + * + * @param string $uri + * The URI or path. + * + * @return string|false + * The directory component of the path or URI, or FALSE if it could not + * be determined. + */ + protected function getDirectory($uri) { + $dir = $this->fileSystem->dirname($uri); + if (substr($dir, -3) == '://') { + return $this->fileSystem->realpath($dir); + } + return $dir; + } + + /** + * Determines if the source and destination URIs represent identical paths. + * + * If either URI is a remote stream, will return FALSE. + * + * @param string $source + * The source URI. + * @param string $destination + * The destination URI. + * + * @return bool + * TRUE if the source and destination URIs refer to the same physical path, + * otherwise FALSE. + */ + protected function isLocationUnchanged($source, $destination) { + if ($this->isLocalUri($source) && $this->isLocalUri($destination)) { + return $this->fileSystem->realpath($source) === $this->fileSystem->realpath($destination); + } + else { + return FALSE; + } + } + + /** + * Determines if the given URI or path is considered local. + * + * A URI or path is considered local if it either has no scheme component, + * or the scheme is implemented by a stream wrapper which extends + * \Drupal\Core\StreamWrapper\LocalStream. + * + * @param string $uri + * The URI or path to test. + * + * @return bool + */ + protected function isLocalUri($uri) { + $scheme = $this->fileSystem->uriScheme($uri); + return $scheme === FALSE || $this->streamWrapperManager->getViaScheme($scheme) instanceof LocalStream; + } + +} diff --git a/core/modules/file/src/Plugin/migrate/process/UrlEncode.php b/core/modules/file/src/Plugin/migrate/process/UrlEncode.php new file mode 100644 index 0000000..f137130 --- /dev/null +++ b/core/modules/file/src/Plugin/migrate/process/UrlEncode.php @@ -0,0 +1,36 @@ + 0) { + $components = explode('/', $value); + foreach ($components as $key => $component) { + // urlencode() would convert spaces to + signs. + $components[$key] = rawurlencode($component); + } + $value = implode('/', $components); + // Actually, we don't want certain characters encoded. + $value = str_replace(['%3A', '%3F', '%26'], [':', '?', '&'], $value); + } + return $value; + } + +} diff --git a/core/modules/file/src/Plugin/migrate/source/d7/File.php b/core/modules/file/src/Plugin/migrate/source/d7/File.php index db55a65..247a6cf 100644 --- a/core/modules/file/src/Plugin/migrate/source/d7/File.php +++ b/core/modules/file/src/Plugin/migrate/source/d7/File.php @@ -84,9 +84,7 @@ public function prepareRow(Row $row) { // At this point, $path could be an absolute path or a relative path, // depending on how the scheme's variable was set. So we need to shear out // the source_base_path in order to make them all relative. - // @todo https://www.drupal.org/node/2577871 Don't depend on destination - // configuration and figure out if this is even needed at all? - $path = str_replace($this->migration->getDestinationConfiguration()['source_base_path'], NULL, $path); + $path = str_replace($this->configuration['constants']['source_base_path'], NULL, $path); $row->setSourceProperty('filepath', $path); return parent::prepareRow($row); } diff --git a/core/modules/file/tests/src/Kernel/Migrate/CopyFileProcessPluginTest.php b/core/modules/file/tests/src/Kernel/Migrate/CopyFileProcessPluginTest.php new file mode 100644 index 0000000..3a76bb2 --- /dev/null +++ b/core/modules/file/tests/src/Kernel/Migrate/CopyFileProcessPluginTest.php @@ -0,0 +1,148 @@ +container->get('stream_wrapper_manager')->registerWrapper('public', 'Drupal\Core\StreamWrapper\PublicStream', StreamWrapperInterface::NORMAL); + } + + /** + * Test successful imports/copies. + */ + public function testSuccessfulCopies() { + foreach ($this->localFileDataCopyProvider() as $data) { + list($source_path, $destination_path, $expected) = $data; + $this->doImport($source_path, $destination_path); + $message = $expected ? sprintf('File %s exists', $destination_path) : sprintf('File %s does not exist', $destination_path); + $this->assertSame($expected, is_file($destination_path), $message); + // Make sure we didn't accidentally do a move. + $message = $expected ? sprintf('File %s exists', $source_path) : sprintf('File %s does not exist', $source_path); + $this->assertSame($expected, is_file($source_path), $message); + } + } + + /** + * The data provider for testing the file destination. + * + * @return array + * An array of file permutations to test. + */ + protected function localFileDataCopyProvider() { + touch('/tmp/test-file.jpg'); + return [ + // Test a local to local copy. + [$this->root . '/core/modules/simpletest/files/image-test.jpg', 'public://file1.jpg', TRUE], + // Test a temporary file using an absolute path. + ['/tmp/test-file.jpg', 'temporary://test.jpg', TRUE], + // Test a temporary file using a relative path. + ['/tmp/test-file.jpg', 'temporary://core/modules/simpletest/files/test.jpg', TRUE], + ]; + } + + /** + * Test successful moves. + */ + public function testSuccessfulMoves() { + foreach ($this->localFileDataMoveProvider() as $data) { + list($source_path, $destination_path, $expected) = $data; + $this->doImport($source_path, $destination_path, ['move' => TRUE]); + $message = $expected ? sprintf('File %s exists', $destination_path) : sprintf('File %s does not exist', $destination_path); + $this->assertSame($expected, is_file($destination_path), $message); + $message = $expected ? sprintf('File %s does not exist', $source_path) : sprintf('File %s exists', $source_path); + $this->assertSame($expected, !is_file($source_path), $message); + } + } + + /** + * The data provider for testing the file destination. + * + * @return array + * An array of file permutations to test. + */ + protected function localFileDataMoveProvider() { + touch('/tmp/test-file.jpg'); + touch('/tmp/test-file2.jpg'); + $local_file = $this->root . '/sites/default/files/source_file.txt'; + touch($local_file); + return [ + // Test a local to local copy. + [$local_file, 'public://file1.jpg', TRUE], + // Test a temporary file using an absolute path. + ['/tmp/test-file.jpg', 'temporary://test.jpg', TRUE], + // Test a temporary file using a relative path. + ['/tmp/test-file2.jpg', 'temporary://core/modules/simpletest/files/test.jpg', TRUE], + ]; + } + + /** + * Test that non-existent files throw an exception. + * + * @expectedException \Drupal\migrate\MigrateException + * + * @expectedExceptionMessage File '/non/existent/file' does not exist + */ + public function testNonExistentSourceFile() { + $source = '/non/existent/file'; + $this->doImport($source, 'public://wontmatter.jpg'); + } + + /** + * Test the 'rename' overwrite mode. + */ + public function testRenameFile() { + $source = 'temporary://baz.txt'; + $destination = 'public://foo.txt'; + $expected_destination = 'public://foo_0.txt'; + touch($source); + touch($destination); + $this->doImport($source, $destination, ['rename' => TRUE]); + $this->assertFileExists($expected_destination); + } + + /** + * Do an import using the destination. + * + * @param string $source_path + * Source path to copy from. + * @param string $destination_path + * The destination path to copy to. + * @param array $configuration + * Process plugin configuration settings. + * + * @throws \Drupal\migrate\MigrateException + */ + protected function doImport($source_path, $destination_path, $configuration = []) { + $this->plugin = FileCopy::create($this->container, $configuration, 'file_copy', []); + $executable = $this->prophesize(MigrateExecutableInterface::class)->reveal(); + $row = new Row([], []); + + $result = $this->plugin->transform([$source_path, $destination_path], $executable, $row, 'foobaz'); + + // The plugin should either throw an exception or return the destination + // path. + $this->assertSame($result, $destination_path); + } + +} diff --git a/core/modules/file/tests/src/Kernel/Migrate/EntityFileTest.php b/core/modules/file/tests/src/Kernel/Migrate/EntityFileTest.php deleted file mode 100644 index 90d968d..0000000 --- a/core/modules/file/tests/src/Kernel/Migrate/EntityFileTest.php +++ /dev/null @@ -1,278 +0,0 @@ -get('stream_wrapper_manager')->registerWrapper('public', 'Drupal\Core\StreamWrapper\PublicStream', StreamWrapperInterface::NORMAL); - $this->destination = new TestEntityFile([]); - $this->installEntitySchema('file'); - - file_put_contents('/tmp/test-file.jpg', ''); - } - - /** - * Test successful imports/copies. - */ - public function testSuccessfulCopies() { - foreach ($this->localFileDataProvider() as $data) { - list($row_values, $destination_path, $expected, $source_base_path) = $data; - - $this->doImport($row_values, $destination_path, $source_base_path); - $message = $expected ? sprintf('File %s exists', $destination_path) : sprintf('File %s does not exist', $destination_path); - $this->assertIdentical($expected, is_file($destination_path), $message); - } - } - - /** - * The data provider for testing the file destination. - * - * @return array - * An array of file permutations to test. - */ - protected function localFileDataProvider() { - return [ - // Test a local to local copy. - [['filepath' => 'core/modules/simpletest/files/image-test.jpg'], 'public://file1.jpg', TRUE, $this->root . '/'], - // Test a temporary file using an absolute path. - [['filepath' => '/tmp/test-file.jpg'], 'temporary://test.jpg', TRUE, ''], - // Test a temporary file using a relative path. - [['filepath' => 'test-file.jpg'], 'temporary://core/modules/simpletest/files/test.jpg', TRUE, '/tmp/'], - // Test a remote path to local. - [['filepath' => 'core/modules/simpletest/files/image-test.jpg'], 'public://remote-file.jpg', TRUE, $this->root . '/'], - // Test a remote path to local inside a folder that doesn't exist. - [['filepath' => 'core/modules/simpletest/files/image-test.jpg'], 'public://folder/remote-file.jpg', TRUE, $this->root . '/'], - ]; - } - - /** - * Test that non-existent files throw an exception. - */ - public function testNonExistentSourceFile() { - $destination = '/non/existent/file'; - try { - // If this test passes, doImport() will raise a MigrateException and - // we'll never reach fail(). - $this->doImport(['filepath' => $destination], 'public://wontmatter.jpg'); - $this->fail('Expected Drupal\migrate\MigrateException when importing ' . $destination); - } - catch (MigrateException $e) { - $this->assertIdentical($e->getMessage(), "File '$destination' does not exist."); - } - } - - /** - * Tests various invocations of the writeFile() method. - */ - public function testWriteFile() { - $plugin = $this->destination; - $method = new \ReflectionMethod($plugin, 'writeFile'); - $method->setAccessible(TRUE); - - touch('temporary://baz.txt'); - - // Moving an actual file should return TRUE. - $plugin->configuration['move'] = TRUE; - $this->assertTrue($method->invoke($plugin, 'temporary://baz.txt', 'public://foo.txt')); - - // Trying to move a non-existent file should return FALSE. - $this->assertFalse($method->invoke($plugin, 'temporary://invalid.txt', 'public://invalid.txt')); - - // Copying over a file that already exists should replace the existing file. - $plugin->configuration['move'] = FALSE; - touch('temporary://baz.txt'); - $this->assertTrue($method->invoke($plugin, 'temporary://baz.txt', 'public://foo.txt')); - // Copying over a file that already exists should rename the resulting file - // if FILE_EXISTS_RENAME is specified. - $method->invoke($plugin, 'temporary://baz.txt', 'public://foo.txt', FILE_EXISTS_RENAME); - $this->assertTrue(file_exists('public://foo_0.txt')); - - // Trying to copy a non-existent file should return FALSE. - $this->assertFalse($method->invoke($plugin, 'temporary://invalid.txt', 'public://invalid.txt')); - } - - /** - * Tests various invocations of the getOverwriteMode() method. - */ - public function testGetOverwriteMode() { - $plugin = $this->destination; - $method = new \ReflectionMethod($plugin, 'getOverwriteMode'); - $method->setAccessible(TRUE); - - $row = new Row([], []); - // If the plugin is not configured to rename the destination file, we should - // always get FILE_EXISTS_REPLACE. - $this->assertIdentical(FILE_EXISTS_REPLACE, $method->invoke($plugin, $row)); - - // When the plugin IS configured to rename the destination file, it should - // return FILE_EXISTS_RENAME if the destination entity already exists, - // and FILE_EXISTS_REPLACE otherwise. - $plugin->configuration['rename'] = TRUE; - $plugin->storage = \Drupal::entityManager()->getStorage('file'); - /** @var \Drupal\file\FileInterface $file */ - $file = $plugin->storage->create(); - touch('public://foo.txt'); - $file->setFileUri('public://foo.txt'); - $file->save(); - $row->setDestinationProperty($plugin->storage->getEntityType()->getKey('id'), $file->id()); - $this->assertIdentical(FILE_EXISTS_RENAME, $method->invoke($plugin, $row)); - unlink('public://foo.txt'); - } - - /** - * Tests various invocations of the getDirectory() method. - */ - public function testGetDirectory() { - $plugin = $this->destination; - $method = new \ReflectionMethod($plugin, 'getDirectory'); - $method->setAccessible(TRUE); - - $this->assertSame('public://foo', $method->invoke($plugin, 'public://foo/baz.txt')); - $this->assertSame('/path/to', $method->invoke($plugin, '/path/to/foo.txt')); - // A directory like public:// (no path) needs to resolve to a physical path. - $fs = \Drupal::getContainer()->get('file_system'); - $this->assertSame($fs->realpath('public://'), $method->invoke($plugin, 'public://foo.txt')); - } - - /** - * Tests various invocations of the isLocationUnchanged() method. - */ - public function testIsLocationUnchanged() { - $plugin = $this->destination; - $method = new \ReflectionMethod($plugin, 'isLocationUnchanged'); - $method->setAccessible(TRUE); - - $temporary_file = '/tmp/foo.txt'; - touch($temporary_file); - $this->assertTrue($method->invoke($plugin, $temporary_file, 'temporary://foo.txt')); - unlink($temporary_file); - } - - /** - * Tests various invocations of the isLocalUri() method. - */ - public function testIsLocalUri() { - $plugin = $this->destination; - $method = new \ReflectionMethod($plugin, 'isLocalUri'); - $method->setAccessible(TRUE); - - $this->assertTrue($method->invoke($plugin, 'public://foo.txt')); - $this->assertTrue($method->invoke($plugin, 'public://path/to/foo.txt')); - $this->assertTrue($method->invoke($plugin, 'temporary://foo.txt')); - $this->assertTrue($method->invoke($plugin, 'temporary://path/to/foo.txt')); - $this->assertTrue($method->invoke($plugin, 'foo.txt')); - $this->assertTrue($method->invoke($plugin, '/path/to/files/foo.txt')); - $this->assertTrue($method->invoke($plugin, 'relative/path/to/foo.txt')); - $this->assertFalse($method->invoke($plugin, 'http://www.example.com/foo.txt')); - } - - /** - * Do an import using the destination. - * - * @param array $row_values - * An array of row values. - * @param string $destination_path - * The destination path to copy to. - * @param string $source_base_path - * The source base path. - * @return array - * An array of saved entities ids. - * - * @throws \Drupal\migrate\MigrateException - */ - protected function doImport($row_values, $destination_path, $source_base_path = '') { - $row = new Row($row_values, []); - $row->setDestinationProperty('uri', $destination_path); - $this->destination->configuration['source_base_path'] = $source_base_path; - - // Importing asserts there are no errors, then we just check the file has - // been copied into place. - return $this->destination->import($row, array()); - } - -} - -class TestEntityFile extends EntityFile { - - /** - * This is needed to be passed to $this->save(). - * - * @var \Drupal\Core\Entity\ContentEntityInterface - */ - public $mockEntity; - - /** - * Make this public for easy writing during tests. - * - * @var array - */ - public $configuration; - - /** - * @var \Drupal\Core\Entity\EntityStorageInterface - */ - public $storage; - - public function __construct($configuration = []) { - $configuration += array( - 'source_base_path' => '', - 'source_path_property' => 'filepath', - 'destination_path_property' => 'uri', - 'move' => FALSE, - 'urlencode' => FALSE, - ); - $this->configuration = $configuration; - // We need a mock entity to be passed to save to prevent strict exceptions. - $this->mockEntity = EntityTest::create(); - $this->streamWrapperManager = \Drupal::getContainer()->get('stream_wrapper_manager'); - $this->fileSystem = \Drupal::getContainer()->get('file_system'); - } - - /** - * {@inheritdoc} - */ - protected function getEntity(Row $row, array $old_destination_id_values) { - return $this->mockEntity; - } - - /** - * {@inheritdoc} - */ - protected function save(ContentEntityInterface $entity, array $old_destination_id_values = array()) {} - -} diff --git a/core/modules/file/tests/src/Kernel/Migrate/d6/FileMigrationTestTrait.php b/core/modules/file/tests/src/Kernel/Migrate/d6/FileMigrationTestTrait.php index ef713c8..23bacf7 100644 --- a/core/modules/file/tests/src/Kernel/Migrate/d6/FileMigrationTestTrait.php +++ b/core/modules/file/tests/src/Kernel/Migrate/d6/FileMigrationTestTrait.php @@ -14,15 +14,23 @@ protected function setUpMigratedFiles() { $this->installEntitySchema('file'); $this->installConfig(['file']); - /** @var \Drupal\migrate\Plugin\MigrationInterface $migration */ - $migration_plugin_manager = $this->container->get('plugin.manager.migration'); + $this->executeMigration('d6_file'); + } - /** @var \Drupal\migrate\Plugin\migration $migration */ - $migration = $migration_plugin_manager->createInstance('d6_file'); - $source = $migration->getSourceConfiguration(); - $source['site_path'] = 'core/modules/simpletest'; - $migration->set('source', $source); - $this->executeMigration($migration); + /** + * {@inheritdoc} + */ + protected function processMigration($migration) { + // File migrations need a source_base_path. + // @see MigrateUpgradeRunBatch::run + $destination = $migration->getDestinationConfiguration(); + if ($destination['plugin'] === 'entity:file') { + // Make sure we have a single trailing slash. + $source = $migration->getSourceConfiguration(); + $source['site_path'] = 'core/modules/simpletest'; + $source['constants']['source_base_path'] = DRUPAL_ROOT . '/'; + $migration->set('source', $source); + } } } diff --git a/core/modules/file/tests/src/Kernel/Migrate/d6/MigrateFileTest.php b/core/modules/file/tests/src/Kernel/Migrate/d6/MigrateFileTest.php index 887fb73..7e1cff9 100644 --- a/core/modules/file/tests/src/Kernel/Migrate/d6/MigrateFileTest.php +++ b/core/modules/file/tests/src/Kernel/Migrate/d6/MigrateFileTest.php @@ -72,9 +72,8 @@ public function testFiles() { $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. - $migration_plugin_manager = $this->container->get('plugin.manager.migration'); \Drupal::database() - ->truncate($migration_plugin_manager->createInstance('d6_file')->getIdMap()->mapTableName()) + ->truncate($this->getMigration('d6_file')->getIdMap()->mapTableName()) ->execute(); // Update the file_directory_path. @@ -89,16 +88,11 @@ public function testFiles() { ->condition('name', 'file_directory_temp') ->execute(); - $migration = $migration_plugin_manager->createInstance('d6_file'); - $this->executeMigration($migration); + $this->executeMigration('d6_file'); $file = File::load(2); $this->assertIdentical('public://core/modules/simpletest/files/image-2.jpg', $file->getFileUri()); - // 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)); diff --git a/core/modules/file/tests/src/Kernel/Migrate/d7/MigrateFileTest.php b/core/modules/file/tests/src/Kernel/Migrate/d7/MigrateFileTest.php index 9750da3..240c9e0 100644 --- a/core/modules/file/tests/src/Kernel/Migrate/d7/MigrateFileTest.php +++ b/core/modules/file/tests/src/Kernel/Migrate/d7/MigrateFileTest.php @@ -33,17 +33,11 @@ protected function setUp() { /** @var \Drupal\migrate\Plugin\Migration $migration */ $migration = $this->getMigration('d7_file'); - // Set the destination plugin's source_base_path configuration value, which + // Set the source plugin's source_base_path configuration value, which // would normally be set by the user running the migration. - $migration->set('destination', [ - 'plugin' => 'entity:file', - // Note that source_base_path must include a trailing slash because it's - // prepended directly to the value of the source path property. - 'source_base_path' => $fs->realpath('public://') . '/', - // This is set in the migration's YAML file, but we need to repeat it - // here because all the destination configuration must be set at once. - 'source_path_property' => 'filepath', - ]); + $source = $migration->getSourceConfiguration(); + $source['constants']['source_base_path'] = $fs->realpath('public://'); + $migration->set('source', $source); $this->executeMigration($migration); } diff --git a/core/modules/file/tests/src/Unit/Plugin/migrate/process/UrlEncodeTest.php b/core/modules/file/tests/src/Unit/Plugin/migrate/process/UrlEncodeTest.php new file mode 100644 index 0000000..39a6ba4 --- /dev/null +++ b/core/modules/file/tests/src/Unit/Plugin/migrate/process/UrlEncodeTest.php @@ -0,0 +1,59 @@ + 'test', + ]; + + /** + * Cover various encoding scenarios. + */ + public function testUrls() { + $values = [ + // A URL with no characters requiring encoding. + 'http://example.com/normal_url.html' => 'http://example.com/normal_url.html', + // The definitive use case - encoding spaces in URLs. + 'http://example.com/url with spaces.html' => 'http://example.com/url%20with%20spaces.html', + // Local filespecs should not be transformed, with or without spaces. + '/tmp/normal.txt' => '/tmp/normal.txt', + '/tmp/with spaces.txt' => '/tmp/with spaces.txt', + // Make sure URL characters (:, ?, &) are not encoded but others are. + 'https://example.com/?a=b@c&d=e+f%' => 'https://example.com/?a%3Db%40c&d%3De%2Bf%25', + ]; + + foreach ($values as $input => $output) { + $this->assertEquals($output, $this->doTransform($input)); + } + } + + /** + * Perform the urlencode process plugin over the given value. + * + * @param string $value + * Url to be encoded. + * + * @return string + * Encoded url. + */ + protected function doTransform($value) { + $executable = new MigrateExecutable($this->getMigration(), new MigrateMessage()); + $row = new Row([], []); + + return (new UrlEncode([], 'urlencode', [])) + ->transform($value, $executable, $row, 'foobaz'); + } + +} diff --git a/core/modules/file/tests/src/Unit/Plugin/migrate/source/d7/FileTest.php b/core/modules/file/tests/src/Unit/Plugin/migrate/source/d7/FileTest.php index 13659d5..6a97e73 100644 --- a/core/modules/file/tests/src/Unit/Plugin/migrate/source/d7/FileTest.php +++ b/core/modules/file/tests/src/Unit/Plugin/migrate/source/d7/FileTest.php @@ -25,6 +25,9 @@ class FileTest extends MigrateSqlSourceTestCase { 'id' => 'test', 'source' => array( 'plugin' => 'd7_file', + 'constants' => array( + 'source_base_path' => '/path/to/files', + ), // Used by testFilteringByScheme(). 'scheme' => array( 'public', @@ -33,7 +36,6 @@ class FileTest extends MigrateSqlSourceTestCase { ), 'destination' => array( 'plugin' => 'entity:file', - 'source_base_path' => '/path/to/files', ), ); diff --git a/core/modules/migrate/tests/src/Kernel/MigrateTestBase.php b/core/modules/migrate/tests/src/Kernel/MigrateTestBase.php index 4ba4d2b..d18d6e1 100644 --- a/core/modules/migrate/tests/src/Kernel/MigrateTestBase.php +++ b/core/modules/migrate/tests/src/Kernel/MigrateTestBase.php @@ -137,6 +137,16 @@ protected function prepareMigrations(array $id_mappings) { } /** + * Modify a migration's configuration before executing it. + * + * @param \Drupal\migrate\Plugin\MigrationInterface $migration + * The migration to execute. + */ + protected function processMigration($migration) { + // Default implementation for test classes not requiring modification. + } + + /** * Executes a single migration. * * @param string|\Drupal\migrate\Plugin\MigrationInterface $migration @@ -152,6 +162,8 @@ protected function executeMigration($migration) { if ($this instanceof MigrateDumpAlterInterface) { static::migrateDumpAlter($this); } + + $this->processMigration($this->migration); (new MigrateExecutable($this->migration, $this))->import(); } diff --git a/core/modules/migrate_drupal_ui/src/MigrateUpgradeRunBatch.php b/core/modules/migrate_drupal_ui/src/MigrateUpgradeRunBatch.php index 2916347..6ad8957 100644 --- a/core/modules/migrate_drupal_ui/src/MigrateUpgradeRunBatch.php +++ b/core/modules/migrate_drupal_ui/src/MigrateUpgradeRunBatch.php @@ -111,8 +111,9 @@ public static function run($initial_ids, $operation, $config, &$context) { if ($destination['plugin'] === 'entity:file') { // Make sure we have a single trailing slash. $source_base_path = rtrim($config['source_base_path'], '/') . '/'; - $destination['source_base_path'] = $source_base_path; - $migration->set('destination', $destination); + $source = $migration->getSourceConfiguration(); + $source['constants']['source_base_path'] = $source_base_path; + $migration->set('source', $source); } if ($migration) { diff --git a/core/modules/user/migration_templates/d6_user_picture_file.yml b/core/modules/user/migration_templates/d6_user_picture_file.yml index e4d572a..3677373 100644 --- a/core/modules/user/migration_templates/d6_user_picture_file.yml +++ b/core/modules/user/migration_templates/d6_user_picture_file.yml @@ -6,21 +6,39 @@ source: plugin: d6_user_picture_file constants: is_public: true + # source_base_path must be set by the tool configuring this migration. It + # represents the fully qualified path relative to which uris in the files + # table are specified, and must end with a /. + source_base_path: '' process: filename: filename uid: uid - uri: + source_full_path: + - + plugin: concat + delimiter: / + source: + - constants/source_base_path + - picture + - + plugin: urlencode + destination_full_path: plugin: file_uri source: - picture - file_directory_path - temp_directory_path - 'constants/is_public' + uri: + plugin: file_copy + source: + - '@source_full_path' + - '@destination_full_path' destination: plugin: entity:file source_path_property: picture migration_dependencies: - # Every migration that saves into {file_managed} must have the d6_file - # migration as an optional dependency to ensure it runs first. + # Every migration that references a file by fid should specify d6_file as an + # optional dependency. optional: - d6_file diff --git a/core/modules/user/tests/src/Kernel/Migrate/d6/MigrateUserPictureFileTest.php b/core/modules/user/tests/src/Kernel/Migrate/d6/MigrateUserPictureFileTest.php index 652e575..b3b6a64 100644 --- a/core/modules/user/tests/src/Kernel/Migrate/d6/MigrateUserPictureFileTest.php +++ b/core/modules/user/tests/src/Kernel/Migrate/d6/MigrateUserPictureFileTest.php @@ -3,6 +3,7 @@ namespace Drupal\Tests\user\Kernel\Migrate\d6; use Drupal\file\Entity\File; +use Drupal\Tests\file\Kernel\Migrate\d6\FileMigrationTestTrait; use Drupal\Tests\migrate_drupal\Kernel\d6\MigrateDrupal6TestBase; /** @@ -12,6 +13,8 @@ */ class MigrateUserPictureFileTest extends MigrateDrupal6TestBase { + use FileMigrationTestTrait; + /** * {@inheritdoc} */ @@ -19,13 +22,7 @@ protected function setUp() { parent::setUp(); $this->installEntitySchema('file'); - - /** @var \Drupal\migrate\Plugin\MigrationInterface $migration */ - $migration = $this->getMigration('d6_user_picture_file'); - $source = $migration->getSourceConfiguration(); - $source['site_path'] = 'core/modules/simpletest'; - $migration->set('source', $source); - $this->executeMigration($migration); + $this->executeMigration('d6_user_picture_file'); } /** diff --git a/core/modules/user/tests/src/Kernel/Migrate/d6/MigrateUserTest.php b/core/modules/user/tests/src/Kernel/Migrate/d6/MigrateUserTest.php index e5a4b86..9317834 100644 --- a/core/modules/user/tests/src/Kernel/Migrate/d6/MigrateUserTest.php +++ b/core/modules/user/tests/src/Kernel/Migrate/d6/MigrateUserTest.php @@ -3,6 +3,7 @@ namespace Drupal\Tests\user\Kernel\Migrate\d6; use Drupal\migrate\MigrateExecutable; +use Drupal\Tests\file\Kernel\Migrate\d6\FileMigrationTestTrait; use Drupal\user\Entity\User; use Drupal\file\Entity\File; use Drupal\Core\Database\Database; @@ -16,6 +17,8 @@ */ class MigrateUserTest extends MigrateDrupal6TestBase { + use FileMigrationTestTrait; + /** * {@inheritdoc} */