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

Make configuration and dependency injection container immutable #876

Merged
merged 2 commits into from
Dec 3, 2019
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.');
}
}
55 changes: 39 additions & 16 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 @@ -35,6 +36,7 @@
use Psr\Log\LoggerInterface;
use Psr\Log\NullLogger;
use Symfony\Component\Stopwatch\Stopwatch as SymfonyStopwatch;
use function array_key_exists;
use function preg_quote;
use function sprintf;

Expand All @@ -45,10 +47,12 @@
*/
class DependencyFactory
{
public const MIGRATIONS_SORTER = 'Doctrine\Migrations\MigrationsSorter';

/** @var Configuration */
private $configuration;

/** @var object[] */
/** @var object[]|callable[] */
private $dependencies = [];

/** @var LoggerInterface */
Expand All @@ -57,12 +61,12 @@ class DependencyFactory
/** @var Connection */
private $connection;

/** @var callable */
private $sorter;

/** @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 All @@ -81,6 +98,13 @@ private function getConnection() : Connection
return $this->connection;
}

public function getSorter() : ?callable
{
return $this->getDependency(self::MIGRATIONS_SORTER, static function () {
return null;
});
}

public function getEventDispatcher() : EventDispatcher
{
return $this->getDependency(EventDispatcher::class, function () : EventDispatcher {
Expand Down Expand Up @@ -179,11 +203,6 @@ public function getMigrationsFinder() : MigrationFinder
});
}

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

public function getMigrationRepository() : MigrationRepository
{
return $this->getDependency(MigrationRepository::class, function () : MigrationRepository {
Expand All @@ -192,14 +211,18 @@ public function getMigrationRepository() : MigrationRepository
$this->getConfiguration()->getMigrationDirectories(),
$this->getMigrationsFinder(),
new MigrationFactory($this->getConnection(), $this->getLogger()),
$this->sorter
$this->getSorter()
);
});
}

public function setMetadataStorageConfiguration(MetadataStorageConfiguration $metadataStorageConfigration) : void
/**
* @param object|callable $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 Expand Up @@ -347,12 +370,12 @@ public function getRollup() : Rollup
/**
* @return mixed
*/
private function getDependency(string $className, callable $callback)
private function getDependency(string $id, callable $callback)
{
if (! isset($this->dependencies[$className])) {
$this->dependencies[$className] = $callback();
if (! array_key_exists($id, $this->dependencies)) {
$this->dependencies[$id] = $callback();
}

return $this->dependencies[$className];
return $this->dependencies[$id];
}
}
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
Loading