feature #30579 Using AMQP auto-setup in all cases, not just in debug …

…(weaverryan)

This PR was merged into the 4.3-dev branch.

Discussion
----------

Using AMQP auto-setup in all cases, not just in debug

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes and no
| New feature?  | no
| BC breaks?    | yes
| Deprecations? | no->
| Tests pass?   | yes
| Fixed tickets | Related to #29476
| License       | MIT
| Doc PR        | TODO

Currently AMQP does auto-setup of queues/exchanges in dev-mode only. That's a problem for 2 reasons:

1) Behavior in prod is drastically different... and actually... there's not currently a way I know of (easily) to set things up on prod.

2) One of the properties of AMQP is that you typically DO want things to be set up at runtime, as you need them - you usually *do* want auto-setup.

This changes the behavior to auto-setup true always.

Commits
-------

503c209 Using AMQP auto-setup in all cases, not just in debug

src/Symfony/Bundle/FrameworkBundle/Resources/config/messenger.xml

@@ -61,7 +61,6 @@
         <service id="messenger.transport.amqp.factory" class="Symfony\Component\Messenger\Transport\AmqpExt\AmqpTransportFactory">
             <tag name="messenger.transport_factory" />
             <argument type="service" id="messenger.transport.serializer" />
-            <argument>%kernel.debug%</argument>
         </service>
     </services>
 </container>

src/Symfony/Component/Messenger/CHANGELOG.md

@@ -13,6 +13,11 @@ CHANGELOG
  * [BC BREAK] If listening to exceptions while using `AmqpSender` or `AmqpReceiver`, `\AMQPException` is
    no longer thrown in favor of `TransportException`.
  * Deprecated `LoggingMiddleware`, pass a logger to `SendMessageMiddleware` instead.
+ * [BC BREAK] `Connection::__construct()` and `Connection::fromDsn()`
+   both no longer have `$isDebug` arguments.
+ * [BC BREAK] The Amqp Transport now automatically sets up the exchanges
+   and queues by default. Previously, this was done when in "debug" mode
+   only. Pass the `auto-setup` connection option to control this.
 
 4.2.0
 -----

src/Symfony/Component/Messenger/Tests/Transport/AmqpExt/AmqpTransportFactoryTest.php

@@ -22,8 +22,7 @@ class AmqpTransportFactoryTest extends TestCase
     public function testSupportsOnlyAmqpTransports()
     {
         $factory = new AmqpTransportFactory(
-            $this->getMockBuilder(SerializerInterface::class)->getMock(),
-            true
+            $this->getMockBuilder(SerializerInterface::class)->getMock()
         );
 
         $this->assertTrue($factory->supports('amqp://localhost', []));
@@ -34,11 +33,10 @@ public function testSupportsOnlyAmqpTransports()
     public function testItCreatesTheTransport()
     {
         $factory = new AmqpTransportFactory(
-            $serializer = $this->getMockBuilder(SerializerInterface::class)->getMock(),
-            true
+            $serializer = $this->getMockBuilder(SerializerInterface::class)->getMock()
         );
 
-        $expectedTransport = new AmqpTransport(Connection::fromDsn('amqp://localhost', ['foo' => 'bar'], true), $serializer);
+        $expectedTransport = new AmqpTransport(Connection::fromDsn('amqp://localhost', ['foo' => 'bar']), $serializer);
 
         $this->assertEquals($expectedTransport, $factory->createTransport('amqp://localhost', ['foo' => 'bar']));
     }

src/Symfony/Component/Messenger/Tests/Transport/AmqpExt/ConnectionTest.php

@@ -128,7 +128,7 @@ public function testSetsParametersOnTheQueueAndExchange()
                     'alternate-exchange' => 'alternate',
                 ],
             ],
-        ], true, $factory);
+        ], $factory);
         $connection->publish('body');
     }
 
@@ -171,9 +171,11 @@ public function testItUsesANormalConnectionByDefault()
             $amqpExchange = $this->getMockBuilder(\AMQPExchange::class)->disableOriginalConstructor()->getMock()
         );
 
+        // makes sure the channel looks connected, so it's not re-created
+        $amqpChannel->expects($this->once())->method('isConnected')->willReturn(true);
         $amqpConnection->expects($this->once())->method('connect');
 
-        $connection = Connection::fromDsn('amqp://localhost/%2f/messages', [], false, $factory);
+        $connection = Connection::fromDsn('amqp://localhost/%2f/messages', [], $factory);
         $connection->publish('body');
     }
 
@@ -186,13 +188,15 @@ public function testItAllowsToUseAPersistentConnection()
             $amqpExchange = $this->getMockBuilder(\AMQPExchange::class)->disableOriginalConstructor()->getMock()
         );
 
+        // makes sure the channel looks connected, so it's not re-created
+        $amqpChannel->expects($this->once())->method('isConnected')->willReturn(true);
         $amqpConnection->expects($this->once())->method('pconnect');
 
-        $connection = Connection::fromDsn('amqp://localhost/%2f/messages?persistent=true', [], false, $factory);
+        $connection = Connection::fromDsn('amqp://localhost/%2f/messages?persistent=true', [], $factory);
         $connection->publish('body');
     }
 
-    public function testItSetupsTheConnectionWhenDebug()
+    public function testItSetupsTheConnectionByDefault()
     {
         $factory = new TestAmqpFactory(
             $amqpConnection = $this->getMockBuilder(\AMQPConnection::class)->disableOriginalConstructor()->getMock(),
@@ -206,7 +210,7 @@ public function testItSetupsTheConnectionWhenDebug()
         $amqpQueue->expects($this->once())->method('declareQueue');
         $amqpQueue->expects($this->once())->method('bind')->with('exchange_name', 'my_key');
 
-        $connection = Connection::fromDsn('amqp://localhost/%2f/messages?queue[routing_key]=my_key', [], true, $factory);
+        $connection = Connection::fromDsn('amqp://localhost/%2f/messages?queue[routing_key]=my_key', [], $factory);
         $connection->publish('body');
     }
 
@@ -224,13 +228,13 @@ public function testItCanDisableTheSetup()
         $amqpQueue->expects($this->never())->method('declareQueue');
         $amqpQueue->expects($this->never())->method('bind');
 
-        $connection = Connection::fromDsn('amqp://localhost/%2f/messages?queue[routing_key]=my_key', ['auto-setup' => 'false'], true, $factory);
+        $connection = Connection::fromDsn('amqp://localhost/%2f/messages?queue[routing_key]=my_key', ['auto-setup' => 'false'], $factory);
         $connection->publish('body');
 
-        $connection = Connection::fromDsn('amqp://localhost/%2f/messages?queue[routing_key]=my_key', ['auto-setup' => false], true, $factory);
+        $connection = Connection::fromDsn('amqp://localhost/%2f/messages?queue[routing_key]=my_key', ['auto-setup' => false], $factory);
         $connection->publish('body');
 
-        $connection = Connection::fromDsn('amqp://localhost/%2f/messages?queue[routing_key]=my_key&auto-setup=false', [], true, $factory);
+        $connection = Connection::fromDsn('amqp://localhost/%2f/messages?queue[routing_key]=my_key&auto-setup=false', [], $factory);
         $connection->publish('body');
     }
 
@@ -248,7 +252,7 @@ public function testPublishWithQueueOptions()
         $amqpExchange->expects($this->once())->method('publish')
             ->with('body', null, 1, ['delivery_mode' => 2, 'headers' => ['token' => 'uuid', 'type' => '*']]);
 
-        $connection = Connection::fromDsn('amqp://localhost/%2f/messages?queue[attributes][delivery_mode]=2&queue[attributes][headers][token]=uuid&queue[flags]=1', [], true, $factory);
+        $connection = Connection::fromDsn('amqp://localhost/%2f/messages?queue[attributes][delivery_mode]=2&queue[attributes][headers][token]=uuid&queue[flags]=1', [], $factory);
         $connection->publish('body', $headers);
     }
 }

src/Symfony/Component/Messenger/Transport/AmqpExt/AmqpTransportFactory.php

@@ -24,17 +24,15 @@
 class AmqpTransportFactory implements TransportFactoryInterface
 {
     private $serializer;
-    private $debug;
 
-    public function __construct(SerializerInterface $serializer = null, bool $debug = false)
+    public function __construct(SerializerInterface $serializer = null)
     {
         $this->serializer = $serializer ?? new PhpSerializer();
-        $this->debug = $debug;
     }
 
     public function createTransport(string $dsn, array $options): TransportInterface
     {
-        return new AmqpTransport(Connection::fromDsn($dsn, $options, $this->debug), $this->serializer);
+        return new AmqpTransport(Connection::fromDsn($dsn, $options), $this->serializer);
     }
 
     public function supports(string $dsn, array $options): bool

src/Symfony/Component/Messenger/Transport/AmqpExt/Connection.php

@@ -36,7 +36,6 @@ class Connection
     private $connectionCredentials;
     private $exchangeConfiguration;
     private $queueConfiguration;
-    private $debug;
     private $amqpFactory;
 
     /**
@@ -54,16 +53,15 @@ class Connection
      */
     private $amqpQueue;
 
-    public function __construct(array $connectionCredentials, array $exchangeConfiguration, array $queueConfiguration, bool $debug = false, AmqpFactory $amqpFactory = null)
+    public function __construct(array $connectionCredentials, array $exchangeConfiguration, array $queueConfiguration, AmqpFactory $amqpFactory = null)
     {
         $this->connectionCredentials = $connectionCredentials;
-        $this->debug = $debug;
         $this->exchangeConfiguration = $exchangeConfiguration;
         $this->queueConfiguration = $queueConfiguration;
         $this->amqpFactory = $amqpFactory ?: new AmqpFactory();
     }
 
-    public static function fromDsn(string $dsn, array $options = [], bool $debug = false, AmqpFactory $amqpFactory = null): self
+    public static function fromDsn(string $dsn, array $options = [], AmqpFactory $amqpFactory = null): self
     {
         if (false === $parsedUrl = parse_url($dsn)) {
             throw new InvalidArgumentException(sprintf('The given AMQP DSN "%s" is invalid.', $dsn));
@@ -104,7 +102,7 @@ public static function fromDsn(string $dsn, array $options = [], bool $debug = f
             $queueOptions['arguments'] = self::normalizeQueueArguments($queueOptions['arguments']);
         }
 
-        return new self($amqpOptions, $exchangeOptions, $queueOptions, $debug, $amqpFactory);
+        return new self($amqpOptions, $exchangeOptions, $queueOptions, $amqpFactory);
     }
 
     private static function normalizeQueueArguments(array $arguments): array
@@ -129,7 +127,7 @@ private static function normalizeQueueArguments(array $arguments): array
      */
     public function publish(string $body, array $headers = []): void
     {
-        if ($this->debug && $this->shouldSetup()) {
+        if ($this->shouldSetup()) {
             $this->setup();
         }
 
@@ -146,7 +144,7 @@ public function publish(string $body, array $headers = []): void
      */
     public function get(): ?\AMQPEnvelope
     {
-        if ($this->debug && $this->shouldSetup()) {
+        if ($this->shouldSetup()) {
             $this->setup();
         }
 
@@ -256,6 +254,14 @@ private function clear(): void
 
     private function shouldSetup(): bool
     {
-        return !\array_key_exists('auto-setup', $this->connectionCredentials) || !\in_array($this->connectionCredentials['auto-setup'], [false, 'false'], true);
+        if (!\array_key_exists('auto-setup', $this->connectionCredentials)) {
+            return true;
+        }
+
+        if (\in_array($this->connectionCredentials['auto-setup'], [false, 'false'], true)) {
+            return false;
+        }
+
+        return true;
     }
 }