Drupal
/
drupal
mirror of https://git.drupalcode.org/project/drupal.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Issue #3021434 by voleger, Peter Majmesku, andypost, Berdir, kim.pepper: Properly deprecate drupal_unlink()

merge-requests/1119/head
Lee Rowlands 2019-04-26 12:44:21 +10:00
parent 073cc74c4d
commit 870b5646b2
No known key found for this signature in database
GPG Key ID: 2B829A3DF9204DC4
19 changed files with 105 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
core/includes/file.inc
View File

@ -1091,12 +1091,14 @@ function drupal_chmod($uri, $mode = NULL) {
/**
* Deletes a file.
*
* @deprecated in Drupal 8.0.x-dev, will be removed before Drupal 9.0.0.
* @deprecated in Drupal 8.0.0, will be removed before Drupal 9.0.0.
* Use \Drupal\Core\File\FileSystem::unlink().
*
* @see \Drupal\Core\File\FileSystem::unlink()
* @see https://www.drupal.org/node/2418133
*/
function drupal_unlink($uri, $context = NULL) {
@trigger_error('drupal_unlink() is deprecated in Drupal 8.0.0, will be removed before Drupal 9.0.0. Use \Drupal\Core\File\FileSystemInterface::unlink(). See https://www.drupal.org/node/2418133.', E_USER_DEPRECATED);
return \Drupal::service('file_system')->unlink($uri, $context);
}

3
core/includes/install.core.inc
View File

@ -2085,6 +2085,7 @@ function install_check_requirements($install_state) {
'description_default' => t('The default settings file does not exist.'),
'title' => t('Settings file'),
];
$file_system = \Drupal::service('file_system');
foreach ($default_files as $default_file_info) {
$readable = FALSE;
@ -2146,7 +2147,7 @@ function install_check_requirements($install_state) {
// file we just created and force the administrator to log on to the
// server and create it manually.
else {
$deleted = @drupal_unlink($file);
$deleted = @$file_system->unlink($file);
// We expect deleting the file to be successful (since we just
// created it ourselves above), but if it fails somehow, we set a
// variable so we can display a one-time error message to the

2
core/lib/Drupal/Core/File/FileSystem.php
View File

@ -411,7 +411,7 @@ class FileSystem implements FileSystemInterface {
if (!@rename($real_source, $real_destination)) {
// Fall back to slow copy and unlink procedure. This is necessary for
// renames across schemes that are not local, or where rename() has not
// been implemented. It's not necessary to use drupal_unlink() as the
// been implemented. It's not necessary to use FileSystem::unlink() as the
// Windows issue has already been resolved above.
if (!@copy($real_source, $real_destination)) {
$this->logger->error("The specified file '%source' could not be moved to '%destination'.", [

30
core/lib/Drupal/Core/FileTransfer/Local.php
View File

@ -2,11 +2,35 @@
namespace Drupal\Core\FileTransfer;
use Drupal\Core\DependencyInjection\DependencySerializationTrait;
use Drupal\Core\File\FileSystemInterface;
/**
* Defines the local connection class for copying files as the httpd user.
*/
class Local extends FileTransfer implements ChmodInterface {
use DependencySerializationTrait;
/**
* The file system service.
*
* @var \Drupal\Core\File\FileSystemInterface
*/
protected $fileSystem;
/**
* {@inheritdoc}
*/
public function __construct($jail, FileSystemInterface $file_system = NULL) {
parent::__construct($jail);
if (!isset($file_system)) {
@trigger_error('The $file_system parameter was added in Drupal 8.8.0 and will be required in 9.0.0. See https://www.drupal.org/node/3021434.', E_USER_DEPRECATED);
$file_system = \Drupal::service('file_system');
}
$this->fileSystem = $file_system;
}
/**
* {@inheritdoc}
*/
@ -18,7 +42,7 @@ class Local extends FileTransfer implements ChmodInterface {
* {@inheritdoc}
*/
public static function factory($jail, $settings) {
return new Local($jail);
return new Local($jail, \Drupal::service('file_system'));
}
/**
@ -56,7 +80,7 @@ class Local extends FileTransfer implements ChmodInterface {
}
}
elseif ($file->isFile()) {
if (@!drupal_unlink($filename)) {
if (@!$this->fileSystem->unlink($filename)) {
throw new FileTransferException('Cannot remove file %file.', NULL, ['%file' => $filename]);
}
}
@ -70,7 +94,7 @@ class Local extends FileTransfer implements ChmodInterface {
* {@inheritdoc}
*/
protected function removeFileJailed($file) {
if (@!drupal_unlink($file)) {
if (@!$this->fileSystem->unlink($file)) {
throw new FileTransferException('Cannot remove file %file.', NULL, ['%file' => $file]);
}
}

12
core/lib/Drupal/Core/StreamWrapper/LocalStream.php
View File

@ -374,7 +374,7 @@ abstract class LocalStream implements StreamWrapperInterface {
*/
public function unlink($uri) {
$this->uri = $uri;
return drupal_unlink($this->getLocalPath());
return $this->getFileSystem()->unlink($this->getLocalPath());
}
/**
@ -572,4 +572,14 @@ abstract class LocalStream implements StreamWrapperInterface {
return TRUE;
}
/**
* Returns file system service.
*
* @return \Drupal\Core\File\FileSystemInterface
* The file system service.
*/
private function getFileSystem() {
return \Drupal::service('file_system');
}
}

3
core/modules/color/color.module
View File

@ -424,11 +424,12 @@ function color_scheme_form_submit($form, FormStateInterface $form_state) {
}
}
$file_system = \Drupal::service('file_system');
// Delete old files.
$files = $config->get('files');
if (isset($files)) {
foreach ($files as $file) {
@drupal_unlink($file);
@$file_system->unlink($file);
}
}
if (isset($file) && $file = dirname($file)) {

22
core/modules/config/src/Form/ConfigImportForm.php
View File

@ -4,6 +4,7 @@ namespace Drupal\config\Form;
use Drupal\Core\Archiver\ArchiveTar;
use Drupal\Core\Config\StorageInterface;
use Drupal\Core\File\FileSystemInterface;
use Drupal\Core\Form\FormBase;
use Drupal\Core\Form\FormStateInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;
@ -22,14 +23,28 @@ class ConfigImportForm extends FormBase {
*/
protected $configStorage;
/**
* The file system service.
*
* @var \Drupal\Core\File\FileSystemInterface
*/
protected $fileSystem;
/**
* Constructs a new ConfigImportForm.
*
* @param \Drupal\Core\Config\StorageInterface $config_storage
* The configuration storage.
* @param \Drupal\Core\File\FileSystemInterface $file_system
* The file system service.
*/
public function __construct(StorageInterface $config_storage) {
public function __construct(StorageInterface $config_storage, FileSystemInterface $file_system = NULL) {
$this->configStorage = $config_storage;
if (!isset($file_system)) {
@trigger_error('The $file_system parameter was added in Drupal 8.8.0 and will be required in 9.0.0. See https://www.drupal.org/node/3021434.', E_USER_DEPRECATED);
$file_system = \Drupal::service('file_system');
}
$this->fileSystem = $file_system;
}
/**
@ -37,7 +52,8 @@ class ConfigImportForm extends FormBase {
*/
public static function create(ContainerInterface $container) {
return new static(
$container->get('config.storage.sync')
$container->get('config.storage.sync'),
$container->get('file_system')
);
}
@ -106,7 +122,7 @@ class ConfigImportForm extends FormBase {
catch (\Exception $e) {
$this->messenger()->addError($this->t('Could not extract the contents of the tar file. The error message is <em>@message</em>', ['@message' => $e->getMessage()]));
}
drupal_unlink($path);
$this->fileSystem->unlink($path);
}
}

2
core/modules/file/tests/src/Kernel/ValidatorTest.php
View File

@ -106,7 +106,7 @@ class ValidatorTest extends FileManagedUnitTestBase {
$errors = file_validate_image_resolution($this->image, '-10x-5');
$this->assertEqual(count($errors), 1, 'An error reported for an oversized image that can not be scaled down.', 'File');
drupal_unlink('temporary://druplicon.png');
\Drupal::service('file_system')->unlink('temporary://druplicon.png');
}
else {
// TODO: should check that the error is returned if no toolkit is available.

9
core/modules/locale/tests/src/Functional/LocaleExportTest.php
View File

@ -41,15 +41,16 @@ class LocaleExportTest extends BrowserTestBase {
* Test exportation of translations.
*/
public function testExportTranslation() {
$file_system = \Drupal::service('file_system');
// First import some known translations.
// This will also automatically add the 'fr' language.
$name = \Drupal::service('file_system')->tempnam('temporary://', "po_") . '.po';
$name = $file_system->tempnam('temporary://', "po_") . '.po';
file_put_contents($name, $this->getPoFile());
$this->drupalPostForm('admin/config/regional/translate/import', [
'langcode' => 'fr',
'files[file]' => $name,
], t('Import'));
drupal_unlink($name);
$file_system->unlink($name);
// Get the French translations.
$this->drupalPostForm('admin/config/regional/translate/export', [
@ -62,14 +63,14 @@ class LocaleExportTest extends BrowserTestBase {
$this->assertRaw('msgstr "lundi"', 'French translations present in exported file.');
// Import some more French translations which will be marked as customized.
$name = \Drupal::service('file_system')->tempnam('temporary://', "po2_") . '.po';
$name = $file_system->tempnam('temporary://', "po2_") . '.po';
file_put_contents($name, $this->getCustomPoFile());
$this->drupalPostForm('admin/config/regional/translate/import', [
'langcode' => 'fr',
'files[file]' => $name,
'customized' => 1,
], t('Import'));
drupal_unlink($name);
$file_system->unlink($name);
// Create string without translation in the locales_source table.
$this->container

5
core/modules/locale/tests/src/Functional/LocaleImportFunctionalTest.php
View File

@ -374,11 +374,12 @@ class LocaleImportFunctionalTest extends BrowserTestBase {
* (optional) Additional options to pass to the translation import form.
*/
public function importPoFile($contents, array $options = []) {
$name = \Drupal::service('file_system')->tempnam('temporary://', "po_") . '.po';
$file_system = \Drupal::service('file_system');
$name = $file_system->tempnam('temporary://', "po_") . '.po';
file_put_contents($name, $contents);
$options['files[file]'] = $name;
$this->drupalPostForm('admin/config/regional/translate/import', $options, t('Import'));
drupal_unlink($name);
$file_system->unlink($name);
}
/**

5
core/modules/locale/tests/src/Functional/LocalePluralFormatTest.php
View File

@ -352,11 +352,12 @@ class LocalePluralFormatTest extends BrowserTestBase {
* Additional options to pass to the translation import form.
*/
public function importPoFile($contents, array $options = []) {
$name = \Drupal::service('file_system')->tempnam('temporary://', "po_") . '.po';
$file_system = \Drupal::service('file_system');
$name = $file_system->tempnam('temporary://', "po_") . '.po';
file_put_contents($name, $contents);
$options['files[file]'] = $name;
$this->drupalPostForm('admin/config/regional/translate/import', $options, t('Import'));
drupal_unlink($name);
$file_system->unlink($name);
}
/**

5
core/modules/system/tests/src/Functional/Theme/TwigTransTest.php
View File

@ -192,6 +192,7 @@ class TwigTransTest extends BrowserTestBase {
* Helper function: install languages.
*/
protected function installLanguages() {
$file_system = \Drupal::service('file_system');
foreach ($this->languages as $langcode => $name) {
// Generate custom .po contents for the language.
$contents = $this->poFileContents($langcode);
@ -209,7 +210,7 @@ class TwigTransTest extends BrowserTestBase {
$this->assertRaw('"edit-languages-' . $langcode . '-weight"', 'Language code found.');
// Import the custom .po contents for the language.
$filename = \Drupal::service('file_system')->tempnam('temporary://', "po_") . '.po';
$filename = $file_system->tempnam('temporary://', "po_") . '.po';
file_put_contents($filename, $contents);
$options = [
'files[file]' => $filename,
@ -217,7 +218,7 @@ class TwigTransTest extends BrowserTestBase {
'customized' => TRUE,
];
$this->drupalPostForm('admin/config/regional/translate/import', $options, t('Import'));
drupal_unlink($filename);
$file_system->unlink($filename);
}
}
$this->container->get('language_manager')->reset();

2
core/modules/update/src/Form/UpdateManagerInstall.php
View File

@ -234,7 +234,7 @@ class UpdateManagerInstall extends FormBase {
// update_authorize_run_install() directly.
if (fileowner($project_real_location) == fileowner($this->sitePath) && !$test_authorize) {
$this->moduleHandler->loadInclude('update', 'inc', 'update.authorize');
$filetransfer = new Local($this->root);
$filetransfer = new Local($this->root, \Drupal::service('file_system'));
$response = call_user_func_array('update_authorize_run_install', array_merge([$filetransfer], $arguments));
if ($response instanceof Response) {
$form_state->setResponse($response);

2
core/modules/update/src/Form/UpdateReady.php
View File

@ -153,7 +153,7 @@ class UpdateReady extends FormBase {
// and invoke update_authorize_run_update() directly.
if (fileowner($project_real_location) == fileowner($this->sitePath)) {
$this->moduleHandler->loadInclude('update', 'inc', 'update.authorize');
$filetransfer = new Local($this->root);
$filetransfer = new Local($this->root, \Drupal::service('file_system'));
$response = update_authorize_run_update($filetransfer, $updates);
if ($response instanceof Response) {
$form_state->setResponse($response);

2
core/modules/update/tests/modules/update_test/src/TestFileTransferWithSettingsForm.php
View File

@ -22,7 +22,7 @@ class TestFileTransferWithSettingsForm extends Local {
* A new Drupal\update_test\TestFileTransferWithSettingsForm object.
*/
public static function factory($jail, $settings) {
return new static($jail);
return new static($jail, \Drupal::service('file_system'));
}
/**

3
core/modules/update/update.manager.inc
View File

@ -309,6 +309,7 @@ function update_manager_batch_project_get($project, $url, &$context) {
* @see install_check_requirements()
*/
function update_manager_local_transfers_allowed() {
$file_system = \Drupal::service('file_system');
// Compare the owner of a webserver-created temporary file to the owner of
// the configuration directory to determine if local transfers will be
// allowed.
@ -318,7 +319,7 @@ function update_manager_local_transfers_allowed() {
// Clean up. If this fails, we can ignore it (since this is just a temporary
// file anyway).
@drupal_unlink($temporary_file);
@$file_system->unlink($temporary_file);
return $local_transfers_allowed;
}

4
core/tests/Drupal/KernelTests/Core/File/DirectoryTest.php
View File

@ -72,7 +72,7 @@ class DirectoryTest extends FileTestBase {
// Make sure directory actually exists.
$this->assertTrue(is_dir($directory), 'Directory actually exists.', 'File');
$file_system = \Drupal::service('file_system');
if (substr(PHP_OS, 0, 3) != 'WIN') {
// PHP on Windows doesn't support any kind of useful read-only mode for
// directories. When executing a chmod() on a directory, PHP only sets the
@ -92,7 +92,7 @@ class DirectoryTest extends FileTestBase {
$this->assertDirectoryPermissions($directory, 0777, 'file_chmod_directory setting is respected.');
// Remove .htaccess file to then test that it gets re-created.
@drupal_unlink(file_default_scheme() . '://.htaccess');
@$file_system->unlink(file_default_scheme() . '://.htaccess');
$this->assertFalse(is_file(file_default_scheme() . '://.htaccess'), 'Successfully removed the .htaccess file in the files directory.', 'File');
file_ensure_htaccess();
$this->assertTrue(is_file(file_default_scheme() . '://.htaccess'), 'Successfully re-created the .htaccess file in the files directory.', 'File');

16
core/tests/Drupal/KernelTests/Core/File/FileSystemDeprecationTest.php
View File

@ -3,6 +3,7 @@
namespace Drupal\KernelTests\Core\File;
use Drupal\KernelTests\KernelTestBase;
use org\bovigo\vfs\vfsStream;
/**
* Tests deprecations in file.inc.
@ -146,4 +147,19 @@ class FileSystemDeprecationTest extends KernelTestBase {
$this->assertNotNull(drupal_tempnam('temporary://', 'file'));
}
/**
* Tests deprecation of the drupal_unlink() function.
*
* @expectedDeprecation drupal_unlink() is deprecated in Drupal 8.0.0, will be removed before Drupal 9.0.0. Use \Drupal\Core\File\FileSystemInterface::unlink(). See https://www.drupal.org/node/2418133.
*/
public function testUnlink() {
vfsStream::setup('dir');
vfsStream::create(['test.txt' => 'asdf']);
$uri = 'vfs://dir/test.txt';
$this->assertFileExists($uri);
drupal_unlink($uri);
$this->assertFileNotExists($uri);
}
}

3
core/tests/Drupal/KernelTests/Core/File/ReadOnlyStreamWrapperTest.php
View File

@ -46,6 +46,7 @@ class ReadOnlyStreamWrapperTest extends FileTestBase {
$uri = $this->scheme . '://' . $filename;
\Drupal::service('stream_wrapper_manager')->getViaScheme($this->scheme);
$file_system = \Drupal::service('file_system');
// Attempt to open a file in read/write mode
$handle = @fopen($uri, 'r+');
$this->assertFalse($handle, 'Unable to open a file for reading and writing with the read-only stream wrapper.');
@ -79,7 +80,7 @@ class ReadOnlyStreamWrapperTest extends FileTestBase {
// Test the rename() function
$this->assertFalse(@rename($uri, $this->scheme . '://newname.txt'), 'Unable to rename files using the read-only stream wrapper.');
// Test the unlink() function
$this->assertTrue(@drupal_unlink($uri), 'Able to unlink file using read-only stream wrapper.');
$this->assertTrue(@$file_system->unlink($uri), 'Able to unlink file using read-only stream wrapper.');
$this->assertTrue(file_exists($filepath), 'Unlink File was not actually deleted.');
// Test the mkdir() function by attempting to create a directory.