[feature/controller] Controller

[imkingdavid]

http://tracker.phpbb.com/browse/PHPBB3-10864

[imkingdavid]

I know we wanted to use dependency injection instead of globals, but I'm not sure what that would look like here.

[EXreaction]

public function __construct(phpbb_user $user)
{
$this->user = $user;
}

[igorw]

In that case having a base class makes no sense. Each controller should assign its own dependencies.

[igorw]

Lots of the logic in our kernel could possibly be replaced with existing symfony classes. Take a look at YOLO for an example of how the HttpKernel, EventDispatcher, RouterListener and RouteCollection work together. And we could possibly use the YamlFileLoader from the routing component.

[imkingdavid]

@igorw I am using HttpKernel, RouteCollection and YamlFileLoader already. I didn't understand the ways it worked with the EventDispatcher, so I didn't do that, but I could study it some more to try to understand it and then integrate it. However, what I have right now works fine.

[igorw]

Needs an exception message explaining the problem.

EDIT: Or since this is the base controller we can either: add a construtor which injects the template or inject the container (the latter is what sf2 does).

[imkingdavid]

@igorw Any comments? I have implemented HttpKernel, EventDispatcher, RouterListener and RouteCollection, among others, and it all works without errors.

There are probably ways to make the current implementation better, but what I have currently works as intended, as far as I can tell.

While it is not a blocker for 3.1, I would like to get this merged so that people don't get used to the front-controller system that is currently in the core and then merge this later so they have to change yet again.

[igorw]

We should probably also call $kernel->finish($request, $response);. Perhaps we can even move exit_handler() to a listener for the kernel.finish event.

[p]

Read the entire diff. Impressive. Not seeing any obvious deficiencies but it is now more symfony than phpbb, and some java/enterprisiness is migrating over this way as well (new GetResponseForControllerResultEvent, yay!). If it works you have my support for merging it.

[p]

Please fix tests first obviously.

[imkingdavid]

This will need to be changed because the kernel no longer exists as a service because we are now using the HttpKernel Symfony component instead of our own kernel class.

[imkingdavid]

@igorw is it possible to specify a Symfony component as a service? If so, that would make this much easier; otherwise, I have to somehow get the HttpKernel object injected for this method.

[imkingdavid]

@p thanks, I'll be working more on this today to clean up some stuff I missed last night, so hopefully it can be merged soon.

[imkingdavid]

@naderman @igorw We need to discuss how to implement the listener for this as well as for the exception handler.

[p]

It should work. Check other tests.

[igorw]

What oleg said. This should not be a problem.

[imkingdavid]

Replaced by #1016