Unverified Commit a6d53088 authored by alexpott's avatar alexpott
Browse files

Issue #2877839 by Jo Fitzgerald, edysmp, heddn, Nebel54, phenaproxima,...

Issue #2877839 by Jo Fitzgerald, edysmp, heddn, Nebel54, phenaproxima, alexpott, quietone: Reuse option in FileCopy migrate process plugin not work with remote files
parent a15fd23c
......@@ -6,7 +6,6 @@
use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
use Drupal\migrate\MigrateException;
use Drupal\migrate\MigrateExecutableInterface;
use Drupal\migrate\ProcessPluginBase;
use Drupal\migrate\Row;
use GuzzleHttp\Client;
use Symfony\Component\DependencyInjection\ContainerInterface;
......@@ -19,8 +18,12 @@
* - destination URI, e.g. 'public://images/foo.img'
*
* Available configuration keys:
* - rename: (optional) If set, a unique destination URI is generated. If not
* set, the destination URI will be overwritten if it exists.
* - file_exists: (optional) Replace behavior when the destination file already
* exists:
* - 'replace' - (default) Replace the existing file.
* - 'rename' - Append _{incrementing number} until the filename is
* unique.
* - 'use existing' - Do nothing and return FALSE.
* - guzzle_options: (optional)
* @link http://docs.guzzlephp.org/en/latest/request-options.html Array of request options for Guzzle. @endlink
*
......@@ -42,7 +45,7 @@
* source:
* - source_url
* - destination_uri
* rename: true
* file_exists: rename
* @endcode
*
* This will download source_url to destination_uri and ensure that the
......@@ -53,7 +56,7 @@
* id = "download"
* )
*/
class Download extends ProcessPluginBase implements ContainerFactoryPluginInterface {
class Download extends FileProcessBase implements ContainerFactoryPluginInterface {
/**
* The file system service.
......@@ -85,7 +88,6 @@ class Download extends ProcessPluginBase implements ContainerFactoryPluginInterf
*/
public function __construct(array $configuration, $plugin_id, array $plugin_definition, FileSystemInterface $file_system, Client $http_client) {
$configuration += [
'rename' => FALSE,
'guzzle_options' => [],
];
parent::__construct($configuration, $plugin_id, $plugin_definition);
......@@ -118,10 +120,12 @@ public function transform($value, MigrateExecutableInterface $migrate_executable
list($source, $destination) = $value;
// Modify the destination filename if necessary.
$replace = !empty($this->configuration['rename']) ?
FILE_EXISTS_RENAME :
FILE_EXISTS_REPLACE;
$final_destination = file_destination($destination, $replace);
$final_destination = file_destination($destination, $this->configuration['file_exists']);
// Reuse if file exists.
if (!$final_destination) {
return $destination;
}
// Try opening the file first, to avoid calling file_prepare_directory()
// unnecessarily. We're suppressing fopen() errors because we want to try
......
......@@ -9,7 +9,6 @@
use Drupal\migrate\MigrateException;
use Drupal\migrate\MigrateExecutableInterface;
use Drupal\migrate\Plugin\MigrateProcessInterface;
use Drupal\migrate\ProcessPluginBase;
use Drupal\migrate\Row;
use Symfony\Component\DependencyInjection\ContainerInterface;
......@@ -26,10 +25,12 @@
* Available configuration keys:
* - move: (optional) Boolean, if TRUE, move the file, otherwise copy the file.
* Defaults to FALSE.
* - rename: (optional) Boolean, if TRUE, rename the file by appending a number
* until the name is unique. Defaults to FALSE.
* - reuse: (optional) Boolean, if TRUE, reuse the current file in its existing
* location rather than move/copy/rename the file. Defaults to FALSE.
* - file_exists: (optional) Replace behavior when the destination file already
* exists:
* - 'replace' - (default) Replace the existing file.
* - 'rename' - Append _{incrementing number} until the filename is
* unique.
* - 'use existing' - Do nothing and return FALSE.
*
* Examples:
*
......@@ -48,7 +49,7 @@
* id = "file_copy"
* )
*/
class FileCopy extends ProcessPluginBase implements ContainerFactoryPluginInterface {
class FileCopy extends FileProcessBase implements ContainerFactoryPluginInterface {
/**
* The stream wrapper manager service.
......@@ -90,8 +91,6 @@ class FileCopy extends ProcessPluginBase implements ContainerFactoryPluginInterf
public function __construct(array $configuration, $plugin_id, array $plugin_definition, StreamWrapperManagerInterface $stream_wrappers, FileSystemInterface $file_system, MigrateProcessInterface $download_plugin) {
$configuration += [
'move' => FALSE,
'rename' => FALSE,
'reuse' => FALSE,
];
parent::__construct($configuration, $plugin_id, $plugin_definition);
$this->streamWrapperManager = $stream_wrappers;
......@@ -109,7 +108,7 @@ public static function create(ContainerInterface $container, array $configuratio
$plugin_definition,
$container->get('stream_wrapper_manager'),
$container->get('file_system'),
$container->get('plugin.manager.migrate.process')->createInstance('download')
$container->get('plugin.manager.migrate.process')->createInstance('download', $configuration)
);
}
......@@ -150,7 +149,7 @@ public function transform($value, MigrateExecutableInterface $migrate_executable
}
}
$final_destination = $this->writeFile($source, $destination, $this->getOverwriteMode());
$final_destination = $this->writeFile($source, $destination, $this->configuration['file_exists']);
if ($final_destination) {
return $final_destination;
}
......@@ -181,24 +180,6 @@ protected function writeFile($source, $destination, $replace = FILE_EXISTS_REPLA
return $function($source, $destination, $replace);
}
/**
* Determines how to handle file conflicts.
*
* @return int
* FILE_EXISTS_REPLACE (default), FILE_EXISTS_RENAME, or FILE_EXISTS_ERROR
* depending on the current configuration.
*/
protected function getOverwriteMode() {
if (!empty($this->configuration['rename'])) {
return FILE_EXISTS_RENAME;
}
if (!empty($this->configuration['reuse'])) {
return FILE_EXISTS_ERROR;
}
return FILE_EXISTS_REPLACE;
}
/**
* Returns the directory component of a URI or path.
*
......
<?php
namespace Drupal\migrate\Plugin\migrate\process;
use Drupal\migrate\ProcessPluginBase;
/**
* Provides functionality for file process plugins.
*
* Available configuration keys:
* - file_exists: (optional) Replace behavior when the destination file already
* exists:
* - 'replace' - (default) Replace the existing file.
* - 'rename' - Append _{incrementing number} until the filename is
* unique.
* - 'use existing' - Do nothing and return FALSE.
*/
abstract class FileProcessBase extends ProcessPluginBase {
/**
* Constructs a file process plugin.
*
* @param array $configuration
* The plugin configuration.
* @param string $plugin_id
* The plugin ID.
* @param mixed $plugin_definition
* The plugin definition.
*/
public function __construct(array $configuration, $plugin_id, array $plugin_definition) {
if (array_key_exists('file_exists', $configuration)) {
switch ($configuration['file_exists']) {
case 'use existing':
$configuration['file_exists'] = FILE_EXISTS_ERROR;
break;
case 'rename':
$configuration['file_exists'] = FILE_EXISTS_RENAME;
break;
default:
$configuration['file_exists'] = FILE_EXISTS_REPLACE;
}
}
if (array_key_exists('reuse', $configuration)) {
@trigger_error("Using the key 'reuse' is deprecated, use 'file_exists' => 'use existing' instead. See https://www.drupal.org/node/2981389.", E_USER_DEPRECATED);
if (!empty($configuration['reuse'])) {
$configuration['file_exists'] = FILE_EXISTS_ERROR;
}
}
if (array_key_exists('rename', $configuration)) {
@trigger_error("Using the key 'rename' is deprecated, use 'file_exists' => 'rename' instead. See https://www.drupal.org/node/2981389.", E_USER_DEPRECATED);
if (!empty($configuration['rename'])) {
$configuration['file_exists'] = FILE_EXISTS_RENAME;
}
}
$configuration += ['file_exists' => FILE_EXISTS_REPLACE];
parent::__construct($configuration, $plugin_id, $plugin_definition);
}
}
......@@ -51,7 +51,7 @@ public function testNonDestructiveDownload() {
$destination_uri = $this->createUri('another_existing_file.txt');
// Test non-destructive download.
$actual_destination = $this->doTransform($destination_uri, ['rename' => TRUE]);
$actual_destination = $this->doTransform($destination_uri, ['file_exists' => 'rename']);
$this->assertSame('public://another_existing_file_0.txt', $actual_destination, 'Import returned a renamed destination');
$this->assertFileExists($actual_destination, 'Downloaded file was created');
}
......
......@@ -9,6 +9,7 @@
use Drupal\migrate\MigrateExecutableInterface;
use Drupal\migrate\Plugin\MigrateProcessInterface;
use Drupal\migrate\Row;
use GuzzleHttp\Client;
/**
* Tests the file_copy process plugin.
......@@ -76,30 +77,52 @@ public function testSuccessfulCopies() {
/**
* Test successful file reuse.
*
* @dataProvider providerSuccessfulReuse
*
* @param string $source_path
* Source path to copy from.
* @param string $destination_path
* The destination path to copy to.
*/
public function testSuccessfulReuse() {
$source_path = $this->root . '/core/modules/simpletest/files/image-test.jpg';
$destination_path = 'public://file1.jpg';
$file_reuse = file_unmanaged_copy($source_path, $destination_path);
public function testSuccessfulReuse($source_path, $destination_path) {
$file_reuse = $this->doTransform($source_path, $destination_path);
clearstatcache(TRUE, $destination_path);
$timestamp = (new \SplFileInfo($file_reuse))->getMTime();
$this->assertInternalType('int', $timestamp);
// We need to make sure the modified timestamp on the file is sooner than
// the attempted migration.
sleep(1);
$configuration = ['reuse' => TRUE];
$configuration = ['file_exists' => 'use existing'];
$this->doTransform($source_path, $destination_path, $configuration);
clearstatcache(TRUE, $destination_path);
$modified_timestamp = (new \SplFileInfo($destination_path))->getMTime();
$this->assertEquals($timestamp, $modified_timestamp);
$configuration = ['reuse' => FALSE];
$this->doTransform($source_path, $destination_path, $configuration);
$this->doTransform($source_path, $destination_path);
clearstatcache(TRUE, $destination_path);
$modified_timestamp = (new \SplFileInfo($destination_path))->getMTime();
$this->assertGreaterThan($timestamp, $modified_timestamp);
}
/**
* Provides the source and destination path files.
*/
public function providerSuccessfulReuse() {
return [
[
'local_source_path' => static::getDrupalRoot() . '/core/modules/simpletest/files/image-test.jpg',
'local_destination_path' => 'public://file1.jpg',
],
[
'remote_source_path' => 'https://www.drupal.org/favicon.ico',
'remote_destination_path' => 'public://file2.jpg',
],
];
}
/**
* Test successful moves.
*/
......@@ -179,7 +202,7 @@ public function testRenameFile() {
$source = $this->createUri(NULL, NULL, 'temporary');
$destination = $this->createUri('foo.txt', NULL, 'public');
$expected_destination = 'public://foo_0.txt';
$actual_destination = $this->doTransform($source, $destination, ['rename' => TRUE]);
$actual_destination = $this->doTransform($source, $destination, ['file_exists' => 'rename']);
$this->assertFileExists($expected_destination, 'File was renamed on import');
$this->assertSame($actual_destination, $expected_destination, 'The importer returned the renamed filename.');
}
......@@ -222,6 +245,9 @@ public function testDownloadRemoteUri() {
* The URI of the copied file.
*/
protected function doTransform($source_path, $destination_path, $configuration = []) {
// Prepare a mock HTTP client.
$this->container->set('http_client', $this->createMock(Client::class));
$plugin = FileCopy::create($this->container, $configuration, 'file_copy', []);
$executable = $this->prophesize(MigrateExecutableInterface::class)->reveal();
$row = new Row([], []);
......
<?php
namespace Drupal\Tests\migrate\Unit\process;
use Drupal\Core\File\FileSystemInterface;
use Drupal\Core\StreamWrapper\StreamWrapperManagerInterface;
use Drupal\migrate\Plugin\migrate\process\FileCopy;
use Drupal\migrate\Plugin\MigrateProcessInterface;
/**
* Flag for dealing with existing files: Appends number until name is unique.
*/
define('FILE_EXISTS_RENAME', 0);
/**
* Flag for dealing with existing files: Replace the existing file.
*/
define('FILE_EXISTS_REPLACE', 1);
/**
* Flag for dealing with existing files: Do nothing and return FALSE.
*/
define('FILE_EXISTS_ERROR', 2);
/**
* Tests the file copy process plugin.
*
* @group migrate
* @group legacy
*
* @coversDefaultClass \Drupal\migrate\Plugin\migrate\process\FileCopy
*/
class FileCopyTest extends MigrateProcessTestCase {
/**
* Tests that the rename configuration key will trigger a deprecation notice.
*
* @dataProvider providerDeprecationNoticeRename
*
* @param array $configuration
* The plugin configuration.
* @param $expected
* The expected value of the plugin configuration.
*
* @expectedDeprecation Using the key 'rename' is deprecated, use 'file_exists' => 'rename' instead. See https://www.drupal.org/node/2981389.
*/
public function testDeprecationNoticeRename($configuration, $expected) {
$this->assertPlugin($configuration, $expected);
}
/**
* Data provider for testDeprecationNoticeRename.
*/
public function providerDeprecationNoticeRename() {
return [
[['rename' => TRUE], FILE_EXISTS_RENAME],
[['rename' => FALSE], FILE_EXISTS_REPLACE],
];
}
/**
* Tests that the reuse configuration key will trigger a deprecation notice.
*
* @dataProvider providerDeprecationNoticeReuse
*
* @param array $configuration
* The plugin configuration.
* @param $expected
* The expected value of the plugin configuration.
*
* @expectedDeprecation Using the key 'reuse' is deprecated, use 'file_exists' => 'use existing' instead. See https://www.drupal.org/node/2981389.
*/
public function testDeprecationNoticeReuse($configuration, $expected) {
$this->assertPlugin($configuration, $expected);
}
/**
* Data provider for testDeprecationNoticeReuse.
*/
public function providerDeprecationNoticeReuse() {
return [
[['reuse' => TRUE], FILE_EXISTS_ERROR],
[['reuse' => FALSE], FILE_EXISTS_REPLACE],
];
}
/**
* Tests that the plugin constructor correctly sets the configuration.
*
* @dataProvider providerFileProcessBaseConstructor
*
* @param array $configuration
* The plugin configuration.
* @param $expected
* The expected value of the plugin configuration.
*/
public function testFileProcessBaseConstructor($configuration, $expected) {
$this->assertPlugin($configuration, $expected);
}
/**
* Data provider for testFileProcessBaseConstructor.
*/
public function providerFileProcessBaseConstructor() {
return [
[['file_exists' => 'replace'], FILE_EXISTS_REPLACE],
[['file_exists' => 'rename'], FILE_EXISTS_RENAME],
[['file_exists' => 'use existing'], FILE_EXISTS_ERROR],
[['file_exists' => 'foobar'], FILE_EXISTS_REPLACE],
[[], FILE_EXISTS_REPLACE],
];
}
/**
* Creates a TestFileCopy process plugin.
*
* @param array $configuration
* The plugin configuration.
* @param $expected
* The expected value of the plugin configuration.
*/
protected function assertPlugin($configuration, $expected) {
$stream_wrapper_manager = $this->prophesize(StreamWrapperManagerInterface::class)->reveal();
$file_system = $this->prophesize(FileSystemInterface::class)->reveal();
$download_plugin = $this->prophesize(MigrateProcessInterface::class)->reveal();
$this->plugin = new TestFileCopy($configuration, 'test', [], $stream_wrapper_manager, $file_system, $download_plugin);
$plugin_config = $this->plugin->getConfiguration();
$this->assertArrayHasKey('file_exists', $plugin_config);
$this->assertSame($expected, $plugin_config['file_exists']);
}
}
/**
* Class for testing FileCopy.
*/
class TestFileCopy extends FileCopy {
/**
* Gets this plugin's configuration.
*
* @return array
* An array of this plugin's configuration.
*/
public function getConfiguration() {
return $this->configuration;
}
}
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment