From e3ceb190af269cafca7fe73399f62d995b96a6b9 Mon Sep 17 00:00:00 2001 From: Nathaniel Catchpole Date: Fri, 21 Jul 2017 09:32:10 +0100 Subject: [PATCH] Issue #2801777 by Berdir, Wim Leers, Pol, alexpott, dawehner, Jo Fitzgerald, Munavijayalakshmi, poornima.n, ifrik, Bojhan, catch: Prevent drupal from deleting temporary files --- .../file/config/install/file.settings.yml | 2 +- .../file/config/schema/file.schema.yml | 3 ++ core/modules/file/file.install | 12 ++++++ core/modules/file/file.services.yml | 2 +- .../FileUsage/DatabaseFileUsageBackend.php | 6 ++- .../file/src/FileUsage/FileUsageBase.php | 26 ++++++++++++ .../file/src/Tests/FileFieldRevisionTest.php | 5 +++ .../file/src/Tests/FileListingTest.php | 5 +++ .../src/Tests/FileOnTranslatedEntityTest.php | 5 +++ .../file/src/Tests/FilePrivateTest.php | 4 ++ ...mporaryDeletionConfigurationUpdateTest.php | 40 +++++++++++++++++++ .../file/tests/src/Kernel/DeleteTest.php | 5 +++ .../file/tests/src/Kernel/UsageTest.php | 26 +++++++++++- .../src/Tests/ImageOnTranslatedEntityTest.php | 3 ++ .../system/src/Form/FileSystemForm.php | 4 +- .../user/src/Tests/UserPictureTest.php | 6 +++ 16 files changed, 148 insertions(+), 6 deletions(-) create mode 100644 core/modules/file/src/Tests/Update/FileUsageTemporaryDeletionConfigurationUpdateTest.php diff --git a/core/modules/file/config/install/file.settings.yml b/core/modules/file/config/install/file.settings.yml index e65227724a24..9191a0b8004e 100644 --- a/core/modules/file/config/install/file.settings.yml +++ b/core/modules/file/config/install/file.settings.yml @@ -3,4 +3,4 @@ description: length: 128 icon: directory: 'core/modules/file/icons' - +make_unused_managed_files_temporary: false diff --git a/core/modules/file/config/schema/file.schema.yml b/core/modules/file/config/schema/file.schema.yml index b9f8918f6331..42d57d9a64ec 100644 --- a/core/modules/file/config/schema/file.schema.yml +++ b/core/modules/file/config/schema/file.schema.yml @@ -21,6 +21,9 @@ file.settings: directory: type: path label: 'Directory' + make_unused_managed_files_temporary: + type: boolean + label: 'Controls if unused files should be marked temporary' field.storage_settings.file: type: base_entity_reference_field_settings diff --git a/core/modules/file/file.install b/core/modules/file/file.install index 9134a25e9e29..48d2bb2d81fb 100644 --- a/core/modules/file/file.install +++ b/core/modules/file/file.install @@ -116,3 +116,15 @@ function file_requirements($phase) { return $requirements; } + +/** + * Prevent unused files from being deleted. + */ +function file_update_8300() { + // Disable deletion of unused permanent files. + \Drupal::configFactory()->getEditable('file.settings') + ->set('make_unused_managed_files_temporary', FALSE) + ->save(); + + return t('Files that have no remaining usages are no longer deleted by default.'); +} diff --git a/core/modules/file/file.services.yml b/core/modules/file/file.services.yml index 1c463afd5ade..03eb7bccf77b 100644 --- a/core/modules/file/file.services.yml +++ b/core/modules/file/file.services.yml @@ -1,6 +1,6 @@ services: file.usage: class: Drupal\file\FileUsage\DatabaseFileUsageBackend - arguments: ['@database'] + arguments: ['@database', 'file_usage', '@config.factory'] tags: - { name: backend_overridable } diff --git a/core/modules/file/src/FileUsage/DatabaseFileUsageBackend.php b/core/modules/file/src/FileUsage/DatabaseFileUsageBackend.php index cfb0c447a685..2b1995437e29 100644 --- a/core/modules/file/src/FileUsage/DatabaseFileUsageBackend.php +++ b/core/modules/file/src/FileUsage/DatabaseFileUsageBackend.php @@ -2,6 +2,7 @@ namespace Drupal\file\FileUsage; +use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\Core\Database\Connection; use Drupal\file\FileInterface; @@ -32,8 +33,11 @@ class DatabaseFileUsageBackend extends FileUsageBase { * information. * @param string $table * (optional) The table to store file usage info. Defaults to 'file_usage'. + * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory + * (optional) The config factory. */ - public function __construct(Connection $connection, $table = 'file_usage') { + public function __construct(Connection $connection, $table = 'file_usage', ConfigFactoryInterface $config_factory = NULL) { + parent::__construct($config_factory); $this->connection = $connection; $this->tableName = $table; diff --git a/core/modules/file/src/FileUsage/FileUsageBase.php b/core/modules/file/src/FileUsage/FileUsageBase.php index c90359be8b09..ba59cd54aa29 100644 --- a/core/modules/file/src/FileUsage/FileUsageBase.php +++ b/core/modules/file/src/FileUsage/FileUsageBase.php @@ -2,6 +2,7 @@ namespace Drupal\file\FileUsage; +use Drupal\Core\Config\ConfigFactoryInterface; use Drupal\file\FileInterface; /** @@ -9,6 +10,27 @@ use Drupal\file\FileInterface; */ abstract class FileUsageBase implements FileUsageInterface { + /** + * The config factory. + * + * @var \Drupal\Core\Config\ConfigFactoryInterface + */ + protected $configFactory; + + /** + * Creates a FileUsageBase object. + * + * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory + * (optional) The config factory. Defaults to NULL and will use + * \Drupal::configFactory() instead. + * + * @deprecated The $config_factory parameter will become required in Drupal + * 9.0.0. + */ + public function __construct(ConfigFactoryInterface $config_factory = NULL) { + $this->configFactory = $config_factory ?: \Drupal::configFactory(); + } + /** * {@inheritdoc} */ @@ -24,6 +46,10 @@ abstract class FileUsageBase implements FileUsageInterface { * {@inheritdoc} */ public function delete(FileInterface $file, $module, $type = NULL, $id = NULL, $count = 1) { + // Do not actually mark files as temporary when the behavior is disabled. + if (!$this->configFactory->get('file.settings')->get('make_unused_managed_files_temporary')) { + return; + } // If there are no more remaining usages of this file, mark it as temporary, // which result in a delete through system_cron(). $usage = \Drupal::service('file.usage')->listUsage($file); diff --git a/core/modules/file/src/Tests/FileFieldRevisionTest.php b/core/modules/file/src/Tests/FileFieldRevisionTest.php index 7b7b4d3c7a2f..04fbcb8767f6 100644 --- a/core/modules/file/src/Tests/FileFieldRevisionTest.php +++ b/core/modules/file/src/Tests/FileFieldRevisionTest.php @@ -22,6 +22,11 @@ class FileFieldRevisionTest extends FileFieldTestBase { * should be deleted also. */ public function testRevisions() { + // This test expects unused managed files to be marked as a temporary file + // and then deleted up by file_cron(). + $this->config('file.settings') + ->set('make_unused_managed_files_temporary', TRUE) + ->save(); $node_storage = $this->container->get('entity.manager')->getStorage('node'); $type_name = 'article'; $field_name = strtolower($this->randomMachineName()); diff --git a/core/modules/file/src/Tests/FileListingTest.php b/core/modules/file/src/Tests/FileListingTest.php index e348949a737d..cca21c5a4c65 100644 --- a/core/modules/file/src/Tests/FileListingTest.php +++ b/core/modules/file/src/Tests/FileListingTest.php @@ -30,6 +30,11 @@ class FileListingTest extends FileFieldTestBase { protected function setUp() { parent::setUp(); + // This test expects unused managed files to be marked as a temporary file. + $this->config('file.settings') + ->set('make_unused_managed_files_temporary', TRUE) + ->save(); + $this->adminUser = $this->drupalCreateUser(['access files overview', 'bypass node access']); $this->baseUser = $this->drupalCreateUser(); $this->createFileField('file', 'node', 'article', [], ['file_extensions' => 'txt png']); diff --git a/core/modules/file/src/Tests/FileOnTranslatedEntityTest.php b/core/modules/file/src/Tests/FileOnTranslatedEntityTest.php index 5f1e7073df82..b8a22ae01b57 100644 --- a/core/modules/file/src/Tests/FileOnTranslatedEntityTest.php +++ b/core/modules/file/src/Tests/FileOnTranslatedEntityTest.php @@ -29,6 +29,11 @@ class FileOnTranslatedEntityTest extends FileFieldTestBase { protected function setUp() { parent::setUp(); + // This test expects unused managed files to be marked as temporary a file. + $this->config('file.settings') + ->set('make_unused_managed_files_temporary', TRUE) + ->save(); + // Create the "Basic page" node type. // @todo Remove the disabling of new revision creation in // https://www.drupal.org/node/1239558. diff --git a/core/modules/file/src/Tests/FilePrivateTest.php b/core/modules/file/src/Tests/FilePrivateTest.php index 6808a4782b7e..0174a89dc8ba 100644 --- a/core/modules/file/src/Tests/FilePrivateTest.php +++ b/core/modules/file/src/Tests/FilePrivateTest.php @@ -27,6 +27,10 @@ class FilePrivateTest extends FileFieldTestBase { node_access_test_add_field(NodeType::load('article')); node_access_rebuild(); \Drupal::state()->set('node_access_test.private', TRUE); + // This test expects unused managed files to be marked as a temporary file. + $this->config('file.settings') + ->set('make_unused_managed_files_temporary', TRUE) + ->save(); } /** diff --git a/core/modules/file/src/Tests/Update/FileUsageTemporaryDeletionConfigurationUpdateTest.php b/core/modules/file/src/Tests/Update/FileUsageTemporaryDeletionConfigurationUpdateTest.php new file mode 100644 index 000000000000..f1fef088cfe4 --- /dev/null +++ b/core/modules/file/src/Tests/Update/FileUsageTemporaryDeletionConfigurationUpdateTest.php @@ -0,0 +1,40 @@ +databaseDumpFiles = [ + __DIR__ . '/../../../../system/tests/fixtures/update/drupal-8.bare.standard.php.gz', + ]; + } + + /** + * Tests that block context mapping is updated properly. + */ + public function testUpdateHookN() { + $this->assertIdentical($this->config('file.settings')->get('make_unused_managed_files_temporary'), NULL); + $this->runUpdates(); + $this->assertIdentical($this->config('file.settings')->get('make_unused_managed_files_temporary'), FALSE); + $this->assertResponse('200'); + } + +} diff --git a/core/modules/file/tests/src/Kernel/DeleteTest.php b/core/modules/file/tests/src/Kernel/DeleteTest.php index 3b868c0ad20f..de5a5bc045d8 100644 --- a/core/modules/file/tests/src/Kernel/DeleteTest.php +++ b/core/modules/file/tests/src/Kernel/DeleteTest.php @@ -28,6 +28,11 @@ class DeleteTest extends FileManagedUnitTestBase { * Tries deleting a file that is in use. */ public function testInUse() { + // This test expects unused managed files to be marked as a temporary file + // and then deleted up by file_cron(). + $this->config('file.settings') + ->set('make_unused_managed_files_temporary', TRUE) + ->save(); $file = $this->createFile(); $file_usage = $this->container->get('file.usage'); $file_usage->add($file, 'testing', 'test', 1); diff --git a/core/modules/file/tests/src/Kernel/UsageTest.php b/core/modules/file/tests/src/Kernel/UsageTest.php index 672de0fb5a8f..f95e9cd48821 100644 --- a/core/modules/file/tests/src/Kernel/UsageTest.php +++ b/core/modules/file/tests/src/Kernel/UsageTest.php @@ -74,11 +74,34 @@ class UsageTest extends FileManagedUnitTestBase { $this->assertEqual($usage[2]->count, 2, 'Correct count'); } + /** + * Tests file usage deletion when files are made temporary. + */ + public function testRemoveUsageTemporary() { + $this->config('file.settings') + ->set('make_unused_managed_files_temporary', TRUE) + ->save(); + $file = $this->doTestRemoveUsage(); + $this->assertTrue($file->isTemporary()); + } + + /** + * Tests file usage deletion when files are made temporary. + */ + public function testRemoveUsageNonTemporary() { + $this->config('file.settings') + ->set('make_unused_managed_files_temporary', FALSE) + ->save(); + $file = $this->doTestRemoveUsage(); + $this->assertFalse($file->isTemporary()); + } + /** * Tests \Drupal\file\FileUsage\DatabaseFileUsageBackend::delete(). */ - public function testRemoveUsage() { + public function doTestRemoveUsage() { $file = $this->createFile(); + $file->setPermanent(); $file_usage = $this->container->get('file.usage'); db_insert('file_usage') ->fields([ @@ -116,6 +139,7 @@ class UsageTest extends FileManagedUnitTestBase { ->execute() ->fetchField(); $this->assertIdentical(FALSE, $count, 'Decrementing non-exist record complete.'); + return $file; } /** diff --git a/core/modules/image/src/Tests/ImageOnTranslatedEntityTest.php b/core/modules/image/src/Tests/ImageOnTranslatedEntityTest.php index e1f70d78a9b0..c5f16cff07c4 100644 --- a/core/modules/image/src/Tests/ImageOnTranslatedEntityTest.php +++ b/core/modules/image/src/Tests/ImageOnTranslatedEntityTest.php @@ -29,6 +29,9 @@ class ImageOnTranslatedEntityTest extends ImageFieldTestBase { protected function setUp() { parent::setUp(); + // This test expects unused managed files to be marked as a temporary file. + $this->config('file.settings')->set('make_unused_managed_files_temporary', TRUE)->save(); + // Create the "Basic page" node type. // @todo Remove the disabling of new revision creation in // https://www.drupal.org/node/1239558. diff --git a/core/modules/system/src/Form/FileSystemForm.php b/core/modules/system/src/Form/FileSystemForm.php index ff0b7924337a..d3a6caed0a64 100644 --- a/core/modules/system/src/Form/FileSystemForm.php +++ b/core/modules/system/src/Form/FileSystemForm.php @@ -125,10 +125,10 @@ class FileSystemForm extends ConfigFormBase { $period[0] = t('Never'); $form['temporary_maximum_age'] = [ '#type' => 'select', - '#title' => t('Delete orphaned files after'), + '#title' => t('Delete temporary files after'), '#default_value' => $config->get('temporary_maximum_age'), '#options' => $period, - '#description' => t('Orphaned files are not referenced from any content but remain in the file system and may appear in administrative listings. Warning: If enabled, orphaned files will be permanently deleted and may not be recoverable.'), + '#description' => t('Temporary files are not referenced, but are in the file system and therefore may show up in administrative lists. Warning: If enabled, temporary files will be permanently deleted and may not be recoverable.'), ]; return parent::buildForm($form, $form_state); diff --git a/core/modules/user/src/Tests/UserPictureTest.php b/core/modules/user/src/Tests/UserPictureTest.php index 6d546f0b4fb6..d8504133bff2 100644 --- a/core/modules/user/src/Tests/UserPictureTest.php +++ b/core/modules/user/src/Tests/UserPictureTest.php @@ -33,6 +33,12 @@ class UserPictureTest extends WebTestBase { protected function setUp() { parent::setUp(); + // This test expects unused managed files to be marked temporary and then + // cleaned up by file_cron(). + $this->config('file.settings') + ->set('make_unused_managed_files_temporary', TRUE) + ->save(); + $this->webUser = $this->drupalCreateUser([ 'access content', 'access comments',