diff -u b/core/modules/file/src/Plugin/migrate/process/FileCopy.php b/core/modules/file/src/Plugin/migrate/process/FileCopy.php --- b/core/modules/file/src/Plugin/migrate/process/FileCopy.php +++ b/core/modules/file/src/Plugin/migrate/process/FileCopy.php @@ -22,17 +22,32 @@ class FileCopy extends ProcessPluginBase implements ContainerFactoryPluginInterface { /** + * The stream wrapper manager service. + * * @var \Drupal\Core\StreamWrapper\StreamWrapperManagerInterface */ protected $streamWrapperManager; /** + * The file system service. + * * @var \Drupal\Core\File\FileSystemInterface */ protected $fileSystem; /** - * {@inheritdoc} + * Constructs a file_copy process plugin. + * + * @param array $configuration + * The plugin configuration. + * @param string $plugin_id + * The plugin ID. + * @param mixed $plugin_definition + * The plugin definition. + * @param \Drupal\Core\StreamWrapper\StreamWrapperManagerInterface $stream_wrappers + * The stream wrapper manager service. + * @param \Drupal\Core\File\FileSystemInterface $file_system + * The file system service. */ public function __construct(array $configuration, $plugin_id, array $plugin_definition, StreamWrapperManagerInterface $stream_wrappers, FileSystemInterface $file_system) { $configuration += array( @@ -78,7 +93,9 @@ return $destination; } - $replace = $this->getOverwriteMode($row); + $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; } diff -u b/core/modules/file/src/Plugin/migrate/process/UrlEncode.php b/core/modules/file/src/Plugin/migrate/process/UrlEncode.php --- b/core/modules/file/src/Plugin/migrate/process/UrlEncode.php +++ b/core/modules/file/src/Plugin/migrate/process/UrlEncode.php @@ -20,16 +20,15 @@ */ public function transform($value, MigrateExecutableInterface $migrate_executable, Row $row, $destination_property) { // Only apply to a full URL. - if (strpos($value, '://')) { + if (strpos($value, '://') > 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', ':', $value); - $value = str_replace('%3F', '?', $value); - $value = str_replace('%26', '&', $value); + $value = str_replace(['%3A', '%3F', '%26'], [':', '?', '&'], $value); } return $value; } diff -u b/core/modules/file/tests/src/Kernel/Migrate/CopyFileProcessPluginTest.php b/core/modules/file/tests/src/Kernel/Migrate/CopyFileProcessPluginTest.php --- b/core/modules/file/tests/src/Kernel/Migrate/CopyFileProcessPluginTest.php +++ b/core/modules/file/tests/src/Kernel/Migrate/CopyFileProcessPluginTest.php @@ -1,17 +1,11 @@ get('stream_wrapper_manager')->registerWrapper('public', 'Drupal\Core\StreamWrapper\PublicStream', StreamWrapperInterface::NORMAL); + $this->container->get('stream_wrapper_manager')->registerWrapper('public', 'Drupal\Core\StreamWrapper\PublicStream', StreamWrapperInterface::NORMAL); + $this->container->get('stream_wrapper_manager')->registerWrapper('temporary', 'Drupal\Core\StreamWrapper\TemporaryStream', StreamWrapperInterface::NORMAL); } /** * Test successful imports/copies. + * + * @dataProvider localFileDataCopyProvider */ - 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); - } + public function testSuccessfulCopies($source_path, $destination_path, $expected) { + $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); } /** @@ -58,7 +49,7 @@ * @return array * An array of file permutations to test. */ - protected function localFileDataCopyProvider() { + public function localFileDataCopyProvider() { touch('/tmp/test-file.jpg'); return [ // Test a local to local copy. @@ -72,17 +63,15 @@ /** * Test successful moves. + * + * @dataProvider localFileDataMoveProvider */ - 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); - } + public function testSuccessfulMoves($source_path, $destination_path, $expected) { + $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); } /** @@ -91,7 +80,7 @@ * @return array * An array of file permutations to test. */ - protected function localFileDataMoveProvider() { + public function localFileDataMoveProvider() { touch('/tmp/test-file.jpg'); touch('/tmp/test-file2.jpg'); $local_file = $this->root . '/sites/default/files/source_file.txt'; @@ -108,18 +97,13 @@ /** * 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'; - try { - // If this test passes, doImport() will raise a MigrateException and - // we'll never reach fail(). - $this->doImport($source, 'public://wontmatter.jpg'); - $this->fail('Expected Drupal\migrate\MigrateException when importing ' . $source); - } - catch (MigrateException $e) { - $this->assertIdentical($e->getMessage(), "File '$source' does not exist."); - } + $this->doImport($source, 'public://wontmatter.jpg'); } /** @@ -132,7 +116,7 @@ touch($source); touch($destination); $this->doImport($source, $destination, ['rename' => TRUE]); - $this->assertTrue(is_file($expected_destination)); + $this->assertFileExists($expected_destination); } /** @@ -151,7 +135,7 @@ * @throws \Drupal\migrate\MigrateException */ protected function doImport($source_path, $destination_path, $configuration = []) { - $this->plugin = FileCopy::create(\Drupal::getContainer(), $configuration, 'file_copy', []); + $this->plugin = FileCopy::create($this->container, $configuration, 'file_copy', []); $executable = $this->prophesize(MigrateExecutableInterface::class)->reveal(); $row = new Row([], []); diff -u b/core/modules/migrate/tests/src/Kernel/MigrateTestBase.php b/core/modules/migrate/tests/src/Kernel/MigrateTestBase.php --- b/core/modules/migrate/tests/src/Kernel/MigrateTestBase.php +++ b/core/modules/migrate/tests/src/Kernel/MigrateTestBase.php @@ -137,12 +137,13 @@ } /** - * Post-process a migration before executing it. + * 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. } /**