array_filter warning [#629800]

  $field_permissions = (isset($field['field_permissions']) ? array_filter($field['field_permissions']) : array());

For whatever reason, sometimes $field['field_permissions'] is not an array but it is set (not sure how that came to be).
So instead of that:

  if (!is_array($field['field_permissions'])) {
    return; 
  }
  
  $field_permissions = array_filter($field['field_permissions']);

Comment	File	Size	Author
#1	629800_field_permissions_array_filter.patch	677 bytes	hefox
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

File	Size
629800_field_permissions_array_filter.patch	677 bytes

Comment #2

markus_petrux CreditAttribution: markus_petrux commented 11 November 2009 at 17:21

Status:

Active

» Postponed (maintainer needs more info)

This issue may happen if you have upgraded from a previous dev version to the latest. There was a commit where I implemented several permission types and checkboxes to select each one independently from teh field settings form, and that patch included a new hook_update_N() implementation to update the settings of the fields that were already configured with previous settings format. If you run update.php, then this problem with array_filter() should be fixed.

Comment #3

hefox CreditAttribution: hefox commented 11 November 2009 at 17:45

My database updates are up to date; I updated via drush update field_permissions likely and there's no pending updates, so would that still be an issue since the update feels like it'd have run? I could re-run the update, but being that I have done that update it seems like there may be other ways for it to become set but not an array. (I believe updating from dev to 1.0 would run the 6001 update)

When I did an print_r() and echo isset() on the $field['field_permissions'] the majority of them came back with nothing (no output) but 1 for isset.

Comment #4

hefox CreditAttribution: hefox commented 11 November 2009 at 19:13

Status:

Postponed (maintainer needs more info)

» Fixed

K, got over my stubborness and (re?) ran the update and the warning is gone (beta1 to beta2 perhaps didn't trigger the update?). Will poke back if I see the warning again. (Being that the update changed it to array, if someone else other the update caused the warning than it should re-appear).

(I assume isset is less intense than is_array and that is why you don't want to change it to is_array?)

Comment #5

markus_petrux CreditAttribution: markus_petrux commented 11 November 2009 at 19:37

Ah, thanks for the feedback.

(I think isset() is a little bit faster, but the real reason is: if is_array() is not needed, then don't use it, makes code more readable, IMHO too)

Comment #6

hefox CreditAttribution: hefox commented 11 November 2009 at 21:44

Status:

Fixed

» Active

It's back

Changed some settings on some fields and display settings, and the warning is back; may be a symptom of a larger problem :/ .

I believe I also was changing some display settings before seeing the warning the first time.

Comment #7

markus_petrux CreditAttribution: markus_petrux commented 13 November 2009 at 14:18

Status:

Active

» Fixed

Hi,

I've been able to reproduce this. By visiting "Display fields" screen and saving. All field that do not have the 'field_permissions' attribute end up with an empty string.

Trying to find the cause, it seems to me the problem is in CCK itself. The following function in includes/content.crud.inc:

function _content_field_write($field, $op = 'update') {
  // Rearrange the data to create the global_settings array.
  $field['global_settings'] = array();
  $setting_names = (array) module_invoke($field['module'], 'field_settings', 'save', $field);
  drupal_alter('field_settings', $setting_names, 'save', $field);

  foreach ($setting_names as $setting) {
    // Unlike _content_field_instance_write() and 'widget_settings', 'global_settings'
    // is never preexisting, so we take no particular precautions here.
    $field['global_settings'][$setting] = isset($field[$setting]) ? $field[$setting] : '';
    unset($field[$setting]);
  }
  // 'columns' is a reserved word in MySQL4, so our column is named 'db_columns'.
  $field['db_columns'] = $field['columns'];

  switch ($op) {
    case 'create':
      drupal_write_record(content_field_tablename(), $field);
      break;
    case 'update':
      drupal_write_record(content_field_tablename(), $field, 'field_name');
      break;
  }
  unset($field['db_columns']);
  return $field;
}

Anyway, I think it will be faster for the purpose to fix this in Field Permissions, and check for is_array() as you initially suggested. I have committed a slight variation of your patch, however.

http://drupal.org/cvs?commit=288586

Thank you very much!