Active
Project:
Drupal core
Version:
main
Component:
base system
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
9 Jun 2014 at 09:34 UTC
Updated:
7 Jun 2024 at 13:20 UTC
Jump to comment: Most recent
The DrupalKernel currently has too many responsibilities, and has many qualities of a "god class".
Maybe the Kernel already is too big in Symfony, but it certainly is in Drupal.
There is a number of issues that deal with DrupalKernel refactoring, but they do not really name this problem, and they do not provide a big picture.
The responsibilities will change with issues like #2016629: Refactor bootstrap to better utilize the kernel and #2021959: Refactor module handling responsibilities out of DrupalKernel. It is also likely to grow further as we split up the procedural files.
Here is a list of responsibilities that DrupalKernel combined from different versions:
Bootstrap / shutdown:
- bootEnvironment() (#2016629)
- initializeRequestGlobals() (#2016629)
- initializeCookieGlobals() (#2016629)
- boot()
- shutdown(), terminate()
Site management:
- findSitePath()
- manage site directory
- initialization of site config
Site config management:
- storage() (#2016629)
- getConfigStorage() (#2016629)
Request handling:
- preHandle() (#2016629)
- handlePageCache() (#2016629)
- handle()
- prepareLegacyRequest() (#3453216: Clean up outdated mentions of prepareLegacyRequest)
Container management:
- service provider management
- service provider discovery
- container initialization
- container dumping
- service persistance
Module management:
- moduleData()
- module update
- module namespaces for the container
- module namespaces registration in the class loader
(While #2021959 proposes to moves most of this into the module handler, I think that could also be split up further)
This is too much for one class to handle. Seriously.
The term "Kernel" itself already suggests that it is going to have too many responsibilities..
(I could go on about how small a class should be for my taste, but I would prefer to leave that to IRC or other places)
So, what about the Symfony Kernel?
DrupalKernel does not have much in common with the symfony kernel anymore, so who cares.
But as a comparison, here are the responsibilities of abstract class \Symfony\Component\HttpKernel\Kernel:
- boot, terminate, shutdown
- request handling
- locateResource()
- bundle management
- container managment
- stripComments() (ouch, this one seems totally out of place)
This is already too much for my taste. But the implementation of these aspects look a lot simpler in Symfony. I assume it is ok, assuming that the average developer won't spend too much time debugging this class.
In Drupal, the Kernel is much bigger, it is intertwined with an ugly environment full of legacy global state, and there are a lot more unknowns and runtime dynamics.
Proposed resolution:
Slice it up, step by step, into components such as ContainerManager, ServiceProviderManager, ServiceProviderDiscovery, RequestHandler, SitePathFinder, etc.
There does not even need to be a DrupalKernel class at all, if all the tasks go into separate components.
To let these components play together, I would personally favor a solution like #2272129: [meta] Introduce a low-level DIC to organize low-level services, but I am open to other ideas, and I would like to keep these two issues separate.
Another solution would be to keep the DrupalKernel class, but only use it as a "hub" to get to dedicated separate components.
Comments
Comment #1
donquixote commentedComment #2
donquixote commentedComment #13
joachim commented+1
Additionally, some of DrupalKernel's functionality is intended to be called statically, BEFORE you've instantiated it -- site management methods in particular.
Comment #15
bradjones1The DrupalKernel class itself is also... problematic. It is extended (e.g.,
InstallerKernel,UpdateKernel, and more) however the "base"DrupalKernelis a super-duper privileged class that is instantiated by name in two key places -index.phpanddrupal_rebuild(). In a perfect world, the base class would be abstract and the currentDrupalKernelwould be renamed something likeDrupalAppKernel.But I digress - the current arrangement makes it difficult to do wild things like customizing your own kernel (e.g., to do some forward-compatibility stuff in Symfony) because even if you update
index.phpto use your kernel class, it's still calling the privilegedDrupalKernelby name in the rebuild function. (Ideally there should be only one way to resolve the kernel class.)I understand this is rather esoteric and only affects the rare customer who is doing crazy custom stuff like this... but it also is a legitimate code smell.
Comment #18
andypostUpdated IS as few methods gone already