{"action":"created","issue":{"url":"https://api.github.com/repos/silverstripe/silverstripe-framework/issues/6681","repository_url":"https://api.github.com/repos/silverstripe/silverstripe-framework","labels_url":"https://api.github.com/repos/silverstripe/silverstripe-framework/issues/6681/labels{/name}","comments_url":"https://api.github.com/repos/silverstripe/silverstripe-framework/issues/6681/comments","events_url":"https://api.github.com/repos/silverstripe/silverstripe-framework/issues/6681/events","html_url":"https://github.com/silverstripe/silverstripe-framework/issues/6681","id":212270917,"number":6681,"title":"RFC: App object to encapsulate global state and services","user":{"login":"chillu","id":111025,"avatar_url":"https://avatars0.githubusercontent.com/u/111025?v=3","gravatar_id":"","url":"https://api.github.com/users/chillu","html_url":"https://github.com/chillu","followers_url":"https://api.github.com/users/chillu/followers","following_url":"https://api.github.com/users/chillu/following{/other_user}","gists_url":"https://api.github.com/users/chillu/gists{/gist_id}","starred_url":"https://api.github.com/users/chillu/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/chillu/subscriptions","organizations_url":"https://api.github.com/users/chillu/orgs","repos_url":"https://api.github.com/users/chillu/repos","events_url":"https://api.github.com/users/chillu/events{/privacy}","received_events_url":"https://api.github.com/users/chillu/received_events","type":"User","site_admin":false},"labels":[{"id":386316041,"url":"https://api.github.com/repos/silverstripe/silverstripe-framework/labels/affects/v4","name":"affects/v4","color":"5319e7","default":false},{"id":86042174,"url":"https://api.github.com/repos/silverstripe/silverstripe-framework/labels/change/major","name":"change/major","color":"e99695","default":false},{"id":182941401,"url":"https://api.github.com/repos/silverstripe/silverstripe-framework/labels/effort/hard","name":"effort/hard","color":"f9d0c4","default":false},{"id":182941354,"url":"https://api.github.com/repos/silverstripe/silverstripe-framework/labels/impact/high","name":"impact/high","color":"eb6420","default":false},{"id":34209120,"url":"https://api.github.com/repos/silverstripe/silverstripe-framework/labels/type/enhancement","name":"type/enhancement","color":"0e8a16","default":false}],"state":"open","locked":false,"assignee":{"login":"chillu","id":111025,"avatar_url":"https://avatars0.githubusercontent.com/u/111025?v=3","gravatar_id":"","url":"https://api.github.com/users/chillu","html_url":"https://github.com/chillu","followers_url":"https://api.github.com/users/chillu/followers","following_url":"https://api.github.com/users/chillu/following{/other_user}","gists_url":"https://api.github.com/users/chillu/gists{/gist_id}","starred_url":"https://api.github.com/users/chillu/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/chillu/subscriptions","organizations_url":"https://api.github.com/users/chillu/orgs","repos_url":"https://api.github.com/users/chillu/repos","events_url":"https://api.github.com/users/chillu/events{/privacy}","received_events_url":"https://api.github.com/users/chillu/received_events","type":"User","site_admin":false},"assignees":[{"login":"chillu","id":111025,"avatar_url":"https://avatars0.githubusercontent.com/u/111025?v=3","gravatar_id":"","url":"https://api.github.com/users/chillu","html_url":"https://github.com/chillu","followers_url":"https://api.github.com/users/chillu/followers","following_url":"https://api.github.com/users/chillu/following{/other_user}","gists_url":"https://api.github.com/users/chillu/gists{/gist_id}","starred_url":"https://api.github.com/users/chillu/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/chillu/subscriptions","organizations_url":"https://api.github.com/users/chillu/orgs","repos_url":"https://api.github.com/users/chillu/repos","events_url":"https://api.github.com/users/chillu/events{/privacy}","received_events_url":"https://api.github.com/users/chillu/received_events","type":"User","site_admin":false},{"login":"tractorcow","id":936064,"avatar_url":"https://avatars1.githubusercontent.com/u/936064?v=3","gravatar_id":"","url":"https://api.github.com/users/tractorcow","html_url":"https://github.com/tractorcow","followers_url":"https://api.github.com/users/tractorcow/followers","following_url":"https://api.github.com/users/tractorcow/following{/other_user}","gists_url":"https://api.github.com/users/tractorcow/gists{/gist_id}","starred_url":"https://api.github.com/users/tractorcow/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tractorcow/subscriptions","organizations_url":"https://api.github.com/users/tractorcow/orgs","repos_url":"https://api.github.com/users/tractorcow/repos","events_url":"https://api.github.com/users/tractorcow/events{/privacy}","received_events_url":"https://api.github.com/users/tractorcow/received_events","type":"User","site_admin":false}],"milestone":{"url":"https://api.github.com/repos/silverstripe/silverstripe-framework/milestones/38","html_url":"https://github.com/silverstripe/silverstripe-framework/milestone/38","labels_url":"https://api.github.com/repos/silverstripe/silverstripe-framework/milestones/38/labels","id":1858534,"number":38,"title":"Recipe 4.0.0-beta1","description":"Feature-complete release of SilverStripe 4.","creator":{"login":"sminnee","id":59968,"avatar_url":"https://avatars2.githubusercontent.com/u/59968?v=3","gravatar_id":"","url":"https://api.github.com/users/sminnee","html_url":"https://github.com/sminnee","followers_url":"https://api.github.com/users/sminnee/followers","following_url":"https://api.github.com/users/sminnee/following{/other_user}","gists_url":"https://api.github.com/users/sminnee/gists{/gist_id}","starred_url":"https://api.github.com/users/sminnee/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sminnee/subscriptions","organizations_url":"https://api.github.com/users/sminnee/orgs","repos_url":"https://api.github.com/users/sminnee/repos","events_url":"https://api.github.com/users/sminnee/events{/privacy}","received_events_url":"https://api.github.com/users/sminnee/received_events","type":"User","site_admin":false},"open_issues":30,"closed_issues":46,"state":"open","created_at":"2016-06-30T22:49:57Z","updated_at":"2017-05-24T00:43:33Z","due_on":"2017-06-30T07:00:00Z","closed_at":null},"comments":31,"created_at":"2017-03-06T22:31:43Z","updated_at":"2017-05-24T04:21:29Z","closed_at":null,"body":"## Acceptance Criteria\r\n\r\n * There is clear documentation on how to instantiate SS core classes and their dependencies (when to use `new` vs. dependency injection)\r\n * There is a way to replace dependencies during early bootstrap (e.g. caching layers initialised before injector kicks in)\r\n * Interoperability with other libraries is maintained by minimizing use of SS-specific references like `Injector` and `Config` in core classes\r\n * Configuration can easily be mocked directly on core class instances (without relying on `Config::nest()` global state)\r\n * It is clear which dependencies a class relies on without reading the entire class implementation\r\n * There are no directly overlapping APIs (`DataModel` method arguments)\r\n * The concept is applied to all core modules\r\n * The upgrade path is clearly documented\r\n\r\n## Excludes\r\n\r\n * Converting static methods on core classes to instance methods\r\n * Converting core classes to interfaces\r\n * Global variable state (e.g. $_SESSION, $_REQUEST) for initial implementation.\r\n\r\n## Notes\r\n\r\n * @sminnee has started a draft RFC for this: https://gist.github.com/sminnee/7fd620a026d551730a088d5e82e542f0\r\n * @tractorcow has posted a follow up RFC alternative:\r\n  https://gist.github.com/tractorcow/c0656a334c13eac8ae252610d226a1c6\r\n * PSR-11 [discourages](https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-11-container-meta.md#4-recommended-usage-container-psr-and-the-service-locator) the direct use of service locators in implementations\r\n * Symfony exposes [services](http://symfony.com/doc/current/service_container.html) service locators (e.g. `$this->app->get('my-dependency')`)\r\n\r\n## Related\r\n\r\n * [Issue: Tighten up usage of dependency injection in core](https://github.com/silverstripe/silverstripe-framework/issues/1678)\r\n * [RFC: Make Injector comply with PSR-11](https://github.com/silverstripe/silverstripe-framework/issues/6594)\r\n * [PSR-11 meta](https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-11-container-meta.md)\r\n * [Issue: Injector factories should be passed the full service definition](https://github.com/silverstripe/silverstripe-framework/issues/4442)\r\n * [Use PSR-7 for HTTPRequest/HTTPResponse](https://github.com/silverstripe/silverstripe-framework/issues/6514)\r\n * [Config should be injectable](https://github.com/silverstripe/silverstripe-framework/issues/4493)\r\n * [Allow config-cache customisation via custom bootloader/app object](https://github.com/silverstripe/silverstripe-framework/issues/3940)\r\n \r\n## Tasks\r\n\r\n  - [ ] Agree on RFC and implementation goals\r\n  - [ ] TODO: Subtask implementation"},"comment":{"url":"https://api.github.com/repos/silverstripe/silverstripe-framework/issues/comments/303613729","html_url":"https://github.com/silverstripe/silverstripe-framework/issues/6681#issuecomment-303613729","issue_url":"https://api.github.com/repos/silverstripe/silverstripe-framework/issues/6681","id":303613729,"user":{"login":"chillu","id":111025,"avatar_url":"https://avatars0.githubusercontent.com/u/111025?v=3","gravatar_id":"","url":"https://api.github.com/users/chillu","html_url":"https://github.com/chillu","followers_url":"https://api.github.com/users/chillu/followers","following_url":"https://api.github.com/users/chillu/following{/other_user}","gists_url":"https://api.github.com/users/chillu/gists{/gist_id}","starred_url":"https://api.github.com/users/chillu/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/chillu/subscriptions","organizations_url":"https://api.github.com/users/chillu/orgs","repos_url":"https://api.github.com/users/chillu/repos","events_url":"https://api.github.com/users/chillu/events{/privacy}","received_events_url":"https://api.github.com/users/chillu/received_events","type":"User","site_admin":false},"created_at":"2017-05-24T04:21:29Z","updated_at":"2017-05-24T04:21:29Z","body":"I'm still trying to get my head around why we need an intermediary `App` object that's not a kernel (= doesn't handle HTTP requests), and is also not a full service locator (we've still got `Injector`). And why Symfony doesn't need these contortions.\r\n\r\nI've written up a deep dive into Symfony's kernel ([highlevel docs](http://symfony.com/doc/current/reference/configuration/kernel.html) and [working example](http://symfony.com/doc/current/components/http_kernel.html#http-kernel-working-example)), might be of benefit to others:\r\n\r\nSymfony boots via [app.php](https://github.com/symfony/symfony-demo/blob/master/web/app.php) or [app_dev.php](https://github.com/symfony/symfony-demo/blob/master/web/app_dev.php), which uses PSR autoloading to find a project specific [AppKernel](https://github.com/symfony/symfony-demo/blob/master/app/AppKernel.php) that extends the [Kernel](https://github.com/symfony/symfony/blob/3.4/src/Symfony/Component/HttpKernel/Kernel.php) component. Most of the logic is in the [Kernel](https://github.com/symfony/symfony/blob/3.4/src/Symfony/Component/HttpKernel/Kernel.php) base class. The `AppKernel` registers modules (based on `getEnvironment()`), and sets early boot config like cache and log directories. The `Kernel` also [creates](https://github.com/symfony/symfony/blob/3.4/src/Symfony/Component/HttpKernel/Kernel.php#L536) the service injection container. This calls a [buildContainer()](https://github.com/symfony/symfony/blob/3.4/src/Symfony/Component/HttpKernel/Kernel.php#L655) which looks like it would do the equivalent of our `ConfigManifest` compilation. Since this is all in a parent class of the project specific `AppKernel`, you can override `buildContainer()` in your own project for really deep customisations. The `AppKernel` has a [HTTPKernel]((https://github.com/symfony/symfony/blob/3.4/src/Symfony/Component/HttpKernel/HttpKernel.php)) dependency through the service injection container, which takes care of the actual request handling. `app.php` calls `$kernel->handle($request)`, which comes with a default [ControllerResolver](https://github.com/symfony/symfony/blob/3.4/src/Symfony/Component/HttpKernel/Controller/ControllerResolver.php) (the equivalent of our `Director::handleRequest()`).\r\n\r\nSo `Kernel` the closest equivalent to our `App` object, but without any setters or aliases, just a `getContainer()` method. From what I can tell, it doesn't get passed around deeper into the execution, but you can retrieve it via `$this->container->get('http_kernel')`.\r\n\r\nDifferences of Kernel to the proposed `App` implementation:\r\n * No setters. \r\n * Doesn't know about sessions\r\n * Doens't assume you need a database connection (presumably that dependency is on the controller logic)\r\n * Can't be nested (or statically track different instances of itself)\r\n * Doesn't have service getter shortcuts\r\n\r\nI think we should strip down the proposed API to it's essentials. This acceptance criteria is the at the core of it: \"There is a way to replace dependencies during early bootstrap (e.g. caching layers initialised before injector kicks in)\". The value in this card isn't in creating a slightly less hacky global state repo, but rather a stable API that allows devs to customise the bootstrap without copying hundreds of lines from `main.php` and `Core.php`.\r\n\r\nI think this can be achieved by implementing something similar to the [KernelInterface](https://github.com/symfony/symfony/blob/3.4/src/Symfony/Component/HttpKernel/KernelInterface.php) with a project specific [AppKernel](https://github.com/symfony/symfony/blob/3.4/src/Symfony/Component/HttpKernel/Kernel.php#L655). Since `AppKernel` can project specific, you can register custom services in a well structured fashion (accessing only instance state) through `boot()` (and utilities like `initializeContainer()`). If you consider that scope creep for this card, then maybe we shouldn't build the `App` object - because it sounds like a temporary API to me: It should really be a `Kernel` object which can handle requests.\r\n\r\nThere's a great [tutorial series](https://knpuniversity.com/screencast/symfony-journey/httpkernel-handle) about Symfony's kernel as well - hit \"next chapter\" at the bottom for more content."}}
{"actor":{"display_login":"chillu"}}