Skip to content
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

[Messenger] Wire the transaction middleware factory when component is enabled #817

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 6 additions & 1 deletion .travis.yml
Expand Up @@ -45,7 +45,12 @@ jobs:
- php: 7.1
env: stability=RC SYMFONY_DEPRECATIONS_HELPER=weak SYMFONY_VERSION="3.4.*"
install:
- composer config minimum-stability RC
- travis_retry composer update -n --prefer-dist

- php: 7.1
env: stability=dev SYMFONY_DEPRECATIONS_HELPER=weak SYMFONY_VERSION="4.1.*"
install:
- composer require --dev "symfony/messenger" --no-update
- travis_retry composer update -n --prefer-dist

- php: 7.2
Expand Down
27 changes: 27 additions & 0 deletions DependencyInjection/Compiler/MessengerPass.php
@@ -0,0 +1,27 @@
<?php

namespace Doctrine\Bundle\DoctrineBundle\DependencyInjection\Compiler;

use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\Messenger\MessageBusInterface;

/**
* Class for Symfony Messenger component integrations
*/
class MessengerPass implements CompilerPassInterface
{
/**
* {@inheritDoc}
*/
public function process(ContainerBuilder $container)
{
if ($container->hasAlias('message_bus') && is_subclass_of($container->findDefinition('message_bus')->getClass(), MessageBusInterface::class)) {
return;
}

// Remove wired services if the Messenger component actually isn't enabled:
$container->removeDefinition('doctrine.orm.messenger.middleware_factory.transaction');
$container->removeDefinition('messenger.middleware.doctrine_transaction_middleware');
}
}
12 changes: 12 additions & 0 deletions DependencyInjection/DoctrineExtension.php
Expand Up @@ -9,6 +9,7 @@
use Doctrine\ORM\Version;
use Symfony\Bridge\Doctrine\DependencyInjection\AbstractDoctrineExtension;
use Symfony\Bridge\Doctrine\Form\Type\DoctrineType;
use Symfony\Bridge\Doctrine\Messenger\DoctrineTransactionMiddlewareFactory;
use Symfony\Component\Config\FileLocator;
use Symfony\Component\DependencyInjection\Alias;
use Symfony\Component\DependencyInjection\ChildDefinition;
Expand All @@ -20,6 +21,7 @@
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\DependencyInjection\ServiceLocator;
use Symfony\Component\Form\AbstractType;
use Symfony\Component\Messenger\MessageBusInterface;

/**
* DoctrineExtension is an extension for the Doctrine DBAL and ORM library.
Expand Down Expand Up @@ -388,6 +390,16 @@ protected function ormLoad(array $config, ContainerBuilder $container)
->addTag(ServiceRepositoryCompilerPass::REPOSITORY_SERVICE_TAG);
}

// If the Messenger component is installed and the doctrine transaction middleware factory is available, wire it:
if (interface_exists(MessageBusInterface::class) && class_exists(DoctrineTransactionMiddlewareFactory::class)) {
$loader = new XmlFileLoader($container, new FileLocator(__DIR__ . '/../Resources/config'));
$loader->load('messenger.xml');

$container->getDefinition('messenger.middleware.doctrine_transaction_middleware')
->replaceArgument(0, $config['default_entity_manager'])
;
}

/*
* Compatibility for Symfony 3.2 and lower: gives the service a default argument.
* When DoctrineBundle requires 3.3 or higher, this can be moved to an anonymous
Expand Down
2 changes: 2 additions & 0 deletions DoctrineBundle.php
Expand Up @@ -3,6 +3,7 @@
namespace Doctrine\Bundle\DoctrineBundle;

use Doctrine\Bundle\DoctrineBundle\DependencyInjection\Compiler\EntityListenerPass;
use Doctrine\Bundle\DoctrineBundle\DependencyInjection\Compiler\MessengerPass;
use Doctrine\Bundle\DoctrineBundle\DependencyInjection\Compiler\ServiceRepositoryCompilerPass;
use Doctrine\Common\Util\ClassUtils;
use Doctrine\ORM\Proxy\Autoloader;
Expand Down Expand Up @@ -38,6 +39,7 @@ public function build(ContainerBuilder $container)
$container->addCompilerPass(new DoctrineValidationPass('orm'));
$container->addCompilerPass(new EntityListenerPass());
$container->addCompilerPass(new ServiceRepositoryCompilerPass());
$container->addCompilerPass(new MessengerPass());
}

/**
Expand Down
20 changes: 20 additions & 0 deletions Resources/config/messenger.xml
@@ -0,0 +1,20 @@
<?xml version="1.0" ?>

<container xmlns="http://symfony.com/schema/dic/services"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
<services>
<service id="doctrine.orm.messenger.middleware_factory.transaction" class="Symfony\Bridge\Doctrine\Messenger\DoctrineTransactionMiddlewareFactory" public="false">
<argument type="service" id="doctrine" />
</service>

<!--
The following service isn't prefixed by the "doctrine.orm" namespace in order for end-users to just use
the "doctrine_transaction_middleware" shortcut in message buses middleware config
-->
<service id="messenger.middleware.doctrine_transaction_middleware" class="Symfony\Bridge\Doctrine\Messenger\DoctrineTransactionMiddleware" abstract="true" public="false">
<factory service="doctrine.orm.messenger.middleware_factory.transaction" method="createMiddleware" />
<argument /> <!-- default entity manager -->
</service>
</services>
</container>
68 changes: 68 additions & 0 deletions Tests/DependencyInjection/Compiler/MessengerPassTest.php
@@ -0,0 +1,68 @@
<?php

namespace DependencyInjection\Compiler;

use Doctrine\Bundle\DoctrineBundle\DependencyInjection\Compiler\MessengerPass;
use PHPUnit\Framework\TestCase;
use Symfony\Component\Config\FileLocator;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
use Symfony\Component\Messenger\MessageBus;
use Symfony\Component\Messenger\MessageBusInterface;

class MessengerPassTest extends TestCase
{
protected function setUp()
{
if (interface_exists(MessageBusInterface::class)) {
return;
}

$this->markTestSkipped('Symfony Messenger component is not installed');
}

public function testRemovesDefinitionsWhenMessengerComponentIsDisabled()
{
$pass = new MessengerPass();
$container = new ContainerBuilder();
$loader = new XmlFileLoader($container, new FileLocator(__DIR__ . '/../../../Resources/config'));
$loader->load('messenger.xml');

$pass->process($container);

$this->assertFalse($container->hasDefinition('doctrine.orm.messenger.middleware_factory.transaction'));
$this->assertFalse($container->hasDefinition('messenger.middleware.doctrine_transaction_middleware'));
}

public function testRemoveDefinitionsWhenHasAliasButNotMessengerComponent()
{
$pass = new MessengerPass();
$container = new ContainerBuilder();
$loader = new XmlFileLoader($container, new FileLocator(__DIR__ . '/../../../Resources/config'));
$loader->load('messenger.xml');

$container->register('some_other_bus', \stdClass::class);
$container->setAlias('message_bus', 'some_other_bus');

$pass->process($container);

$this->assertFalse($container->hasDefinition('doctrine.orm.messenger.middleware_factory.transaction'));
$this->assertFalse($container->hasDefinition('messenger.middleware.doctrine_transaction_middleware'));
}

public function testDoesNotRemoveDefinitionsWhenMessengerComponentIsEnabled()
{
$pass = new MessengerPass();
$container = new ContainerBuilder();
$loader = new XmlFileLoader($container, new FileLocator(__DIR__ . '/../../../Resources/config'));
$loader->load('messenger.xml');

$container->register('messenger.bus.default', MessageBus::class);
$container->setAlias('message_bus', 'messenger.bus.default');

$pass->process($container);

$this->assertTrue($container->hasDefinition('doctrine.orm.messenger.middleware_factory.transaction'));
$this->assertTrue($container->hasDefinition('messenger.middleware.doctrine_transaction_middleware'));
}
}
26 changes: 26 additions & 0 deletions Tests/DependencyInjection/DoctrineExtensionTest.php
Expand Up @@ -18,6 +18,7 @@
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\Messenger\MessageBusInterface;

class DoctrineExtensionTest extends TestCase
{
Expand Down Expand Up @@ -676,6 +677,31 @@ public function testAnnotationsBundleMappingDetectionWithVendorNamespace()
$this->assertEquals('Fixtures\Bundles\Vendor\AnnotationsBundle\Entity', $calls[0][1][1]);
}

public function testMessengerIntegration()
{
if (! interface_exists(MessageBusInterface::class)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd argue that it should be a dev-dependency, otherwise it's untested.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, I though symfony/symfony was used on CI for specific versions. So I expected it to be available in the new build.
The issue with adding it into the dev-dependencies (see first commit) is that it'll conflict with lowest dependencies tested for this packages due to the component requirements. So it must be explicitly added on a specific build. WDYT?

Copy link
Contributor

@Majkl578 Majkl578 May 14, 2018

Choose a reason for hiding this comment

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

You can override it for now using ^4.1@beta, the @beta part would be dropped once 4.1 is stable. But yes, it wouldn't allow SF pre-3.4. Since there is only one (or two) CI job to test SF 2.x, I think it'd be better to drop this dependency only in such job.
(Another argument for dropping 2.x support soon.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would actually require dropping symfony/messenger for every build without PHP 7.1, a minimum-stability: dev or with a specific SYMFONY_VERSION except for the new 4.1-beta2 one, as the component is only available since 4.1 and the required classes would be part of the beta2 (so it'll be quite a mess).
For now, I've just added the symfony/messenger component on the new build, so this build will pass once the related PR on symfony/symfony is merged.
But if there is a better strategy to you, please feel free to push the required changes to my branch :) Thank you!

$this->markTestSkipped('Symfony Messenger component is not installed');
}

$container = $this->getContainer();
$extension = new DoctrineExtension();

$config = BundleConfigurationBuilder::createBuilder()
->addBaseConnection()
->addEntityManager([
'default_entity_manager' => 'default',
'entity_managers' => [
'default' => [],
],
])
->build();
$extension->load([$config], $container);

$this->assertNotNull($container->getDefinition('doctrine.orm.messenger.middleware_factory.transaction'));
$this->assertNotNull($middlewarePrototype = $container->getDefinition('messenger.middleware.doctrine_transaction_middleware'));
$this->assertSame('default', $middlewarePrototype->getArgument(0));
}

public function testCacheConfiguration()
{
$container = $this->getContainer();
Expand Down