diff -u b/core/modules/file/migration_templates/d6_file.yml b/core/modules/file/migration_templates/d6_file.yml --- b/core/modules/file/migration_templates/d6_file.yml +++ b/core/modules/file/migration_templates/d6_file.yml @@ -1,5 +1,5 @@ -# Every migration that references a file by fid should specify this migration -# as an optional dependency. +# Every migration that references a file by Drupal 6 fid should specify this +# migration as an optional dependency. id: d6_file label: Files migration_tags: @@ -8,8 +8,9 @@ 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 /. + # represents the fully qualified path relative to which URIs in the files + # table are specified, and must end with a /. See source_full_path + # configuration in this migration's process pipeline as an example. source_base_path: '' process: fid: fid diff -u b/core/modules/file/migration_templates/d7_file.yml b/core/modules/file/migration_templates/d7_file.yml --- b/core/modules/file/migration_templates/d7_file.yml +++ b/core/modules/file/migration_templates/d7_file.yml @@ -1,5 +1,5 @@ -# Every migration that references a file by fid should specify this migration -# as an optional dependency. +# Every migration that references a file by Drupal 7 fid should specify this +# migration as an optional dependency. id: d7_file label: Files migration_tags: @@ -9,7 +9,8 @@ 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 /. + # table are specified, and must end with a /. See source_full_path + # configuration in this migration's process pipeline as an example. source_base_path: '' process: fid: fid diff -u b/core/modules/file/src/Plugin/migrate/destination/EntityFile.php b/core/modules/file/src/Plugin/migrate/destination/EntityFile.php --- b/core/modules/file/src/Plugin/migrate/destination/EntityFile.php +++ b/core/modules/file/src/Plugin/migrate/destination/EntityFile.php @@ -3,7 +3,11 @@ 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\migrate\Plugin\MigrationInterface; use Drupal\migrate\Row; use Drupal\migrate\MigrateException; use Drupal\migrate\Plugin\migrate\destination\EntityContentBase; @@ -18,7 +22,16 @@ /** * Column name of the file URI. */ - const URI_COLUMN = 'uri'; + protected $uri_column; + + /** + * @inheritdoc + */ + public function __construct(array $configuration, $plugin_id, $plugin_definition, MigrationInterface $migration, EntityStorageInterface $storage, array $bundles, EntityManagerInterface $entity_manager, FieldTypePluginManagerInterface $field_type_manager) { + parent::__construct($configuration, $plugin_id, $plugin_definition, $migration, $storage, $bundles, $entity_manager, $field_type_manager); + $this->uri_column = isset($configuration['uri_column']) ? $configuration['uri_column'] : 'uri'; + } + /** * {@inheritdoc} @@ -31,9 +44,9 @@ } // 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]); + // we're loading the matching URI. + $destination = $row->getDestinationProperty($this->uri_column); + $entity = $this->storage->loadByProperties([$this->uri_column => $destination]); if ($entity) { return reset($entity); } @@ -47,11 +60,11 @@ */ protected function processStubRow(Row $row) { // We stub the uri value ourselves so we can create a real stub file for it. - if (!$row->getDestinationProperty(static::URI_COLUMN)) { + if (!$row->getDestinationProperty($this->uri_column)) { $field_definitions = $this->entityManager ->getFieldDefinitions($this->storage->getEntityTypeId(), $this->getKey('bundle')); - $value = UriItem::generateSampleValue($field_definitions[static::URI_COLUMN]); + $value = UriItem::generateSampleValue($field_definitions[$this->uri_column]); if (empty($value)) { throw new MigrateException('Stubbing failed, unable to generate value for field uri'); } @@ -60,10 +73,10 @@ // 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[static::URI_COLUMN]->getSetting('max_length')); + $value = Unicode::substr($value, 0, $field_definitions[$this->uri_column]->getSetting('max_length')); // Create a real file, so File::preSave() can do filesize() on it. touch($value); - $row->setDestinationProperty(static::URI_COLUMN, $value); + $row->setDestinationProperty($this->uri_column, $value); } parent::processStubRow($row); } diff -u b/core/modules/file/tests/src/Kernel/Migrate/d6/FileMigrationTestTrait.php b/core/modules/file/tests/src/Kernel/Migrate/d6/FileMigrationTestTrait.php --- b/core/modules/file/tests/src/Kernel/Migrate/d6/FileMigrationTestTrait.php +++ b/core/modules/file/tests/src/Kernel/Migrate/d6/FileMigrationTestTrait.php @@ -1,6 +1,7 @@ getDestinationConfiguration(); @@ -28,7 +29,7 @@ // Make sure we have a single trailing slash. $source = $migration->getSourceConfiguration(); $source['site_path'] = 'core/modules/simpletest'; - $source['constants']['source_base_path'] = DRUPAL_ROOT . '/'; + $source['constants']['source_base_path'] = \Drupal::root() . '/'; $migration->set('source', $source); } } diff -u b/core/modules/file/tests/src/Unit/Plugin/migrate/process/UrlEncodeTest.php b/core/modules/file/tests/src/Unit/Plugin/migrate/process/UrlEncodeTest.php --- b/core/modules/file/tests/src/Unit/Plugin/migrate/process/UrlEncodeTest.php +++ b/core/modules/file/tests/src/Unit/Plugin/migrate/process/UrlEncodeTest.php @@ -14,29 +14,35 @@ */ class UrlEncodeTest extends MigrateTestCase { + /** + * @inheritdoc + */ protected $migrationConfiguration = [ 'id' => 'test', ]; /** - * Cover various encoding scenarios. + * The data provider for testing URL encoding scenarios. + * + * @return array + * An array of URLs to test. */ - 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', - ]; + public function urlDataProvider() { + return array( + 'A URL with no characters requiring encoding' => array('http://example.com/normal_url.html', 'http://example.com/normal_url.html'), + 'The definitive use case - encoding spaces in URLs' => array('http://example.com/url with spaces.html', 'http://example.com/url%20with%20spaces.html'), + 'Local filespecs without spaces should not be transformed' => array('/tmp/normal.txt', '/tmp/normal.txt'), + 'Local filespecs with spaces should not be transformed' => array('/tmp/with spaces.txt', '/tmp/with spaces.txt'), + 'Make sure URL characters (:, ?, &) are not encoded but others are.' => array('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)); - } + /** + * Cover various encoding scenarios. + * @dataProvider urlDataProvider + */ + public function testUrls($input, $output) { + $this->assertEquals($output, $this->doTransform($input)); } /** diff -u b/core/modules/migrate/src/Plugin/migrate/process/FileCopy.php b/core/modules/migrate/src/Plugin/migrate/process/FileCopy.php --- b/core/modules/migrate/src/Plugin/migrate/process/FileCopy.php +++ b/core/modules/migrate/src/Plugin/migrate/process/FileCopy.php @@ -85,7 +85,7 @@ // 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."); + throw new MigrateException("File '$source' does not exist"); } // If the start and end file is exactly the same, there is nothing to do. @@ -100,13 +100,13 @@ return $destination; } $dir = $this->getDirectory($destination); - if (!file_prepare_directory($dir, FILE_CREATE_DIRECTORY)) { - throw new MigrateException("Could not create directory '$dir'"); + if (!file_prepare_directory($dir, FILE_CREATE_DIRECTORY | FILE_MODIFY_PERMISSIONS)) { + throw new MigrateException("Could not create or write to directory '$dir'"); } if ($this->writeFile($source, $destination, $replace)) { return $destination; } - throw new MigrateException("File $source could not be copied to $destination."); + throw new MigrateException("File $source could not be copied to $destination"); } /** diff -u b/core/modules/migrate/src/Plugin/migrate/process/UrlEncode.php b/core/modules/migrate/src/Plugin/migrate/process/UrlEncode.php --- b/core/modules/migrate/src/Plugin/migrate/process/UrlEncode.php +++ b/core/modules/migrate/src/Plugin/migrate/process/UrlEncode.php @@ -3,8 +3,10 @@ namespace Drupal\migrate\Plugin\migrate\process; use Drupal\migrate\MigrateExecutableInterface; +use Drupal\migrate\MigrateException; use Drupal\migrate\ProcessPluginBase; use Drupal\migrate\Row; +use GuzzleHttp\Psr7\Uri; /** * Apply urlencoding to a URI. @@ -23,15 +25,34 @@ */ public function transform($value, MigrateExecutableInterface $migrate_executable, Row $row, $destination_property) { // Only apply to a full URL. - if (strpos($value, '://') > 0) { - $components = explode('/', $value); - foreach ($components as $key => $component) { - // urlencode() would convert spaces to + signs. - $components[$key] = rawurlencode($component); + if (is_string($value) && strpos($value, '://') > 0) { + // URL encode everything after the hostname. + $parsed_url = parse_url($value); + // Fail on seriously malformed URLs. + if ($parsed_url === FALSE) { + throw new MigrateException("Value '$value' is not a valid URL"); } - $value = implode('/', $components); - // Do not encode URL syntax characters. - $value = str_replace(['%3A', '%3F', '%26'], [':', '?', '&'], $value); + // Iterate over specific pieces of the URL rawurlencoding each one. + $url_parts_to_encode = array('path', 'query', 'fragment'); + foreach ($parsed_url as $parsed_url_key => $parsed_url_value) { + if (in_array($parsed_url_key, $url_parts_to_encode)) { + // urlencode() would convert spaces to + signs. + $urlencoded_parsed_url_value = rawurlencode($parsed_url_value); + // Restore special characters depending on which part of the URL this is. + switch ($parsed_url_key) { + case 'query': + $urlencoded_parsed_url_value = str_replace('%26', '&', $urlencoded_parsed_url_value); + break; + + case 'path': + $urlencoded_parsed_url_value = str_replace('%2F', '/', $urlencoded_parsed_url_value); + break; + } + + $parsed_url[$parsed_url_key] = $urlencoded_parsed_url_value; + } + } + $value = (string)Uri::fromParts($parsed_url); } return $value; } 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 @@ -7,6 +7,8 @@ use Drupal\migrate\MigrateExecutable; use Drupal\migrate\MigrateMessageInterface; use Drupal\migrate\Plugin\MigrateIdMapInterface; +use Drupal\migrate\Plugin\Migration; +use Drupal\migrate\Plugin\MigrationInterface; use Drupal\migrate\Row; /** @@ -142,7 +144,7 @@ * @param \Drupal\migrate\Plugin\MigrationInterface $migration * The migration to execute. */ - protected function processMigration($migration) { + protected function processMigration(MigrationInterface $migration) { // Default implementation for test classes not requiring modification. } diff -u b/core/modules/migrate/tests/src/Unit/process/CopyFileTest.php b/core/modules/migrate/tests/src/Unit/process/CopyFileTest.php --- b/core/modules/migrate/tests/src/Unit/process/CopyFileTest.php +++ b/core/modules/migrate/tests/src/Unit/process/CopyFileTest.php @@ -2,45 +2,57 @@ namespace Drupal\Tests\migrate\Unit\process; -use Drupal\Core\StreamWrapper\StreamWrapperInterface; +use Drupal\Core\File\FileSystemInterface; +use Drupal\KernelTests\Core\File\FileTestBase; use Drupal\migrate\Plugin\migrate\process\FileCopy; use Drupal\migrate\MigrateExecutableInterface; use Drupal\migrate\Row; -use Drupal\KernelTests\KernelTestBase; /** * Tests the copy_file process plugin. * * @group file */ -class CopyFileTest extends KernelTestBase { +class CopyFileTest extends FileTestBase { /** * {@inheritdoc} */ - public static $modules = ['system']; + public static $modules = array('system'); + + /** + * @var FileSystemInterface + */ + protected $fileSystem; /** * {@inheritdoc} */ protected function setUp() { parent::setUp(); - $this->container->get('stream_wrapper_manager')->registerWrapper('public', 'Drupal\Core\StreamWrapper\PublicStream', StreamWrapperInterface::NORMAL); + $this->filesystem = $this->container->get('file_system'); + } + /** + * Test successful copies from a remote source. + */ + public function testSuccessfulRemoteCopy() { + $destination_path = 'temporary://favicon.ico'; + $this->doImport('https://www.drupal.org/favicon.ico', $destination_path); + $message = sprintf('File %s exists', $destination_path); + $this->assertTrue(is_file($destination_path), $message); } /** * Test successful imports/copies. + * @dataProvider fileDataCopyProvider */ - public function testSuccessfulCopies() { - foreach ($this->localFileDataCopyProvider() as $data) { - list($source_path, $destination_path, $expected) = $data; + public function testSuccessfulCopies($data) { + list($source_path, $destination_path) = $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); + $message = sprintf('File %s exists', $destination_path); + $this->assertTrue(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); - } + $this->assertTrue(is_file($source_path), $message); } /** @@ -49,30 +61,27 @@ * @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], - ]; + public function fileDataCopyProvider() { + $file = $this->createUri(NULL, NULL, 'temporary'); + $file_absolute = $this->fileSystem->realpath($file); + return array( + 'local to local copy' => array($this->root . '/core/modules/simpletest/files/image-test.jpg', 'public://file1.jpg'), + 'Temporary file using an absolute path' => array($file_absolute, 'temporary://test.jpg'), + 'Temporary file using a relative path' => array($file_absolute, 'temporary://core/modules/simpletest/files/test.jpg'), + ); } /** * 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($data) { + list($source_path, $destination_path) = $data; + $this->doImport($source_path, $destination_path, ['move' => TRUE]); + $message = sprintf('File %s exists', $destination_path); + $this->assertTrue(is_file($destination_path), $message); + $message = sprintf('File %s does not exist', $source_path); + $this->assertTrue(!is_file($source_path), $message); } /** @@ -81,19 +90,18 @@ * @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], - ]; + public function localFileDataMoveProvider() { + $file_1 = $this->createUri(NULL, NULL, 'temporary'); + $file_1_absolute = $this->fileSystem->realpath($file_1); + $file_2 = $this->createUri(NULL, NULL, 'temporary'); + $file_2_absolute = $this->fileSystem->realpath($file_2); + $local_file = $this->createUri(NULL, NULL, 'public'); + + return array( + 'Local to local copy' => array($local_file, 'public://file1.jpg'), + 'Temporary file using an absolute path.' => array($file_1_absolute, 'temporary://test.jpg'), + 'Temporary file using a relative path' => array($file_2_absolute, 'temporary://core/modules/simpletest/files/test.jpg'), + ); } /** @@ -112,11 +120,9 @@ * Test the 'rename' overwrite mode. */ public function testRenameFile() { - $source = 'temporary://baz.txt'; - $destination = 'public://foo.txt'; + $source = $this->createUri(NULL, NULL, 'temporary'); + $destination = $this->createUri('foo.txt', NULL, 'public'); $expected_destination = 'public://foo_0.txt'; - touch($source); - touch($destination); $this->doImport($source, $destination, ['rename' => TRUE]); $this->assertFileExists($expected_destination); } diff -u b/core/modules/migrate/tests/src/Unit/process/UrlEncodeTest.php b/core/modules/migrate/tests/src/Unit/process/UrlEncodeTest.php --- b/core/modules/migrate/tests/src/Unit/process/UrlEncodeTest.php +++ b/core/modules/migrate/tests/src/Unit/process/UrlEncodeTest.php @@ -14,39 +14,45 @@ */ class UrlEncodeTest extends MigrateTestCase { + /** + * @inheritdoc + */ protected $migrationConfiguration = [ 'id' => 'test', ]; /** - * Cover various encoding scenarios. + * The data provider for testing URL encoding scenarios. + * + * @return array + * An array of URLs to test. */ - 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', - ]; + public function urlDataProvider() { + return array( + 'A URL with no characters requiring encoding' => array('http://example.com/normal_url.html', 'http://example.com/normal_url.html'), + 'The definitive use case - encoding spaces in URLs' => array('http://example.com/url with spaces.html', 'http://example.com/url%20with%20spaces.html'), + 'Local filespecs without spaces should not be transformed' => array('/tmp/normal.txt', '/tmp/normal.txt'), + 'Local filespecs with spaces should not be transformed' => array('/tmp/with spaces.txt', '/tmp/with spaces.txt'), + 'Make sure URL characters (:, ?, &) are not encoded but others are.' => array('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)); - } + /** + * Cover various encoding scenarios. + * @dataProvider urlDataProvider + */ + public function testUrls($input, $output) { + $this->assertEquals($output, $this->doTransform($input)); } /** * Perform the urlencode process plugin over the given value. * * @param string $value - * Url to be encoded. + * URL to be encoded. * * @return string - * Encoded url. + * Encoded URL. */ protected function doTransform($value) { $executable = new MigrateExecutable($this->getMigration(), new MigrateMessage()); diff -u b/core/modules/user/migration_templates/d6_user_picture_file.yml b/core/modules/user/migration_templates/d6_user_picture_file.yml --- b/core/modules/user/migration_templates/d6_user_picture_file.yml +++ b/core/modules/user/migration_templates/d6_user_picture_file.yml @@ -7,7 +7,7 @@ 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 + # represents the fully qualified path relative to which URIs in the files # table are specified, and must end with a /. source_base_path: '' process: @@ -38,7 +38,7 @@ plugin: entity:file source_path_property: picture migration_dependencies: - # Every migration that references a file by fid should specify d6_file as an + # Every migration that references a file by Drupal 6 fid should specify d6_file as an # optional dependency. optional: - d6_file