merged branch davedevelopment/controller-services-cookbook (PR #512)
This PR was squashed before being merged into the master branch (closes #512). Commits ------- ce057326 ServiceControllerServiceProvider documentation Discussion ---------- ServiceControllerServiceProvider documentation Refs #511, I tried to change it to a PR via the API, but no dice. --------------------------------------------------------------------------- by davedevelopment at 2012-10-15T21:28:06Z Just a first draft, feedback appreciated, I'll be reading it through again tomorrow for a fresh take. --------------------------------------------------------------------------- by igorw at 2012-10-15T21:53:06Z Looks good to me. Since this is one of the common criticisms of silex from DI enthusiasts, maybe we should just support this feature out of the box. @fabpot thoughts? --------------------------------------------------------------------------- by davedevelopment at 2012-10-15T22:23:55Z @stof there you go, compromising my principles for the sake of Open Source😢 --------------------------------------------------------------------------- by igorw at 2012-10-15T22:29:58Z i-know-that-feel.gif --------------------------------------------------------------------------- by Mparaiso at 2012-10-17T17:59:04Z What's the benefit of using that technique over ControllerProviders ? some declouping can be made at the controller instanciation : like ```php $app->mount("/article",new App\Controller\ArticleController($app['article_manager'],$app['user_manager']) ); ``` --------------------------------------------------------------------------- by igorw at 2012-10-17T18:08:41Z @Mparaiso laziness: the services are only created if the controller gets called. --------------------------------------------------------------------------- by davedevelopment at 2012-10-18T22:31:13Z @Mparaiso having been doing this for a couple of weeks now, I think the another benefit is the way I've been writing the controllers. I like that my routes are decoupled from the controllers and, done right, the controllers can be completely framework agnostic. Frameworks should be an implementation detail. Previously, my feelings were that the best controllers, simply delegated to a service. Now my controllers are a service, and the routing does the delegating. --------------------------------------------------------------------------- by igorw at 2012-11-04T16:57:20Z @fabpot I would like to hear your opinion on adding controllers as services to core. --------------------------------------------------------------------------- by fabpot at 2012-11-05T07:36:46Z Oops, when writing my previous comment, I mixed two PRs in my mind. I'm in fact -1 for this. --------------------------------------------------------------------------- by davedevelopment at 2012-11-05T09:22:55Z I wouldn't push for this to be in core, I'd love it to be there for my own selfish reasons, but I understand it's not everyone's cup of tea. Let's put any finishes on the cookbook and get it merged. Perhaps in time we may see more demand for it and then we can consider getting it in the core? --------------------------------------------------------------------------- by igorw at 2012-11-05T13:10:36Z Well, then let's not lose time arguing and just merge this into the docs. --------------------------------------------------------------------------- by ChrisRiddell at 2012-11-06T10:07:21Z I am all for this cookbook, I'll be using this for my next project, having a bit more structure as a full framework without a full framework is nice. --------------------------------------------------------------------------- by fabpot at 2012-11-06T10:41:17Z There is no link between "having a bit more structure" and "using controllers as services". You can just create classes for your controllers without adding them to the service container. --------------------------------------------------------------------------- by ChrisRiddell at 2012-11-06T11:14:54Z Okay had to read the requested cookbook a number of times but I sort understand now. 1) What's the major advantage in doing this? 2) so overriding the controller resolver only gives you better looking route defining? --------------------------------------------------------------------------- by davedevelopment at 2012-11-06T11:36:17Z @ChrisRiddell it's a lot about personal preference. I prefer this as it promotes Dependency Injection over Service Location, which, without sounding like a dick, you can read about all over the web, I'd start with http://martinfowler.com/articles/injection.html#ServiceLocatorVsDependencyInjection, then try the google. --------------------------------------------------------------------------- by simensen at 2012-11-09T03:15:35Z👍 I was just asking about this tonight. I think this would be very useful to have *at least* as a cookbook entry. What reasons would there be to not add this to core? I'm curious to what sort of downsides there might be to allowing it. As a point of reference, I'd like to present one of my earliest attempts to try and figure out how I wanted to use Silex and how supporting this out of the box could have changed my code. Originally (currently) I had to duplicate the method signature for each of my controller actions: ```php <?php use Symfony\Component\HttpFoundation\Request; /** * API App * * @author Beau Simensen <beau@dflydev.com> */ class Api extends AbstractApp { const ROUTE_ROOT = 'my_api_root'; const ROUTE_AUTH_ROOT = 'my_api_auth_root'; const ROUTE_AUTH_AUTHENTICATE = 'my_api_auth_authenticate'; const ROUTE_ROUTE_ROOT = 'my_api_route_root'; const ROUTE_ROUTE_CREATE = 'my_api_route_create'; const ROUTE_ROUTE_FIND = 'my_api_route_find'; const ROUTE_ROUTE_DETAIL = 'my_api_route_detail'; const ROUTE_SERVICE_ROOT = 'my_api_service_root'; const ROUTE_SERVICE_CREATE = 'my_api_service_create'; const ROUTE_SERVICE_FIND = 'my_api_service_find'; const ROUTE_SERVICE_DETAIL = 'my_api_service_detail'; protected function configure() { parent::configure(); $app = $this; $app['my.api.controller.rootController'] = $app->share(function() use ($app) { return new Api\Controller\RootController($app); }); $app['my.api.controller.authController'] = $app->share(function() use ($app) { return new Api\Controller\AuthController($app); }); $app['my.api.controller.routeController'] = $app->share(function() use ($app) { return new Api\Controller\RouteController($app); }); $app['my.api.controller.serviceController'] = $app->share(function() use ($app) { return new Api\Controller\ServiceController($app); }); $app->get('/', function() use ($app) { return $app['my.api.controller.rootController']->rootAction(); })->bind(self::ROUTE_ROOT); $app->get('/auth', function() use ($app) { return $app['my.api.controller.authController']->rootAction(); })->bind(self::ROUTE_AUTH_ROOT); $app->post('/auth/authenticate', function(Request $request) use ($app) { return $app['my.api.controller.authController']->authenticateAction($request); })->bind(self::ROUTE_AUTH_AUTHENTICATE); $app->get('/route', function() use ($app) { return $app['my.api.controller.routeController']->rootAction(); })->bind(self::ROUTE_ROUTE_ROOT); $app->post('/route/routes', function(Request $request) use ($app) { return $app['my.api.controller.routeController']->createAction($request); })->bind(self::ROUTE_ROUTE_CREATE); $app->post('/route/find', function(Request $request) use ($app) { return $app['my.api.controller.routeController']->findAction($request); })->bind(self::ROUTE_ROUTE_FIND); $app->get('/route/routes/{routeId}', function(Request $request, $routeId) use ($app) { return $app['my.api.controller.routeController']->detailAction($request, $routeId); })->bind(self::ROUTE_ROUTE_DETAIL); $app->get('/service', function(Request $request) use ($app) { return $app['my.api.controller.serviceController']->rootAction($request); })->bind(self::ROUTE_SERVICE_ROOT); $app->post('/service/services', function(Request $request, $serviceKey) use ($app) { return $app['my.api.controller.serviceController']->createAction($request, $serviceKey); })->bind(self::ROUTE_SERVICE_CREATE); $app->post('/service/find', function(Request $request) use ($app) { return $app['my.api.controller.serviceController']->findAction($request); })->bind(self::ROUTE_SERVICE_FIND); $app->get('/service/services/{serviceKey}', function(Request $request, $serviceKey) use ($app) { return $app['my.api.controller.serviceController']->detailAction($request, $serviceKey); })->bind(self::ROUTE_SERVICE_DETAIL); } } ``` Using controllers as a service correctly, I can cut the amount of times I define my method signature in half and it looks more readable: ```php <?php use Symfony\Component\HttpFoundation\Request; /** * API App * * @author Beau Simensen <beau@dflydev.com> */ class Api extends AbstractApp { const ROUTE_ROOT = 'my_api_root'; const ROUTE_AUTH_ROOT = 'my_api_auth_root'; const ROUTE_AUTH_AUTHENTICATE = 'my_api_auth_authenticate'; const ROUTE_ROUTE_ROOT = 'my_api_route_root'; const ROUTE_ROUTE_CREATE = 'my_api_route_create'; const ROUTE_ROUTE_FIND = 'my_api_route_find'; const ROUTE_ROUTE_DETAIL = 'my_api_route_detail'; const ROUTE_SERVICE_ROOT = 'my_api_service_root'; const ROUTE_SERVICE_CREATE = 'my_api_service_create'; const ROUTE_SERVICE_FIND = 'my_api_service_find'; const ROUTE_SERVICE_DETAIL = 'my_api_service_detail'; protected function configure() { parent::configure(); $app = $this; $app['my.api.controller.rootController'] = $app->share(function() use ($app) { return new Api\Controller\RootController($app); }); $app['my.api.controller.authController'] = $app->share(function() use ($app) { return new Api\Controller\AuthController($app); }); $app['my.api.controller.routeController'] = $app->share(function() use ($app) { return new Api\Controller\RouteController($app); }); $app['my.api.controller.serviceController'] = $app->share(function() use ($app) { return new Api\Controller\ServiceController($app); }); $app->get('/', 'my.api.controller.rootController:rootAction')->bind(self::ROUTE_ROOT); $app->get('/auth', 'my.api.controller.authController:rootAction') ->bind(self::ROUTE_AUTH_ROOT); $app->post('/auth/authenticate', 'my.api.controller.authController:authenticateAction') ->bind(self::ROUTE_AUTH_AUTHENTICATE); $app->get('/route', 'my.api.controller.routeController:rootAction') ->bind(self::ROUTE_ROUTE_ROOT); $app->post('/route/routes', 'my.api.controller.routeController:createAction') ->bind(self::ROUTE_ROUTE_CREATE); $app->post('/route/find', 'my.api.controller.routeController:findAction') ->bind(self::ROUTE_ROUTE_FIND); $app->get('/route/routes/{routeId}', 'my.api.controller.routeController:detailAction') ->bind(self::ROUTE_ROUTE_DETAIL); $app->get('/service', 'my.api.controller.serviceController:rootAction') ->bind(self::ROUTE_SERVICE_ROOT); $app->post('/service/services', 'my.api.controller.serviceController:createAction') ->bind(self::ROUTE_SERVICE_CREATE); $app->post('/service/find', 'my.api.controller.serviceController:findAction') ->bind(self::ROUTE_SERVICE_FIND); $app->get('/service/services/{serviceKey}', 'my.api.controller.serviceController:detailAction') ->bind(self::ROUTE_SERVICE_DETAIL); } } ``` In any event, this is a great extension. :) Whether it ends up in core or not I'll need to add this to my toolbox. --------------------------------------------------------------------------- by spantaleev at 2012-11-09T08:58:46Z To move my "controllers" to methods on an object, I've been instantiating the controller object and registering the routes like this: $controller = new NewsController($app); $collection->get('/index', array($controller, 'indexAction'))->bind('news.index'); $collection->get('/view/{id}', array($controller, 'viewAction'))->bind('news.view'); This way, at least I don't have to define the method signatures twice and pass around the arguments manually, as @simensen originally did. It does have some overhead, because the controller object needs to be created. It also looks a bit ugly passing the callbacks like that. Seems like the "controller.service:action" method is cleaner and flexible enough to allow injecting the whole container or the individual controller dependencies. With my way of doing things (above), I can't afford to inject individual dependencies into the controller class even if I wanted to (because all controller objects in the app are always created, which will lead to a lot of unnecessary services being created as well). It would be nice to have something standard like this in core, instead of having everyone override the controller resolver or use their own ugly way around the problem. I can see how going the "class controllers" route can complicate things though - it could lead to talks about adding a ContainerAware base controller class, etc. --------------------------------------------------------------------------- by ChrisRiddell at 2012-11-09T10:12:46Z @simensen I am very interested in how you setup your project like that, If your configure is protected i am assuming you extend each AbstractApp with Silex and run the configure method. I am asking this as the way you have setup Silex by the look of things is very clean, I start playing around with using Controllers and it got messy fast. On Topic: I think something like this as core, would be as it gives you the option of using controller classes or route callbacks, But keep it simple by not going down the path of ContainerAware etc. --------------------------------------------------------------------------- by stof at 2012-11-09T23:37:47Z @ChrisRiddell I encourage you to read https://igor.io/2012/11/09/scaling-silex.html --------------------------------------------------------------------------- by simensen at 2012-12-13T12:44:38Z @fabpot What reasons do you have for being👎 on this? I'm curious as to what sort of downsides you see. I'm👍 on having this just be a part of Silex and not requiring people to use an optional decorator or copy and paste cookbook code. --------------------------------------------------------------------------- by davedevelopment at 2013-01-04T01:50:52Z @fabpot that last commit moved to a composition based solution, do you think we could get this in now? --------------------------------------------------------------------------- by igorw at 2013-01-04T01:59:56Z A very strong👍 from me. --------------------------------------------------------------------------- by ChrisRiddell at 2013-01-04T02:10:04Z👍 --------------------------------------------------------------------------- by simensen at 2013-01-04T02:28:57Z👍 I'm still curious to know what kind of downsides there are to having Silex do this natively but if it has to be a cookbook entry to get the word out I'm all for it going in as-is. :) --------------------------------------------------------------------------- by igorw at 2013-01-06T14:27:39Z @fabpot can you please re-review this? IMO it would be a valuable addition to the docs. --------------------------------------------------------------------------- by fabpot at 2013-01-07T11:00:15Z I'm back! I really like the new implementation that uses composition. Thanks @davedevelopment. After thinking about this topic a lot during my holidays, here are my new thoughts (new year, new thoughts ;)): * As the new implementation is quite straightforward, I'm +1 for moving the code to Silex core (the cookbook can just mention how to enable it then). * The cookbook should IMO emphasise one of the big benefit (already mentioned above): creating controllers that are framework agnostic. At first, it does not seem like a big deal, but when you want to publish some code that needs to be reused, it becomes crucial. I worked on a project a few days ago and I was able to create controllers that are compatible with Symfony (full-stack), Silex, and Drupal; that's a big deal! --------------------------------------------------------------------------- by davedevelopment at 2013-01-07T11:26:44Z Awesome, this has made my morning. I'll write another PR, with the decorator and some tests, then modify this PR to emphasise the major benefits (especially the framework agnostic part) and explain how to enable the decorator. --------------------------------------------------------------------------- by jmontoyaa at 2013-01-07T11:28:50Z Great news! --------------------------------------------------------------------------- by igorw at 2013-01-07T15:24:37Z @fabpot awesome, glad to hear! --------------------------------------------------------------------------- by simensen at 2013-01-07T17:17:49Z @fabpot Hurray! :)
Showing
Please register or sign in to comment