Commit c848fb6b authored by catch's avatar catch

Issue #3039026 by kim.pepper, Berdir, Peter Majmesku, vadim.hirbu, YurkinPark,...

Issue #3039026 by kim.pepper, Berdir, Peter Majmesku, vadim.hirbu, YurkinPark, andypost, dww, claudiu.cristea, alexpott, jibran, dawehner, mikelutz: Deprecate file_directory_temp() and move to FileSystem service
parent 3a6e13de
......@@ -15,7 +15,6 @@
use Drupal\Core\File\FileSystemInterface;
use Drupal\Core\Site\Settings;
use Drupal\Core\StreamWrapper\PrivateStream;
use Drupal\Core\StreamWrapper\PublicStream;
use Drupal\Core\StreamWrapper\StreamWrapperManager;
/**
......@@ -1138,30 +1137,16 @@ function drupal_tempnam($directory, $prefix) {
*
* @return mixed|null
* A string containing the path to the temporary directory.
*
* @deprecated in drupal:8.8.0 and is removed from drupal:9.0.0. Use
* \Drupal\Core\File\FileSystemInterface::getTempDirectory() instead.
*
* @see \Drupal\Core\File\FileSystemInterface::getTempDirectory()
* @see https://www.drupal.org/node/3039255
*/
function file_directory_temp() {
$temporary_directory = \Drupal::config('system.file')->get('path.temporary');
if (empty($temporary_directory)) {
// Needs set up.
$config = \Drupal::configFactory()->getEditable('system.file');
$temporary_directory = ComponentFileSystem::getOsTemporaryDirectory();
if (empty($temporary_directory)) {
// If no directory has been found default to 'files/tmp'.
$temporary_directory = PublicStream::basePath() . '/tmp';
// Windows accepts paths with either slash (/) or backslash (\), but will
// not accept a path which contains both a slash and a backslash. Since
// the 'file_public_path' variable may have either format, we sanitize
// everything to use slash which is supported on all platforms.
$temporary_directory = str_replace('\\', '/', $temporary_directory);
}
// Save the path of the discovered directory. Do not check config schema on
// save.
$config->set('path.temporary', (string) $temporary_directory)->save(TRUE);
}
return $temporary_directory;
@trigger_error('file_directory_temp() is deprecated in drupal:8.8.0 and is removed from drupal:9.0.0. Use \Drupal\Core\File\FileSystemInterface::getTempDirectory() instead. See https://www.drupal.org/node/3039255', E_USER_DEPRECATED);
return \Drupal::service('file_system')->getTempDirectory();
}
/**
......
......@@ -27,7 +27,6 @@
use Drupal\Core\Site\Settings;
use Drupal\Core\StringTranslation\Translator\FileTranslation;
use Drupal\Core\StackMiddleware\ReverseProxyMiddleware;
use Drupal\Core\StreamWrapper\PublicStream;
use Drupal\Core\Extension\ExtensionDiscovery;
use Drupal\Core\DependencyInjection\ContainerBuilder;
use Drupal\Core\Url;
......@@ -1100,21 +1099,6 @@ function install_base_system(&$install_state) {
// Install system.module.
drupal_install_system($install_state);
// Prevent the installer from using the system temporary directory after the
// system module has been installed.
if (drupal_valid_test_ua()) {
// While the temporary directory could be preset/enforced in settings.php
// like the public files directory, some tests expect it to be configurable
// in the UI. If declared in settings.php, they would no longer be
// configurable. The temporary directory needs to match what is set in each
// test types ::prepareEnvironment() step.
$temporary_directory = dirname(PublicStream::basePath()) . '/temp';
\Drupal::service('file_system')->prepareDirectory($temporary_directory, FileSystemInterface::MODIFY_PERMISSIONS | FileSystemInterface::CREATE_DIRECTORY);
\Drupal::configFactory()->getEditable('system.file')
->set('path.temporary', $temporary_directory)
->save();
}
// Call file_ensure_htaccess() to ensure that all of Drupal's standard
// directories (e.g., the public files directory and config directory) have
// appropriate .htaccess files. These directories will have already been
......
......@@ -2,6 +2,7 @@
namespace Drupal\Core\File;
use Drupal\Component\FileSystem\FileSystem as FileSystemComponent;
use Drupal\Component\Utility\Unicode;
use Drupal\Core\File\Exception\DirectoryNotReadyException;
use Drupal\Core\File\Exception\FileException;
......@@ -11,6 +12,7 @@
use Drupal\Core\File\Exception\NotRegularDirectoryException;
use Drupal\Core\File\Exception\NotRegularFileException;
use Drupal\Core\Site\Settings;
use Drupal\Core\StreamWrapper\PublicStream;
use Drupal\Core\StreamWrapper\StreamWrapperManager;
use Drupal\Core\StreamWrapper\StreamWrapperManagerInterface;
use Psr\Log\LoggerInterface;
......@@ -626,6 +628,44 @@ public function createFilename($basename, $directory) {
return $destination;
}
/**
* {@inheritdoc}
*/
public function getTempDirectory() {
// Use settings.
$temporary_directory = $this->settings->get('file_temp_path');
if (!empty($temporary_directory)) {
return $temporary_directory;
}
// Fallback to config for Backwards compatibility.
// This service is lazy-loaded and not injected, as the file_system service
// is used in the install phase before config_factory service exists. It
// will be removed before Drupal 9.0.0.
if (\Drupal::hasContainer()) {
$temporary_directory = \Drupal::config('system.file')->get('path.temporary');
if (!empty($temporary_directory)) {
@trigger_error("The 'system.file' config 'path.temporary' is deprecated in drupal:8.8.0 and is removed from drupal:9.0.0. Set 'file_temp_path' in settings.php instead. See https://www.drupal.org/node/3039255", E_USER_DEPRECATED);
return $temporary_directory;
}
}
// Fallback to OS default.
$temporary_directory = FileSystemComponent::getOsTemporaryDirectory();
if (empty($temporary_directory)) {
// If no directory has been found default to 'files/tmp'.
$temporary_directory = PublicStream::basePath() . '/tmp';
// Windows accepts paths with either slash (/) or backslash (\), but
// will not accept a path which contains both a slash and a backslash.
// Since the 'file_public_path' variable may have either format, we
// sanitize everything to use slash which is supported on all platforms.
$temporary_directory = str_replace('\\', '/', $temporary_directory);
}
return $temporary_directory;
}
/**
* {@inheritdoc}
*/
......
......@@ -472,6 +472,18 @@ public function createFilename($basename, $directory);
*/
public function getDestinationFilename($destination, $replace);
/**
* Gets the path of the configured temporary directory.
*
* If the path is not set, it will fall back to the OS-specific default if
* set, otherwise a directory under the public files directory. It will then
* set this as the configured directory.
*
* @return string
* A string containing the path to the temporary directory.
*/
public function getTempDirectory();
/**
* Finds all files that match a given mask in a given directory.
*
......
......@@ -37,7 +37,7 @@ public function getDescription() {
* {@inheritdoc}
*/
public function getDirectoryPath() {
return file_directory_temp();
return \Drupal::service('file_system')->getTempDirectory();
}
/**
......
......@@ -91,6 +91,10 @@ protected function prepareSettings() {
'value' => $this->privateFilesDirectory,
'required' => TRUE,
];
$settings['settings']['file_temp_path'] = (object) [
'value' => $this->tempFilesDirectory,
'required' => TRUE,
];
// Save the original site directory path, so that extensions in the
// site-specific directory can still be discovered in the test site
// environment.
......
......@@ -121,13 +121,13 @@ public function __construct(StorageInterface $target_storage, StorageInterface $
*/
public function downloadExport() {
try {
$this->fileSystem->delete(file_directory_temp() . '/config.tar.gz');
$this->fileSystem->delete($this->fileSystem->getTempDirectory() . '/config.tar.gz');
}
catch (FileException $e) {
// Ignore failed deletes.
}
$archiver = new ArchiveTar(file_directory_temp() . '/config.tar.gz', 'gz');
$archiver = new ArchiveTar($this->fileSystem->getTempDirectory() . '/config.tar.gz', 'gz');
// Add all contents of the export storage to the archive.
foreach ($this->exportStorage->listAll() as $name) {
$archiver->addString("$name.yml", Yaml::encode($this->exportStorage->read($name)));
......
......@@ -226,7 +226,7 @@ public function testExportImportCollections() {
// Export the configuration.
$this->drupalPostForm('admin/config/development/configuration/full/export', [], 'Export');
$this->tarball = $this->getSession()->getPage()->getContent();
$filename = file_directory_temp() . '/' . $this->randomMachineName();
$filename = \Drupal::service('file_system')->getTempDirectory() . '/' . $this->randomMachineName();
file_put_contents($filename, $this->tarball);
// Set up the active storage collections to test import.
......
......@@ -3,6 +3,7 @@
namespace Drupal\Tests\config\Functional;
use Drupal\Core\Archiver\Tar;
use Drupal\Core\File\FileSystemInterface;
use Drupal\Core\Serialization\Yaml;
use Drupal\Tests\BrowserTestBase;
......@@ -57,7 +58,10 @@ public function testExport() {
$this->assertTrue($header_match, "Header with filename matches the expected format.");
// Extract the archive and verify it's not empty.
$file_path = file_directory_temp() . '/config.tar.gz';
$file_system = \Drupal::service('file_system');
assert($file_system instanceof FileSystemInterface);
$temp_directory = $file_system->getTempDirectory();
$file_path = $temp_directory . '/config.tar.gz';
$archiver = new Tar($file_path);
$archive_contents = $archiver->listContents();
$this->assert(!empty($archive_contents), 'Downloaded archive file is not empty.');
......@@ -75,8 +79,8 @@ public function testExport() {
// Ensure the test configuration override is in effect but was not exported.
$this->assertIdentical(\Drupal::config('system.maintenance')->get('message'), 'Foo');
$archiver->extract(file_directory_temp(), ['system.maintenance.yml']);
$file_contents = file_get_contents(file_directory_temp() . '/' . 'system.maintenance.yml');
$archiver->extract($temp_directory, ['system.maintenance.yml']);
$file_contents = file_get_contents($temp_directory . '/' . 'system.maintenance.yml');
$exported = Yaml::decode($file_contents);
$this->assertNotIdentical($exported['message'], 'Foo');
......
......@@ -101,11 +101,6 @@ public function testFiles() {
->fields(['value' => serialize('files/test')])
->condition('name', 'file_directory_path')
->execute();
Database::getConnection('default', 'migrate')
->update('variable')
->fields(['value' => serialize(file_directory_temp())])
->condition('name', 'file_directory_temp')
->execute();
$this->executeMigration('d6_file');
......
allow_insecure_uploads: false
default_scheme: 'public'
path:
temporary: ''
temporary_maximum_age: 21600
......@@ -6,11 +6,9 @@ migration_tags:
source:
plugin: variable
variables:
- file_directory_temp
- allow_insecure_uploads
source_module: system
process:
'path/temporary': file_directory_temp
allow_insecure_uploads:
plugin: static_map
source: allow_insecure_uploads
......
......@@ -7,7 +7,6 @@ source:
plugin: variable
variables:
- allow_insecure_uploads
- file_temporary_path
source_module: system
process:
allow_insecure_uploads:
......@@ -16,7 +15,6 @@ process:
map:
0: FALSE
1: TRUE
'path/temporary': file_temporary_path
destination:
plugin: config
config_name: system.file
......@@ -4,6 +4,7 @@
use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\Datetime\DateFormatterInterface;
use Drupal\Core\File\FileSystemInterface;
use Drupal\Core\Form\FormStateInterface;
use Drupal\Core\StreamWrapper\PrivateStream;
use Drupal\Core\StreamWrapper\PublicStream;
......@@ -33,6 +34,13 @@ class FileSystemForm extends ConfigFormBase {
*/
protected $streamWrapperManager;
/**
* The file system.
*
* @var \Drupal\Core\File\FileSystemInterface
*/
protected $fileSystem;
/**
* Constructs a FileSystemForm object.
*
......@@ -42,11 +50,18 @@ class FileSystemForm extends ConfigFormBase {
* The date formatter service.
* @param \Drupal\Core\StreamWrapper\StreamWrapperManagerInterface $stream_wrapper_manager
* The stream wrapper manager.
* @param \Drupal\Core\File\FileSystemInterface $file_system
* The file system.
*/
public function __construct(ConfigFactoryInterface $config_factory, DateFormatterInterface $date_formatter, StreamWrapperManagerInterface $stream_wrapper_manager) {
public function __construct(ConfigFactoryInterface $config_factory, DateFormatterInterface $date_formatter, StreamWrapperManagerInterface $stream_wrapper_manager, FileSystemInterface $file_system = NULL) {
parent::__construct($config_factory);
$this->dateFormatter = $date_formatter;
$this->streamWrapperManager = $stream_wrapper_manager;
if (!$file_system) {
@trigger_error('Calling FileSystemForm::__construct() without the $file_system argument is deprecated in drupal:8.8.0. The $file_system argument will be required in drupal:9.0.0. See https://www.drupal.org/node/3039255', E_USER_DEPRECATED);
$file_system = \Drupal::service('file_system');
}
$this->fileSystem = $file_system;
}
/**
......@@ -56,7 +71,8 @@ public static function create(ContainerInterface $container) {
return new static(
$container->get('config.factory'),
$container->get('date.formatter'),
$container->get('stream_wrapper_manager')
$container->get('stream_wrapper_manager'),
$container->get('file_system')
);
}
......@@ -101,12 +117,10 @@ public function buildForm(array $form, FormStateInterface $form_state) {
];
$form['file_temporary_path'] = [
'#type' => 'textfield',
'#type' => 'item',
'#title' => t('Temporary directory'),
'#default_value' => $config->get('path.temporary'),
'#maxlength' => 255,
'#description' => t('A local file system path where temporary files will be stored. This directory should not be accessible over the web.'),
'#after_build' => ['system_check_directory'],
'#markup' => $this->fileSystem->getTempDirectory(),
'#description' => t('A local file system path where temporary files will be stored. This directory should not be accessible over the web. This must be changed in settings.php.'),
];
// Any visible, writeable wrapper can potentially be used for the files
// directory, including a remote file system that integrates with a CDN.
......@@ -141,7 +155,6 @@ public function buildForm(array $form, FormStateInterface $form_state) {
*/
public function submitForm(array &$form, FormStateInterface $form_state) {
$config = $this->config('system.file')
->set('path.temporary', $form_state->getValue('file_temporary_path'))
->set('temporary_maximum_age', $form_state->getValue('temporary_maximum_age'));
if ($form_state->hasValue('file_default_scheme')) {
......
......@@ -529,6 +529,38 @@ function system_requirements($phase) {
}
}
// Test that path.temporary config is not set.
if ($phase == 'runtime') {
if (!Settings::get('file_temp_path')) {
$filesystem_config = \Drupal::config('system.file');
if ($temp_path = $filesystem_config->get('path.temporary')) {
$requirements['temp_directory'] = [
'title' => t('Temporary Directory'),
'severity' => REQUIREMENT_WARNING,
'value' => 'Deprecated configuration',
'description' => [
[
'#markup' => t('You are using deprecated configuration for the temporary files path.'),
'#suffix' => ' ',
],
],
];
if ($temp_path === FileSystem::getOsTemporaryDirectory()) {
$requirements['temp_directory']['description'][] = [
'#markup' => t('Your temporary directory configuration matches the OS default and can be safely removed.'),
'#suffix' => ' ',
];
}
else {
$requirements['temp_directory']['description'][] = [
'#markup' => t('Remove the configuration and add the following to <code>settings.php</code>. <code>$settings["file_temp_path"] = "%temp_path"', ['%temp_path' => $temp_path]),
'#suffix' => ' ',
];
}
}
}
}
// Report cron status.
if ($phase == 'runtime') {
$cron_config = \Drupal::config('system.cron');
......@@ -596,7 +628,7 @@ function system_requirements($phase) {
// By default no private files directory is configured. For private files
// to be secure the admin needs to provide a path outside the webroot.
PrivateStream::basePath(),
file_directory_temp(),
\Drupal::service('file_system')->getTempDirectory(),
];
}
......@@ -617,8 +649,8 @@ function system_requirements($phase) {
if ($file_private_path = Settings::get('file_private_path')) {
$directories[] = $file_private_path;
}
if (!empty($GLOBALS['config']['system.file']['path']['temporary'])) {
$directories[] = $GLOBALS['config']['system.file']['path']['temporary'];
if (Settings::get('file_temp_path')) {
$directories[] = Settings::get('file_temp_path');
}
else {
// If the temporary directory is not overridden use an appropriate
......@@ -2312,3 +2344,16 @@ function system_update_8702() {
}
\Drupal::entityTypeManager()->useCaches(TRUE);
}
/**
* Remove 'path.temporary' config if redundant.
*/
function system_update_8801() {
// If settings is already being used, or the config is set to the OS default,
// clear the config value.
$config = Drupal::configFactory()->getEditable('system.file');
if (Settings::get('file_temp_path') || $config->get('path.temporary') === FileSystem::getOsTemporaryDirectory()) {
$config->clear('path.temporary')
->save(TRUE);
}
}
......@@ -27,7 +27,6 @@ public function testFileConfigurationPage() {
// upon form submission.
$file_path = $this->publicFilesDirectory;
$fields = [
'file_temporary_path' => $file_path . '/file_config_page_test/temporary',
'file_default_scheme' => 'private',
];
......
......@@ -47,9 +47,6 @@ class MigrateSystemConfigurationTest extends MigrateDrupal6TestBase {
'allow_insecure_uploads' => TRUE,
// default_scheme is not handled by the migration.
'default_scheme' => 'public',
'path' => [
'temporary' => 'files/temp',
],
// temporary_maximum_age is not handled by the migration.
'temporary_maximum_age' => 21600,
],
......
......@@ -45,9 +45,6 @@ class MigrateSystemConfigurationTest extends MigrateDrupal7TestBase {
'allow_insecure_uploads' => TRUE,
// default_scheme is not handled by the migration.
'default_scheme' => 'public',
'path' => [
'temporary' => '/tmp',
],
// temporary_maximum_age is not handled by the migration.
'temporary_maximum_age' => 21600,
],
......
......@@ -190,6 +190,10 @@ protected function setUp() {
$sync_directory = Settings::get('config_sync_directory');
\Drupal::service('file_system')->prepareDirectory($sync_directory, FileSystemInterface::CREATE_DIRECTORY | FileSystemInterface::MODIFY_PERMISSIONS);
// Ensure the default temp directory exist and is writable. The configured
// temp directory may be removed during update.
\Drupal::service('file_system')->prepareDirectory($this->tempFilesDirectory, FileSystemInterface::CREATE_DIRECTORY | FileSystemInterface::MODIFY_PERMISSIONS);
// Set the container. parent::rebuildAll() would normally do this, but this
// not safe to do here, because the database has not been updated yet.
$this->container = \Drupal::getContainer();
......
......@@ -2,6 +2,7 @@
namespace Drupal\KernelTests\Core\File;
use Drupal\Component\FileSystem\FileSystem;
use Drupal\Component\Render\FormattableMarkup;
use Drupal\Component\PhpStorage\FileStorage;
use Drupal\Core\File\Exception\FileException;
......@@ -167,15 +168,12 @@ public function testFileDestination() {
}
/**
* Ensure that the file_directory_temp() function always returns a value.
* Ensure that the getTempDirectory() method always returns a value.
*/
public function testFileDirectoryTemp() {
// Start with an empty variable to ensure we have a clean slate.
$config = $this->config('system.file');
$config->set('path.temporary', '')->save();
$tmp_directory = file_directory_temp();
$this->assertEqual(empty($tmp_directory), FALSE, 'file_directory_temp() returned a non-empty value.');
$this->assertEqual($config->get('path.temporary'), $tmp_directory);
$tmp_directory = \Drupal::service('file_system')->getTempDirectory();
$this->assertNotEmpty($tmp_directory);
$this->assertEquals($tmp_directory, FileSystem::getOsTemporaryDirectory());
}
/**
......
......@@ -46,7 +46,7 @@ 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() {
$source = file_directory_temp() . '/example.txt';
$source = \Drupal::service('file_system')->getTempDirectory() . '/example.txt';
file_put_contents($source, 'example');
$filename = file_unmanaged_copy($source);
$this->assertEquals('public://example.txt', $filename);
......@@ -67,6 +67,7 @@ public function testDeprecatedUnmanagedFileDeleteRecursive() {
}
/**
* @expectedDeprecation file_directory_temp() is deprecated in drupal:8.8.0 and is removed from drupal:9.0.0. Use \Drupal\Core\File\FileSystemInterface::getTempDirectory() instead. See https://www.drupal.org/node/3039255
* @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() {
......
<?php
namespace Drupal\KernelTests\Core\File;
use Drupal\Component\FileSystem\FileSystem;
use Drupal\KernelTests\KernelTestBase;
/**
* @group File
* @group legacy
*/
class FileSystemRequirementsTest extends KernelTestBase {
/**
* Modules to enable.
*
* @var array
*/
public static $modules = ['system'];
/**
* {@inheritdoc}
*/
protected $strictConfigSchema = FALSE;
/**
* {@inheritdoc}
*/
protected function setUp() {
parent::setUp();
$this->setInstallProfile('standard');
}
/**
* Tests requirements warnings.
*
* @expectedDeprecation The 'system.file' config 'path.temporary' is deprecated in drupal:8.8.0 and is removed from drupal:9.0.0. Set 'file_temp_path' in settings.php instead. See https://www.drupal.org/node/3039255
*/
public function testFileSystemRequirements() {
$this->config('system.file')
->set('path.temporary', $this->randomMachineName())
->save(TRUE);
$requirements = $this->checkSystemRequirements();
$this->assertEquals('Deprecated configuration', (string) $requirements['temp_directory']['value']);
$this->assertEquals('You are using deprecated configuration for the temporary files path.', (string) $requirements['temp_directory']['description'][0]['#markup']);
$this->assertStringStartsWith('Remove the configuration and add the following', (string) $requirements['temp_directory']['description'][1]['#markup']);
$this->config('system.file')
->set('path.temporary', FileSystem::getOsTemporaryDirectory())
->save(TRUE);
$requirements = $this->checkSystemRequirements();
$this->assertEquals('Deprecated configuration', (string) $requirements['temp_directory']['value']);
$this->assertEquals('You are using deprecated configuration for the temporary files path.', (string) $requirements['temp_directory']['description'][0]['#markup']);
$this->assertEquals('Your temporary directory configuration matches the OS default and can be safely removed.', (string) $requirements['temp_directory']['description'][1]['#markup']);
}
/**
* Tests if settings are set, there are not warnings.
*/
public function testSettingsExist() {
$this->setSetting('file_temp_path', $this->randomMachineName());
$requirements = $this->checkSystemRequirements();
$this->assertArrayNotHasKey('temp_directory', $requirements);
}
/**