Entity context: Impossible to load entities with non-numeric IDs

Would you be able, willing to create a patch?

It seems likely that this code derived from the node context originally and that did require numeric entity IDs. Since every entity in core uses numeric IDs, it is unsurprising that such a thing was never caught.

Yes, it suffers from the same problem:

  elseif (is_numeric($data)) {
    $id = $data;
    $data = entity_load($entity_type, array($id));
    $data = !empty($data[$id]) ? $data[$id] : FALSE;
  }

Since the checks prior to it are is_array and is_object already, anything else is probably legit as an entity id, so just changing that to an else is probably fine.

-  elseif (is_numeric($data)) {
+  else {
     $id = $data;
-    $data = entity_load($entity_type, array($id));
-    $data = !empty($data[$id]) ? $data[$id] : FALSE;
   }
 
-  if (is_array($data)) {
-    $data = entity_load($entity_type, array($id));
-    $data = !empty($data[$id]) ? $data[$id] : FALSE;
-  }
+  $data = entity_load($entity_type, array($id));
+  $data = !empty($data[$id]) ? $data[$id] : FALSE;

Unless I misinterpret, that will fail -- you'll entity load, and then entity load again immediately. $data being an array is still an important gate there.

If $data is an object, meaning an already loaded entity was passed in, you then perform a second entity_load. That is definitely not the right behavior.

+  if(isset($entities) && !empty($entities)) {

No need for the isset here; !empty() covers it. Also spacing problem: if should have a space.

I don't see any validation for an enitity actually being loaded. It's pretty easy to get invalid entity IDs over time, due to deletions or just bad data, so we need to ensure that a failed entity load returns an empty context.

Sorry! Been basically out of commission the last 2 weeks. Super busy. :/

Look sokay!

Side note that entity IDs cannot and should not be non-numeric - this is an assumption coming from core that Entity API provides ways to violate. See #1003788: PostgreSQL: PDOException:Invalid text representation when attempting to load an entity with a string or non-scalar ID as to why this is not allowed.

This needs to be reverted or re-add the numeric checking. To clarify: the core patch #1003788: PostgreSQL: PDOException:Invalid text representation when attempting to load an entity with a string or non-scalar ID once committed the entity_load() function will not work for entities with non-numeric IDs.

Blargh.

Ok, reverted.

Side note that entity IDs cannot and should not be non-numeric - this is an assumption coming from core that Entity API provides ways to violate.

That's not true. Only fieldable entities require numeric ids, others don't. Also see the docs at hook_entity_info() and #1003788-67: PostgreSQL: PDOException:Invalid text representation when attempting to load an entity with a string or non-scalar ID.

To clarify: the core patch #1003788: PDOException:Invalid text representation when attempting to load an entity with a string ID once committed the entity_load() function will not work for entities with non-numeric IDs.

It's about to fix the default implementation which requires numeric ids - that doesn't mean the whole API does so though. Thus, entity_load() is going to remain working with non-numeric ids.

The assumption made that non-numeric IDs will work has already caused problems with other contrib modules - see #1343934: Meta Tags & WYSIWYG Incompatibility?. so sorry, I just don't think that supporting machine names as IDs for entities is a valid assumption with Drupal 7.

Dave, this is not just about machine names for entities, it's about the restriction in general. Consider integrating Drupal with a REST web service, such that the REST resources are entities. In that case the URL or at least the trailing path of it would have to act as identifier, but needs not be an integer at all. In order to be able to implement use-cases like this it's crucial that the API doesn't pose such restrictions which a remote source might not fulfill.

We encountered this problem where we were building a relationship via a machine name. I've attached the patch that fixed it for us. It's essentially the same as the previous patches, though I've only dealt with context creation, not arguments.

The main differences are that we do a check in entity info to make sure that there is a machine name for the entity type and that the following wont work with machine names as entity_load() keys by numeric ids:

$data = !empty($data[$id]) ? $data[$id] : FALSE;

As such, I'm using:

$data = reset($data);

I think this is fine, as in this case we either have an array with a single item, or an empty array. Reset will return the item or FALSE respectively, so it has the same result. I wonder if it's worth changing to that across the board?

I wonder if it's worth tweaking the rest of the function so that it's agnostic whether the argument is an id or a name, rather than using $id. I've also not done anything to allow it to load via an array/object on machine names.

If it looks like this approach is a good one, let me know and I can write a patch to deal with the rest of is as well.

As for the argument, do we want to have a separate entity_name argument handler, rather than making entity_id cope with machine names?

Oops, forgot to attach the patch! Marked for review for testing purposes, though will want more work before being committed.

The reason this issue was reverted (#1003788: PostgreSQL: PDOException:Invalid text representation when attempting to load an entity with a string or non-scalar ID) was never committed.

Profile2 supports non-numeric entity IDs. While added a context via the UI gives you an autocomplete that translates it into a numeric ID, the argument programmatically adds the context with no such allowances.

I haven't revisited the context code, but here's a fix for the argument.

Unlike the above patches, this doesn't need any specific checking, since it just uses reset().

Tested, get's rid of some notices, no side effects including nausea, dizziness, fainting, etc

Added file names and another place that may need updating

This patch fix non-numeric issue with arguments. Not sure if we need id-regexp functionality as ctools_argument_entity_id_settings_form has never called(and even if it will be called, it doesn't work).

I am having the same issue.
We are using entities for our backend API, which has non-numerical ID's.
A bit confused about this issue, is there consensus? Will this be allowed?
We are patching Ctools now internally for this in our make file.
It would be nice to know if it's just about submitting a working patch or not to get this pushed upstream.

Tom

The 3 year old patch in #29 to entity_id.inc applied cleanly to the latest ctools 7.x-1.x-dev and (if applicable) still needs review.

Checking patch plugins/arguments/entity_id.inc...
Hunk #1 succeeded at 70 (offset -1 lines).
Applied patch plugins/arguments/entity_id.inc cleanly.