A fieldable entity type without a bundle breaks Field UI integration for all bundles of entity types that alphabetically follow it.

Lazy bit of coding on my part.

Found by agentrickard because domain has no bundle.
It doesn't occur in core because User is the only entity type that doesn't have a bundle, and there are no fieldable entity types that come after U.

The $defaults array is never reset, so it is wrong populated into the routes that follow, breaking their placeholder expansion.

I'll write tests tonight.

CommentFileSizeAuthor
#14 field-ui-1986888-14-FAIL.patch3.55 KBtim.plunkett
#14 field-ui-1986888-14-PASS.patch4.28 KBtim.plunkett
#11 field-ui-1986888-11.patch5.81 KBtim.plunkett
#8 field-ui-1986888-8.patch5.08 KBtim.plunkett
#7 field-ui-1986888-7-FAIL.patch4.88 KBtim.plunkett
#7 field-ui-1986888-7-PASS.patch5.61 KBtim.plunkett
#6 field-ui-1986888-6.patch5.61 KBtim.plunkett
#4 field-ui-1986888-4-FAIL.patch6.97 KBtim.plunkett
#4 field-ui-1986888-4-PASS.patch7.7 KBtim.plunkett
#3 field_ui_1986888-entity-no-bundle-do-not-test.patch1.09 KBagentrickard
field-ui-bundle-disaster.patch747 bytestim.plunkett
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard commented

Confirmed, and the patch fixes it. Lets see if I can write that test.

yched’s picture

yched CreditAttribution: yched commented

Different bug, but was mentioned in the same IRC discussion : #1986902: Menu Rebuilding to late causing RouteNotFoundException during install

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard commented
FileSize
field_ui_1986888-entity-no-bundle-do-not-test.patch1.09 KB

Well, here's a start, but it looks like the entity system silently dies if we don't add all the new crufty ho-hah to really define the fake entity type.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Priority: Normal » Major
Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
field-ui-1986888-4-PASS.patch7.7 KB
field-ui-1986888-4-FAIL.patch6.97 KB

Whew. That's a lot of overhead.
Possibly the largest test-to-bugfix ratio I've ever done? 222:1

Since this means any module A-M that provides an entity type with no bundles, calling this major.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented

This is going to be much smaller after #1982088: Remove hook_entity_bundle_info()'s need for 'real path'. I'll post a reroll when that's in.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
FileSize
field-ui-1986888-6.patch5.61 KB

Don't need the .install or .module anymore.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
FileSize
field-ui-1986888-7-PASS.patch5.61 KB
field-ui-1986888-7-FAIL.patch4.88 KB

This fails locally for me...

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
FileSize
field-ui-1986888-8.patch5.08 KB

Well, I get this locally.
Fatal error: Allowed memory size of 6442450944 bytes exhausted (tried to allocate 76 bytes) in /Users/tim/www/d8/core/lib/Drupal/Core/Routing/RouteProvider.php on line 122

That's a fair amount of RAM. 6GB. More than the bots. So.

Drupal test run
---------------

Tests to be run:
 - Field UI routes (Drupal\field_ui\Tests\FieldUIRouteTest)

Test run started:
 Sunday, May 5, 2013 - 08:23

Test summary
------------

Field UI routes 10 passes, 1 fail, and 0 exceptions

Test run duration: 10 sec

Here is the same test but also with node, since that's how we reproduced it originally. This should fail.

Status: Needs review » Needs work

The last submitted patch, field-ui-1986888-8.patch, failed testing.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard commented

I was having the same issue before applying that patch -- running out of memory during the routing phase.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Needs work » Needs review
FileSize
field-ui-1986888-11.patch5.81 KB

Well ideally #7 would have failed the same, but this is just as well. Here's #8 (test) + OP (fix).

yched’s picture

yched CreditAttribution: yched commented

Just wondering what the 'aaa' and 'bbb' in entity type names and route_base_path are there for ?
Also wondering whether it would make more sense to reuse the test entity types defined by entity_test.module - by default they have no bundles, and have an API to create bundles.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Needs review » Needs work

I was using the aaa and bbb because the bug is based on alphabetical order. But yeah, I think I will just reuse the entity_test ones in conjunction with node.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Needs work » Needs review
FileSize
field-ui-1986888-14-PASS.patch4.28 KB
field-ui-1986888-14-FAIL.patch3.55 KB

While the entity_test entity types have no bundles created, they do have a bundle key. Not having one is a criteria to make this thing fail.

So this is what I've got, I think we're good enough to go with this.

yched’s picture

yched CreditAttribution: yched commented

entity_test entity types have a bundle key. Not having one is a criteria to make this thing fail

Too bad. This should probably be stated in the patch then, to avoid later merging by a good soul :-/

damiankloip’s picture

damiankloip CreditAttribution: damiankloip commented
Status: Needs review » Reviewed & tested by the community

This makes sense, and is exposed nicely with the test.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed a371a7b and pushed to 8.x. Thanks!

Added type: module to core/modules/field_ui/tests/modules/field_ui_test/field_ui_test.info.yml during commit...

Status: Fixed » Closed (fixed)

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