Autowiring for all! (mautic#11587)

* Trying to autowire all services * Moving BatchIdToEntityHelper to DTO namespace which is excuded from autowiring * Autowire all services in app/bundles except some classes that are not services like exception and DTO objects * Commands are autowired automatically. Existing services must set at least an alias for BC * Moving BatchIdToEntityHelper back and rather excluding it in services.php as it would require more refactoring * Allow bundles to configure their own services and use default configuration if not to keep BC * Removing an empty file * Fixing types * Controllers must be excluded by default until we refactor them to use DI * Moving campaign-related DI excludes from global config to CampaignBundle * Moving email-related DI excludes from global config to EmailBundle * Moving report-related DI excludes from global config to ReportBundle * Moving form-related DI excludes from global config to FormBundle * Moving channel-related DI excludes from global config to ChannelBundle * Moving a DTO class from Model to DTO dir to avoid autowiring error * Moving api-related DI excludes from global config to ApiBundle * Moving install-related DI excludes from global config to InstallBundle * Moving integrations-related DI excludes from global config to IntegrationslBundle * Moving plugin-related DI excludes from global config to PluginBundle * Moving queue-related DI excludes from global config to QueueBundle * Moving stats-related DI excludes from global config to StatsBundle * Configuring plugins to be autowireable * Fixing types * Updating class comments * Fixing CRM bundle DI config * Get the autowired service if it exists rather than always creating it * Removing duplicated service definition and moving repository services in one place to avoid further duplicates * Moving legacy service configuration from the MauticCoreExtension to ServicePass * Configuring Doctrine repositories as services visible by autowiring * Removing all command services from config.php files and relying on autowiring and autoconfiguration * Removing subscriber service definitions part 1 * Rewriting useless unit test of a repostitory method to a functional test * checkGenericClassInNonGenericObjectType rule is causing big troubles with antipaterns like getting a repository out of EM. * Type fixes * Fixing repository tests after they started extending ServiceEntityRepository * Removing subscriber service definitions part 2 * Fixing report repositories * Fixing FrequencyRuleRepository service * Another round of repository test fixes * Using FQCN instead of mautic.http.client.mock_handler in tests * Moving overwriting of the HTTP client for tests from config to a compiler pass because I moved the legacy service configuration from MauticCoreExtension to ServicePass and so it was called after the config_test.php service and did not overwrite the service as it was created afterwards. * Moving overwriding of the PipeDrive HTTP client from config_test to its own compiler pass to be executed at the right time and also for separation of concerns * Fixing logger methods after the namespace was updated * The font URLs have now escaped slashes * Enabling autowiring for controllers * Leaving autowiring of controllers for another PR * Removing form type service definitions part 1 the aliases were refactored for Symfony 4 support * Removing form type service definitions part 2 * Autoconfiguring Mautic Models * Converting some services from legacy DI to autowiring keeping aliases for BC * Documenting the changes in the upgrading doc * Removing option to set service aliases in config.php. They should be defined in services.php instead * Resolving "42x: Referencing controllers with MauticLeadBundle:Lead:contactStats is deprecated since Symfony 4.1, use "Mautic\LeadBundle\Controller\LeadController::contactStatsAction" instead." * Fixing unsubscribing from a channel * Loading citrix repositories * Removing twig extension definitions as those are now autowired and autoconfigured. They are not used as services * Configuring the AssetsHelper to be set properly * Fixing autowirng for new exception after rebase * Excluding new DTOs recently added to 5.x
mlahlouh · Nov 11, 2022 · 41f5c8a · 41f5c8a
1 parent 19c0a13
commit 41f5c8a
Show file tree

Hide file tree

Showing 148 changed files with 1,901 additions and 4,418 deletions.
diff --git a/UPGRADE-5.0.md b/UPGRADE-5.0.md
@@ -8,6 +8,9 @@
     * Command `Mautic\LeadBundle\Command\CheckQueryBuildersCommand` and the methods it use:
         * `Mautic\LeadBundle\Model\ListModel::getVersionNew()`
         * `Mautic\LeadBundle\Model\ListModel::getVersionOld()`
+*   Services
+    * Repository service `mautic.user.token.repository` for `Mautic\UserBundle\Entity\UserTokenRepository` was removed as it was duplicated. Use `mautic.user.repository.user_token` instead.
+    * In tests replace `self::$container->get('mautic.http.client.mock_handler')` with `self::$container->get(\GuzzleHttp\Handler\MockHandler::class)` to get HTTP client mock handler.
 *   Other
     * `Mautic\UserBundle\Security\Firewall\AuthenticationListener::class` no longer implements the deprecated `Symfony\Component\Security\Http\Firewall\ListenerInterface` and was made final. The `public function handle(GetResponseEvent $event)` method was changed to `public function __invoke(RequestEvent $event): void` to support Symfony 5.
     * `Mautic\IntegrationsBundle\Configuration\PluginConfiguration` removed - we don't use it
@@ -24,3 +27,19 @@
         * `Mautic\UserBundle\Entity\User::isEnabled()`
     * Two French regions were updates based on ISO_3166-2 (Val-d\'Oise, La Réunion). If you use it in API, please change values to Val d\'Oise or Réunion
     * `AbstractMauticTestCase::loadFixtures` and `AbstractMauticTestCase::loadFixtureFiles` now accept only two arguments: `array $fixtures` and `bool $append`. If you need to use old parameters - refer to the documentation of `LiipTestFixturesBundle`
+
+# Dependency injection improvements
+
+Mautic 5 adds support for Symfony's [autowiring](https://symfony.com/doc/5.4/service_container/autowiring.html) and [autoconfigure](https://symfony.com/doc/4.4/service_container.html#the-autoconfigure-option) for services.
+
+Advantages:
+- New services no longer need to have any definition in the app/bundles/*Bundle/Config/config.php. Symfony will guess what services are needed in the services by types of arguments in the constructor.
+- Services that aren't used in other services as dependencies like **subscribers**, **commands** and **form types** were deleted completely.
+- Existing service definitions can be reduced to setting just the string alias to keep backward compatibility and controllers working.
+- `app/config/services.php` is automatically configuring all bundles including plugins so if the bundle doesn't do anything uncommon then it should work out of the box.
+- The legacy services definitions in `*Bundle/Config/config.php` file are still working but will be removed in Mautic 6.
+
+Possible backward compatibility breaks:
+- If your plugin does break it may be using some value objects out of common places. Get inspiration in existing `plugins/*Bundle/Config/services.php` to exclude the folders or files from autowiring.
+- Some services might need to be configured. For example if they need a config parameter in the constructor. Follow [the official Symfony docs](https://symfony.com/doc/5.4/service_container.html#explicitly-configuring-services-and-arguments) to configure such services.
+- Start converting your controllers to support DI over loading services from container as that is an anti-pattern that Symfony no longer supports. That is the reason why all the services are set as public so the old controllers can still work. This will change throughout the life of Mautic 5 and will be removed in Mautic 6. See https://symfony.com/doc/current/controller/service.html
diff --git a/app/AppKernel.php b/app/AppKernel.php
@@ -4,6 +4,7 @@
 use Mautic\CoreBundle\Release\ThisRelease;
 use Mautic\QueueBundle\Queue\QueueProtocol;
 use Symfony\Component\Config\Loader\LoaderInterface;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\HttpFoundation\RedirectResponse;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
@@ -219,6 +220,12 @@ public function registerBundles(): array
         return $bundles;
     }
 
+    protected function build(ContainerBuilder $container): void
+    {
+        $container->registerForAutoconfiguration(\Mautic\CoreBundle\Model\MauticModelInterface::class)
+            ->addTag(\Mautic\CoreBundle\DependencyInjection\Compiler\ModelPass::TAG);
+    }
+
     /**
      * {@inheritdoc}
      */

diff --git a/app/bundles/ApiBundle/Config/config.php b/app/bundles/ApiBundle/Config/config.php
@@ -73,54 +73,6 @@
                 ],
             ],
         ],
-        'events' => [
-            'mautic.api.subscriber' => [
-                'class'     => \Mautic\ApiBundle\EventListener\ApiSubscriber::class,
-                'arguments' => [
-                    'mautic.helper.core_parameters',
-                    'translator',
-                ],
-            ],
-            'mautic.api.client.subscriber' => [
-                'class'     => \Mautic\ApiBundle\EventListener\ClientSubscriber::class,
-                'arguments' => [
-                    'mautic.helper.ip_lookup',
-                    'mautic.core.model.auditlog',
-                ],
-            ],
-            'mautic.api.configbundle.subscriber' => [
-                'class' => \Mautic\ApiBundle\EventListener\ConfigSubscriber::class,
-            ],
-            'mautic.api.search.subscriber' => [
-                'class'     => \Mautic\ApiBundle\EventListener\SearchSubscriber::class,
-                'arguments' => [
-                    'mautic.api.model.client',
-                    'mautic.security',
-                    'mautic.helper.templating',
-                ],
-            ],
-            'mautic.api.rate_limit_generate_key.subscriber' => [
-              'class'     => \Mautic\ApiBundle\EventListener\RateLimitGenerateKeySubscriber::class,
-              'arguments' => [
-                'mautic.helper.core_parameters',
-              ],
-            ],
-        ],
-        'forms' => [
-            'mautic.form.type.apiclients' => [
-                'class'     => \Mautic\ApiBundle\Form\Type\ClientType::class,
-                'arguments' => [
-                    'request_stack',
-                    'translator',
-                    'validator',
-                    'session',
-                    'router',
-                ],
-            ],
-            'mautic.form.type.apiconfig' => [
-                'class' => 'Mautic\ApiBundle\Form\Type\ConfigType',
-            ],
-        ],
         'helpers' => [
             'mautic.api.helper.entity_result' => [
                 'class' => \Mautic\ApiBundle\Helper\EntityResultHelper::class,

diff --git a/app/bundles/ApiBundle/Config/services.php b/app/bundles/ApiBundle/Config/services.php
@@ -0,0 +1,25 @@
+<?php
+
+declare(strict_types=1);
+
+use Mautic\CoreBundle\DependencyInjection\MauticCoreExtension;
+use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
+
+return function (ContainerConfigurator $configurator) {
+    $services = $configurator->services()
+        ->defaults()
+        ->autowire()
+        ->autoconfigure()
+        ->public();
+
+    $excludes = [
+        'Serializer/Driver',
+        'Serializer/Exclusion',
+        'Helper/BatchIdToEntityHelper.php',
+    ];
+
+    $services->load('Mautic\\ApiBundle\\', '../')
+        ->exclude('../{'.implode(',', array_merge(MauticCoreExtension::DEFAULT_EXCLUDES, $excludes)).'}');
+
+    $services->load('Mautic\\ApiBundle\\Entity\\oAuth2\\', '../Entity/oAuth2/*Repository.php');
+};
diff --git a/app/bundles/ApiBundle/DependencyInjection/MauticApiExtension.php b/app/bundles/ApiBundle/DependencyInjection/MauticApiExtension.php
@@ -0,0 +1,22 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Mautic\ApiBundle\DependencyInjection;
+
+use Symfony\Component\Config\FileLocator;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Extension\Extension;
+use Symfony\Component\DependencyInjection\Loader\PhpFileLoader;
+
+class MauticApiExtension extends Extension
+{
+    /**
+     * @param mixed[] $configs
+     */
+    public function load(array $configs, ContainerBuilder $container): void
+    {
+        $loader = new PhpFileLoader($container, new FileLocator(__DIR__.'/../Config'));
+        $loader->load('services.php');
+    }
+}
diff --git a/app/bundles/ApiBundle/Form/Type/ClientType.php b/app/bundles/ApiBundle/Form/Type/ClientType.php
@@ -15,7 +15,7 @@
 use Symfony\Component\Form\FormEvent;
 use Symfony\Component\Form\FormEvents;
 use Symfony\Component\HttpFoundation\RequestStack;
-use Symfony\Component\HttpFoundation\Session\Session;
+use Symfony\Component\HttpFoundation\Session\SessionInterface;
 use Symfony\Component\OptionsResolver\OptionsResolver;
 use Symfony\Component\Routing\RouterInterface;
 use Symfony\Component\Validator\Validator\ValidatorInterface;
@@ -44,15 +44,15 @@ class ClientType extends AbstractType
     private $requestStack;
 
     /**
-     * @var Session
+     * @var SessionInterface
      */
     private $session;
 
     public function __construct(
         RequestStack $requestStack,
         TranslatorInterface $translator,
         ValidatorInterface $validator,
-        Session $session,
+        SessionInterface $session,
         RouterInterface $router
     ) {
         $this->translator   = $translator;

diff --git a/app/bundles/AssetBundle/Config/config.php b/app/bundles/AssetBundle/Config/config.php
@@ -61,167 +61,13 @@
                 ],
             ],
         ],
-        'events' => [
-            'mautic.asset.subscriber' => [
-                'class'     => \Mautic\AssetBundle\EventListener\AssetSubscriber::class,
-                'arguments' => [
-                    'mautic.helper.ip_lookup',
-                    'mautic.core.model.auditlog',
-                ],
-            ],
-            'mautic.asset.pointbundle.subscriber' => [
-                'class'     => \Mautic\AssetBundle\EventListener\PointSubscriber::class,
-                'arguments' => [
-                    'mautic.point.model.point',
-                ],
-            ],
-            'mautic.asset.formbundle.subscriber' => [
-                'class'     => Mautic\AssetBundle\EventListener\FormSubscriber::class,
-                'arguments' => [
-                    'mautic.asset.model.asset',
-                    'translator',
-                    'mautic.helper.template.analytics',
-                    'templating.helper.assets',
-                    'mautic.helper.theme',
-                    'mautic.helper.templating',
-                    'mautic.helper.core_parameters',
-                ],
-            ],
-            'mautic.asset.campaignbundle.subscriber' => [
-                'class'     => \Mautic\AssetBundle\EventListener\CampaignSubscriber::class,
-                'arguments' => [
-                    'mautic.campaign.executioner.realtime',
-                ],
-            ],
-            'mautic.asset.reportbundle.subscriber' => [
-                'class'     => \Mautic\AssetBundle\EventListener\ReportSubscriber::class,
-                'arguments' => [
-                    'mautic.lead.model.company_report_data',
-                    'mautic.asset.repository.download',
-                ],
-            ],
-            'mautic.asset.builder.subscriber' => [
-                'class'     => \Mautic\AssetBundle\EventListener\BuilderSubscriber::class,
-                'arguments' => [
-                    'mautic.security',
-                    'mautic.asset.helper.token',
-                    'mautic.tracker.contact',
-                    'mautic.helper.token_builder.factory',
-                ],
-            ],
-            'mautic.asset.leadbundle.subscriber' => [
-                'class'     => \Mautic\AssetBundle\EventListener\LeadSubscriber::class,
-                'arguments' => [
-                    'mautic.asset.model.asset',
-                    'translator',
-                    'router',
-                    'mautic.asset.repository.download',
-                ],
-            ],
-            'mautic.asset.pagebundle.subscriber' => [
-                'class' => \Mautic\AssetBundle\EventListener\PageSubscriber::class,
-            ],
-            'mautic.asset.emailbundle.subscriber' => [
-                'class' => \Mautic\AssetBundle\EventListener\EmailSubscriber::class,
-            ],
-            'mautic.asset.configbundle.subscriber' => [
-                'class' => \Mautic\AssetBundle\EventListener\ConfigSubscriber::class,
-            ],
-            'mautic.asset.search.subscriber' => [
-                'class'     => \Mautic\AssetBundle\EventListener\SearchSubscriber::class,
-                'arguments' => [
-                    'mautic.asset.model.asset',
-                    'mautic.security',
-                    'mautic.helper.user',
-                    'mautic.helper.templating',
-                ],
-            ],
-            'mautic.asset.stats.subscriber' => [
-                'class'     => \Mautic\AssetBundle\EventListener\StatsSubscriber::class,
-                'arguments' => [
-                    'mautic.security',
-                    'doctrine.orm.entity_manager',
-                ],
-            ],
-            'oneup_uploader.pre_upload' => [
-                'class'     => \Mautic\AssetBundle\EventListener\UploadSubscriber::class,
-                'arguments' => [
-                    'mautic.helper.core_parameters',
-                    'mautic.asset.model.asset',
-                    'mautic.core.validator.file_upload',
-                ],
-            ],
-            'mautic.asset.dashboard.subscriber' => [
-                'class'     => \Mautic\AssetBundle\EventListener\DashboardSubscriber::class,
-                'arguments' => [
-                    'mautic.asset.model.asset',
-                    'router',
-                ],
-            ],
-            'mautic.asset.subscriber.determine_winner' => [
-                'class'     => \Mautic\AssetBundle\EventListener\DetermineWinnerSubscriber::class,
-                'arguments' => [
-                    'doctrine.orm.entity_manager',
-                    'translator',
-                ],
-            ],
-        ],
-        'forms' => [
-            'mautic.form.type.asset' => [
-                'class'     => \Mautic\AssetBundle\Form\Type\AssetType::class,
-                'arguments' => [
-                    'translator',
-                    'mautic.asset.model.asset',
-                ],
-            ],
-            'mautic.form.type.pointaction_assetdownload' => [
-                'class' => \Mautic\AssetBundle\Form\Type\PointActionAssetDownloadType::class,
-            ],
-            'mautic.form.type.campaignevent_assetdownload' => [
-                'class' => \Mautic\AssetBundle\Form\Type\CampaignEventAssetDownloadType::class,
-            ],
-            'mautic.form.type.formsubmit_assetdownload' => [
-                'class' => \Mautic\AssetBundle\Form\Type\FormSubmitActionDownloadFileType::class,
-            ],
-            'mautic.form.type.assetlist' => [
-                'class'     => \Mautic\AssetBundle\Form\Type\AssetListType::class,
-                'arguments' => [
-                    'mautic.security',
-                    'mautic.asset.model.asset',
-                    'mautic.helper.user',
-                ],
-            ],
-            'mautic.form.type.assetconfig' => [
-                'class' => \Mautic\AssetBundle\Form\Type\ConfigType::class,
-            ],
-        ],
         'others' => [
             'mautic.asset.upload.error.handler' => [
                 'class'     => \Mautic\AssetBundle\ErrorHandler\DropzoneErrorHandler::class,
                 'arguments' => 'mautic.factory',
             ],
             // Override the DropzoneController
             'oneup_uploader.controller.dropzone.class' => \Mautic\AssetBundle\Controller\UploadController::class,
-            'mautic.asset.helper.token'                => [
-                'class'     => \Mautic\AssetBundle\Helper\TokenHelper::class,
-                'arguments' => 'mautic.asset.model.asset',
-            ],
-        ],
-        'models' => [
-            'mautic.asset.model.asset' => [
-                'class'     => \Mautic\AssetBundle\Model\AssetModel::class,
-                'arguments' => [
-                    'mautic.lead.model.lead',
-                    'mautic.category.model.category',
-                    'request_stack',
-                    'mautic.helper.ip_lookup',
-                    'mautic.helper.core_parameters',
-                    'mautic.lead.service.device_creator_service',
-                    'mautic.lead.factory.device_detector_factory',
-                    'mautic.lead.service.device_tracking_service',
-                    'mautic.tracker.contact',
-                ],
-            ],
         ],
         'fixtures' => [
             'mautic.asset.fixture.asset' => [

diff --git a/app/bundles/AssetBundle/Config/services.php b/app/bundles/AssetBundle/Config/services.php
@@ -0,0 +1,25 @@
+<?php
+
+declare(strict_types=1);
+
+use Mautic\CoreBundle\DependencyInjection\MauticCoreExtension;
+use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
+
+return function (ContainerConfigurator $configurator) {
+    $services = $configurator->services()
+        ->defaults()
+        ->autowire()
+        ->autoconfigure()
+        ->public();
+
+    $excludes = [
+        'Controller/UploadController.php',
+    ];
+
+    $services->load('Mautic\\AssetBundle\\', '../')
+        ->exclude('../{'.implode(',', array_merge(MauticCoreExtension::DEFAULT_EXCLUDES, $excludes)).'}');
+
+    $services->load('Mautic\\AssetBundle\\Entity\\', '../Entity/*Repository.php');
+    $services->alias('mautic.asset.helper.token', \Mautic\AssetBundle\Helper\TokenHelper::class);
+    $services->alias('mautic.asset.model.asset', \Mautic\AssetBundle\Model\AssetModel::class);
+};
diff --git a/app/bundles/AssetBundle/DependencyInjection/MauticAssetExtension.php b/app/bundles/AssetBundle/DependencyInjection/MauticAssetExtension.php
@@ -0,0 +1,22 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Mautic\AssetBundle\DependencyInjection;
+
+use Symfony\Component\Config\FileLocator;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Extension\Extension;
+use Symfony\Component\DependencyInjection\Loader\PhpFileLoader;
+
+class MauticAssetExtension extends Extension
+{
+    /**
+     * @param mixed[] $configs
+     */
+    public function load(array $configs, ContainerBuilder $container): void
+    {
+        $loader = new PhpFileLoader($container, new FileLocator(__DIR__.'/../Config'));
+        $loader->load('services.php');
+    }
+}