Problem/Motivation

When trying to use the \Drupal\file\Plugin\Field\FieldType\FileItem::generateSampleValue() method (for example with Devel Generate), we get a Fatal error because the file cannot be moved to its destination.

That can happen because of two things:

  • the $destination variable is not set properly (it's missing a slash)
  • the destination directory does not exists (yet)

Steps to reproduce

  1. Install Drupal standard with devel generate
  2. Create a Test content type
  3. Add a file field
  4. Generate content for your content type

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Bug because Core code should not throw Fatal errors
Issue priority Normal because it only affects contrib (devel generate) in certain circumstances
Prioritized changes The main goal of this issue is bug fixing
Disruption Not disruptive as it only fixes a method that Core never uses

Proposed resolution

Fix the destination and call file_prepare_directory()

Remaining tasks

Contributor tasks needed
Task Novice task? Contributor instructions Complete?
Create a patch Instructions
Update the issue summary noting if allowed during the beta Instructions Done
Add steps to reproduce the issue Novice Instructions Done

User interface changes

None.

API changes

None.

Data model changes

None.

CommentFileSizeAuthor
#7 2532280-7-file-item-sample.patch2.57 KBmr.baileys
#7 2532280-7-file-item-sample-test-only.patch1.6 KBmr.baileys
#5 2532280-5-file-sample-generation.patch1.58 KBmr.baileys
#5 2532280-5-file-sample-generation-test-only.patch620 bytesmr.baileys
#1 fix_file_generation-2532280-1.patch994 bytesduaelfr

Comments

duaelfr’s picture

duaelfr
he/him
Primary language French
Location Montpellier, France
commented
Assigned: duaelfr » Unassigned
Status: Active » Needs review
StatusFileSize
new fix_file_generation-2532280-1.patch994 bytes

That patch fixes the issue.

cilefen’s picture

cilefen commented
Priority: Normal » Major

PHP fatals are Major.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

Seems fine for me. I'm not sure whether we need test coverage for that kind of functionality.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

Given this was a fatal I think we really do want test coverage here.

mr.baileys’s picture

mr.baileys
Primary language Dutch
Location 🇧🇪 (Ghent)
commented
Status: Needs work » Needs review
Issue tags: -Needs tests
StatusFileSize
new 2532280-5-file-sample-generation-test-only.patch620 bytes
new 2532280-5-file-sample-generation.patch1.58 KB

Sample value generation is already tested in \Drupal\file\Tests\FileItemTest:

// Test the generateSampleValue() method.
$entity = entity_create('entity_test');
$entity->file_test->generateSampleItems();
$this->entityValidateAndSave($entity);

However, the fatal error is only thrown when a directory is specified (and does not yet exist). I changed the field configuration for the file field in FileItemTest, this should trigger the failure when the patch is not applied.

mr.baileys’s picture

mr.baileys
Primary language Dutch
Location 🇧🇪 (Ghent)
commented
Status: Needs review » Needs work

Well, that is unexpected -- obviously needs work since the test-only patch succeeds -- will try and take a look later today...

mr.baileys’s picture

mr.baileys
Primary language Dutch
Location 🇧🇪 (Ghent)
commented
Status: Needs work » Needs review
StatusFileSize
new 2532280-7-file-item-sample-test-only.patch1.6 KB
new 2532280-7-file-item-sample.patch2.57 KB

The last submitted patch, 7: 2532280-7-file-item-sample-test-only.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 7: 2532280-7-file-item-sample.patch, failed testing.

Status: Needs work » Needs review

DuaelFr queued 7: 2532280-7-file-item-sample.patch for re-testing.

The last submitted patch, 7: 2532280-7-file-item-sample-test-only.patch, failed testing.

duaelfr’s picture

duaelfr
he/him
Primary language French
Location Montpellier, France
commented
Status: Needs review » Reviewed & tested by the community

Bugfix already RTBC'ed by @dawehner in #3.
Tests added by @mr.baileys in #7 look good to me.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed 852a07d and pushed to 8.0.x. Thanks!

  • alexpott committed 852a07d on 8.0.x 
    Issue #2532280 by mr.baileys, DuaelFr: Fatal error while trying to use \...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.