Problem/Motivation

The methods loadRevision() / loadMultipleRevisions() allowed NULL values as input without throwing an error or notice, and just returned NULL if no entity was found (which is obvious since there can't be an entity with a NULL ID).

In #3019030: Ensure loadMultipleRevisions() returns the revisions in the same order as the passed IDs an array_flip() call was added to loadMultipleRevisions(), which now shows a warning array_flip(): Can only flip STRING and INTEGER values!.

The methods load() and loadMultiple() already had this "issue", since loadMultiple() also contains a call to array_flip().

While you could argue the calling code should not call the method with "garbage" input, this means that calling code always needs to do 2 checks:
1. Is the input a string or int.
2. Is the return value of the specific load method not NULL.

It would be nice if you could call load() / loadMultiple() / loadRevision() / loadMultipleRevisions() without having to check the input, and let the load methods return NULL if it can't find an entity. I've seen methods (like ContentEntityStorageBase::getLatestTranslationAffectedRevisionId()) that could return a entity ID or NULL and I think the DX would be a bit better if you don't have to do the extra check.

Steps to reproduce

drush php-eval "Drupal::entityTypeManager()->getStorage('node')->loadMultiple(['x'=>['x']]);"

Proposed resolution

Check the input for load() / loadMultiple() / loadRevision() / loadMultipleRevisions() and simply return NULL if it's not a string or int.

Remaining tasks

Discuss
Patch
Review
Commit

User interface changes

API changes

Data model changes

Release notes snippet

Comments

seanB created an issue. See original summary.

seanb’s picture

seanb
Primary language Dutch
Location Netherlands
commented
Title: Improve DX of load / loadMultiple / loadRevisionload / MultipleRevisions » Improve DX of load / loadMultiple / loadRevisionload / loadMultipleRevisions
jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented

This may be a duplicate of #3035980: Provide a better error when a NULL is passed to EntityStorageBase::load(), although I don't think that patch currently covers the revision storage.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

manuel.adan’s picture

manuel.adan
Location 🌌
commented
Related issues: +#2762737: ContentEntityStorageBase::loadRevision(NULL) returns an entity

Seems dup of #2762737: ContentEntityStorageBase::loadRevision(NULL) returns an entity.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented

We can probably address these methods the same as was done in #3019030: Ensure loadMultipleRevisions() returns the revisions in the same order as the passed IDs which was to use assert(). Note that this should no longer be an issue on loadMultipleRevisions since it is type-hinted to take array as its parameter, so passing null in there already results in immediate feedback to the developer.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

joachim’s picture

joachim commented

Duplicate of #3221534: throw an exception when IDs passed to loadMultiple() are badly formed?

seanb’s picture

seanb
Primary language Dutch
Location Netherlands
commented

I guess so. Same problem, different solution :)

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone credited damondt.

quietone’s picture

quietone commented
Category: Feature request » Task
Issue summary: View changes

Closed #3048490: Warning: array_flip(): Can only flip STRING and INTEGER values! as a duplicate and adding credit. Updated IS with STR from @damondt in the other issue.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.