The class EntityConverter should be using hasDefinition() instead of getDefinition().
See https://github.com/socketwench/flag-drupal8/pull/22, when a route is defined with a non-existent entity type this will fail.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ja_ca’s picture

Status: Active » Needs review
FileSize
747 bytes

Fixed.

ja_ca’s picture

Title: Non-existent entity type » EntityConverter uses getDefinition() instead of hasDefinition()
dawehner’s picture

Component: configuration entity system » routing system
Status: Needs review » Reviewed & tested by the community

Seems legit.

Berdir’s picture

Could possibly use a unit test but I'm not sure how useful that would really be, a unit test would *not* have caught this bug, because we changed the behavior of getDefinition() since this was written and nobody would have thought to update a mock implementation of getDefinition() to throw an exception.

Maybe an integration test, but again, there's not really something that you can test for other than being able to build the routes and it not resulting in a exception that aborts the whole process (which shouldn't be possible, but that's another topic again), but:

What's very weird and we couldn't really explain is that this only happened if applies() was called through the ViewsUI enhancer and not when called directly.

dawehner’s picture

Well yeah some sort of integration test could be usuable, do you know exactly how this happened? What let the views UI enhancer
upcast with the wrong entity type? This sounds like a case when route rebuild happens before the entity definition (static/non-static) cache is cleared.

Talking about the views UI one, I really don't like the fact that it is extending the entity one, this is a misuse of inheritance if you ask me.

Berdir’s picture

What happened is that there was bundle_entity_type = "invalid_entity_type" annotation key on an entity type, that resulted in an explicit type => 'entity:invalid_entity_type' in the field_ui manage fields route (as it needs to be defined manually because the defaults don't apply for routes not defined in routes.yml). Then it tried to check if that can be enhanced.

That said, I don't know why it only happend through ViewsUIEnhancer, the problem did not occur in tests even if they enabled field_ui.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 013a6e8 and pushed to 8.x. Thanks!

  • alexpott committed 013a6e8 on 8.x
    Issue #2295647 by ja_ca: Fixed EntityConverter uses getDefinition()...

Status: Fixed » Closed (fixed)

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