From d1db7c785c147c1a52d4b3f8db74c5d02fdd6ff4 Mon Sep 17 00:00:00 2001
From: Lee Rowlands <lee.rowlands@previousnext.com.au>
Date: Fri, 22 Mar 2019 07:56:13 +1000
Subject: [PATCH] Issue #3038988 by Berdir, seanB: Remove support for empty
destination in FileSystem::copy(), FileSystem::move() and
FileSystem::saveData()
---
core/includes/file.inc | 20 +++++++++++++++++--
core/lib/Drupal/Core/File/FileSystem.php | 14 ++++---------
.../Drupal/Core/File/FileSystemInterface.php | 16 ++++++---------
.../src/Functional/ImageAdminStylesTest.php | 2 +-
.../FunctionalJavascript/MediaLibraryTest.php | 14 ++++++-------
.../system/src/Form/ThemeSettingsForm.php | 4 ++--
.../UpdateDeleteFileIfStaleTest.php | 2 +-
.../Core/File/FileSaveDataTest.php | 4 ----
.../Core/File/FileSystemDeprecationTest.php | 13 +++++++++---
9 files changed, 49 insertions(+), 40 deletions(-)
diff --git a/core/includes/file.inc b/core/includes/file.inc
index ded0f8c89107..178d7f61f6e2 100644
--- a/core/includes/file.inc
+++ b/core/includes/file.inc
@@ -465,7 +465,13 @@ function file_valid_uri($uri) {
function file_unmanaged_copy($source, $destination = NULL, $replace = FILE_EXISTS_RENAME) {
@trigger_error('file_unmanaged_copy() is deprecated in Drupal 8.7.0 and will be removed before Drupal 9.0.0. Use \Drupal\Core\File\FileSystemInterface::copy(). See https://www.drupal.org/node/3006851.', E_USER_DEPRECATED);
try {
- return \Drupal::service('file_system')->copy($source, $destination, $replace);
+ $file_system = \Drupal::service('file_system');
+
+ // Build a destination URI if necessary.
+ if (!isset($destination)) {
+ $destination = file_build_uri($file_system->basename($source));
+ }
+ return $file_system->copy($source, $destination, $replace);
}
catch (FileException $e) {
return FALSE;
@@ -643,7 +649,13 @@ function file_destination($destination, $replace) {
function file_unmanaged_move($source, $destination = NULL, $replace = FILE_EXISTS_RENAME) {
@trigger_error('file_unmanaged_move() is deprecated in Drupal 8.7.0 and will be removed before Drupal 9.0.0. Use \Drupal\Core\File\FileSystemInterface::move(). See https://www.drupal.org/node/3006851.', E_USER_DEPRECATED);
try {
- return \Drupal::service('file_system')->move($source, $destination, $replace);
+ $file_system = \Drupal::service('file_system');
+
+ // Build a destination URI if necessary.
+ if (!isset($destination)) {
+ $destination = file_build_uri($file_system->basename($source));
+ }
+ return $file_system->move($source, $destination, $replace);
}
catch (FileException $e) {
return FALSE;
@@ -921,6 +933,10 @@ function drupal_move_uploaded_file($filename, $uri) {
function file_unmanaged_save_data($data, $destination = NULL, $replace = FILE_EXISTS_RENAME) {
@trigger_error('file_unmanaged_save_data() is deprecated in Drupal 8.7.0 and will be removed before Drupal 9.0.0. Use \Drupal\Core\File\FileSystemInterface::saveData(). See https://www.drupal.org/node/3006851.', E_USER_DEPRECATED);
try {
+ // Build a destination URI if necessary.
+ if (!isset($destination)) {
+ $destination = file_default_scheme() . '://';
+ }
return \Drupal::service('file_system')->saveData($data, $destination, $replace);
}
catch (FileWriteException $e) {
diff --git a/core/lib/Drupal/Core/File/FileSystem.php b/core/lib/Drupal/Core/File/FileSystem.php
index d2e1e3d977a1..ebb113c38044 100644
--- a/core/lib/Drupal/Core/File/FileSystem.php
+++ b/core/lib/Drupal/Core/File/FileSystem.php
@@ -311,7 +311,7 @@ public function validScheme($scheme) {
/**
* {@inheritdoc}
*/
- public function copy($source, $destination = NULL, $replace = self::EXISTS_RENAME) {
+ public function copy($source, $destination, $replace = self::EXISTS_RENAME) {
$this->prepareDestination($source, $destination, $replace);
// Perform the copy operation.
@@ -393,7 +393,7 @@ public function deleteRecursive($path, callable $callback = NULL) {
/**
* {@inheritdoc}
*/
- public function move($source, $destination = NULL, $replace = self::EXISTS_RENAME) {
+ public function move($source, $destination, $replace = self::EXISTS_RENAME) {
$this->prepareDestination($source, $destination, $replace);
// Ensure compatibility with Windows.
@@ -449,8 +449,7 @@ public function move($source, $destination = NULL, $replace = self::EXISTS_RENAM
* @param string|null $destination
* A URI containing the destination that $source should be moved/copied to.
* The URI may be a bare filepath (without a scheme) and in that case the
- * default scheme (file://) will be used. If this value is omitted, Drupal's
- * default files scheme will be used, usually "public://".
+ * default scheme (file://) will be used.
* @param int $replace
* Replace behavior when the destination file already exists:
* - FILE_EXISTS_REPLACE - Replace the existing file.
@@ -481,11 +480,6 @@ protected function prepareDestination($source, &$destination, $replace) {
}
}
- // Build a destination URI if necessary.
- if (!isset($destination)) {
- $destination = file_build_uri($this->basename($source));
- }
-
// Prepare the destination directory.
if ($this->prepareDirectory($destination)) {
// The destination is already a directory, so append the source basename.
@@ -529,7 +523,7 @@ protected function prepareDestination($source, &$destination, $replace) {
/**
* {@inheritdoc}
*/
- public function saveData($data, $destination = NULL, $replace = self::EXISTS_RENAME) {
+ public function saveData($data, $destination, $replace = self::EXISTS_RENAME) {
// Write the data to a temporary file.
$temp_name = $this->tempnam('temporary://', 'file');
if (file_put_contents($temp_name, $data) === FALSE) {
diff --git a/core/lib/Drupal/Core/File/FileSystemInterface.php b/core/lib/Drupal/Core/File/FileSystemInterface.php
index 262666d92a67..c2adfedaa60f 100644
--- a/core/lib/Drupal/Core/File/FileSystemInterface.php
+++ b/core/lib/Drupal/Core/File/FileSystemInterface.php
@@ -282,8 +282,7 @@ public function validScheme($scheme);
* A string specifying the filepath or URI of the source file.
* @param string $destination
* A URI containing the destination that $source should be copied to. The
- * URI may be a bare filepath (without a scheme). If this value is omitted,
- * Drupal's default files scheme will be used, usually "public://".
+ * URI may be a bare filepath (without a scheme).
* @param int $replace
* Replace behavior when the destination file already exists:
* - FileManagerInterface::FILE_EXISTS_REPLACE - Replace the existing file.
@@ -297,7 +296,7 @@ public function validScheme($scheme);
* @throws \Drupal\Core\File\Exception\FileException
* Implementation may throw FileException or its subtype on failure.
*/
- public function copy($source, $destination = NULL, $replace = self::EXISTS_RENAME);
+ public function copy($source, $destination, $replace = self::EXISTS_RENAME);
/**
* Deletes a file without database changes or hook invocations.
@@ -355,8 +354,7 @@ public function deleteRecursive($path, callable $callback = NULL);
* @param string $destination
* A URI containing the destination that $source should be moved to. The
* URI may be a bare filepath (without a scheme) and in that case the
- * default scheme (file://) will be used. If this value is omitted, Drupal's
- * default files scheme will be used, usually "public://".
+ * default scheme (public://) will be used.
* @param int $replace
* Replace behavior when the destination file already exists:
* - FILE_EXISTS_REPLACE - Replace the existing file.
@@ -372,7 +370,7 @@ public function deleteRecursive($path, callable $callback = NULL);
*
* @see https://bugs.php.net/bug.php?id=60456
*/
- public function move($source, $destination = NULL, $replace = self::EXISTS_RENAME);
+ public function move($source, $destination, $replace = self::EXISTS_RENAME);
/**
* Saves a file to the specified destination without invoking file API.
@@ -385,9 +383,7 @@ public function move($source, $destination = NULL, $replace = self::EXISTS_RENAM
* A string containing the contents of the file.
* @param string $destination
* A string containing the destination location. This must be a stream
- * wrapper URI. If no value is provided, a randomized name will be generated
- * and the file will be saved using Drupal's default files scheme, usually
- * "public://".
+ * wrapper URI.
* @param int $replace
* Replace behavior when the destination file already exists:
* - FILE_EXISTS_REPLACE - Replace the existing file.
@@ -403,7 +399,7 @@ public function move($source, $destination = NULL, $replace = self::EXISTS_RENAM
*
* @see file_save_data()
*/
- public function saveData($data, $destination = NULL, $replace = self::EXISTS_RENAME);
+ public function saveData($data, $destination, $replace = self::EXISTS_RENAME);
/**
* Checks that the directory exists and is writable.
diff --git a/core/modules/image/tests/src/Functional/ImageAdminStylesTest.php b/core/modules/image/tests/src/Functional/ImageAdminStylesTest.php
index 66a3e54b61bc..c40e5465e081 100644
--- a/core/modules/image/tests/src/Functional/ImageAdminStylesTest.php
+++ b/core/modules/image/tests/src/Functional/ImageAdminStylesTest.php
@@ -33,7 +33,7 @@ public function createSampleImage(ImageStyleInterface $style) {
if (!isset($file_path)) {
$files = $this->drupalGetTestFiles('image');
$file = reset($files);
- $file_path = \Drupal::service('file_system')->copy($file->uri);
+ $file_path = \Drupal::service('file_system')->copy($file->uri, 'public://');
}
return $style->buildUrl($file_path) ? $file_path : FALSE;
diff --git a/core/modules/media_library/tests/src/FunctionalJavascript/MediaLibraryTest.php b/core/modules/media_library/tests/src/FunctionalJavascript/MediaLibraryTest.php
index 3b63ef521187..ff16acf4e032 100644
--- a/core/modules/media_library/tests/src/FunctionalJavascript/MediaLibraryTest.php
+++ b/core/modules/media_library/tests/src/FunctionalJavascript/MediaLibraryTest.php
@@ -769,7 +769,7 @@ public function testWidgetUpload() {
$assert_session->pageTextContains('Add or select media');
$page->clickLink('Type Three');
$assert_session->assertWaitOnAjaxRequest();
- $png_uri_2 = $file_system->copy($png_image->uri);
+ $png_uri_2 = $file_system->copy($png_image->uri, 'public://');
$page->attachFileToField('Add files', $this->container->get('file_system')->realpath($png_uri_2));
$assert_session->assertWaitOnAjaxRequest();
$page->fillField('Alternative text', $this->randomString());
@@ -793,7 +793,7 @@ public function testWidgetUpload() {
$checkbox->click();
$assert_session->pageTextContains('1 item selected');
$assert_session->hiddenFieldValueEquals('current_selection', $selected_item_id);
- $png_uri_3 = $file_system->copy($png_image->uri);
+ $png_uri_3 = $file_system->copy($png_image->uri, 'public://');
$page->attachFileToField('Add files', $this->container->get('file_system')->realpath($png_uri_3));
$assert_session->assertWaitOnAjaxRequest();
$assert_session->checkboxChecked("Select $existing_media_name");
@@ -843,18 +843,18 @@ public function testWidgetUpload() {
// Assert media type four should only allow jpg files by trying a png file
// first.
- $png_uri_4 = $file_system->copy($png_image->uri);
+ $png_uri_4 = $file_system->copy($png_image->uri, 'public://');
$page->attachFileToField('Add file', $file_system->realpath($png_uri_4));
$assert_session->assertWaitOnAjaxRequest();
$assert_session->pageTextContains('Only files with the following extensions are allowed');
// Assert that jpg files are accepted by type four.
- $jpg_uri_2 = $file_system->copy($jpg_image->uri);
+ $jpg_uri_2 = $file_system->copy($jpg_image->uri, 'public://');
$page->attachFileToField('Add file', $file_system->realpath($jpg_uri_2));
$assert_session->assertWaitOnAjaxRequest();
$page->fillField('Alternative text', $this->randomString());
// The type_four media type has another optional image field.
$assert_session->pageTextContains('Extra Image');
- $jpg_uri_3 = $file_system->copy($jpg_image->uri);
+ $jpg_uri_3 = $file_system->copy($jpg_image->uri, 'public://');
$page->attachFileToField('Extra Image', $this->container->get('file_system')->realpath($jpg_uri_3));
$assert_session->assertWaitOnAjaxRequest();
// Ensure that the extra image was uploaded to the correct directory.
@@ -885,7 +885,7 @@ public function testWidgetUpload() {
$checkbox->click();
$assert_session->hiddenFieldValueEquals('current_selection', $selected_item_id);
$this->assertTrue($assert_session->fieldExists('Add files')->hasAttribute('multiple'));
- $png_uri_5 = $file_system->copy($png_image->uri);
+ $png_uri_5 = $file_system->copy($png_image->uri, 'public://');
$page->attachFileToField('Add files', $this->container->get('file_system')->realpath($png_uri_5));
$assert_session->assertWaitOnAjaxRequest();
// Assert the pre-selected items are shown.
@@ -951,7 +951,7 @@ public function testWidgetUpload() {
$filenames = [];
$remote_paths = [];
foreach (range(1, 3) as $i) {
- $path = $file_system->copy($png_image->uri);
+ $path = $file_system->copy($png_image->uri, 'public://');
$filenames[] = $file_system->basename($path);
$remote_paths[] = $driver->uploadFileAndGetRemoteFilePath($file_system->realpath($path));
}
diff --git a/core/modules/system/src/Form/ThemeSettingsForm.php b/core/modules/system/src/Form/ThemeSettingsForm.php
index ce242ee4a95b..83a8d1563425 100644
--- a/core/modules/system/src/Form/ThemeSettingsForm.php
+++ b/core/modules/system/src/Form/ThemeSettingsForm.php
@@ -463,7 +463,7 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
// and use it in place of the default theme-provided file.
try {
if (!empty($values['logo_upload'])) {
- $filename = $this->fileSystem->copy($values['logo_upload']->getFileUri());
+ $filename = $this->fileSystem->copy($values['logo_upload']->getFileUri(), file_default_scheme() . '://');
$values['default_logo'] = 0;
$values['logo_path'] = $filename;
}
@@ -473,7 +473,7 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
}
try {
if (!empty($values['favicon_upload'])) {
- $filename = $this->fileSystem->copy($values['favicon_upload']->getFileUri());
+ $filename = $this->fileSystem->copy($values['favicon_upload']->getFileUri(), file_default_scheme() . '://');
$values['default_favicon'] = 0;
$values['favicon_path'] = $filename;
$values['toggle_favicon'] = 1;
diff --git a/core/modules/update/tests/src/Functional/UpdateDeleteFileIfStaleTest.php b/core/modules/update/tests/src/Functional/UpdateDeleteFileIfStaleTest.php
index b468485f54f1..f7d875faacbf 100644
--- a/core/modules/update/tests/src/Functional/UpdateDeleteFileIfStaleTest.php
+++ b/core/modules/update/tests/src/Functional/UpdateDeleteFileIfStaleTest.php
@@ -27,7 +27,7 @@ protected function setUp() {
* Tests the deletion of stale files.
*/
public function testUpdateDeleteFileIfStale() {
- $file_name = \Drupal::service('file_system')->saveData($this->randomMachineName());
+ $file_name = \Drupal::service('file_system')->saveData($this->randomMachineName(), 'public://');
$this->assertNotNull($file_name);
// During testing the file change and the stale checking occurs in the same
diff --git a/core/tests/Drupal/KernelTests/Core/File/FileSaveDataTest.php b/core/tests/Drupal/KernelTests/Core/File/FileSaveDataTest.php
index 75b4c4d0b91e..b2ba943ef0e9 100644
--- a/core/tests/Drupal/KernelTests/Core/File/FileSaveDataTest.php
+++ b/core/tests/Drupal/KernelTests/Core/File/FileSaveDataTest.php
@@ -19,10 +19,6 @@ public function testFileSaveData() {
// No filename.
/** @var \Drupal\Core\File\FileSystemInterface $file_system */
$file_system = \Drupal::service('file_system');
- $filepath = $file_system->saveData($contents);
- $this->assertTrue($filepath, 'Unnamed file saved correctly.');
- $this->assertEqual(file_uri_scheme($filepath), file_default_scheme(), "File was placed in Drupal's files directory.");
- $this->assertEqual($contents, file_get_contents($filepath), 'Contents of the file are correct.');
// Provide a filename.
$filepath = $file_system->saveData($contents, 'public://asdf.txt', FILE_EXISTS_REPLACE);
diff --git a/core/tests/Drupal/KernelTests/Core/File/FileSystemDeprecationTest.php b/core/tests/Drupal/KernelTests/Core/File/FileSystemDeprecationTest.php
index 97112e36bba3..bc18bd3f7cef 100644
--- a/core/tests/Drupal/KernelTests/Core/File/FileSystemDeprecationTest.php
+++ b/core/tests/Drupal/KernelTests/Core/File/FileSystemDeprecationTest.php
@@ -30,7 +30,10 @@ public function testDeprecatedFileMoveUploadedFile() {
* @expectedDeprecation file_unmanaged_copy() is deprecated in Drupal 8.7.0 and will be removed before Drupal 9.0.0. Use \Drupal\Core\File\FileSystemInterface::copy(). See https://www.drupal.org/node/3006851.
*/
public function testDeprecatedUnmanagedFileCopy() {
- $this->assertNotNull(file_unmanaged_copy(NULL));
+ $source = file_directory_temp() . '/example.txt';
+ file_put_contents($source, 'example');
+ $filename = file_unmanaged_copy($source);
+ $this->assertEquals('public://example.txt', $filename);
}
/**
@@ -51,7 +54,10 @@ public function testDeprecatedUnmanagedFileDeleteRecursive() {
* @expectedDeprecation file_unmanaged_move() is deprecated in Drupal 8.7.0 and will be removed before Drupal 9.0.0. Use \Drupal\Core\File\FileSystemInterface::move(). See https://www.drupal.org/node/3006851.
*/
public function testDeprecatedUnmanagedFileMove() {
- $this->assertNotNull(file_unmanaged_move(NULL));
+ $source = file_directory_temp() . '/example.txt';
+ file_put_contents($source, 'example');
+ $filename = file_unmanaged_move($source);
+ $this->assertEquals('public://example.txt', $filename);
}
/**
@@ -65,7 +71,8 @@ public function testDeprecatedUnmanagedPrepare() {
* @expectedDeprecation file_unmanaged_save_data() is deprecated in Drupal 8.7.0 and will be removed before Drupal 9.0.0. Use \Drupal\Core\File\FileSystemInterface::saveData(). See https://www.drupal.org/node/3006851.
*/
public function testDeprecatedUnmanagedSaveData() {
- $this->assertNotNull(file_unmanaged_save_data(NULL));
+ $filename = file_unmanaged_save_data('example');
+ $this->assertStringMatchesFormat('public://file%s', $filename);
}
/**
--
GitLab