fix(EntityLoad): Return NULL on NULL entity IDs when composing entity load

[klausi]

We saw the error array_flip(): Can only flip STRING and INTEGER values! in Drupal\Core\Entity\EntityStorageBase->loadMultiple(), this happens when a NULL ID is passed into the EntityLoad dataproducer (for example when composing data producers)

Make this more robust by checking for NULL.

[rthideaway]

to me this makes perfect sense

[otrolopezmas]

Hey!

Great that you found this.

I think for the test it would suffice with just adding a new test checking out for NULLs. See

/**
   * @covers \Drupal\graphql\Plugin\GraphQL\DataProducer\Entity\EntityLoad::resolve
   */
  public function testResolveEntityLoadWithNullId(): void {
    $result = $this->executeDataProducer('entity_load', [
      'type' => $this->node->getEntityTypeId(),
      'id' => NULL,
    ]);

    $this->assertNull($result);
  }

At the same time, I am not completely sure but I think I got the same problem with LoadMultiple? In that case we would need to do some kind of checking if there're NULL presents in the resolver in src/Plugin/GraphQL/DataProducer/Entity/EntityLoadMultiple.php. And of course we would need to do a similar test like

  /**
   * @covers \Drupal\graphql\Plugin\GraphQL\DataProducer\Entity\EntityLoadMultiple::resolve
   */
  public function testResolveEntityLoadMultipleWithNullIds(): void {
    $result = $this->executeDataProducer('entity_load_multiple', [
      'type' => $this->node1->getEntityTypeId(),
      'bundles' => [$this->node1->bundle(), $this->node2->bundle()],
      'ids' => [$this->node1->id(), $this->node2->id(), NULL],
      // @todo We need to set these default values here to make the access
      // handling work. Ideally that should not be needed.
      'access' => TRUE,
      'access_operation' => 'view',
    ]);

   $this->assertNull($result);
  }

What do you think? Would we need more test covering some kind of composing.

[klausi]

Thanks, yes I think that test case would be enough for EntityLoad. Can you make a new pull request so that we can see the tests run?

EntityLoadMultiple: right, would make sense to run the default array_filter() on the passed IDs to remove any NULL or falsy values. Your test for that makes sense!

[klausi]

Added a test case and merged this, thanks!