Improve performance of getReferencableEntities()

Version 1.6 released

Has anyone tested this with the 1.6 release? Automated tests are passing nicely.

pushed this into 7.x-1.x dev

see
🌱 Plan for 7.x-1.10 Active

Great job everyone above,

this patch/fix also resolves a memory allocation error.

I noticed a server crash with memory allocation failure after consuming 4 gigabytes of ram.

The 1.7 release solves the bug thanks to the above patch, along with installing the latest entity module

🐛 Fatal error, allowed memory size exhausted, after update to 7.x-1.5 Fixed

https://www.drupal.org/project/entityreference/releases/7.x-1.7 →

When we used 7.x-1.6, we did not have any errors caused by this module. After updating this module to 7.x-1.7, we got this error:

EntityMalformedException: Missing bundle property on entity of type taxonomy_term. in entity_extract_ids() (line 8090 of
/.../includes/common.inc).

To get rid of this error, we load an entity inside the loop. Please see the attached patch.

@joseph.olstad, in addition to what @fralui (I work with fralui, full disclosure) mentioned above there is also a security issue here too because entity_load() will add a security access check tag and translation tag capabilities, example for taxonomy terms:

/modules/taxonomy/taxonomy.module:1254

  protected function buildQuery($ids, $conditions = array(), $revision_id = FALSE) {
    $query = parent::buildQuery($ids, $conditions, $revision_id);
    $query->addTag('translatable');
    $query->addTag('taxonomy_term_access');

Which is true for other entity types as well.

So quick fix is revert, or consider the patch middle ground that solves the bundle issue where the entity key's bundle is not on the base table that was db_select'd.

For this patch, wouldn't it be better to check to see if the bundle property is missing first before loading?

I'll roll a new patch

New patch

actually, my patch is not any better than patch 37, in fact bundle will never be set in this case.

@joelpittet, if you have access to this environment I'd like to run a test or two and inspect the $entity variable returned on line 150

$query->execute() as $entity

what would be the value of $entity when this crashes?

what would be the value of $target_type when this crashes?

It would be great to find a better solution than reverting since this fixes a critical memory allocation error on my clients environment and it was reported above to fix memory allocation issues in other environments.

might not be possible to do so easily however maybe worth a bit of effort.

@joelpittet, @franceslui, will this patch work for you?

As for the "security issue", I'd like to see a real-world scenario for this use case. There's multiple levels of access checks and we're trying to balance out performance.

@joelpittet

I did a little bit of debugging on my own environment and tested this code which assumes first taxonomy_term tid = 1 , then node nid =1.

  $entity_id = 1;
  $target_type = 'taxonomy_term';
  $loaded_entities = entity_load($target_type, array($entity_id));
  $entity = reset($loaded_entities);
  list($id,, $bundle) = entity_extract_ids('taxonomy_term', $entity);
  echo "\n";
  echo print_r(entity_extract_ids('taxonomy_term', $entity), TRUE);
  echo "\n";
  echo "id: " . $id;
  echo "\n";
  echo "bundle: " . $bundle;
  echo "\n";

  $target_type = 'node';
  $loaded_entities = entity_load($target_type, array($entity_id));
  $entity = reset($loaded_entities);
  list($id,, $bundle) = entity_extract_ids('node', $entity);
  echo "\n";
  echo print_r(entity_extract_ids('node', $entity), TRUE);
  echo "\n";
  echo "id: " . $id;
  echo "\n";
  echo "bundle: " . $bundle;
  echo "\n";

The output was as follows:

Array
(
    [0] => 1
    [1] => 
    [2] => tags
)

 id: 1
 bundle: tags

Array
(
    [0] => 1
    [1] => 1
    [2] => article
)

 id: 1
 bundle: article

What entity type is causing this problem on your end, I tested taxonomy_term and node?

@joelpittet @franceslui, please test patch 43

@joseph.olsad #43 entity_extract_ids is where the error is happening so that won't fix the problem.

@joseph.olsad Thanks for taking some of your weekend to look into this, much appreciated.

The crux of this is that db_select() was used to replace entity_load() so while db_select will be hugely faster, that is mostly due to the lack of hooks that could be fired off to do things to "load" the entity.

$entity = stdClass when db_select() builds it.
$entity = *EntityController class when entity_load() finishes with it.

And in our case taxonomy_term_data table doesn't store the bundle name, it stores vid, and needs those extra hooks to join the vocabulary table.

@fralui, you probably need to add a empty check after the entity_load in case it was not loaded due to access control or something for #37

@joseph.olstad oh I think I see where you might be confused, the code we care about is the part at the end of the committed patch from #31

-      $entities = entity_load($target_type, array_keys($result));
-      foreach($entities as $entity) {
+
+      // Do not load the entire entity for performance reasons.
+      $entity_info = entity_get_info($target_type);
+      $query = db_select($entity_info['base table'], 'e');
+      $query->fields('e');
+      $query->condition('e.' . $entity_info['entity keys']['id'], array_keys($result), 'IN');
+
+      foreach($query->execute() as $entity) {

You see where there is no entity_load()

If this only gets a partial revert or better patch, maybe it should live over here 🐛 EntityMalformedException: Missing bundle property error when entity reference selection mode is set to Views: Filter by an entity reference view Fixed ?

@joelpittet, please try

#3348756-04: EntityMalformedException: Missing bundle property error when entity reference selection mode is set to Views: Filter by an entity reference view →

alternatively, this approach may also resolve the issue for you:

#3348756-06: EntityMalformedException: Missing bundle property error when entity reference selection mode is set to Views: Filter by an entity reference view →

If someone experiences what was described in #37, please try this patch:
#3348756-19: EntityMalformedException: Missing bundle property error when entity reference selection mode is set to Views: Filter by an entity reference view →

If I understood correctly I believe that @joelpittet is working on an automated test to make sure that we fix this issue for good.

Marking this back as fixed as all the work is happening in the other issue mentioned in #53

Thanks to the UBC team, especially Joelpittet and @franceslui, 1.8 has the regression fix, a usability improvement and added test coverage.
https://www.drupal.org/project/entityreference/releases/7.x-1.8 →

here's the usability improvement:
🐛 Improve the ENTER keydown behaviour of ER autocomplete Fixed

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

please try the latest release

https://www.drupal.org/project/entityreference/releases/7.x-1.9 →