file_field_update() triggers a full entity load durung entity_save() [#644338]

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.

Comment	File	Size	Author
#7	file_field_load_on_update-644338-7.patch	1.08 KB	yched
#7
#5	file_field_load_on_update.patch	1.06 KB	yched
#5

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Anonymous (not verified) CreditAttribution: Anonymous commented 30 November 2009 at 00:48

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

Comment #2

sun.core CreditAttribution: sun.core commented 14 February 2010 at 04:33

Priority:

Critical

» Normal

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

Comment #3

catch

he/him

English

CreditAttribution: catch commented 8 March 2010 at 08:24

#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.

Comment #4

yched CreditAttribution: yched commented 8 March 2010 at 23:47

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().