I get the following PHP notices when deleting files using file_delete():

Warning: array_fill() [function.array-fill]: Number of elements must be positive in EntityTranslationDefaultHandler->removeTranslation() (line 460 of sites/all/modules/entity_translation/includes/translation.handler.inc).
Warning: array_combine() expects parameter 2 to be array, boolean given in EntityTranslationDefaultHandler->removeTranslation() (line 462 of sites/all/modules/entity_translation/includes/translation.handler.inc).
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dave Reid’s picture

Files in question had no entity_translation data, which is a common occurrence.

Dave Reid’s picture

Status: Active » Needs review
FileSize
708 bytes

Patch provided which at least to me fixes the issue without any harm.

Dave Reid’s picture

The alternative would be to do this:

$values = !empty($keys) ? array_fill(0, count($keys), $hook_info) : array();

But it seems pointless to execute the else statement if the translation 'data' array is empty.

plach’s picture

Status: Needs review » Reviewed & tested by the community

Thanks, the patch in itself looks good but it's a bit weird that the data field is not defined, see EntityTranslationDefaultHandler::loadMultiple(). Does that happen with a new entity?

plach’s picture

Status: Reviewed & tested by the community » Needs review

Actually I'm wondering whether it would make more sense to ensure that translations are defined when setting the wrapped entity in the translation handler.

plach’s picture

Does this fix the issue?

Dave Reid’s picture

No. $file->translations does exist, it's just $file->translations['data'] is an empty array because it has no data (entity created prior to entity_translation). The problem is array_fill cannot be called to generate an empty array.

plach’s picture

Title: PHP notices when deleting files » Ensure that empty translation data is handled properly
FileSize
1.96 KB

Ah ok, let's combine the two patches then.

plach’s picture

Status: Needs review » Fixed

Committed and pushed, thanks.

Dave Reid’s picture

Hrm, I'm just not seeing that the code from #6 is even necessary, but I guess it can't hurt. For reference, this is the debug() output of a file from file_load() that has no entity translation records:

stdClass::__set_state(array(
   'fid' => '2',
   'uid' => '1',
   'filename' => 'Hello.txt',
   'uri' => 'public://Hello_0.txt',
   'filemime' => 'text/plain',
   'filesize' => '0',
   'status' => '1',
   'timestamp' => '1343687273',
   'type' => 'text',
   'rdf_mapping' => 
  array (
  ),
   'translations' => 
  stdClass::__set_state(array(
     'original' => NULL,
     'data' => 
    array (
    ),
  )),
))
plach’s picture

OK, that looks fine. The code in #6 is to account for situations where one creates an entity and wraps it in a translation handler before saving it for the first time.

Status: Fixed » Closed (fixed)

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

  • Commit 3cefb78 on 7.x-1.x, et-permissions-1829630, factory, et-fc, revisions by plach:
    Issue #1728648 by plach, Dave Reid: Fixed Ensure that empty translation...

  • Commit 3cefb78 on 7.x-1.x, et-permissions-1829630, factory, et-fc, revisions, workbench by plach:
    Issue #1728648 by plach, Dave Reid: Fixed Ensure that empty translation...