file_cron() will log an error if the file it's trying to delete does not exist. However, if it does not exist, it would be more appropriate to delete the entity and log a warning.

Suggesting change from:
function file_cron()
...
if (empty($references)) {
if (file_exists($file->getFileUri())) {
$file->delete();
}
else {
\Drupal::logger('file system')->error('Could not delete temporary file "%path" during garbage collection', array('%path' => $file->getFileUri()));
}
}

to:

if (empty($references)) {
if (file_exists($file->getFileUri())) {
$file->delete();
}
else {
//go ahead and delete the entity, even though the file does not exist
$file->delete();
\Drupal::logger('file system')->warning('During garbage collection the temporary file "%path" did not exist.', array('%path' => $file->getFileUri()));
}
}

CommentFileSizeAuthor
#11 drupal-add-warning-2760347-11.D8.patch2.96 KBthelmer
#3 add-warning-2760347-3.patch653 byteskamalrajsahu21

Comments

ecrutchfield created an issue. See original summary.

ecrutchfield’s picture

ecrutchfield commented
Issue summary: View changes
kamalrajsahu21’s picture

kamalrajsahu21 commented
StatusFileSize
new add-warning-2760347-3.patch653 bytes

Here is the path with warning message changed to "Could not find temporary file "%path" during garbage collection".

kamalrajsahu21’s picture

kamalrajsahu21 commented
Status: Active » Needs review
ecrutchfield’s picture

ecrutchfield commented

Thanks for taking a look at the issue, and certainly changing the message is a good first step. But why wouldn't you go ahead and delete the file from the table if it wasn't found?

kamalrajsahu21’s picture

kamalrajsahu21 commented

I believe it's good to delete from table if file is not found because there is no use of it.

ecrutchfield’s picture

ecrutchfield commented

Sorry, I misread your patch so that looks good to me. Thanks for taking care of that.

ecrutchfield’s picture

ecrutchfield commented

I've reviewed the patch and all looks good. Do I need to do something with the issue status?

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

thelmer’s picture

thelmer commented
Assigned: Unassigned » thelmer
Status: Needs review » Needs work

Needs test, will start working an this

thelmer’s picture

thelmer commented
Assigned: thelmer » Unassigned
Status: Needs work » Reviewed & tested by the community
StatusFileSize
new drupal-add-warning-2760347-11.D8.patch2.96 KB
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs review
Issue tags: +Needs subsystem maintainer review
Related issues: +#2801777: Give users the option to prevent drupal from automatically marking unused files as temporary

@thelmer, thanks for working on this issue. After working on patch the correct status to get a review is needs review.

Also given #2801777: Give users the option to prevent drupal from automatically marking unused files as temporary we need sub system maintainer review. 

+++ b/core/modules/file/tests/src/Kernel/DeleteTest.php
@@ -10,10 +10,11 @@
+
...
-  function testUnused() {
+  public function testUnused() {

@@ -27,7 +28,7 @@ function testUnused() {
-  function testInUse() {
+  public function testInUse() {

@@ -63,10 +64,35 @@ function testInUse() {
-    // file_cron() loads
+    // file_cron() loads.

Out of scope chnages - coding standards should be fixed in other issues.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Closed (duplicate)

I merged the test into #2760347: file_cron does not remove orphaned entries and requested that you get credited there.

kalistos’s picture

kalistos commented

Berdir pointed incorrect link for duplicated issue. Correct link: #2802803: Temporary files whose files are missing on the disk result in never-ending error log messages