file_field_update() triggers a full entity load durung entity_save()

Posted by yched on November 27, 2009 at 1:18am

Issue Summary

file_field_update() (file field's implementation of hook_field_update()) checks if a value has been removed from the previous values, to keep track of file references.

It hardocodes a check on $object->revision. See #644332: Do we need to abstract $object->revision ('new revision') property about that.

More problematic, to compare new values with previous values, it does a full load the current object.

a) I'm not sure the current code is the correct way of doing that - could use the opinion of an entity API guru.

  $load_function = $obj_type . '_load';
  $original = $load_function($id);

(this is the main reason I'm marking this 'critical')

b) entity_save() triggering an entity_load() doesn't sound too great. Reacting on values being removed is a legitimate use case for 'resource reference' field types, I'm wondering if we can make this better.

Comments

#1

Posted by beejeebus on November 30, 2009 at 12:48am

subscribe. still getting my head around the fields code, came across this function here #618654: (Fixed, but needs cleanup) File and image fields are treated as temporary files and automatically deleted after six hours.

#2

Posted by sun.core on February 14, 2010 at 4:33am

Priority:

critical

» normal

Not critical. Please read and understand Priority levels of Issues, thanks.

#3

Posted by catch on March 8, 2010 at 8:24am

#733756: image_field_insert() makes no sense and causes a memory leak with devel_generate() was a memory leak on inserts caused by this, although that was fixed by completely removing image_field_insert().

Subscribing to this one.

#4

Posted by yched on March 8, 2010 at 11:47pm

I think that instead of a full entity load, the code could use field_attach_load() with $options = array('field_id' => $field['id']) to get the current values of the field for the object. This is guaranteed to work whatever the entity type, and would avoid triggering a full entity load() during an entity update().