Register entity controllers with the DIC

What about register each controller direclty in the controller? E.g.

entity.storage.ENTITY_TYPE
entity.form.ENTITY_TYPE
entity.render.ENTITY_TYPE
entity.access.ENTITY_TYPE
entity.query.ENTITY_TYPE

We could still keep our procedural wrappes and access the DIC there. That should make conversion easy.

Related issue: #1807042: Reorganizie entity storage/list/form/render controller annotation keys

I wonder how this relates to #1763974: Convert entity type info into plugins ?

This isn't really WSCCI, but good framework practice. So, tag Crell? :-P

In any case, +1. I like the naming pattern from #3, as well.

We can keep the wrappers for now, but should remove them during code slush. They're all new in D8, which means we don't need them for BC. I'd rather minimize the number of procedural wrappers we have and encourage people to realize they're using the DIC, or better yet make their code injected in the first place.

Also, we will want to do things like convert the controllers in the DIC to be fully injected, so they don't have db_* calls in them, and such. Let's get the patch green first before we do that, though. (That's maybe a follow up, albeit one we need to make sure to not forget.)

What sun said.

Don't know much about plugins yet, but I'm not sure how this plays together. The DIC is compiled now, but would be built based on plugins? Which means that if a plugin definition is changed/altered, we need to rebuild the container? Not sure if that makes sense.

And if the controllers/handlers are not part of the plugin definition, who would be responsible for registering them?

Also, the way this is currently implemented doesn't allow to remove __construct() from the controllers, because we require that they all have the same constructor and makes passing in services like the database impossible because we don't know which services a controller might need.

This and the plugin patch combined will result in the following definition chain:

plugin definition (+possible alters) <- cache (requires database by default) <- built DIC <- compiled DIC.

That means that we need to fetch a service to create the service definition, a service that needs the database, so we need to initalize that that early. Which will no longer be possible with the current implementation of #1811730: Refactor Database to rely on the DIC solely because the database then requires a parameter that is only set later on.

Second, once the DIC is compiled, it's going to be really complicated to change the plugin definition. To be able to access a new entity type, you need to clear both the plugin definition cache and rebuild the DIC at the same time, in the correct order.

The cache backend issue is not solved, and will not work that way after the bootstrap kernel is gone. See the discussion in #1810912: [meta] Decide on pluggability and the current implementation of the keyvalue backends in #1809206: KeyValueFactory hard-codes DatabaseStorage.

Something constructive to end this post, something that *might* work better would be the following DIC definition:

$container->register('entity.node.storage')
  ->setFactoryService('entity_manager')
  ->setFactoryMethod('getController')
  ->addArgument('node')
  ->addArgument('storage');

That solves the problem partially because we would no longer have hardcoded controller class names in the compiled DIC but a call to another service. I'm using something like that in the database DIC issue. But probably not everything, e.g. being able to override that for different constructor arguments.

shameless plug for the people subscribed here : #1832418: Possibility of stale cache in EntityManager::$controllers

I'd prefer to approach this more like #1831264: Use the Entity manager to create new controller instances.

I'd prefer to approach this more like #1831264: Use the Entity manager to create new controller instances.

+1.
Keeping the controllers close to where we collect the metadata they created from makes it easier to do the household (like cleanup when the base metadata is refreshed)