[EXPERIMENTAL][DI] Add tail injection for methods + autowiring #11

nicolas-grekas · 2017-02-06T14:36:40Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	-
License	MIT
Doc PR	-

ping @fabpot @simensen

services:
    foo:
        class: Foo
        tails:
            method1: { 1: 'bar', 2: '@bar' }

will create an inheritance proxy overriding Foo::method1, making its args 2 & 3 optional, with the configured values being used as defaults, keeping only argument 1 mandatory (if it was in the original declaration of course).

Allows Laravel like controllers (function indexAction(Request $request, PostRepository $postRepository) with $postRepository injection), but with full Symfony-style flexibility (ie you can/have to exactly choose where+what to inject).
Note that this can not be achieved via a 3.2 ArgumentResolver : only compile time can achieve the required level of configurability - not runtime, which ArgumentResolver is only about.
Works with any service really of course.
Reuses the inheritance proxy generation already in place for getter injection.

nicolas-grekas · 2017-02-06T14:39:35Z

Note that autowiring will be added later - the PR is big enough.
The way I see it requires symfony#21194
And declaration in Yaml would look like:

services:
  App\Controller\FooController:
    autowire:
      - !tail '*Action'

stof · 2017-02-06T17:19:43Z

this requires that the arguments are at the end, right ?

nicolas-grekas · 2017-02-06T17:20:30Z

yes, it throws if there are remaining required tail args.

stof · 2017-02-06T17:29:29Z

src/Symfony/Component/DependencyInjection/ContainerBuilder.php

        }
+        if ($type = method_exists($r, 'getReturnType') ? $r->getReturnType() : null) {
+            $type = ': '.($type->allowsNull() ? '?' : '').self::generateTypeHint($type, $r);


I vote for moving the building of the dumped return type outside this method. I already found the reference argument harder to read previously, but it becomes worse when nesting the resolution by reference in another method. We already return all the necessary info anyway, as this logic depends only on the reflector anyway

stof · 2017-02-07T08:58:03Z

src/Symfony/Component/DependencyInjection/ContainerBuilder.php

+            $code[] = sprintf('%s => %s', var_export($k, true), self::export($v));
+        }
+
+        return sprintf('array(%s)', implode(', ', $code));


isn't it exactly what var_export($array) does ?

with one difference: this generates a single line export, while var_export splits arrays on several lines

nicolas-grekas · 2017-02-07T10:15:58Z

PR is ready, with tests and autowiring.

nicolas-grekas · 2017-02-07T10:22:54Z

src/Symfony/Component/DependencyInjection/ChildDefinition.php

@@ -162,11 +162,21 @@ public function setDeprecated($boolean = true, $template = null)
    /**
     * {@inheritdoc}
     */
-    public function setAutowiredMethods(array $autowiredMethods)
+    public function setAutowiredCalls(array $autowiredCalls)


@dunglas @stof if you're ok with this rename, I can submit a refactoring PR first, to reduce the diff of this PR, OK for you?

looks good to me. And doing it now is still possible as the existing name is not yet released

nicolas-grekas · 2017-02-13T14:30:53Z

"partial" renamed to "tail"

stof · 2017-02-13T14:49:27Z

src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php

@@ -33,6 +33,7 @@ class AutowirePass extends AbstractRecursivePass
     * @internal
     */
    const MODE_OPTIONAL = 2;
+    const MODE_TAIL = 3;


should also be @internal

stof · 2017-02-13T14:50:06Z

@nicolas-grekas why is this PR opened on your fork rather than on the symfony repo ? this looks weird

nicolas-grekas · 2017-02-17T23:32:30Z

@stof because this is going to be even more surprising than getter injection to some :)
I'd like the idea to be confirmed first with a small core-team related group.
What do you think yourself of the usefulness of this?

…s that work on PHP5

…by regular type-hints that work on PHP5 (nicolas-grekas) This PR was merged into the 3.3-dev branch. Discussion ---------- [DI] Replace PHP7-conditional return-type checks by regular type-hints that work on PHP5 | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Commits ------- 372ff7c [DI] Replace PHP7-conditional return-type checks by regular type-hints that work on PHP5

…sertion (wouterj) This PR was merged into the 3.2 branch. Discussion ---------- [PHPunitBridge] Count @expectedDeprecation as an assertion | Q | A | ------------- | --- | Branch? | 3.2 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - By adding `addToAssertionCount()`, the `@expectedDeprecation` annotation is recognized as an assertion by PHPUnit. This means PHPUnit will not report the test as risky because it does not contain any assertion. Commits ------- ba5c0f4 Count @expectedDeprecation as an assertion

* 3.2: Count @expectedDeprecation as an assertion

…luz) This PR was squashed before being merged into the 3.3-dev branch (closes symfony#21893). Discussion ---------- Added a castToArray() config helper | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony#21817 | License | MIT | Doc PR | - Commits ------- 3a589df Added a castToArray() config helper

Basically, the formatter now uses the VarDumper & uses more significant colors and has a more compact / readable format (IMHO).

…yrixx) This PR was merged into the 3.3-dev branch. Discussion ---------- [Bridge/Monolog] Enhance the Console Handler | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | yes | Tests pass? | - | Fixed tickets | - | License | MIT | Doc PR | - --- I extracted and enhanced the formatting logic from symfony#21080. Basically, The formatter now use the VarDumper & use more significant colors and has a more compact / readable format (IMHO). --- I used the following code to generate before/after screenshots. ```php protected function execute(InputInterface $input, OutputInterface $output) { $logger = $this->getContainer()->get('logger'); $filesystem = $this->getContainer()->get('filesystem'); $anObject = new \stdClass; $anObject->firstPpt = 'foo'; $anObject->secondePpt = 'bar'; $logger->log('notice', 'Hello {who}', [ 'who' => 'Wold', 'an_object' => $anObject, 'file_system' => $filesystem, ]); $r = new \ReflectionClass(LogLevel::class); foreach ($r->getConstants() as $level) { $logger = $logger->withName($level); $logger->log($level, 'log at level {level}', [ 'level' => $level, ]); } } ``` before: ![screenshot12](https://cloud.githubusercontent.com/assets/408368/23183075/0bb21f80-f87b-11e6-8123-f6da70f2493b.png) After: ![screenshot11](https://cloud.githubusercontent.com/assets/408368/23182971/b4022ed8-f87a-11e6-9d3b-de1a9d4ce9aa.png) Commits ------- b663ab5 [Bridge/Monolog] Enhanced the Console Handler

…hen publishing a message. (jwage) This PR was squashed before being merged into the 6.4 branch. Discussion ---------- [Messenger] [Amqp] Handle AMQPConnectionException when publishing a message. | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | Fix symfony#36538 Fix symfony#48241 | License | MIT If you have a message handler that dispatches messages to another queue, you can encounter `AMQPConnectionException` with the message "Library error: a SSL error occurred" or "a socket error occurred" depending on if you are using tls or not or if you are running behind a load balancer or not. You can manually reproduce this issue by dispatching a message where the handler then dispatches another message to a different queue, then go to rabbitmq admin and close the connection manually, then dispatch another message and when the message handler goes to dispatch the other message, you will get this exception: ``` a socket error occurred #0 /vagrant/vendor/symfony/amqp-messenger/Transport/AmqpTransport.php(60): Symfony\Component\Messenger\Bridge\Amqp\Transport\AmqpSender->send() #1 /vagrant/vendor/symfony/messenger/Middleware/SendMessageMiddleware.php(62): Symfony\Component\Messenger\Bridge\Amqp\Transport\AmqpTransport->send() #2 /vagrant/vendor/symfony/messenger/Middleware/FailedMessageProcessingMiddleware.php(34): Symfony\Component\Messenger\Middleware\SendMessageMiddleware->handle() #3 /vagrant/vendor/symfony/messenger/Middleware/DispatchAfterCurrentBusMiddleware.php(61): Symfony\Component\Messenger\Middleware\FailedMessageProcessingMiddleware->handle() #4 /vagrant/vendor/symfony/messenger/Middleware/RejectRedeliveredMessageMiddleware.php(41): Symfony\Component\Messenger\Middleware\DispatchAfterCurrentBusMiddleware->handle() #5 /vagrant/vendor/symfony/messenger/Middleware/AddBusNameStampMiddleware.php(37): Symfony\Component\Messenger\Middleware\RejectRedeliveredMessageMiddleware->handle() #6 /vagrant/vendor/symfony/messenger/Middleware/TraceableMiddleware.php(40): Symfony\Component\Messenger\Middleware\AddBusNameStampMiddleware->handle() #7 /vagrant/vendor/symfony/messenger/MessageBus.php(70): Symfony\Component\Messenger\Middleware\TraceableMiddleware->handle() #8 /vagrant/vendor/symfony/messenger/TraceableMessageBus.php(38): Symfony\Component\Messenger\MessageBus->dispatch() #9 /vagrant/src/Messenger/MessageBus.php(37): Symfony\Component\Messenger\TraceableMessageBus->dispatch() #10 /vagrant/vendor/symfony/mailer/Mailer.php(66): App\Messenger\MessageBus->dispatch() #11 /vagrant/src/Mailer/Mailer.php(83): Symfony\Component\Mailer\Mailer->send() #12 /vagrant/src/Mailer/Mailer.php(96): App\Mailer\Mailer->send() #13 /vagrant/src/MessageHandler/Trading/StrategySubscriptionMessageHandler.php(118): App\Mailer\Mailer->sendEmail() #14 /vagrant/src/MessageHandler/Trading/StrategySubscriptionMessageHandler.php(72): App\MessageHandler\Trading\StrategySubscriptionMessageHandler->handle() #15 /vagrant/vendor/symfony/messenger/Middleware/HandleMessageMiddleware.php(152): App\MessageHandler\Trading\StrategySubscriptionMessageHandler->__invoke() #16 /vagrant/vendor/symfony/messenger/Middleware/HandleMessageMiddleware.php(91): Symfony\Component\Messenger\Middleware\HandleMessageMiddleware->callHandler() #17 /vagrant/vendor/symfony/messenger/Middleware/SendMessageMiddleware.php(71): Symfony\Component\Messenger\Middleware\HandleMessageMiddleware->handle() #18 /vagrant/vendor/symfony/messenger/Middleware/FailedMessageProcessingMiddleware.php(34): Symfony\Component\Messenger\Middleware\SendMessageMiddleware->handle() #19 /vagrant/vendor/symfony/messenger/Middleware/DispatchAfterCurrentBusMiddleware.php(68): Symfony\Component\Messenger\Middleware\FailedMessageProcessingMiddleware->handle() #20 /vagrant/vendor/symfony/messenger/Middleware/RejectRedeliveredMessageMiddleware.php(41): Symfony\Component\Messenger\Middleware\DispatchAfterCurrentBusMiddleware->handle() #21 /vagrant/vendor/symfony/messenger/Middleware/AddBusNameStampMiddleware.php(37): Symfony\Component\Messenger\Middleware\RejectRedeliveredMessageMiddleware->handle() #22 /vagrant/vendor/symfony/messenger/Middleware/TraceableMiddleware.php(40): Symfony\Component\Messenger\Middleware\AddBusNameStampMiddleware->handle() #23 /vagrant/vendor/symfony/messenger/MessageBus.php(70): Symfony\Component\Messenger\Middleware\TraceableMiddleware->handle() #24 /vagrant/vendor/symfony/messenger/TraceableMessageBus.php(38): Symfony\Component\Messenger\MessageBus->dispatch() #25 /vagrant/vendor/symfony/messenger/RoutableMessageBus.php(54): Symfony\Component\Messenger\TraceableMessageBus->dispatch() #26 /vagrant/vendor/symfony/messenger/Worker.php(162): Symfony\Component\Messenger\RoutableMessageBus->dispatch() #27 /vagrant/vendor/symfony/messenger/Worker.php(109): Symfony\Component\Messenger\Worker->handleMessage() #28 /vagrant/vendor/symfony/messenger/Command/ConsumeMessagesCommand.php(238): Symfony\Component\Messenger\Worker->run() #29 /vagrant/vendor/symfony/console/Command/Command.php(326): Symfony\Component\Messenger\Command\ConsumeMessagesCommand->execute() #30 /vagrant/vendor/symfony/console/Application.php(1096): Symfony\Component\Console\Command\Command->run() #31 /vagrant/vendor/symfony/framework-bundle/Console/Application.php(126): Symfony\Component\Console\Application->doRunCommand() #32 /vagrant/vendor/symfony/console/Application.php(324): Symfony\Bundle\FrameworkBundle\Console\Application->doRunCommand() #33 /vagrant/vendor/symfony/framework-bundle/Console/Application.php(80): Symfony\Component\Console\Application->doRun() #34 /vagrant/vendor/symfony/console/Application.php(175): Symfony\Bundle\FrameworkBundle\Console\Application->doRun() #35 /vagrant/vendor/symfony/runtime/Runner/Symfony/ConsoleApplicationRunner.php(49): Symfony\Component\Console\Application->run() #36 /vagrant/vendor/autoload_runtime.php(29): Symfony\Component\Runtime\Runner\Symfony\ConsoleApplicationRunner->run() #37 /vagrant/bin/console(11): require_once('...') #38 {main} ``` TODO: - [x] Add test for retry logic when publishing messages Commits ------- f123370 [Messenger] [Amqp] Handle AMQPConnectionException when publishing a message.

…nal::readFromProcess` (fritzmg) This PR was merged into the 5.4 branch. Discussion ---------- [Console] Suppress `proc_open` errors within `Terminal::readFromProcess` | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | | License | MIT When instantiating `SymfonyStyle` in a command it will try to determine the maximum width of the current console interface. https://github.com/symfony/symfony/blob/6687e4ea35f45ebd73fb0315938103628cfb13a0/src/Symfony/Component/Console/Style/SymfonyStyle.php#L53 This will execute `stty -a | grep columns` down the line. Access to `stty` might be disallowed however, resulting in the following error: ``` ErrorException: Warning: proc_open(): Exec failed: Permission denied #16 /vendor/symfony/console/Terminal.php(220): Symfony\Component\Console\Terminal::readFromProcess #15 /vendor/symfony/console/Terminal.php(204): Symfony\Component\Console\Terminal::getSttyColumns #14 /vendor/symfony/console/Terminal.php(170): Symfony\Component\Console\Terminal::initDimensionsUsingStty #13 /vendor/symfony/console/Terminal.php(153): Symfony\Component\Console\Terminal::initDimensions #12 /vendor/symfony/console/Terminal.php(94): Symfony\Component\Console\Terminal::getWidth #11 /vendor/symfony/console/Style/SymfonyStyle.php(55): Symfony\Component\Console\Style\SymfonyStyle::__construct #10 /vendor/symfony/messenger/Command/ConsumeMessagesCommand.php(136): Symfony\Component\Messenger\Command\ConsumeMessagesCommand::interact #9 /vendor/symfony/console/Command/Command.php(311): Symfony\Component\Console\Command\Command::run ``` (Stack Trace actually from Symfony 6) The phpDoc of `Terminal::getSttyColumns` states > Runs and parses stty -a if it's available, _suppressing any error output_. The latter might refer to `['suppress_errors' => true]` (though I am not sure) - which is a Windows only functionality. In any case, since `Terminal::readFromProcess` already checks for ```php if (!$process = proc_open(…)) { return null; } ``` and ```php if (!\is_resource($process)) { return null; } ``` upstream in Symfony 6/7, indicating that `proc_open` might fail - this error can additionally be suppressed using `@`. Besides, `Process::start` also uses ``@proc_open`` (added in symfony@099481f "Prevent warning in proc_open()"). Commits ------- 575249a suppress proc_open errors

stof reviewed Feb 6, 2017

View reviewed changes

nicolas-grekas force-pushed the di-partials branch 3 times, most recently from 054a1cc to a0db997 Compare February 7, 2017 08:41

nicolas-grekas changed the title ~~[EXPERIMENTAL][DI] Add partial methods injection~~ [EXPERIMENTAL][DI] Add partial methods injection + autowiring Feb 7, 2017

stof reviewed Feb 7, 2017

View reviewed changes

nicolas-grekas force-pushed the di-partials branch from a0db997 to ac8e268 Compare February 7, 2017 10:14

nicolas-grekas force-pushed the di-partials branch from ac8e268 to 5fcb561 Compare February 7, 2017 10:19

nicolas-grekas commented Feb 7, 2017

View reviewed changes

nicolas-grekas force-pushed the di-partials branch 6 times, most recently from 962229e to 8695bfd Compare February 13, 2017 08:19

nicolas-grekas changed the title ~~[EXPERIMENTAL][DI] Add partial methods injection + autowiring~~ [EXPERIMENTAL][DI] Add tail injection for methods + autowiring Feb 13, 2017

stof reviewed Feb 13, 2017

View reviewed changes

nicolas-grekas force-pushed the di-partials branch 5 times, most recently from a3775d1 to 7c91b1a Compare February 16, 2017 22:37

nicolas-grekas force-pushed the di-partials branch from 7c91b1a to 5c2e5b2 Compare February 18, 2017 09:15

nicolas-grekas force-pushed the di-partials branch from 5c2e5b2 to c591853 Compare February 18, 2017 09:18

nicolas-grekas force-pushed the di-partials branch from c591853 to 6c0987f Compare March 1, 2017 22:04

fabpot and others added 12 commits March 5, 2017 13:52

fixed CS

e4c12da

fix typo

fa451b2

[DI] Replace PHP7-conditional return-type checks by regular type-hint…

372ff7c

…s that work on PHP5

Count @expectedDeprecation as an assertion

ba5c0f4

Merge branch '3.2'

a1a7599

* 3.2: Count @expectedDeprecation as an assertion

Added a castToArray() config helper

3a589df

[Bridge/Monolog] Enhanced the Console Handler

b663ab5

Basically, the formatter now uses the VarDumper & uses more significant colors and has a more compact / readable format (IMHO).

[EXPERIMENTAL][DI] Add tail injection for methods + autowiring

8717654

nicolas-grekas force-pushed the di-partials branch from 6c0987f to 8717654 Compare March 6, 2017 20:45

nicolas-grekas closed this Mar 6, 2017

nicolas-grekas deleted the di-partials branch March 6, 2017 21:34

nicolas-grekas restored the di-partials branch March 6, 2017 21:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[EXPERIMENTAL][DI] Add tail injection for methods + autowiring #11

[EXPERIMENTAL][DI] Add tail injection for methods + autowiring #11

Uh oh!

nicolas-grekas commented Feb 6, 2017 •

edited

Loading

Uh oh!

nicolas-grekas commented Feb 6, 2017 •

edited

Loading

Uh oh!

stof commented Feb 6, 2017

Uh oh!

nicolas-grekas commented Feb 6, 2017

Uh oh!

stof Feb 6, 2017

Uh oh!

nicolas-grekas Feb 6, 2017

Uh oh!

stof Feb 7, 2017

Uh oh!

nicolas-grekas Feb 7, 2017

Uh oh!

nicolas-grekas commented Feb 7, 2017 •

edited

Loading

Uh oh!

nicolas-grekas Feb 7, 2017

Uh oh!

stof Feb 7, 2017

Uh oh!

nicolas-grekas commented Feb 13, 2017

Uh oh!

stof Feb 13, 2017

Uh oh!

stof commented Feb 13, 2017

Uh oh!

nicolas-grekas commented Feb 17, 2017

Uh oh!

Uh oh!

[EXPERIMENTAL][DI] Add tail injection for methods + autowiring #11

[EXPERIMENTAL][DI] Add tail injection for methods + autowiring #11

Uh oh!

Conversation

nicolas-grekas commented Feb 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicolas-grekas commented Feb 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stof commented Feb 6, 2017

Uh oh!

nicolas-grekas commented Feb 6, 2017

Uh oh!

stof Feb 6, 2017

Choose a reason for hiding this comment

Uh oh!

nicolas-grekas Feb 6, 2017

Choose a reason for hiding this comment

Uh oh!

stof Feb 7, 2017

Choose a reason for hiding this comment

Uh oh!

nicolas-grekas Feb 7, 2017

Choose a reason for hiding this comment

Uh oh!

nicolas-grekas commented Feb 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicolas-grekas Feb 7, 2017

Choose a reason for hiding this comment

Uh oh!

stof Feb 7, 2017

Choose a reason for hiding this comment

Uh oh!

nicolas-grekas commented Feb 13, 2017

Uh oh!

stof Feb 13, 2017

Choose a reason for hiding this comment

Uh oh!

stof commented Feb 13, 2017

Uh oh!

nicolas-grekas commented Feb 17, 2017

Uh oh!

Uh oh!

nicolas-grekas commented Feb 6, 2017 •

edited

Loading

nicolas-grekas commented Feb 6, 2017 •

edited

Loading

nicolas-grekas commented Feb 7, 2017 •

edited

Loading