Test unit behavior of EntityStorageBase::load(), loadMultiple() with invalid ID, UUID

I think its Entity::load()/loadMultiple() ?

The loadMuliple case a bit tricky. It takes an array of UUID's as an argument. Should it return NULL even if a single UUID is invalid or NULL in the return array for those UUID's that are invalid/non-existing?

In the case of invalid id's, EntityStorage::loadMultiple(), which is called by Entity::loadMultiple($ids), removes them from the result array, so I think there shouldn't be any need to make changes in loadMultiple() function.

Entity::load($id) makes use of EntityStorage::load() function, which calls EntityStorage::loadMultiple() with a single id in an array as the parameter. EntityStorage::load() returns null if EntityStorage::loadMultiple() returns an empty array, so I think there is no need to make changes in load() function too.

Id, not UUID.

Also, we should return NULL

Hi all,

I'm in DrupalDevDays in Montpellier today (mentoring session)
And I'll try to fix this issue.

Hi, I'm in drupal dev days and i'll try to fix this issue.

In the absence of m0zzy in the mentoring room, I investigated this issue and will post the results soon.

I am unable to reproduce this issue.

Some quick debugging seems to show that there is no issue with load and loadMultiple while using their calling functions (entity_load and entity_load_multiple) which returns their expected results:

• NULL for single entities
• An empty array for multiple entities.

Loading a single entity

$id = entity_load('node', 'foo');
$id2 = entity_load('node', 35453);
$id3 = entity_load('node', NULL);
var_dump($id);
var_dump($id2);
var_dump($id3);

Results in:

null
null
null

Loading multiple entities

$multiple = entity_load_multiple('node', array('foo'));
$multiple2 = entity_load_multiple('node', array());
$multiple3 = entity_load_multiple('node', array(234234));
$multiple4 = entity_load_multiple('node', array(NULL));

var_dump($multiple);
var_dump($multiple2);
var_dump($multiple3);
var_dump($multiple4);

Results in:

array (size=0)
  empty
array (size=0)
  empty
array (size=0)
  empty
array (size=0)
  empty

Postponed for more information and removed the novice tag.

Why: Per #9 issue couldn't be recreated; need more information. Removed novice tag since issue is not actionable by novice.

Novice tag documentation: https://www.drupal.org/core-mentoring/novice-tasks

Can you provide inputs or unit tests which replicate this issue?

@andrewsuth If it's at all possible, could you turn the experimentation in #9 into a runnable test?

That way we can be sure the behavior is correct.

Thanks.

Updated issue summary to reflect that EntityManager::load()/loadMultiple() doesn't exist any more, and is on the storage object.

Rescoping a bit to concentrate on EntityStorageBase.

Since EntityStorageInterface::load() accepts an ID of mixed type, there's no such thing as an invalid parameter to it, or to loadMultiple(), since it accepts an array of the same type. Whether a given ID is of an illegal type would be determined by doLoadMultiple() in the concrete storage class, rather than EntityStorageBase, which is abstract. In such a case, doLoadMultiple() should simply ignore the illegal ID.

This patch contains a test class to cover EntityStorageBase.

It @covers ::load and ::loadMultiple.

For load():

• Return NULL when there are no entities for the given ID.
• Return the entity when its ID exists.

For loadMultiple():

• Return an empty array if there are no results for the given IDs.
• Return an array containing entities matching the given IDs.
• Return an array containing matching entities for queries with partial matches for the given IDs.

Note that we're only testing the behavior of loadMultiple() for entity types without static caching.

Still applies, still passes.

Removed @file per new CS, added keys to data provider arrays for readability.

Changing to a task,

I have also rerolled the patch for 10.0.x

Discussed briefly with @quiteone in #bugsmash about the added value here and it was pointed out that this still checks for logic errors even if the expected value is basically the same as whatever we pass as the return value for doLoadMultiple.

So, more test coverage ++

Committed and pushed 50ddf37fda to 10.1.x and 9909596e5e to 10.0.x and 112de03974 to 9.5.x and 38e9b318a3 to 9.4.x. Thanks!

Backported to 9.4.x to keep test aligned. I ran the test on 9.4.x / php 7.3 locally.