In the latest release of D7 features:
All fields within a feature get updated on field_base feature_revert regardless of whether they are considered to be changed.

Replication Steps:

Create a feature with 10 fields.

Update one field.

Run a feature_revert against field_base definitions for that feature.

Count how many times we run field_info_cache_clear():

  • In this instance it is 11 times.
  • once to Load all the existing field bases up-front so that we don't have to rebuild the cache all the time.
  • one per field

Count how many times we run field_update_field:

  • It should be once, it's 10 times in this instance

Based on the design of the field_Base_features_rebuild:

  • We should run field_info_cache_clear 2 times
    • once to Load all the existing field bases up-front so that we don't have to rebuild the cache all the time.
    • once for the updated field
  • Field_update_field should run once

Why does it happen?

Appears to have been an issue for some time and was most likely introduced by the work in:
#1721926: Field Instance Export "weight" - brings me in a Override ping/pong - quotes and no quotes

Values such as 'cardinality' appear to be strings when rendered by field_info_fields() and integers in feature_export definitions

During features_var_export we convert string values to integers

  else if (is_numeric($var)) {
    $floatval = floatval($var);
    if (is_string($var)) {
      // Do not convert a string to a number if the string
      // representation of that number is not identical to the
      // original value.
      $output = var_export($var, TRUE);
    }
    else {
      $output = $floatval;
    }
  }

During import we compare the value output by field_info_fields to the value in feature_export definition. And this code checks that not only is the value the same but the variable type.

        $array_diff_result = drupal_array_diff_assoc_recursive($field + $existing_field, $existing_field);
        if (!empty($array_diff_result)) {
          try {
            field_update_field($field);
          }
          catch (FieldException $e) {
            watchdog('features', 'Attempt to update field %label failed: %message', array('%label' => $field['field_name'], '%message' => $e->getMessage()), WATCHDOG_ERROR);
          }
        }

cardinatility of -1 does not match cardinality of '-1' and the other 9 fields are updated

What next?

I'm happy to have a look at creating a patch but can I ask, which section of code would be the most appropriate to fix? The export or the import comparison?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Darren Shelley created an issue. See original summary.

Darren Shelley’s picture

Issue summary: View changes
Darren Shelley’s picture

Issue summary: View changes
Darren Shelley’s picture

Patch attached which resolves the issue for me and appears to be inline with how this is handled elsewhere in features.

This brings the fields to be updated more in line with those that would cause the module to be considered overridden.

ciss’s picture

@darren-shelley Assuming you have been using the patch in production for a while now: did it ever cause any new issues for you, especially on field heavy sites and/or across multiple field component changes? Or do you still consider it RTBC from your point of view?

Darren Shelley’s picture

@ciss, I've personally used this in production on one site for 8 months with no ill effects.
Always worth testing on your staging environment first.

jollysolutions’s picture

Status: Needs review » Reviewed & tested by the community

Works fine in production for me

joseph.olstad’s picture

Status: Reviewed & tested by the community » Fixed

Thanks!

Status: Fixed » Closed (fixed)

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