Problem/Motivation

Came across this issue while working on a site that had a node type "404".

JSON:API uses array_keys() on the result of \Drupal\Core\Entity\EntityTypeBundleInfoInterface::getBundleInfo($entity_type_id). getBundleInfo() returns an associative array keyed on bundle name. Unfortunately, array_keys() will return (int) 404 in our case, passing the bundle name as integer down the stack, which in turn triggers some of the 'bundle-ID-is-string-and-not-empty' assertions (for example in isLocatableResourceType and isMutableResourceType:

protected static function isLocatableResourceType(EntityTypeInterface $entity_type, $bundle) {
  assert(is_string($bundle) && !empty($bundle), 'A bundle ID is required. Bundleless entity types should pass the entity type ID again.');
  return $entity_type->getStorageClass() !== ContentEntityNullStorage::class;
}

Proposed resolution

Explictly cast the result of array_keys(bundle_array) to a string to negate the automatic conversion to integer by array_keys().

Remaining tasks

Review & commit

User interface changes

None.

API changes

None.

Data model changes

None.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mr.baileys created an issue. See original summary.

Wim Leers’s picture

Title: Failing assertions when bundle ID contains only numbers. » Failing assertions when bundle ID contains only numbers
Issue tags: +API-First Initiative

Hah, nice edge case!

I see you even provided test coverage! 🥳🙏 Thanks!

DrupalCI failed to test the test-only patch with a "fetch save error" 😲 Never seen that before. Retesting…

Wim Leers’s picture

Priority: Normal » Minor

I think this qualifies as a minor bug since this is a pretty peculiar edge case that very, very few people will ever run into! 😃

The last submitted patch, jsonapi-bundle-assert-test-only.patch, failed testing. View results

Wim Leers’s picture

Status: Needs review » Reviewed & tested by the community
+++ b/core/modules/jsonapi/tests/src/Kernel/ResourceType/ResourceTypeRepositoryTest.php
@@ -102,9 +106,9 @@ public function getProvider() {
-    $this->assertCount(2, $this->resourceTypeRepository->get('node', 'article')->getRelatableResourceTypesByField('field_relationship'));
-    NodeType::create(['type' => 'camelids'])->save();
     $this->assertCount(3, $this->resourceTypeRepository->get('node', 'article')->getRelatableResourceTypesByField('field_relationship'));
+    NodeType::create(['type' => 'camelids'])->save();
+    $this->assertCount(4, $this->resourceTypeRepository->get('node', 'article')->getRelatableResourceTypesByField('field_relationship'));

This looks like it's moving lines, but it's not. @mr.baileys just incremented 2 and 3 to 3 and 4 because of the extra NodeType he added :)

  • larowlan committed cf5ad5f on 8.8.x
    Issue #3075831 by mr.baileys: Failing assertions when bundle ID contains...
larowlan’s picture

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

Committed cf5ad5f and pushed to 8.8.x. Thanks!

Leaving at RTBC for 8.7 for commit after freeze ends

idebr’s picture

#3 Actually we fixed this before in #2900459: InvalidArgumentException: Routing requirement for "_bundle" must be a string. Webform submission entities have a generated bundle that are always interpreted as integers in PHP.

Wim Leers’s picture

D'oh. Thanks @idebr. Looks like this is not super rare then, because Webform is obviously widely used.

So this is just PHP's typecasting getting in the way and us needing to remember to always do the proper casting all over the place. Fascinating.

larowlan’s picture

Status: Reviewed & tested by the community » Needs work

This needs a re-roll for 8.7

Wim Leers’s picture

Issue tags: +Needs reroll
aleevas’s picture

Here is my reroll for 8.7 branch

aleevas’s picture

Status: Needs work » Needs review
Wim Leers’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs reroll

👍 — thanks @aleevas!

(The patch is smaller because the ::testCaching() test coverage did not exist in Drupal 8.7.)

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Fixed

Committed d3e67cc and pushed to 8.7.x. Thanks!

  • Gábor Hojtsy committed d3e67cc on 8.7.x
    Issue #3075831 by mr.baileys: Failing assertions when bundle ID contains...

Status: Fixed » Closed (fixed)

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