file_move() should only be called on managed files. The problem is that the file and image fields call file_move() on a managed file object that has yet to be saved. This causes problems with contrib modules that implement hook_file_delete() because they expect $file->id() to exist. This was fixed in Devel Generate in 7.x-1.x with #2429787: File generation should use file_unmanaged_move() instead of file_move() before the file object is saved

CommentFileSizeAuthor
#46 2550973-46.patch1.03 KBNitin shrivastava
#44 reroll_diff_43-44.txt920 bytesAkram Khan
#44 2550973-44.patch1.03 KBAkram Khan
#43 2550973-43.patch1.04 KBSandeepSingh199
#38 2550973-37.patch2.84 KBMedha Kumari
#35 interdiff-31-35.txt1.97 KBsmustgrave
#35 2550973-35.patch2.84 KBsmustgrave
#34 interdiff-31-34.txt2.35 KBsmustgrave
#34 2550973-34.patch2.23 KBsmustgrave
#31 interdiff-29-31.txt1.17 KBsmustgrave
#31 2550973-31.patch2.99 KBsmustgrave
#29 2550973-29.patch3.14 KBsmustgrave
#28 reroll_diff_12-28.txt3.81 KBimmaculatexavier
#28 2550973-28.patch2.87 KBimmaculatexavier
#12 image_generate_unmanaged_move-2550973-12.patch2.41 KByogeshmpawar
#8 2550973-8-image-generate-unmanaged-move.patch2.41 KBtkuldeep17
#6 2550973-6-image-generate-unmanaged-move.patch2.31 KBtkuldeep17
#2 2550973-image-generate-unmanaged-move.patch1.64 KBDave Reid
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dave Reid created an issue. See original summary.

Dave Reid’s picture

Title: File and image fields' generateSampleValue() should use file_unmanaged_move() before calling $file->save(). » ImageItem::generateSampleValue() should use file_unmanaged_move() before saving the file
Status: Active » Needs review
FileSize
1.64 KB

Seems limited just to Image fields.

Dave Reid’s picture

Component: file system » image.module
Dave Reid’s picture

+++ b/core/modules/image/src/Plugin/Field/FieldType/ImageItem.php
@@ -341,16 +341,20 @@ public static function generateSampleValue(FieldDefinitionInterface $field_defin
         $destination = $destination_dir . '/' . basename($path);
-        $file = file_move($image, $destination, FILE_CREATE_DIRECTORY);
-        $images[$extension][$min_resolution][$max_resolution][$file->id()] = $file;
+        if ($path = file_unmanaged_move($path, $destination)) {

I should just use $destination_dir as the second argument for file_unmanaged_move(). There's no need to construct the exact destination URL.

Status: Needs review » Needs work

The last submitted patch, 2: 2550973-image-generate-unmanaged-move.patch, failed testing.

tkuldeep17’s picture

Status: Needs work » Needs review
FileSize
2.31 KB

@dave patch is failed due to undefined variable `file`, so I have fixed it. and uploading patch.

Status: Needs review » Needs work

The last submitted patch, 6: 2550973-6-image-generate-unmanaged-move.patch, failed testing.

tkuldeep17’s picture

Sorry, I missed to fix file variable at one place. Updating patch again.

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

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

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

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.

Status: Needs review » Needs work

The last submitted patch, 8: 2550973-8-image-generate-unmanaged-move.patch, failed testing.

yogeshmpawar’s picture

Status: Needs work » Needs review
FileSize
2.41 KB

Rerolled the patch #8.

Status: Needs review » Needs work

The last submitted patch, 12: image_generate_unmanaged_move-2550973-12.patch, failed testing.

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.

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

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

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

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

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

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

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

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

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

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

larowlan’s picture

Assigned: Dave Reid » Unassigned
Priority: Normal » Minor
Issue tags: +Needs reroll, +Bug Smash Initiative

Those methods are now on the file repository service, so I suspect this needs a reroll

kim.pepper’s picture

Assigned: Unassigned » Dave Reid
Priority: Minor » Normal

file_unmanaged_move was deprecated in Drupal 8.7.0. See https://www.drupal.org/node/3006851

kim.pepper’s picture

Assigned: Dave Reid » Unassigned
immaculatexavier’s picture

Assigned: Unassigned » immaculatexavier
immaculatexavier’s picture

Assigned: immaculatexavier » Unassigned
Status: Needs work » Needs review

Addressed #23,#24. Rerolled against #12. Attached reroll diff for the same.

immaculatexavier’s picture

Missed to upload patch in #27
Addressed #23,#24. Rerolled against #12. Attached reroll diff for the same.

smustgrave’s picture

Status: Needs review » Needs work

The last submitted patch, 29: 2550973-29.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 31: 2550973-31.patch, failed testing. View results

smustgrave’s picture

So I removed this if statement
if ($path = \Drupal::service('file.repository')->move($path, $destination)) {

because $path couldn't be passed into move(). Wondering if that was the right approach?

smustgrave’s picture

Status: Needs work » Needs review
FileSize
2.23 KB
2.35 KB
smustgrave’s picture

FileSize
2.84 KB
1.97 KB

Forgot to check commit before generating patch.

smustgrave’s picture

Status: Needs review » Needs work

Just realized this has no tests.

Medha Kumari’s picture

Assigned: Unassigned » Medha Kumari
Medha Kumari’s picture

Version: 9.4.x-dev » 9.5.x-dev
Assigned: Medha Kumari » Unassigned
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
2.84 KB

Reroll the patch #35 with Drupal 9.5.x

smustgrave’s picture

Status: Needs review » Needs work

Still no test case. Nor an interdiff so a reroll doesn’t help in this case.

smustgrave’s picture

Category: Bug report » Feature request
Status: Needs work » Needs review

Brought this up in the bugsmash slack channel it seems more like an enhancement than a bug. Moving to Feature request.

Sorry for the confusion.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

larowlan’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests
+++ b/core/modules/image/src/Plugin/Field/FieldType/ImageItem.php
@@ -349,23 +348,21 @@ public static function generateSampleValue(FieldDefinitionInterface $field_defin
+        if ($path = \Drupal::service('file_system')->move($path, $destination)) {
+          $image = File::create([
+            'uri' => $path,
+            'uid' => \Drupal::currentUser()->id(),
+          ]);
+          $image->save();
+          $images[$extension][$min_resolution][$max_resolution][$image->id()] = $image;
+          $file = \Drupal::service('file.repository')->move($image, $destination);

Can we simplify a lot of this by using the file.repository service's writeData method?

\Drupal\Tests\image\Kernel\ImageItemTest::testImageItem has test coverage for this method, so can we expand that to add the missing coverage?

SandeepSingh199’s picture

Status: Needs work » Needs review
FileSize
1.04 KB

With the help of #42, I have created the patch for D10.1.x. Please check & review.

Akram Khan’s picture

smustgrave’s picture

Status: Needs review » Needs work

Thank you for the patches but tests were not added.

No interdiff in 43

And you can check the patch will apply using core scrips adjusting credit

Nitin shrivastava’s picture

try to fix #44 ccf error.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.