Skip to content

[DX] ADR usage #8153

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

Closed
wants to merge 29 commits into from
Closed
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
c8050b1
Adding the ADR approach.
Guikingone Jul 14, 2017
0d0d4cc
Update service.rst
Guikingone Jul 14, 2017
7359837
Update service.rst
Guikingone Jul 14, 2017
885e696
Update service.rst
Guikingone Jul 14, 2017
220021e
Update service.rst
Guikingone Jul 14, 2017
1fda263
Update service.rst
Guikingone Jul 14, 2017
11c77c7
Update service.rst
Guikingone Jul 14, 2017
d21feb2
Update service.rst
Guikingone Jul 15, 2017
a449220
Update service.rst
Guikingone Jul 17, 2017
6d1ed62
Update service.rst
Guikingone Jul 17, 2017
a81b625
Update service.rst
Guikingone Aug 3, 2017
8444e98
Added the dedicated file and link from service.rst
Guikingone Aug 3, 2017
a93fa6c
Update adr.rst
Guikingone Aug 3, 2017
97bcbac
Update adr.rst
Guikingone Aug 3, 2017
72c2c74
Update service.rst
Guikingone Aug 3, 2017
666a89f
Update adr.rst
Guikingone Aug 3, 2017
c8625fb
Update adr.rst
Guikingone Aug 5, 2017
0d2022a
Corrected the index typo.
Guikingone Aug 5, 2017
582f4bf
Update adr.rst
Guikingone Aug 5, 2017
a237312
Update adr.rst
Guikingone Aug 6, 2017
cd46501
Update adr.rst
Guikingone Aug 16, 2017
24416f5
Update adr.rst
Guikingone Sep 12, 2017
dc6ddc6
Update adr.rst
Guikingone Oct 16, 2017
f1aad20
Update adr.rst
Guikingone Oct 16, 2017
16c8472
Update adr.rst
Guikingone Oct 16, 2017
eca9a0a
[ADD](Responder classe)[!P2]
Guikingone Nov 24, 2017
aa0a207
[FIX](Syntax)[!P2]
Guikingone Nov 24, 2017
551aed6
[FIX](ADR Repository)[!P2]
Guikingone Nov 24, 2017
3dacc68
fix(typo): fix on typo and space.
Guikingone Jan 24, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
187 changes: 187 additions & 0 deletions controller/adr.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,187 @@
.. index::
single: Action Domain Responder approach

How to implement the ADR pattern
================================

In Symfony, you're used to implement the MVC pattern and extending the default 'Controller'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'Controller' =>

:class:`Symfony\\Bundle\\FrameworkBundle\\Controller\\Controller`

class, since the 3.3 update, Symfony is capable of using natively the ADR approach.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would put a dot before "since".


Updating your internal logic
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Updating your configuration"?

----------------------------

As you saw from earlier example, you must update the services.yml file in order to
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"As you saw from earlier example" does not mean anything is this context what introducing by:

You just need to update `the services.yml` file in order to auto configure your actions::

(Notice there is no space before the :: .

use the latest features of the DependencyInjection component, this way, here's the updates ::

# ...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be removed?


services:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to add xml and PHP versions too.

_defaults:
autowire: true
autoconfigure: true
public: false

# Allow to load every actions
AppBundle\Action\:
resource: '../../src/AppBundle/Action/'
public: true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This behavior has been modified for 3.3, now all the services tagged with controller.service_arguments are public by definition and according to #8415 we can remove it so.


Once the file is updated, delete your Controller folder and create an Action class using the ADR principles, i.e ::

<?php

namespace AppBundle\Action;

use Twig\Environment;
use Symfony\Component\Templating\EngineInterface;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This use statement is no longer needed.


final class HelloAction
{
private $renderer;

public function __construct(Environment $render)
{
$this->render = $render;
}
Copy link
Member

@yceruto yceruto Sep 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking again about this approach, if we have just one method/controller, shouldn't be simpler inject the required dependencies into __invoke() directly? Thus, we are avoiding define one property per service argument, if so the action service needs to be tagged with controller.service_arguments.

After

final class HelloAction
{
    public function __invoke(Environment $twig): Response
    {
         return new Response($twig->render('default/index.html.twig'));
    }
}


public function __invoke()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should add a return type hint.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

: Response

{
return new Response($this->render->render('default/index.html.twig'));
}
}

.. tip::

As described in the DependencyInjection doc, you must use the __construct() injection
approach, this way, your class is easier to update and keep in sync with the framework internal
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"the framework" => "any framework"

services.

By default, we define the class with the final keyword because this class shouldn't been extended,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"been" => "be"

the logic is pretty simple to understand as you understand the ADR pattern, in fact, the 'Action'
is linked to a single request and his dependencies are linked to this precise Action.

.. tip::

By using the final approach and the private visibility (inside the container), our class
is faster to return and easier to keep out of the framework logic.


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra blank line.

Once this is done, you can define the routes like before using multiples approach :
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In English there is no space before a colon :.


.. configuration-block::

.. code-block:: php-annotations

# src/AppBundle/Action/HelloAction.php
// ...

/**
* @Route("/hello", name="hello")
*/
final class HelloAction
{
// ...
}

.. code-block:: yaml

# app/config/routing.yml
hello:
path: /hello
defaults: { _controller: AppBundle\Action\HelloAction }

.. code-block:: xml

<!-- app/config/routing.xml -->
<?xml version="1.0" encoding="UTF-8" ?>
<routes xmlns="http://symfony.com/schema/routing"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://symfony.com/schema/routing
http://symfony.com/schema/routing/routing-1.0.xsd">

<route id="hello" path="/hello">
<default key="_controller">AppBundle\Action\HelloAction</default>
</route>

</routes>

.. code-block:: php

// app/config/routing.php
$collection->add('hello', new Route('/hello', array(
'_controller' => 'HelloAction::class',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the quotes and add a use statement.

)));

Accessing the request
---------------------

As you can imagine, as the logic evolve, your class isn't capable of accessing
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"isn't" => "is"

the request from simple method injection like this ::

<?php

// ...

public function __invoke(Request $request)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also add a use statement for the Request in this example.

{
return new Response($this->render->render('default/index.html.twig'));
}
}

Like you can easily imagine, the container is the best option to gain access to ths request,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"container" =>

:class:`Symfony\\Component\\HttpFoundation\\RequestStack`

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"container" is confusing here and should be

:class:`Symfony\\Component\Httpfoundation\RequestStack`

typo "ths" => "the".

using this approach, a simple update is recommended ::

<?php

namespace AppBundle\Action;

use Twig\Environment;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\RequestStack;
use Symfony\Component\Templating\EngineInterface;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be removed.


final class HelloAction
{
private $requestStack;

private $renderer;

public function __construct(RequestStack $requestStack, Environment $render)
{
$this->requestStack = $requestStack
$this->render = $render;
}

public function __invoke(Request $request)
{
return new Response($this->render->render('default/index.html.twig'));
}
}

This way, you can easily access to parameters ::

<?php

// ...

public function __construct(RequestStack $requestStack, Environment $render)
{
$this->requestStack = $requestStack
$this->render = $render;
}

public function __invoke(Request $request)
{
$data = $this->requestStack->getCurrentRequest()->get('id');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$data is not used, you should at least pass it to the view.


return new Response($this->render->render('default/index.html.twig'));
}
}

Final though
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought?

------------
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing a dash.


Keep in mind that this approach can be completely different from what you're used to use, in order to
keep your code clean and easy to maintain, we recommend to use this approach only if your code is
decoupled from the internal framework logic (like with Clean Architecture approach) or if you start a new
project and need to keep the logic linked to your business rules.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this last paragraph and the supposed benefits. I don't see how this approach or what you refer to the old one is cleaner or easier. I don't understand why this approach is more decoupled than the other one. I'm not saying this is good or bad, just that from a beginner's perspective, the benefits are not very clear.

5 changes: 5 additions & 0 deletions controller/service.rst
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,11 @@ If your controller implements the ``__invoke()`` method - popular with the
Action-Domain-Response (ADR) pattern, you can simply refer to the service id
(``AppBundle\Controller\HelloController`` or ``app.hello_controller`` for example).

As this approach is evolving faster and can be easily transposed into Symfony, we recommend
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is it evolving?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this stage, not so much, like before, there's was an error in the real sense of my words, ADR still young and the "logic" keep evolving with time and usage by the developers, the true meaning was to say that using ADR still a "WIP" in Symfony and as the "logic" of ADR still evolving by the feedbacks of the developers, his implementation inside Symfony can be modified with future evolution of the framework or pattern.

In fact, it was possible since Symfony 3.1 when recompiling the container (or like @dunglas do it, by injecting the classes as controllers and mapping the return of each one of them) but using it in production is way easier since 3.3 and 3.4 (and even easier in 4.0)

you to read the following part :
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No space before colon.


* :doc:`/controller/adr`

Alternatives to base Controller Methods
---------------------------------------

Expand Down