Hi Everybody,

I found a bug in fields module.
I have a link field that is altered in hook_field_info_alter().
I replace the module of the link field to something else which is working fine.
However after clearing the cache the module in field_config table become again link.
After investigation what is going on I found that on flush cache hook in field module a function field_sync_field_status() is called that always update the module to default one which is not correct (the functionality doesn't call hook_field_info_alter() and the module is not correct).
However I've made a quick patch for this so if anybody needs a fix for this can use it until the core is fixed.

CommentFileSizeAuthor
Field module clear cache patch779 bytesskek

Comments

Anonymous’s picture

Anonymous (not verified) commented

can you help us with a test that illustrates the issue?

also, if you can get to the #drupal-contribute IRC channel, we can help with fixing this issue.

http://drupal.org/irc

skek’s picture

skek commented

@beejeebus

You can easily reproduce this bug.
Download and install i18n module for example. In private we need i18n_taxonomy module.
This module on hook_enable() update the $field['module'] variable for all "taxonomy_term_reference" fields to 'i18n_taxonomy'. 

foreach (field_info_fields() as $fieldname => $field) {
    if (($field['type'] == 'taxonomy_term_reference') && ($field['module'] == 'taxonomy') && ($field['deleted'] == 0)) {
      $field['module'] = 'i18n_taxonomy';
      field_update_field($field);
    }
  }

Also it implements an alter hook for this field and change the module to 'i18n_taxonomy'. 

/**
 * Implements hook_field_info_alter()
 */
function i18n_taxonomy_field_info_alter(&$info) {
  // Change default formatter for term reference fields
  $info['taxonomy_term_reference']['default_formatter'] = 'i18n_taxonomy_term_reference_link';
  // Change module for this field. Basically we take over this field and this means
  // we need to implement a bunch of hooks for fields and options, including hook_field_schema()
  $info['taxonomy_term_reference']['module'] = 'i18n_taxonomy';
  // Sync callback for field translations
  $info['taxonomy_term_reference']['i18n_sync_callback'] = 'i18n_taxonomy_field_prepare_translation';
}

If you clear the cache and hook_flush_caches() is invoked you can check in database that the module has been returned to taxonomy.
This is because the function field_associate_fields() collect the information from hook_field_info() without invoking the hook_field_info_alter().

I hope it is more clear now. Write me if you need my help for this.

catch’s picture

catch
he/him
Primary language English
commented
Version: 7.9 » 8.x-dev
Assigned: skek » Unassigned
Issue tags: +Needs backport to D7

Fixing version.

catch’s picture

catch
he/him
Primary language English
commented
Title: Bug in field_flush_caches() function execution. » field_associate_fields() doesn't invoke alter hooks
xjm’s picture

xjm
she/her
Primary language English
commented
Issue tags: +Needs tests
alan evans’s picture

alan evans commented
Assigned: Unassigned » alan evans

Taking a look - if I've understood this issue correctly, it seems that the attached patch isn't necessarily the right fix either.

alan evans’s picture

alan evans commented
Priority: Major » Normal

So the problem here is basically an inconsistency, but I'm losing confidence that we really gain much by "fixing" it - the fix would be a change in order to support a hack in a single contrib module, and at that a hack which has since been removed from the module that was depending on it. The inconsistency is that the field info API will add the module to the definition of a field, and allow all of this data to be altered before returning it including the "module", which we'd normally expect to be mainly static (just storing the module that defined the field type). However, the other place that module_invoke(..., 'field_info') is called (field_associate_fields), that piece of previously alterable data is not altered before saving it to the DB. This only applies to the "module" column, as that's the only piece of data which is acted-upon in field_associate_fields().

The main motivation behind this issue appears to have been that i18n_taxonomy module is relying on this behaviour, and seems for one thing to be a reasonably popular module in 7 at least (it's not clear to me whether it'll still be needed in 8).

Looking now though, it seems that it hasn't relied on this for a while now, and there is at a glance no other module (at least modules covered by drupalcontrib) altering the module in this hook http://drupalcontrib.org/api/drupal/drupal%21modules%21field%21field.api...

... thinking about it, taking over a field type to such an extent that you need to alter the module almost seems like the kind of hack that we wouldn't really want to support anyway, and if code relies on this working, then it might make more sense to look at what features those modules need and see if those needed features can be supported in some other way (with core changes if needed, but hopefully reasonable, flexible changes, rather than just hack-supporting changes).

Furthermore, documentation including the header of field_associate_fields implies that the module defining a field type remains in control for life: "Allows a module to update the database for fields and columns it controls"

The commit to i18n_taxonomy removing this "hack" was here http://drupalcode.org/project/i18n.git/commitdiff/7f1ecad80992c1f33cf7d1... and issue was here: http://drupal.org/node/1356434

I did start on code for adding this alter into field_associate_fields and a test module for it before I started researching deeper and started developing doubts about it, but I'm going to stash it for the time being unless I can gather more data on why this would be useful.

Could any other contributors to this issue please add further comments on any other motivations that might remain for supporting alteration of the module column in field_associate_fields? Empasizing once again that the only thing that would be usefully altered in this case is the module column - none of the other data is used within that func.

I'm downgrading to normal for now, but I'd rather close this as won't-fix.

andypost’s picture

andypost
he/him
Primary language Russian
commented

seems dropped in d8, so probably better switch to 7.x

xjm’s picture

xjm
she/her
Primary language English
commented
Version: 8.x-dev » 7.x-dev

So why not do so? :)

xjm’s picture

xjm
she/her
Primary language English
commented
Issue tags: -Needs backport to D7

Status: Active » Closed (outdated)

Automatically closed because Drupal 7 security and bugfix support has ended as of 5 January 2025. If the issue verifiably applies to later versions, please reopen with details and update the version.