Problem/Motivation

I'm using a custom entity with a content moderation field. All works fine but after saving a register when the content is loaded I got the following warning message :
Warning: array_flip(): Can only flip STRING and INTEGER values! in Drupal\Core\Entity\ContentEntityStorageBase->loadMultipleRevisions() (line 573 of core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php).

The line 573 is part of a verification to load multiple revisions and the array_flip() function is receiving an array like this

^ array:1 [▼
  0 => 103
]

and after flipping the array it's returning another like this

^ array:1 [▼
  103 => 0
]

In this case the array_flip() is working but is returning the warning message.
$flipped_ids = array_intersect_key(array_flip($revision_ids), $revisions);

Steps to reproduce

  • Create a custom entity
  • add a content moderation field
  • and save some registers.

Proposed resolution

Since I don't detect that flip_array () is receiving a malformed array and if I transform the array manually I get the same results.
I suggest maybe returning a more complete feedback message since I have found a lot of posts with issues with similar messages.

Looking for doesn't skip the verification that ensures that the returned array is ordered the same as the original.

I test using a foreach iteration to do the array flip, I get the same result but I'm avoiding the warning message.

If someone can test the following patch and give me some feedback or suggest an alternative solution it would be great.

CommentFileSizeAuthor
#19 3249389-check-invalid-revision-id-19.patch1.24 KBfranceslui
#17 make-revision-ids-core-check-verify-we-have-non-empty-array.patch1.01 KBjnicola
#4 3249389-array-flip-replaced-by-foreach-iteration.patch985 bytescarlos-perez
#2 3249389-1-array-flip-replaced-by-foreach-iteration.patch985 bytescarlos-perez

Issue fork drupal-3249389

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

carlos-perez created an issue. See original summary.

carlos-perez’s picture

carlos-perez commented
StatusFileSize
new 3249389-1-array-flip-replaced-by-foreach-iteration.patch985 bytes

Looking doesn't skip the validation to ensure that the returned array is ordered the same as the original.

I test flipping the array using a foreach iteration and works well. I'm skipping the warning message returned by the flip_array() function.

If someone can test and give me some feedback or suggest an alternative solution it would be great.

carlos-perez’s picture

carlos-perez commented
Issue summary: View changes
carlos-perez’s picture

carlos-perez commented
StatusFileSize
new 3249389-array-flip-replaced-by-foreach-iteration.patch985 bytes
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Category: Bug report » Support request
Status: Active » Postponed (maintainer needs more info)
Issue tags: -array_flip +Bug Smash Initiative

This happens when you pass NULL to a load operation

Can you post your entity annotation?

carlos-perez’s picture

carlos-perez commented

Hi! I'm using a custom workflow and the patch works fine to me in this case on 9.2.x.

Yes, the problem happens in a load operation
public function loadMultipleRevisions(array $revision_ids)

Is receiving $revision_ids that is something like:

^ array:1 [▼
  0 => 103
]

and flip it to 103=>0.
just like when I load some other type of entity.
But with a warning message included.

I was looking for a malformed array or null values on it, but I didn't find it.

If there is a way to resolve without a patch would be nice.

This is the entity annotation

/**
 * Defines the registration entity class.
 *
 * @ContentEntityType(
 *   id = "registration",
 *   label = @Translation("Registration"),
 *   label_collection = @Translation("Registrations"),
 *   handlers = {
 *     "view_builder" = "Drupal\Core\Entity\EntityViewBuilder",
 *     "list_builder" = "Drupal\my_scheduler\RegistrationListBuilder",
 *     "views_data" = "Drupal\my_scheduler\RegistrationViewData",
 *     "access" = "Drupal\my_scheduler\RegistrationAccessControlHandler",
 *     "form" = {
 *       "add" = "Drupal\my_scheduler\Form\RegistrationForm",
 *       "edit" = "Drupal\my_scheduler\Form\RegistrationForm",
 *       "delete" = "Drupal\Core\Entity\ContentEntityDeleteForm"
 *     },
 *     "route_provider" = {
 *       "html" = "Drupal\Core\Entity\Routing\AdminHtmlRouteProvider",
 *     },
 *    "moderation" = "Drupal\my_scheduler\Entity\RegistrationHandler",
 *   },
 *   base_table = "registration",
 *   revision_table = "registration_revision",
 *   show_revision_ui = TRUE,
 *   admin_permission = "administer registration",
 *   entity_keys = {
 *     "id" = "id",
 *     "revision" = "revision_id",
 *     "label" = "id",
 *     "uuid" = "uuid",
 *     "published" = "status",
 *   },
 *   revision_metadata_keys = {
 *     "revision_user" = "revision_uid",
 *     "revision_created" = "revision_timestamp",
 *     "revision_log_message" = "revision_log"
 *   },
 *   links = {
 *     "add-form" = "/admin/content/registration/add",
 *     "canonical" = "/registration/{registration}",
 *     "edit-form" = "/admin/content/registration/{registration}/edit",
 *     "delete-form" = "/admin/content/registration/{registration}/delete",
 *     "collection" = "/admin/content/registration"
 *   },
 *   field_ui_base_route = "entity.registration.settings"
 * )
 */
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

can you var_export the array and share? there may be some casting going on with the new patch

carlos-perez’s picture

carlos-perez commented

This is the exported var

array ( 0 => 109, )

maybe is expecting an element after the comma.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

that looks like the second entry is NULL somehow?

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

https://3v4l.org/4oWIV would suggest so

carlos-perez’s picture

carlos-perez commented

Yes, thank you so much.
I'll try to track where the array is built.

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

Drupal 9.1.10 (June 4, 2021) and Drupal 9.2.10 (November 24, 2021) were the last bugfix releases of those minor version series. Drupal 9 bug reports should be targeted for the 9.3.x-dev branch from now on, and new development or disruptive changes should 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.

gaurav_manerkar’s picture

gaurav_manerkar commented

I am facing this issue as well after updating to 9.3

cilefen’s picture

cilefen commented
Status: Postponed (maintainer needs more info) » Closed (outdated)

It seems we need more information to provide support for this one. Reopen it if there is some.

jnicola’s picture

jnicola commented
Status: Closed (outdated) » Postponed (maintainer needs more info)

I'm having the same issue creating a group entity in Behat from simple array values and a content moderation state.

jnicola’s picture

jnicola commented

Also, just tested the patch locally. Resolved my issues.

Issue was programmatically creating a pretty simple group from array values. No idea where the array flip complaint came from.

This is likely an issue worth getting some review on. A simple start would be to create a bare bones drupal install with just content moderation and programatically create an entity to see if it can be reproduced, and increase complexity until the warning shows up.

jnicola’s picture

jnicola commented
Version: 9.3.x-dev » 9.5.x-dev
StatusFileSize
new make-revision-ids-core-check-verify-we-have-non-empty-array.patch1.01 KB

Alright, another update.

After experimenting with this, I was able to ascertain that $revision_ids is an array of a single NULL value when programmatically creating content with content moderation on it. In our case it is a group content type. We're experiencing this in 9.5.x, creating a simple group entity with very simple values.

I tried using the foreach patch above, which worked... but when standing the site up for automated testing it would error out on config imports due to memory limitation issues. Not sure how the two relate?

I was able to resolve the issue by instead of using a foreach, adding a few sanity checks prior to the array_flip causing the warning:

if ($revision_ids && !is_null(reset($revision_ids))) {

Attached is the patch, which does not follow the correct issue queue naming conventions, but eh whatever take it out of my bonus that I don't get ;)

Hope this helps somebody, and perhaps with some simpler steps to reproduce and a test we could get this onwards to core.

Version: 9.5.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. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

franceslui’s picture

franceslui commented
Version: 11.x-dev » 10.2.x-dev
Status: Postponed (maintainer needs more info) » Needs review
StatusFileSize
new 3249389-check-invalid-revision-id-19.patch1.24 KB

The current implementation of ContentEntityStorageBase::loadRevision() and ContentEntityStorageBase::loadMultipleRevisions() does not adequately handle invalid or empty revision IDs. This can lead to potential errors or unexpected behavior when these methods are called with invalid parameters.

Solution:
1. Added validation in ContentEntityStorageBase::loadRevision($revision_id) to check if $revision_id is either an integer or a string. If not, the method exits early and returns NULL.
2. Added a check in ContentEntityStorageBase::loadMultipleRevisions(array $revision_ids) to exit early and return the revisions array if no revisions can be loaded, ensuring the method does not proceed with empty data.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work

The Needs Review Queue Bot tested this issue.

While you are making the above changes, we recommend that you convert this patch to a merge request. Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)

Prashant.c made their first commit to this issue’s fork.

Prashant.c opened merge request !8230

prashant.c’s picture

prashant.c
he/him
Primary language English
Location Dharamshala
commented
Status: Needs work » Needs review

Raised a PR by taking the changes from #19.

Changing the status to NR.

Thanks

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs tests

Support requests typically don't have patches or MRs.

Has anyone determined where the issue is happening? Instead of just putting a check that may be masking a larger issue.

franceslui’s picture

franceslui commented
Version: 10.2.x-dev » 11.0.x-dev
Status: Needs work » Postponed (maintainer needs more info)
Related issues: +#3386719: Fatal error when viewing a static_page revision due to core type upcasting change

I concur with @smustgrave’s comment #24 that the check I implemented in my patch might obscure a deeper issue. Therefore, I have removed my patch from this issue. Upon further investigation, I discovered that the problem stems from the static_page module rather than Drupal core. The detailed information can be found in the related issue at #3386719: Fatal error when viewing a static_page revision due to core type upcasting change. You can also find my new patch in my comment at #3386719-4: Fatal error when viewing a static_page revision due to core type upcasting change.

quietone’s picture

quietone commented
Version: 11.0.x-dev » 11.x-dev
smustgrave’s picture

smustgrave commented
Status: Postponed (maintainer needs more info) » Closed (outdated)

Believe the related issue should solve this support request.