#1012888: Error adding products to a order in cart status describes a problem as a result of a call to entity_load with an array of fully loaded entities rather than a list of ids.

The call which raises the error is made by EntityListWrapper::value(). Unlike other methods of EntityListWrapper (set() for instance) value assumes that its input (the ::value() of its parent EntityMetadataWrapper) is an array of ids not an array of fully loaded entities.

The patch adds a check for whether ::value() from the parent object is returning a list of loaded entities.

Comments

fago’s picture

hm, internally parent::value() should be an list of ids, so that patch is not the right fix for your problem.

For a single entity, the API is capable of dealing with objects too, however still the usual representation is its id - e.g. it passes the id to a setter callback of an entity reference property. So maybe you have a getter callback returning the list of values? Then it should return a list of ids.

rszrama’s picture

I'm at a loss here... I don't really understand comment #1, but I'm happy to make any changes necessary to get this working again. I can't really isolate what part of the wrapper process is failing (i.e. is it the initial wrapping, the use of count($wrapper->line_items) on the list in the wrapper, or the foreach($wrapper->line_items as ...) on the list in the wrapper).

All I've really been able to nail down is that with no changes to the Commerce Cart module itself, the changes to entity.wrapper.inc between 1.1.2.2 and 1.1.2.3 seem to be causing the error (reverting to 1.1.2.2 allowed it to function just fine).

Perhaps we do need to change something in our Entity integration now, but that property on the order wrapper is getting defined automatically as it's a line item reference field. The pertinent integration in the Line Item module seems to be:

/**
 * Implements hook_field_info().
 */
function commerce_line_item_field_info() {
  return array(
    'commerce_line_item_reference' => array(
      'label' => t('Line item reference'),
      'description' => t('This field stores the ID of a related line item as an integer value.'),
      'settings' => array(),
      'instance_settings' => array(),
      'default_widget' => 'commerce_line_item_manager',
      'default_formatter' => 'commerce_line_item_reference_view',
      'property_type' => 'commerce_line_item',
      'property_callbacks' => array('commerce_line_item_property_info_callback'),
    ),
  );
}

/**
 * Callback to alter the property info of the reference field.
 *
 * @see commerce_line_item_field_info().
 */
function commerce_line_item_property_info_callback(&$info, $entity_type, $field, $instance, $field_type) {
  $property = &$info[$entity_type]['bundles'][$instance['bundle']]['properties'][$field['field_name']];
  $property['options list'] = 'entity_metadata_field_options_list';
  unset($property['query callback']);
}
fago’s picture

The default field getter just gets the value of your first field column, being 'line_item_id'. So maybe this is sometimes not an id but the full entity, e.g. like that?

$value[] = array('line_item_id' => $line_item->line_item_id);

If that's the case, then it gets full entities instead of ids, what leads to that problems. I've no idea why it worked before, because it was never intended to work like that - although it certainly would be a nice addition. ;)

So we have to options:
* Improve the wrappers to allow a list of entities to be passed as full objects too. For that we would have to check for that in the lists value() method and update $this->data with the list of ids - as parent::value() would return a list of objects. Also, all other uses of parent::value() in the list wrapper need to be replaced by value(array('identifier' => TRUE)) then, such that a list of ids returned.
* Improve the list item property integration to make sure a list of ids is returned. E.g. overwrite the default getter callback 'entity_metadata_field_property_get' with your own that returns a numerically indexed array of entity ids.

rszrama’s picture

Ahh, pcambra reminded me that we do in fact have our own getter for the line item reference field on Orders...

  $properties['line_items'] = array(
    'label' => t('Line items'),
    'description' => t('The line items attached to this order.'),
    'type' => 'list<commerce_line_item>',
    'getter callback' => 'commerce_order_get_properties',
  );

/**
 * Callback for getting order properties.
 * @see commerce_order_entity_property_info()
 */
function commerce_order_get_properties($order, array $options, $name) {
  switch ($name) {
    case 'line_items':
      if (empty($order->line_items[LANGUAGE_NONE])) {
        return array();
      }

      $line_item_ids = array();

      foreach ($order->line_items[LANGUAGE_NONE] as $key => $value) {
        $line_item_ids[] = $value['line_item_id'];
      }

      return commerce_line_item_load_multiple($line_item_ids);
  }
}

I believe we thought that in order to use the line items properly through Rules, we had to 1) explicitly add the field to the order entity property info like node does for the body field and 2) define a getter that returned the line item entity array (id => object). For some reason this worked before, but it looks like we need to revert this to just return an array of IDs (or rather, to not have a custom getter at all).

In fact, what's interesting to me is that in my code comments I said we should use a metadata wrapper to avoid having to hardcode a language constant, but I realized it would be a little redundant to try and use a wrapper when defining the info that makes the wrapper possible. ; )

So do we just need to nuke this getter / will Rules still work with the line item entities, or do we just need to rewrite this getter somehow and work around the language problem?

rszrama’s picture

Ok, digging deeper... what I did was nuke the custom getter (I can't remember why we had it in the first place) and set the getter of the line items property to 'entity_metadata_field_property_get' as you suggested. This results in things working fine as far as I can tell, but I don't know if there's anything else we should be doing... or as I mentioned above, if we should even be explicitly adding the line items property to the order info array. For default / "locked" fields, I wonder if there'd be a way for Rules to automatically represent these fields in data selectors?

Patch is here: http://drupal.org/node/1012888#comment-3899922

fago’s picture

hm, if you have a ususal field on the order, why do you manually define the property info at all? It looks like you have provided the property info integration for the line item field type, so it should automatically generate a property for the field. Any reason to not go with that?

>1) explicitly add the field to the order entity property info like node does for the body field
This is done for nodes such that one needs not check the node type in order to be able to use the body field. But do you have bundles for orders at all?

rszrama’s picture

Well, we do have bundles, but they'll always have a line item reference field on them. The problem is that I can't get the line items of an order to show up in the data selector if I don't explicitly add them in the commerce_order.info.inc.

  $properties['line_items'] = array(
    'label' => t('Line items'),
    'description' => t('The line items attached to this order.'),
    'type' => 'list<commerce_line_item>',
    'getter callback' => 'entity_metadata_field_property_get',
  );

For example, I have a global variable I've added to the site variables for the current user's shopping cart order. So, on a random Rule, in the actions I'm trying to add a loop and over the line items and display the line item ID in a message. I can select the line items if I have that code above, but if I don't it's impossible. I thought perhaps it just wasn't getting defined as a list, so I tried editing this code to no avail:

/**
 * Callback to alter the property info of the reference field.
 *
 * @see commerce_line_item_field_info().
 */
function commerce_line_item_property_info_callback(&$info, $entity_type, $field, $instance, $field_type) {
  $property = &$info[$entity_type]['bundles'][$instance['bundle']]['properties'][$field['field_name']];
  $property['type'] = 'list<commerce_line_item>';
  $property['options list'] = 'entity_metadata_field_options_list';
  unset($property['query callback']);
}

I don't think I wrote that code originally, but maybe I did. ; ) Am I missing something?

fago’s picture

hm, I was only able to find bundles -except for the default bundle - in your code? Anyway, if you have multiple bundles you can make use of the field by first checking for the bundle, e.g. with the data comparison condition or directly checking for the field (Entity has field). Also, if you have a property referencing to a specific bundle of an order, you can just specify that with 'bundle' => value in the property info.
At last, yep you can just add the property info of the field to the main property info of the entity. I'd suggest to output the generated property info for a bundle (e.g. with dpm(entity_get_property_info())) and provide the same information manually - just as for node bodies (see http://drupalcode.org/viewvc/drupal/contributions/modules/entity/modules...).

rszrama’s picture

Status: Needs review » Fixed

Ahh, yeah, at the database level we support multiple order bundles (i.e. commerce_order has a type column), but we aren't really doing anything with it right now in 1.0. I think a good 2.0 target could be to provide some further level of support... but I'm not sure if that'll be wise or not given the possibility of needing to convert from one order type to another. : ?

I finally figured out what you meant with our info declaration... I think it doesn't make sense for us to add field data into the entity property info array, but I do think from a usability standpoint it would be worthwhile for Rules to support showing fields present on every bundle of a particular entity type without having to check the bundle type or for the existence of the field. I'm going to remove our definition for not and just instruct people to use the "Entity has field" condition for now.

In Commerce for practically every entity type there are default locked fields that we put on every bundle, such as price on products, line items on orders, address field on customer profiles, etc. Functionally, these "fields" are properties of the entities, but we don't have to define them as such since Entity adds field data itself. Either Rules could pick up on those fields automatically so they are always available in data selectors or we could perhaps have some way to declare in the entity property array which fields are present on every bundle.

Let me know if I should repost that as a feature request in the Rules queue or if it's better suited for the Entity queue. : )

fago’s picture

>Let me know if I should repost that as a feature request in the Rules queue or if it's better suited for the Entity queue. : )

It's something we should fix in the entity API - as it is just about hook_entity_property_info() which Rules makes use of via the wrappers. Core misses support for having fields available on any bundle, so the entity API just adds the metadata for what is there. However, with modules caring about having the field available everywhere, it's up to them to adjust the property info accordingly as we have done it for node bodies and comment bodies.
So far I'd say, there is nothing wrong with that approach. However, we could make doing so easier - e.g. by moving the property info automatically once a special flag in $instance is set. Though, it's questionable which label we should pick then, as each instance / bundle might have different labels and descriptions. But probably it would be safe to assume they are all equal (as the fields should be locked).

peem83’s picture

Status: Fixed » Needs review
fago’s picture

Status: Needs review » Fixed

Any cause to re-open this issue?

Status: Fixed » Closed (fixed)

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

geek-merlin’s picture

Status: Postponed » Active

Returning a list<node> from a rule and dsm-ing it gave me this so this seems not fixed.

Recoverable fatal error: Object of class stdClass could not be converted to string in DatabaseStatementBase->execute() (line 2139 of /var/www/virtual/clsys/html/clsys/saft/includes/database/database.inc). Backtrace:
PDOStatement->execute(Array) database.inc:2139
DatabaseStatementBase->execute(Array, Array) database.inc:664
DatabaseConnection->query('SELECT revision.vid AS vid, base.uid AS uid, revision.title AS title, revision.log AS log, revision.status AS status, revision.comment AS comment, revision.promote AS promote, revision.sticky AS sticky, revision.ds_switch AS ds_switch, base.nid AS nid, base.type AS type, base.language AS language, base.created AS created, base.changed AS changed, base.tnid AS tnid, base.translate AS translate, revision.timestamp AS revision_timestamp, revision.uid AS revision_uid
FROM 
{node} base
INNER JOIN {node_revision} revision ON revision.vid = base.vid
WHERE  (base.nid IN  (:db_condition_placeholder_0, :db_condition_placeholder_1, :db_condition_placeholder_2, :db_condition_placeholder_3, :db_condition_placeholder_4)) ', Array, Array) select.inc:1264
SelectQuery->execute() entity.inc:196
DrupalDefaultEntityController->load(Array, Array) common.inc:7716
entity_load('node', Array) entity.wrapper.inc:1012
EntityListWrapper->value() devel.module(1415) : eval()'d code:2
eval() devel.module:1415
devel_execute_form_submit(Array, Array) form.inc:1464
form_execute_handlers('submit', Array, Array) form.inc:860
drupal_process_form('devel_execute_form', Array, Array) form.inc:374
drupal_build_form('devel_execute_form', Array) form.inc:131
drupal_get_form('devel_execute_form') 
call_user_func_array('drupal_get_form', Array) menu.inc:517
menu_execute_active_handler() index.php:21
geek-merlin’s picture

Status: Closed (fixed) » Postponed

hmm, it's more complicated than this - need more investigation...

geek-merlin’s picture

yep, in fact i returned fully loaded entities from a property getter.
looks like the mess is here.

fgjohnson@lojoh.ca’s picture

Hi,
If this is not an appropriate forum please excuse. I hope this is the same issue.

Drupal 7.23 is installed SLEZ 11 -sp3 (which uses php5.3).

We are using a plethora of content types that use Entity_API and Field (I think) to reference each other via content creation and for display via Views.

Worked fine for a while as I was developing then it just WSOD with errors similar to the following.
< start >
Fatal error: Class 'SelectQuery' not found in /srv/www/htdocs/includes/database/database.inc on line 812
< end >

What am I missing!

Thanks

fago’s picture

Issue summary: View changes
Status: Active » Closed (fixed)

yep, in fact i returned fully loaded entities from a property getter.
looks like the mess is here.

That's not supported, from the getters/setters you have to use entity IDs if you have lists of entities. The wrappers getters/setters support IDs and entity objects though.

Setting back to fixed.