Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.0] Splitting the HttpKernel component #9406

Closed
fabpot opened this issue Oct 30, 2013 · 32 comments
Closed

[3.0] Splitting the HttpKernel component #9406

fabpot opened this issue Oct 30, 2013 · 32 comments
Labels
HttpKernel RFC RFC = Request For Comments (proposals about features that you want to be discussed)

Comments

@fabpot
Copy link
Member

fabpot commented Oct 30, 2013

The HttpKernel component contains many classes and from time to time, someone wants to split it in several components.

It does not really brings us anything if the split components are not useable by themselves, but separating Kernel and HttpKernel is probably good to at least avoid confusing about the decoupling between bundles and the HTTP kernel itself.

Also, as things have been refactored a lot since 2.0, it's much clearer now to see where things can go in separate components. For instance, moving the Profiler/DataCollector classes to a new component is now possible as other classes do not depend on them anymore (at least in 3.0 as the TraceableEventDispatcher won't depend on the Profiler anymore).

So, here is my initial proposal for Symfony 3.0: we split the HttpKernel component into 3 separate components:

  • Kernel:
    • Bundle/, Config/, DependencyInjection/, CacheClearer/, CacheWarmer/ directories
    • Kernel.php, KernelInterface.php
  • Profiler:
    • Profiler/ and DataCollector/ directories
    • EventListener/ProfilerListener.php
  • HttpKernel:
    • Controller/, Debug/, Event/, EventListener/, Exception/, Fragment/, HttpCache/, Log/ directories
    • Client.php, HttpKernel.php, HttpKernelInterface.php, TerminableInterface.php (to be merge with HttpKernelInterface in 3.0), UriSigner.php
    • rename KernelEvent to HttpKernelEvent and KernelEvents to HttpKernelEvents?
@frastel
Copy link

frastel commented Oct 30, 2013

Great CR!
I totally agree in splitting up this component and divide the logic to seperate smaller components.

Besides these two new components I would suggest one component further more: ContainerBuilder (no good name as this name is also used in a class, but I hope you get the idea) with the extracted logic of initializeContainer, buildContainer, dumpContainer and stripComments . The Kernel then could take this builder and add its registered Bundles.
Building and dumping the Container is a standalone task which should be reusable in plain PHP Projects where the Kernel is not needed.
I had to introduce the DIC in a plain PHP Project some days ago (for preparation of a full-stack migration), didn't wanted to reinvent the wheel and so I have introduced the Kernel there too. However I had to introduce Bundles there too, which was completely useless in this case.

@igorw
Copy link
Contributor

igorw commented Oct 30, 2013

TerminableInterface.php (to be merge with HttpKernelInterface in 3.0)

👎 on this, it would force every user to implement an extra method. From a middleware perspective, almost none of them need terminate() and this would just be an inconvenience for that use case imo.

What I would like to see however, is having the HttpKernelInterface (and TerminableInterface) in a separate package than HttpKernel. This could mean moving it to HttpFoundation. Or it could mean making a separate package containing only the interface.

@webmozart
Copy link
Contributor

What I would like to see however, is having the HttpKernelInterface (and TerminableInterface) in a separate package than HttpKernel. This could mean moving it to HttpFoundation. Or it could mean making a separate package containing only the interface.

ref #6129

@Tobion
Copy link
Contributor

Tobion commented Oct 30, 2013

I think the naming Kernel and HttpKernel is not optimal. Given the naming one would think that the HttpKernel is a specialized Kernel. But in fact, it is the other way round: KernelInterface extends HttpKernelInterface.
So Kernel is more an AppKernel or something.

@simensen
Copy link
Contributor

Would Kernel still depend on HttpKernel or would it be the other way around?

My hopes for Kernel aren't so much that they are just in different packages (though that is a great first step!) but that one can build an application built around Kernel that doesn't have anything at all to do with HTTP (and thus doesn't require getting HttpKernel and its dependencies).

@fabpot
Copy link
Member Author

fabpot commented Oct 30, 2013

In this proposal, Kernel would depend on HttpKernel.

@stof
Copy link
Member

stof commented Oct 30, 2013

regarding your separation of directories, Log should probably not go into HttpKernel in 3.0 but in Profiler as it only contains the deprecated interfaces (gone in 3.0 in favor of PSR-3) and the DebugLoggerInterface (used by the LoggerDaraCollector)

@fabpot
Copy link
Member Author

fabpot commented Oct 30, 2013

@stof I thought about that as well but DebugLoggerInterface is used by other classes. That said, if we can remove this interface altogether and move the data collector to be a proper logger, that would be even better.

@simensen
Copy link
Contributor

In this proposal, Kernel would depend on HttpKernel.

I see.

If I wanted to follow up on either reversing the dependency (HttpKernel depends on Kernel) or finding another way to make sure that Kernel can be used w/o Http*, what would be the best way to go about doing that? Here? Create a new issue?

I think the Kernel is great! I just don't like the fact that it is so tied into Http*. It is very usable outside of the context of HTTP and I think more people would be willing to use it to build their applications if they weren't tied into HTTP.

@stof
Copy link
Member

stof commented Oct 30, 2013

@simensen HttpKenrel has no reason to depend on Kernel. There is no need for bundles and their DIC building to use the HttpKernelInterface (see Silex or StackPhp for examples not relying on Kernel)

@fabpot
Copy link
Member Author

fabpot commented Oct 30, 2013

@simensen I know where you're coming from as you submitted a PR on this exact topic. As explained by @stof, HttpKernel will never depend on Kernel. So, the only option we might discuss is having a Kernel component that optionally depends on HttpKernel.

@webmozart
Copy link
Contributor

I agree with @Tobion that it's confusing that Kernel would depend on HttpKernel. This is a naming issue that we can certainly solve.

@docteurklein
Copy link
Contributor

Moreover, the link between Kernel and HttpKernel is nothing but delegating handle() and terminate() calls. We could easilly separate them and compose with these 2 independant pieces.

@simensen
Copy link
Contributor

HttpKenrel has no reason to depend on Kernel. There is no need for bundles and their DIC building to use the HttpKernelInterface (see Silex or StackPhp for examples not relying on Kernel)

@stof Yes, I'm familiar with both Silex and Stack. I'm also very familiar with the downsides of having to get all of the HttpKernel dependencies even if you don't need them. This proposal fixes the problem for HTTP frameworks that don't want Symfony's kernel, but it does nothing for applications that want to use Symfony's kernel but don't want anything related to HTTP.

And really, Silex and Stack wouldn't need to depend on anything kernel related (HTTP or otherwise) if we consider something like @igorw suggested by moving HttpKernelInterface out of symfony/http-kernel entirely. For example, assume we move HttpKernelInterface (and TerminableInterface) to symfony/http-foundation and renamed it HttpInterface:

<?php

namespace Symfony\Component\HttpFoundation;

interface HttpInterface
{
    const MASTER_REQUEST = 1;
    const SUB_REQUEST = 2;

    public function handle(Request $request, $type = self::MASTER_REQUEST, $catch = true);
}

Then the likes of Silex, Stack, Laravel, Drupal, etc would only need to require symfony/http-foundation to be de facto compatible with Symfony 3. There would be no need to require any of the kernel related packages.


As @Tobion and @bschussek mentioned, I think a lot of the awkwardness might come down to naming. By reading their names, HttpKernel definitely looks to extend Kernel and not the other way around. Having actually read the code, I see that this is not the case. However, I also don't see why this must be the case forever just because that is how it is now?

The biggest reason I can see (currently) is that everyone who wants to be Symfony compatible MUST implement HttpKernelInterface. If we can get around that (either by moving and potentially renaming HttpKernelInterface or by doing something like what was proposed in #6129) then I think it may actually become possible for invert the dependencies or find a way to handle the relationship with composition like @docteurklein suggested.


I know where you're coming from as you submitted a PR on this exact topic. [...] So, the only option we might discuss is having a Kernel component that optionally depends on HttpKernel.

@fabpot Yes. :) I have held off discussing it because I felt that overall people were not interested in making changes like this in Symfony 2 as it would have most likely led to considerable BC breaks. I had hoped that it could be discussed more fully in the context of Symfony 3, though.

If we can really only discuss doing this in the way you propose (Kernel only optionally depending on HttpKernel) I'll focus my energy on that. If there is any chance at all that you'd reconsider, though, please let me know. ;)

@stof
Copy link
Member

stof commented Oct 30, 2013

@simensen Silex depends of all stuff that @fabpot suggested moving into the new HttpKernel after the split, not only on the interface.

and HttpKernelInterface has nothing to do in HttpFoundation IMO. It is not the same feature scope (HttpKernelInterface represents the transformation while HttpFoundation abstracts the input and output)

@Crell
Copy link
Contributor

Crell commented Oct 31, 2013

I fully support breaking up the HttpKernel package for Symfony 3, for all the reasons given.

The point of fully divorcing KernelInterface and HttpKernelInterface is to let people use KernelInterface, bundles, et al for something like Cilex, or some other CLI-only application. If it's a CLI-only application (yes I've written those in PHP, although in my pre-Symfony days) then there's no reason to carry around an HttpKernelInterface dependency, and the entire HttpKernel package. I just want the "AppKernel" capabilities, no HTTP at all.

Beau's point about moving HttpKernelInterface to HttpFoundation actually sits very well with me. IMO, one of the most important parts of Symfony, and its greatest contribution to the PHP realm, is the Request/Response/HttpKernelInterface model. The default implementation is good (Drupal's using it), but that main pipeline is the key. That's a pipeline that should be as easy as humanly possible to leverage and share between projects, with as little overhead as possible. Pulling in the rest of the HttpKernel package for that is overkill. Whether HttpFoundation per se is the ideal place is debatable, but having as lightweight and easily-reusable Request/Response/HttpKernelInterface set as possible is good for PHP as a whole.

@bamarni
Copy link
Contributor

bamarni commented Oct 31, 2013

Very interesting topic, I also think HttpKernelInterface shouldn't be implemented by the Kernel itself, but by a new wrapping class.

Currently its implementation in the Kernel needs a service from the container (http_kernel), which is defined in FrameworkBundle, so the component blindly relies on the fact that a given bundle is registered.

Maybe there should be a dedicated class in FrameworkBundle, the same way that we have HttpCache. Even though its code would be pretty straigthforward, it has the exact same goal than HttpCache : transforming a request to a response.

@evillemez
Copy link

I think @simensen, @frastel & @Crell are all on the right track - and part of the problem is how we've named things. In 2.x Bundles are tied to the concept of a "kernel", which is tied to HTTP. But, really, Bundles are nothing but a mechanism for configuring the DIC, which doesn't really have anything to do with HTTP.

Rather than calling it "Kernel" in 3.x, why not simply call it "Application"? An application component could provide a ContainerAwareWebApplication, which implements HttpKernelInterface by wrapping an instance of HttpKernel, and a ContainerAwareConsoleApplication which does the same from the CLI side. Or not - FrameworkBundle could provide those implementations, or even another component.

One big benefit to this, would be an ApplicationInterface, which just defines the "boot" and "shutdown", and other purely DIC related things. All container loading stuff would go there - it would also provide a way to remove BundleInterface::boot/shutdown. Rather, bundles could register listeners for application.boot and application.shutdown events, the same way they currently do for the kernel.* events. That would be a nice feature, because bundles wouldn't have to be loaded on every HTTP request anymore - they'd only need to load for the purposes of actually building/dumping the container, which is all they really do anyway.

This (or something like it) would clearly separate web stuff vs cli stuff, but expose the same Bundle/DIC functionality equally for both.

@realityking
Copy link
Contributor

Minor point on this topic: I think RegisterListenersPass should be moved to the EventDispatcher component, I recently added HttpKernel to a project just for this one class.

Also the Cache(Warmer/Clearer) classes and interfaces could be useful outside the Kernel but I'm not sure there is a better place for them.

@immutef
Copy link

immutef commented Nov 3, 2013

I'd like to see the bundle subsystem extracted into a component, too.

@Peter-Rene-Menges
Copy link

Take a look at this suggestions: #6082, #6082 (comment)
this issue #9457 that I came up with when discussing the process of automatically installation and registration of bundles.

@o
Copy link
Contributor

o commented Nov 8, 2013

This is great. I implemented my own solution top of HttpKernel and i don't need Bundle functionality, but current component context makes me confused.

👍

@fabpot
Copy link
Member Author

fabpot commented Mar 4, 2014

If we want to make these changes in 3.0, we should start now.

Here is the plan to make upgrading as easy as possible:

  • in 2.5: we move things around but keep the old classes as aliases for new ones (those alias classes are deprecated in 2.5 as well);
  • in 3.0: we remove the alias classes.

This means that upgrading Symfony core can be done now, and people relying on the current architecture will have plenty of time to migrate to the new structure. That way, upgrading to 3.0 won't be a problem.

I've started this move with the easiest part, the HTTP profiler; have a look at PR #10374.

I'm going to send pull requests for other things soon and try to get them in 2.5 as well.

@adamelso
Copy link
Contributor

adamelso commented Mar 5, 2014

As others have mentioned already, I favor the idea of a Kernel that's agnostic of its context.

I once created a command line application using Symfony components but unfortunately I couldn't use the concept of the Kernel, as the ones in Symfony are tied to the context of HTTP.

Instead I simply extended Symfony\Component\Console\Application, made it ContainerAware, and built the Application to act as the Kernel. I could have created my own Kernel class, but if Symfony provides a non-HTTP one, that would be ideal.

@pyrech
Copy link
Contributor

pyrech commented May 31, 2014

I was thinking about a way to have a Kernel (or whatever its name - like AppKernel) "agnostic of its context" (http or cli), inspirated by how Console Application - from FrameworkBundle - works:

  • KernelInterface (or whatever its name - like AppKernelInterface) should no longer extend HttpKernelInterface
  • then no longer handle() or terminate(), nor getHttpKernel() methods in Kernel (or whatever its name - like AppKernel)
  • Create a Symfony\Bundle\FrameworkBundle\HttpKernel\Application which extends HttpKernel. This new class should encapsulate a Kernel instance - as the Symfony\Bundle\FrameworkBundle\Console\Application does - and boot this instance when needed.

To resume, we finish with:

  • A Console component, which is already completely independent
  • A HttpKernel component which only depends on HttpFoundation and deals with Cache, Controller, etc
  • A Kernel (or AppKernel) which handles Bundle, Config, DependencyInjection, etc, and does not depend on HttpKernel nor on Console components
  • A FrameworkBundle with two kinds of Application (one for Console, one for HttpKernel) which both encapsulate a Kernel (or AppKernel) instance and use it internally.

What do you think about this proposal? Please correct me if I'm missing something.

PS: As a nice side-effect, http and cli start to look similar, like these two bootstraps show it:

cli - bin/console

use Symfony\Bundle\FrameworkBundle\Console\Application;
use Symfony\Component\Console\Input\ArgvInput;

$input = new ArgvInput();

// Instantiate the kernel
$kernel = new MyAppKernel('prod', false);

// Compose the Application with the Kernel instance
$application = new Application($kernel);

// Run!
$application->run($input);

http - web/app.php

use Symfony\Bundle\FrameworkBundle\HttpKernel\Application;
use Symfony\Component\HttpFoundation\Request;

// Instantiate the kernel
$kernel = new MyAppKernel('prod', false);
$kernel->loadClassCache();

// Compose the Application with the Kernel instance (eventually decorate the Application with an HttpCache)
$application = new Application($kernel);
//$application = new MyAppCache($application);

// Run!
$request = Request::createFromGlobals();
$response = $application->handle($request);
$response->send();
$application->terminate($request, $response);

@ghost
Copy link

ghost commented Aug 4, 2014

Can't we suggest HttpKernelInterace to php-fig ? seems possible now that PSR-7 is being discussed?

@Crell
Copy link
Contributor

Crell commented Aug 4, 2014

Once PSR-7 is approved I fully support FIG adopting an HttpKernelInterface-esque PSR to follow it.

@mickaelandrieu
Copy link
Contributor

👍
but maybe can we move all Bundle logic to the related FrameworkBundle as this is (should be ?) totaly Symfony2 specific ? What is the interest of a Bundle component outside the framework ?

edit: aww how to manage FrameworkBundle without Bundle registering :/ (inception)

@pyrech
Copy link
Contributor

pyrech commented Nov 11, 2014

Does someone has any opinion on my proposal - 3 comments above - about decoupling kernel from http? :)

@mickaelandrieu
Copy link
Contributor

@pyrech agree in case of console applications, but this represents an important BC.

What are the pros against the cons ?

@pyrech
Copy link
Contributor

pyrech commented Nov 12, 2014

Here the pros I see:

  • allowing people building cli applications to not require the http stuff (HttpFoundation component, HttpKernelInterface, Controller, etc) just to use the bundle ecosystem
  • better separation of concerns: currently the only coupling between HttpKernel and Kernel is that Kernel delegates handle and terminate calls to HttpKernel. IMO it could easily be moved to FrameworkBundle to reduce coupling

cons:

  • The AppKernel (Probably to rename, something like App?) would now extends a FrameworkBundle\HttpKernel\Application instead of a HttpKernel\Kernel
  • Project implementing HttpKernelInterface would probably need to update their Kernel to remove the call to handle and terminate method

Did I miss something else?

@simensen
Copy link
Contributor

Are there more details about what is currently planned for Symfony 3 in how HttpKernel is going to be split out? From the roadmap post it hints that it will be split but it isn't explained in detail:

... (the HttpKernel is going to be split into several smaller ones -- the profiler for instance will be standalone; classes are moved from bundles to components; components are extracted from existing ones, ...)

I am still hoping that someday soon we'll be able to create Symfony Kernel applications that do not need to have any HTTP baggage implied.

If this is the direction Symfony 3 is already heading, awesome! I'd like to learn more. Is there code or ideas written down somewhere that I can take a look? I'm very much hoping that I can remove the HTTP baggage in the next major release of Sculpin.

If this isn't the direction that Symfony 3 is currently going, is it up for discussion? If not, I'll leave it be and deal with whatever. If it is still up for discussion, how can I get involved in that discussion? I think it would be better to get that ball rolling sooner rather than later.


I've let this sit for awhile as I knew I couldn't do anything in 2.x and so Sculpin was stuck with what is there currently. However, with the Symfony 3 on the horizon, I'm going to need to be updating Sculpin to use Symfony 3. Nothing would make me more happy than to be able to base Sculpin 3 around a Symfony Kernel that was just a Kernel and had nothing to do with HTTP at all.

Regardless of how this ultimately gets split out, I'd like to have an idea how I can prepare for working with Symfony 3's new Kernel-related components. Any and all information on the current direction would be greatly appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
HttpKernel RFC RFC = Request For Comments (proposals about features that you want to be discussed)
Projects
None yet
Development

No branches or pull requests