[TASK] Use constructor injection for Container

Resolves: #796

Author: sabbelasichon

Resources/services.php

@@ -76,7 +76,7 @@
         ->share(false);
 
     $services->set(Factory::class)
-        ->args([service(FilesystemInterface::class), service(LoggerInterface::class)]);
+        ->args([service('service_container'), service(FilesystemInterface::class), service(LoggerInterface::class)]);
 
     $services->alias(FactoryInterface::class, Factory::class);
 

composer.json

@@ -16,7 +16,7 @@
         "neos/utility-files": "^7.3.10 || ^8.3.9",
         "symfony/config": "^5.0 || ^6.0 || ^7.0",
         "symfony/console": "^5.0 || ^6.0 || ^7.0",
-        "symfony/dependency-injection": "^5.0 || ^6.0",
+        "symfony/dependency-injection": "^5.0 || ^6.0 || ^7.0",
         "symfony/finder": "^5.1 || ^6.0 || ^7.0",
         "symfony/options-resolver": "^5.0 || ^6.0 || ^7.0",
         "symfony/process": "^5.0 || ^6.0 || ^7.0",

src/Cli/Symfony/ConsoleKernel.php

@@ -17,9 +17,7 @@
 use Symfony\Component\Config\FileLocator;
 use Symfony\Component\Config\Loader\LoaderInterface;
 use Symfony\Component\DependencyInjection\Container;
-use Symfony\Component\DependencyInjection\ContainerAwareInterface;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
-use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\DependencyInjection\Dumper\PhpDumper;
 use Symfony\Component\DependencyInjection\Loader\PhpFileLoader;
 use Symfony\Component\DependencyInjection\Reference;
@@ -47,12 +45,6 @@ private function registerContainerConfiguration(LoaderInterface $loader): void
     private function build(ContainerBuilder $container): void
     {
         $container->addCompilerPass(new CommandsToApplicationCompilerPass());
-        $container->registerForAutoconfiguration(
-            ContainerAwareInterface::class
-        )->addMethodCall(
-            'setContainer',
-            [new Reference(ContainerInterface::class)]
-        );
         $container->registerForAutoconfiguration(
             ShellCommandServiceAwareInterface::class
         )->addMethodCall(

src/Domain/Model/Deployment.php

@@ -12,24 +12,21 @@
 namespace TYPO3\Surf\Domain\Model;
 
 use Neos\Utility\Files;
+use Psr\Container\ContainerInterface;
 use Psr\Log\LoggerAwareInterface;
 use Psr\Log\LoggerInterface;
-use Symfony\Component\DependencyInjection\ContainerAwareInterface;
-use Symfony\Component\DependencyInjection\ContainerAwareTrait;
 use TYPO3\Surf\Domain\Enum\DeploymentStatus;
 use TYPO3\Surf\Exception as SurfException;
 use TYPO3\Surf\Integration\LoggerAwareTrait;
 use UnexpectedValueException;
-use Webmozart\Assert\Assert;
 
 /**
  * This is the base object exposed to a deployment configuration script and serves as a configuration builder and
  * model for an actual deployment.
  */
-class Deployment implements LoggerAwareInterface, ContainerAwareInterface
+class Deployment implements LoggerAwareInterface
 {
     use LoggerAwareTrait;
-    use ContainerAwareTrait;
 
     /**
      * The name of this deployment
@@ -97,9 +94,11 @@ class Deployment implements LoggerAwareInterface, ContainerAwareInterface
     private bool $forceRun = false;
 
     private string $deploymentLockIdentifier;
+    private ContainerInterface $container;
 
-    public function __construct(string $name, string $deploymentLockIdentifier = null)
+    public function __construct(ContainerInterface $container, string $name, string $deploymentLockIdentifier = null)
     {
+        $this->container = $container;
         $this->name = $name;
         $this->status = DeploymentStatus::UNKNOWN();
         $this->releaseIdentifier = date('YmdHis');
@@ -459,8 +458,6 @@ public function getTemporaryPath(): string
 
     public function rollback(bool $dryRun = false): void
     {
-        Assert::notNull($this->container);
-
         $this->logger->notice('Rollback deployment ' . $this->name . ' (' . $this->releaseIdentifier . ')');
 
         /** @var RollbackWorkflow $workflow */
@@ -500,8 +497,6 @@ private function setDeploymentLockIdentifier(?string $deploymentLockIdentifier =
 
     private function createSimpleWorkflow(): SimpleWorkflow
     {
-        Assert::notNull($this->container);
-
         $workflow = $this->container->get(SimpleWorkflow::class);
 
         if (!$workflow instanceof SimpleWorkflow) {

src/Domain/Model/FailedDeployment.php

@@ -11,6 +11,7 @@
 
 namespace TYPO3\Surf\Domain\Model;
 
+use Psr\Container\ContainerInterface;
 use TYPO3\Surf\Domain\Enum\DeploymentStatus;
 
 /**
@@ -20,9 +21,9 @@
  */
 class FailedDeployment extends Deployment
 {
-    public function __construct(string $name)
+    public function __construct(ContainerInterface $container, string $name)
     {
-        parent::__construct($name);
+        parent::__construct($container, $name);
         $this->releaseIdentifier = null;
     }
 

src/Domain/Service/TaskFactory.php

@@ -11,20 +11,24 @@
 
 namespace TYPO3\Surf\Domain\Service;
 
+use Psr\Container\ContainerInterface;
 use Psr\Log\LoggerAwareInterface;
 use Psr\Log\LoggerInterface;
-use Symfony\Component\DependencyInjection\ContainerAwareInterface;
-use Symfony\Component\DependencyInjection\ContainerAwareTrait;
 use TYPO3\Surf\Domain\Model\Task;
 use UnexpectedValueException;
-use Webmozart\Assert\Assert;
 
 /**
  * @final
  */
-class TaskFactory implements ContainerAwareInterface
+class TaskFactory
 {
-    use ContainerAwareTrait;
+
+    private ContainerInterface $container;
+
+    public function __construct(ContainerInterface $container)
+    {
+        $this->container = $container;
+    }
 
     public function createTaskInstance(string $taskName): Task
     {
@@ -33,8 +37,6 @@ public function createTaskInstance(string $taskName): Task
 
     private function createTask(string $taskName): Task
     {
-        Assert::notNull($this->container);
-
         if (! $this->container->has($taskName)) {
             $task = new $taskName();
             if ($task instanceof ShellCommandServiceAwareInterface) {

src/Integration/Factory.php

@@ -14,27 +14,28 @@
 use Monolog\Handler\StreamHandler;
 use Monolog\Logger;
 use Neos\Utility\Files;
+use Psr\Container\ContainerInterface;
+use Psr\Log\LoggerInterface;
 use RuntimeException;
 use Symfony\Component\Console\Output\OutputInterface;
-use Symfony\Component\DependencyInjection\ContainerAwareInterface;
-use Symfony\Component\DependencyInjection\ContainerAwareTrait;
 use TYPO3\Surf\Domain\Filesystem\FilesystemInterface;
 use TYPO3\Surf\Domain\Model\Deployment;
 use TYPO3\Surf\Domain\Model\FailedDeployment;
 use TYPO3\Surf\Exception\InvalidConfigurationException;
 
-class Factory implements FactoryInterface, ContainerAwareInterface
+class Factory implements FactoryInterface
 {
-    use ContainerAwareTrait;
-
     protected OutputInterface $output;
 
-    protected Logger $logger;
+    protected LoggerInterface $logger;
 
     protected FilesystemInterface $filesystem;
 
-    public function __construct(FilesystemInterface $filesystem, Logger $logger)
+    protected ContainerInterface $container;
+
+    public function __construct(ContainerInterface $container, FilesystemInterface $filesystem, LoggerInterface $logger)
     {
+        $this->container = $container;
         $this->filesystem = $filesystem;
         $this->logger = $logger;
     }
@@ -45,7 +46,9 @@ public function getDeployment(string $deploymentName, string $configurationPath
 
         if (! $simulateDeployment) {
             $logFilePath = Files::concatenatePaths([$this->getWorkspacesBasePath($configurationPath), 'logs', $deployment->getName() . '.log']);
-            $this->logger->pushHandler(new StreamHandler($logFilePath));
+            if ($this->logger instanceof Logger) {
+                $this->logger->pushHandler(new StreamHandler($logFilePath));
+            }
         }
 
         $deployment->setForceRun($forceDeployment);
@@ -145,15 +148,14 @@ protected function createDeployment(string $deploymentName, string $path = null)
 
         if (! $this->filesystem->fileExists($deploymentPathAndFilename)) {
             $this->logger->error(sprintf("The deployment file %s does not exist.\n", $deploymentPathAndFilename));
-            $deployment = new FailedDeployment($deploymentName);
+            $deployment = new FailedDeployment($this->container, $deploymentName);
             $deployment->setLogger($this->logger);
 
             return $deployment;
         }
 
-        $deployment = new Deployment($deploymentName);
+        $deployment = new Deployment($this->container, $deploymentName);
         $deployment->setLogger($this->logger);
-        $deployment->setContainer($this->container);
         $deployment->setDeploymentBasePath($deploymentConfigurationPath);
         $deployment->setWorkspacesBasePath($workspacesBasePath);
 

tests/Unit/Command/DescribeCommandTest.php

@@ -42,8 +42,7 @@ class DescribeCommandTest extends TestCase
 
     protected function setUp(): void
     {
-        $this->deployment = new Deployment('TestDeployment');
-        $this->deployment->setContainer(static::getKernel()->getContainer());
+        $this->deployment = new Deployment(static::getKernel()->getContainer(), 'TestDeployment');
 
         $this->node = new Node('TestNode');
         $this->node->setHostname('hostname');

tests/Unit/Domain/Model/DeploymentTest.php

@@ -42,8 +42,7 @@ protected function tearDown(): void
      */
     public function initializeUsesSimpleWorkflowAsDefault(): void
     {
-        $deployment = new Deployment('Test deployment');
-        $deployment->setContainer(static::getKernel()->getContainer());
+        $deployment = new Deployment(static::getKernel()->getContainer(), 'Test deployment');
         $deployment->initialize();
 
         self::assertInstanceOf(SimpleWorkflow::class, $deployment->getWorkflow());
@@ -54,8 +53,7 @@ public function initializeUsesSimpleWorkflowAsDefault(): void
      */
     public function getNodesReturnsNodesFromApplicationsAsSet(): void
     {
-        $deployment = new Deployment('Test deployment');
-        $deployment->setContainer(static::getKernel()->getContainer());
+        $deployment = new Deployment(static::getKernel()->getContainer(), 'Test deployment');
         $application1 = new Application('Test application 1');
         $application2 = new Application('Test application 2');
 
@@ -81,8 +79,7 @@ public function getNodesReturnsNodesFromApplicationsAsSet(): void
      */
     public function constructorCreatesReleaseIdentifier(): void
     {
-        $deployment = new Deployment('Test deployment');
-        $deployment->setContainer(static::getKernel()->getContainer());
+        $deployment = new Deployment(static::getKernel()->getContainer(), 'Test deployment');
 
         $releaseIdentifier = $deployment->getReleaseIdentifier();
 
@@ -98,7 +95,7 @@ public function initializeIsAllowedOnlyOnce(): void
 
         $workflow = new SimpleWorkflow($this->prophesize(TaskManager::class)->reveal());
 
-        $deployment = new Deployment('Test deployment');
+        $deployment = new Deployment(static::getKernel()->getContainer(), 'Test deployment');
         $deployment->setWorkflow($workflow);
         $deployment->initialize();
 
@@ -113,8 +110,7 @@ public function initializeIsAllowedOnlyOnce(): void
      */
     public function deploymentHasDefaultLockIdentifierIfNoIdentifierIsGiven($deploymentLockIdentifier): void
     {
-        $deployment = new Deployment('Some name', $deploymentLockIdentifier);
-        $deployment->setContainer(static::getKernel()->getContainer());
+        $deployment = new Deployment(static::getKernel()->getContainer(), 'Some name', $deploymentLockIdentifier);
 
         self::assertSame($deployment->getReleaseIdentifier(), $deployment->getDeploymentLockIdentifier());
     }
@@ -125,7 +121,7 @@ public function deploymentHasDefaultLockIdentifierIfNoIdentifierIsGiven($deploym
     public function deploymentHasDefinedLockIdentifier(): void
     {
         $deploymentLockIdentifier = 'Deployment lock identifier';
-        $deployment = new Deployment('Some name', $deploymentLockIdentifier);
+        $deployment = new Deployment(static::getKernel()->getContainer(), 'Some name', $deploymentLockIdentifier);
 
         self::assertSame($deploymentLockIdentifier, $deployment->getDeploymentLockIdentifier());
     }
@@ -138,7 +134,7 @@ public function deploymentHasLockIdentifierDefinedByEnvironmentVariable(): void
         $deploymentLockIdentifier = 'Deployment lock identifier';
         putenv(sprintf('SURF_DEPLOYMENT_LOCK_IDENTIFIER=%s', $deploymentLockIdentifier));
 
-        $deployment = new Deployment('Some name');
+        $deployment = new Deployment(static::getKernel()->getContainer(), 'Some name');
 
         self::assertSame($deploymentLockIdentifier, $deployment->getDeploymentLockIdentifier());
     }
@@ -148,7 +144,7 @@ public function deploymentHasLockIdentifierDefinedByEnvironmentVariable(): void
      */
     public function deploymentContainsRelativeProjectRootPathForApplicationReleasePath(): void
     {
-        $deployment = new Deployment('Some name');
+        $deployment = new Deployment(static::getKernel()->getContainer(), 'Some name');
 
         $node = new Node('Node');
         $node->setDeploymentPath('/deployment/path');
@@ -166,7 +162,7 @@ public function deploymentContainsRelativeProjectRootPathForApplicationReleasePa
      */
     public function deploymentContainsChangedRelativeProjectRootPathForApplicationReleasePath(): void
     {
-        $deployment = new Deployment('Some name');
+        $deployment = new Deployment(static::getKernel()->getContainer(), 'Some name');
         $deployment->setRelativeProjectRootPath('htdocs');
 
         $node = new Node('Node');

tests/Unit/Domain/Model/RollbackWorkflowTest.php

@@ -21,9 +21,12 @@
 use TYPO3\Surf\Domain\Service\TaskManager;
 use TYPO3\Surf\Exception as SurfException;
 use TYPO3\Surf\Task\Generic\RollbackTask;
+use TYPO3\Surf\Tests\Unit\KernelAwareTrait;
 
 class RollbackWorkflowTest extends TestCase
 {
+    use KernelAwareTrait;
+
     /**
      * @test
      */
@@ -367,7 +370,7 @@ public function tasksAreExecutedInTheRightOrder(): void
      */
     protected function buildDeployment(array &$executedTasks = []): Deployment
     {
-        $deployment = new Deployment('Test rollback deployment');
+        $deployment = new Deployment(static::getKernel()->getContainer(), 'Test rollback deployment');
         $mockLogger = $this->createMock(LoggerInterface::class);
         // Enable log to console to debug tests
         // $mockLogger->expects(self::any())->method('log')->will($this->returnCallback(function($message) {