Skip to content

Commit

Permalink
make configuration and dependency injection container immutable
Browse files Browse the repository at this point in the history
This avoids having other services hacking into the DI container or config object at runtime (avoiding inconsistent retrieval of services
  • Loading branch information
goetas committed Dec 1, 2019
1 parent 4662196 commit 0bfb8ac
Show file tree
Hide file tree
Showing 8 changed files with 155 additions and 17 deletions.
32 changes: 30 additions & 2 deletions lib/Doctrine/Migrations/Configuration/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Doctrine\Migrations\Configuration;

use Doctrine\Migrations\Configuration\Exception\FrozenConfiguration;
use Doctrine\Migrations\Configuration\Exception\MissingNamespaceConfiguration;
use Doctrine\Migrations\Configuration\Exception\UnknownConfigurationValue;
use Doctrine\Migrations\Exception\MigrationException;
Expand Down Expand Up @@ -49,8 +50,27 @@ final class Configuration
/** @var MetadataStorageConfiguration */
private $metadataStorageConfiguration;

/** @var bool */
private $frozen = false;

public function freeze() : void
{
if (! $this->frozen) {
$this->validate();
}
$this->frozen = true;
}

private function assertNotFrozen() : void
{
if ($this->frozen) {
throw FrozenConfiguration::new();
}
}

public function setMetadataStorageConfiguration(MetadataStorageConfiguration $metadataStorageConfiguration) : void
{
$this->assertNotFrozen();
$this->metadataStorageConfiguration = $metadataStorageConfiguration;
}

Expand All @@ -64,6 +84,7 @@ public function getMigrationClasses() : array

public function addMigrationClass(string $className) : void
{
$this->assertNotFrozen();
$this->migrationClasses[] = $className;
}

Expand All @@ -74,6 +95,7 @@ public function getMetadataStorageConfiguration() : ?MetadataStorageConfiguratio

public function addMigrationsDirectory(string $namespace, string $path) : void
{
$this->assertNotFrozen();
$this->migrationsDirectories[$namespace] = $path;
}

Expand All @@ -87,6 +109,7 @@ public function getMigrationDirectories() : array

public function setName(string $name) : void
{
$this->assertNotFrozen();
$this->name = $name;
}

Expand All @@ -97,6 +120,7 @@ public function getName() : ?string

public function setCustomTemplate(?string $customTemplate) : void
{
$this->assertNotFrozen();
$this->customTemplate = $customTemplate;
}

Expand All @@ -116,6 +140,7 @@ public function areMigrationsOrganizedByYear() : bool
public function setMigrationsAreOrganizedByYear(
bool $migrationsAreOrganizedByYear = true
) : void {
$this->assertNotFrozen();
$this->migrationsAreOrganizedByYear = $migrationsAreOrganizedByYear;
}

Expand All @@ -125,6 +150,7 @@ public function setMigrationsAreOrganizedByYear(
public function setMigrationsAreOrganizedByYearAndMonth(
bool $migrationsAreOrganizedByYearAndMonth = true
) : void {
$this->assertNotFrozen();
$this->migrationsAreOrganizedByYear = $migrationsAreOrganizedByYearAndMonth;
$this->migrationsAreOrganizedByYearAndMonth = $migrationsAreOrganizedByYearAndMonth;
}
Expand All @@ -134,8 +160,7 @@ public function areMigrationsOrganizedByYearAndMonth() : bool
return $this->migrationsAreOrganizedByYearAndMonth;
}

/** @throws MigrationException */
public function validate() : void
private function validate() : void
{
if (count($this->migrationsDirectories) === 0) {
throw MissingNamespaceConfiguration::new();
Expand All @@ -144,6 +169,7 @@ public function validate() : void

public function setIsDryRun(bool $isDryRun) : void
{
$this->assertNotFrozen();
$this->isDryRun = $isDryRun;
}

Expand All @@ -154,6 +180,7 @@ public function isDryRun() : bool

public function setAllOrNothing(bool $allOrNothing) : void
{
$this->assertNotFrozen();
$this->allOrNothing = $allOrNothing;
}

Expand All @@ -174,6 +201,7 @@ public function isDatabasePlatformChecked() : bool

public function setMigrationOrganization(string $migrationOrganization) : void
{
$this->assertNotFrozen();
if (strcasecmp($migrationOrganization, self::VERSIONS_ORGANIZATION_BY_YEAR) === 0) {
$this->setMigrationsAreOrganizedByYear();
} elseif (strcasecmp($migrationOrganization, self::VERSIONS_ORGANIZATION_BY_YEAR_AND_MONTH) === 0) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?php

declare(strict_types=1);

namespace Doctrine\Migrations\Configuration\Exception;

use LogicException;

final class FrozenConfiguration extends LogicException implements ConfigurationException
{
public static function new() : self
{
return new self('The configuration is frozen and cannot be edited anymore.');
}
}
26 changes: 24 additions & 2 deletions lib/Doctrine/Migrations/DependencyFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use Doctrine\DBAL\Connection;
use Doctrine\Migrations\Configuration\Configuration;
use Doctrine\Migrations\Exception\FrozenDependencies;
use Doctrine\Migrations\Exception\MissingDependency;
use Doctrine\Migrations\Finder\GlobFinder;
use Doctrine\Migrations\Finder\MigrationFinder;
Expand Down Expand Up @@ -63,6 +64,9 @@ class DependencyFactory
/** @var EntityManagerInterface|null */
private $em;

/** @var bool */
private $frozen = false;

public function __construct(Configuration $configuration, Connection $connection, ?EntityManagerInterface $em = null, ?LoggerInterface $logger = null)
{
$this->configuration = $configuration;
Expand All @@ -71,6 +75,19 @@ public function __construct(Configuration $configuration, Connection $connection
$this->em = $em;
}

public function freeze() : void
{
$this->frozen = true;
$this->configuration->freeze();
}

private function assertNotFrozen() : void
{
if ($this->frozen) {
throw FrozenDependencies::new();
}
}

public function getConfiguration() : Configuration
{
return $this->configuration;
Expand Down Expand Up @@ -181,6 +198,7 @@ public function getMigrationsFinder() : MigrationFinder

public function setSorter(callable $sorter) : void
{
$this->assertNotFrozen();
$this->sorter = $sorter;
}

Expand All @@ -197,9 +215,13 @@ public function getMigrationRepository() : MigrationRepository
});
}

public function setMetadataStorageConfiguration(MetadataStorageConfiguration $metadataStorageConfigration) : void
/**
* @param mixed $service
*/
public function setService(string $id, $service) : void
{
$this->dependencies[MetadataStorageConfiguration::class] = $metadataStorageConfigration;
$this->assertNotFrozen();
$this->dependencies[$id] = $service;
}

private function getMetadataStorageConfiguration() : MetadataStorageConfiguration
Expand Down
15 changes: 15 additions & 0 deletions lib/Doctrine/Migrations/Exception/FrozenDependencies.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?php

declare(strict_types=1);

namespace Doctrine\Migrations\Exception;

use LogicException;

final class FrozenDependencies extends LogicException implements MigrationException
{
public static function new() : self
{
return new self('The dependencies are frozen and cannot be edited anymore.');
}
}
18 changes: 6 additions & 12 deletions lib/Doctrine/Migrations/Tools/Console/Command/DoctrineCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,6 @@ abstract class DoctrineCommand extends Command
/** @var DependencyFactory|null */
private $dependencyFactory;

public function initialize(
InputInterface $input,
OutputInterface $output
) : void {
$this->initializeDependencies($input, $output);
$this->getDependencyFactory()->getConfiguration()->validate();
}

public function __construct(?string $name = null, ?DependencyFactory $dependencyFactory = null)
{
parent::__construct($name);
Expand Down Expand Up @@ -74,13 +66,14 @@ protected function outputHeader(
$output->writeln('');
}

protected function initializeDependencies(
InputInterface $input,
OutputInterface $output
) : void {
protected function initialize(InputInterface $input, OutputInterface $output) : void
{
if ($this->dependencyFactory !== null) {
$this->dependencyFactory->freeze();

return;
}

$helperSet = $this->getHelperSet() ?: new HelperSet();

if ($helperSet->has('configuration') && $helperSet->get('configuration') instanceof ConfigurationHelper) {
Expand All @@ -104,6 +97,7 @@ protected function initializeDependencies(

$logger = new ConsoleLogger($output);
$this->dependencyFactory = new DependencyFactory($configuration, $connection, $em, $logger);
$this->dependencyFactory->freeze();
}

protected function getDependencyFactory() : DependencyFactory
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
namespace Doctrine\Migrations\Tests\Configuration;

use Doctrine\Migrations\Configuration\Configuration;
use Doctrine\Migrations\Configuration\Exception\FrozenConfiguration;
use Doctrine\Migrations\Configuration\Exception\MissingNamespaceConfiguration;
use Doctrine\Migrations\Configuration\Exception\UnknownConfigurationValue;
use Doctrine\Migrations\Metadata\Storage\MetadataStorageConfiguration;
Expand Down Expand Up @@ -53,7 +54,18 @@ public function testNoNamespaceConfigured() : void
$this->expectExceptionMessage('There are no namespaces configured.');

$config = new Configuration();
$config->validate();
$config->freeze();
}

public function testFreezeConfiguration() : void
{
$config = new Configuration();
$config->addMigrationsDirectory('foo', 'bar');
$config->freeze();

$this->expectException(FrozenConfiguration::class);
$this->expectExceptionMessage('The configuration is frozen and cannot be edited anymore.');
$config->setName('foo');
}

public function testMigrationOrganizationByYear() : void
Expand Down
15 changes: 15 additions & 0 deletions tests/Doctrine/Migrations/Tests/DependencyFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@
use Doctrine\DBAL\Connection;
use Doctrine\Migrations\Configuration\Configuration;
use Doctrine\Migrations\DependencyFactory;
use Doctrine\Migrations\Exception\FrozenDependencies;
use Doctrine\Migrations\Finder\GlobFinder;
use Doctrine\Migrations\Finder\RecursiveRegexFinder;
use PHPUnit\Framework\MockObject\MockObject;
use stdClass;

final class DependencyFactoryTest extends MigrationTestCase
{
Expand All @@ -21,6 +23,19 @@ public function setUp() : void
$this->connection = $this->createMock(Connection::class);
}

public function testFreeze() : void
{
$conf = new Configuration();
$conf->addMigrationsDirectory('foo', 'bar');

$di = new DependencyFactory($conf, $this->connection);
$di->freeze();

$this->expectException(FrozenDependencies::class);
$this->expectExceptionMessage('The dependencies are frozen and cannot be edited anymore.');
$di->setService('foo', new stdClass());
}

public function testFinderForYearMonthStructure() : void
{
$conf = new Configuration();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?php

declare(strict_types=1);

namespace Doctrine\Migrations\Tests\Tools\Console\Command;

use Doctrine\Migrations\DependencyFactory;
use Doctrine\Migrations\Tests\MigrationTestCase;
use Doctrine\Migrations\Tools\Console\Command\DoctrineCommand;
use Symfony\Component\Console\Tester\CommandTester;

class DoctrineCommandTest extends MigrationTestCase
{
public function testCommandFreezes() : void
{
$dependencyFactory = $this->getMockBuilder(DependencyFactory::class)
->disableOriginalConstructor()
->setMethods(['freeze'])
->getMock();

$dependencyFactory
->expects(self::once())
->method('freeze');

$command = $this->getMockBuilder(DoctrineCommand::class)
->setConstructorArgs(['foo', $dependencyFactory])
->setMethods(['execute'])
->getMockForAbstractClass();

$commandTester = new CommandTester($command);

$commandTester->execute(
[],
['interactive' => false]
);
}
}

0 comments on commit 0bfb8ac

Please sign in to comment.